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

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

[Tirthendu Sarkar]

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.

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(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_adminq_cmd.h b/drivers/net/ethernet/intel/i40e/i40e_adminq_cmd.h
index 60f9e0a6aaca..085355e2acac 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_adminq_cmd.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_adminq_cmd.h
@@ -1795,9 +1795,11 @@ I40E_CHECK_CMD_LENGTH(i40e_aqc_an_advt_reg);
 /* Set Loopback mode (0x0618) */
 struct i40e_aqc_set_lb_mode {
 	__le16	lb_mode;
-#define I40E_AQ_LB_PHY_LOCAL	0x01
-#define I40E_AQ_LB_PHY_REMOTE	0x02
-#define I40E_AQ_LB_MAC_LOCAL	0x04
+#define I40E_LEGACY_LOOPBACK_NVM_VER	0x6000
+#define I40E_AQ_LB_MAC_LOCAL		0x01
+#define I40E_AQ_LB_PHY_LOCAL		0x05
+#define I40E_AQ_LB_PHY_REMOTE		0x06
+#define I40E_AQ_LB_MAC_LOCAL_LEGACY   	0x04
 	u8	reserved[14];
 };
 
diff --git a/drivers/net/ethernet/intel/i40e/i40e_common.c b/drivers/net/ethernet/intel/i40e/i40e_common.c
index 4f01e2a6b6bb..8f764ff5c990 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_common.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_common.c
@@ -1830,6 +1830,32 @@ i40e_status i40e_aq_set_phy_int_mask(struct i40e_hw *hw,
 	return status;
 }
 
+/**
+ * i40e_aq_set_mac_loopback
+ * @hw: pointer to the HW struct
+ * @ena_lpbk: Enable or Disable loopback
+ * @cmd_details: pointer to command details structure or NULL
+ *
+ * Enable/disable loopback on a given port
+ */
+i40e_status i40e_aq_set_mac_loopback(struct i40e_hw *hw, bool ena_lpbk,
+				     struct i40e_asq_cmd_details *cmd_details)
+{
+	struct i40e_aq_desc desc;
+	struct i40e_aqc_set_lb_mode *cmd =
+		(struct i40e_aqc_set_lb_mode *)&desc.params.raw;
+
+	i40e_fill_default_direct_cmd_desc(&desc, i40e_aqc_opc_set_lb_modes);
+	if (ena_lpbk) {
+		if (hw->nvm.version <= I40E_LEGACY_LOOPBACK_NVM_VER)
+			cmd->lb_mode = cpu_to_le16(I40E_AQ_LB_MAC_LOCAL_LEGACY);
+		else
+			cmd->lb_mode = cpu_to_le16(I40E_AQ_LB_MAC_LOCAL);
+	}
+
+	return i40e_asq_send_command(hw, &desc, NULL, 0, cmd_details);
+}
+
 /**
  * i40e_aq_set_phy_debug
  * @hw: pointer to the hw struct
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 4880b740fa6e..298fdd19900c 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -12920,6 +12920,29 @@ static void i40e_clear_rss_lut(struct i40e_vsi *vsi)
 	}
 }
 
+/**
+ * i40e_set_loopback - turn on/off loopback mode on underlying PF
+ * @vsi: ptr to VSI
+ * @ena: flag to indicate the on/off setting
+ */
+static int i40e_set_loopback(struct i40e_vsi *vsi, bool ena)
+{
+	bool if_running = netif_running(vsi->netdev) &&
+			  !test_and_set_bit(__I40E_VSI_DOWN, vsi->state);
+	int ret;
+
+	if (if_running)
+		i40e_down(vsi);
+
+	ret = i40e_aq_set_mac_loopback(&vsi->back->hw, ena, NULL);
+	if (ret)
+		netdev_err(vsi->netdev, "Failed to toggle loopback state\n");
+	if (if_running)
+		i40e_up(vsi);
+
+	return ret;
+}
+
 /**
  * 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));
+
 	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;
 
 	netdev->features |= hw_features | NETIF_F_HW_VLAN_CTAG_FILTER;
 	netdev->hw_enc_features |= NETIF_F_TSO_MANGLEID;
diff --git a/drivers/net/ethernet/intel/i40e/i40e_prototype.h b/drivers/net/ethernet/intel/i40e/i40e_prototype.h
index ebdcde6f1aeb..9a71121420c3 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_prototype.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_prototype.h
@@ -105,6 +105,9 @@ enum i40e_status_code i40e_aq_set_phy_config(struct i40e_hw *hw,
 				struct i40e_asq_cmd_details *cmd_details);
 enum i40e_status_code i40e_set_fc(struct i40e_hw *hw, u8 *aq_failures,
 				  bool atomic_reset);
+i40e_status i40e_aq_set_mac_loopback(struct i40e_hw *hw,
+				     bool ena_lpbk,
+				     struct i40e_asq_cmd_details *cmd_details);
 i40e_status i40e_aq_set_phy_int_mask(struct i40e_hw *hw, u16 mask,
 				     struct i40e_asq_cmd_details *cmd_details);
 i40e_status i40e_aq_clear_pxe_mode(struct i40e_hw *hw,
-- 
2.34.1

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

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

[Leon Romanovsky]

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;
>  

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

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

[Alexander Lobakin]

From: Leon Romanovsky <leon@kernel.org>
Date: Tue, 22 Nov 2022 13:07:28 +0200

> 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.

[...]

> > @@ -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?

0 ^ 1 == 1 -> call i40e_set_loopback()
!!(0) == 0 -> disable

> 
> > +
> >  	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;
> >  

Reviewed-by: Alexander Lobakin <alexandr.lobakin@intel.com>

Thanks,
Olek
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

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

[Leon Romanovsky]

On Tue, Nov 22, 2022 at 04:57:59PM +0100, Alexander Lobakin wrote:
> From: Leon Romanovsky <leon@kernel.org>
> Date: Tue, 22 Nov 2022 13:07:28 +0200
> 
> > 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.
> 
> [...]
> 
> > > @@ -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?
> 
> 0 ^ 1 == 1 -> call i40e_set_loopback()
> !!(0) == 0 -> disable
> 

Nice, thanks,
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

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

[Magnus Karlsson]

On Wed, Dec 7, 2022 at 4:37 PM Magnus Karlsson
<magnus.karlsson@gmail.com> wrote:
>
> On Wed, Dec 7, 2022 at 4:01 PM Rout, ChandanX <chandanx.rout@intel.com> wrote:
> >
> > Hi Team,
> > We observed some different result on i40e driver which are as follows
> >
> > Issue Summary: Unable to find loopback test in "ethtool -t <interface> using i40e driver using latest next-queue.
> > Observations:
> > ===========
> > 1. we are able to enable loopback mode on i40e driver.
> > 2. We are unable to find loopback test in "ethtool -t <interface>" command while using i40e driver.
>
> That is correct, there is no loopback test in i40e. We chose not to
> add one since it was broken in ice (until Maciej fixed it), so we
> thought nobody actually used it. Instead, we have a much more thorough
> test shipped in tools/testing/selftests/bpf/xsk* that tests the
> loopback support in more ways than just sending a single message. I
> have run this test using Tirtha's patch and it passes. So can I sign
> off with a Tested-by? Would save you a lot of time, which is good.
> There is no point for you to run the same test as I did again. Just a
> waste of valuable testing time.

Adding intel-wired and Tony who was not on the reply for some reason.

I have now tested the patch and it passes all the tests executed by
tools/testing/selftests/bpf/test_xsk.sh. The script launches over 100
tests that send 1000s of different packets through the loopback
interface and verifies that the packet content is the same as what was
sent and that they are received in order.

Tested-by: Magnus Karlsson <magnus.karlsson@intel.com>

Tony, please pick this up for your next i40e pull request / release.

Thank you: Magnus

> > 3. However, in ice driver we are able to enable loopback mode also we are able to see the loopback test using "ethtool -t <interface>".
> >
> > Note: Detail Observation is attached in excel format.
> >
> > On I40e
> > =======
> > [root@localhost admin]# ethtool -k ens802f3 | grep loopback
> > loopback: off
> > [root@localhost admin]# ethtool -K ens802f3 loopback on
> > [root@localhost admin]# ethtool -k ens802f3 | grep loopback
> > loopback: on
> > [root@localhost admin]# ethtool -t ens802f3 online
> > The test result is PASS
> > The test extra info:
> > Register test  (offline)         0
> > Eeprom test    (offline)         0
> > Interrupt test (offline)         0
> > Link test   (on/offline)         0
> > [root@localhost admin]# ethtool -t ens802f3 offline
> > The test result is PASS
> > The test extra info:
> > Register test  (offline)         0
> > Eeprom test    (offline)         0
> > Interrupt test (offline)         0
> > Link test   (on/offline)         0
> >
> > On ice
> > =====
> > [root@localhost admin]# ethtool -k ens801f0np0 | grep loopback
> > loopback: off
> > [root@localhost admin]# ethtool -K ens801f0np0 loopback on
> > [root@localhost admin]# ethtool -k ens801f0np0 | grep loopback
> > loopback: on
> > [root@localhost admin]# ethtool -t ens801f0np0 online
> > The test result is PASS
> > The test extra info:
> > Register test  (offline)         0
> > EEPROM test    (offline)         0
> > Interrupt test (offline)         0
> > Loopback test  (offline)         0
> > Link test   (on/offline)         0
> > [root@localhost admin]# ethtool -t ens801f0np0 offline
> > The test result is PASS
> > The test extra info:
> > Register test  (offline)         0
> > EEPROM test    (offline)         0
> > Interrupt test (offline)         0
> > Loopback test  (offline)         0
> > Link test   (on/offline)         0
> >
> >
> > Thanks & Regards
> > Chandan Kumar Rout
> >
> > -----Original Message-----
> > From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Lobakin, Alexandr
> > Sent: 22 November 2022 21:28
> > To: Leon Romanovsky <leon@kernel.org>
> > Cc: Sarkar, Tirthendu <tirthendu.sarkar@intel.com>; tirtha@gmail.com; intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; Karlsson, Magnus <magnus.karlsson@intel.com>
> > Subject: Re: [Intel-wired-lan] [PATCH intel-next v4] i40e: allow toggling loopback mode via ndo_set_features callback
> >
> > From: Leon Romanovsky <leon@kernel.org>
> > Date: Tue, 22 Nov 2022 13:07:28 +0200
> >
> > > 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.
> >
> > [...]
> >
> > > > @@ -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?
> >
> > 0 ^ 1 == 1 -> call i40e_set_loopback()
> > !!(0) == 0 -> disable
> >
> > >
> > > > +
> > > >     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;
> > > >
> >
> > Reviewed-by: Alexander Lobakin <alexandr.lobakin@intel.com>
> >
> > Thanks,
> > Olek
> > _______________________________________________
> > Intel-wired-lan mailing list
> > Intel-wired-lan@osuosl.org
> > https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

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

[Tony Nguyen]

On 12/9/2022 1:09 AM, Magnus Karlsson wrote:
> On Wed, Dec 7, 2022 at 4:37 PM Magnus Karlsson
> <magnus.karlsson@gmail.com> wrote:
>
> I have now tested the patch and it passes all the tests executed by
> tools/testing/selftests/bpf/test_xsk.sh. The script launches over 100
> tests that send 1000s of different packets through the loopback
> interface and verifies that the packet content is the same as what was
> sent and that they are received in order.
> 
> Tested-by: Magnus Karlsson <magnus.karlsson@intel.com>
> 
> Tony, please pick this up for your next i40e pull request / release.

Sure. Besides this, I don't have any i40e for net-next currently, but 
I'll send this one out later today to try and make it in before net-next 
closes.

Thanks,
Tony
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan