netdev - Re: [PATCH intel-next v4] i40e: allow toggling loopback mode via ndo_set

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Y3ytcGM2c52lzukO@unreal>
Date:   Tue, 22 Nov 2022 13:07:28 +0200
From:   Leon Romanovsky <leon@...nel.org>
To:     Tirthendu Sarkar <tirthendu.sarkar@...el.com>
Cc:     tirtha@...il.com, magnus.karlsson@...el.com,
        intel-wired-lan@...ts.osuosl.org, anthony.l.nguyen@...el.com,
        maciej.fijalkowski@...el.com, netdev@...r.kernel.org
Subject: Re: [PATCH intel-next v4] i40e: allow toggling loopback mode via
 ndo_set_features callback

On Fri, Nov 18, 2022 at 02:33:06PM +0530, Tirthendu Sarkar wrote:
> Add support for NETIF_F_LOOPBACK. This feature can be set via:
> $ ethtool -K eth0 loopback <on|off>
> 
> This sets the MAC Tx->Rx loopback.
> 
> This feature is used for the xsk selftests, and might have other uses
> too.
> 
> Changelog:
>     v3 -> v4:
>     - Removed unused %_LEGACY macros as suggested by Alexandr Lobakin.
>     - Modified checks for interface bringup and i40e_set_loopback().
>     - Propagating up return value from i40_set_loopback().
> 
>     v2 -> v3:
>      - Fixed loopback macros as per NVM version 6.01+.
>      - Renamed existing macros as *_LEGACY.
>      - Based on NVM verison appropriate macro is used for MAC loopback.
> 
>     v1 -> v2:
>      - Moved loopback to netdev's hardware features as suggested by
>        Alexandr Lobakin.

Please put changelog after --- mark. It doesn't belong to the commit
message.

> 
> Signed-off-by: Tirthendu Sarkar <tirthendu.sarkar@...el.com>
> ---
>  .../net/ethernet/intel/i40e/i40e_adminq_cmd.h |  8 ++++--
>  drivers/net/ethernet/intel/i40e/i40e_common.c | 26 +++++++++++++++++
>  drivers/net/ethernet/intel/i40e/i40e_main.c   | 28 ++++++++++++++++++-
>  .../net/ethernet/intel/i40e/i40e_prototype.h  |  3 ++
>  4 files changed, 61 insertions(+), 4 deletions(-)

<...>

>  /**
>   * i40e_set_features - set the netdev feature flags
>   * @netdev: ptr to the netdev being adjusted
> @@ -12960,6 +12983,9 @@ static int i40e_set_features(struct net_device *netdev,
>  	if (need_reset)
>  		i40e_do_reset(pf, I40E_PF_RESET_FLAG, true);
>  
> +	if ((features ^ netdev->features) & NETIF_F_LOOPBACK)
> +		return i40e_set_loopback(vsi, !!(features & NETIF_F_LOOPBACK));

Don't you need to disable loopback if NETIF_F_LOOPBACK was cleared?

> +
>  	return 0;
>  }
>  
> @@ -13722,7 +13748,7 @@ static int i40e_config_netdev(struct i40e_vsi *vsi)
>  	if (!(pf->flags & I40E_FLAG_MFP_ENABLED))
>  		hw_features |= NETIF_F_NTUPLE | NETIF_F_HW_TC;
>  
> -	netdev->hw_features |= hw_features;
> +	netdev->hw_features |= hw_features | NETIF_F_LOOPBACK;
>