Re: [PATCH intel-next v4] i40e: allow toggling loopback mode via ndo_set_features callback

[Leon Romanovsky <leon@kernel.org>]

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@intel.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;
>