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

Open Source and information security mailing list archives

Message-ID: <CAJ8uoz2_s_hyYZaPt15a9c274UXC-8Ejc2nPAmqMN9437fcciQ@mail.gmail.com>
Date:   Fri, 9 Dec 2022 10:09:00 +0100
From:   Magnus Karlsson <magnus.karlsson@...il.com>
To:     "Rout, ChandanX" <chandanx.rout@...el.com>,
        intel-wired-lan <intel-wired-lan@...ts.osuosl.org>,
        "Nguyen, Anthony L" <anthony.l.nguyen@...el.com>
Cc:     "Lobakin, Alexandr" <alexandr.lobakin@...el.com>,
        Leon Romanovsky <leon@...nel.org>,
        "Fijalkowski, Maciej" <maciej.fijalkowski@...el.com>,
        "Sarkar, Tirthendu" <tirthendu.sarkar@...el.com>,
        "tirtha@...il.com" <tirtha@...il.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "Karlsson, Magnus" <magnus.karlsson@...el.com>,
        "Kuruvinakunnel, George" <george.kuruvinakunnel@...el.com>,
        "Nagaraju, Shwetha" <shwetha.nagaraju@...el.com>,
        "Nagraj, Shravan" <shravan.nagraj@...el.com>
Subject: Re: [Intel-wired-lan] [PATCH intel-next v4] i40e: allow toggling
 loopback mode via ndo_set_features callback

On Wed, Dec 7, 2022 at 4:37 PM Magnus Karlsson
<magnus.karlsson@...il.com> wrote:
>
> On Wed, Dec 7, 2022 at 4:01 PM Rout, ChandanX <chandanx.rout@...el.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@...el.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@...alhost admin]# ethtool -k ens802f3 | grep loopback
> > loopback: off
> > [root@...alhost admin]# ethtool -K ens802f3 loopback on
> > [root@...alhost admin]# ethtool -k ens802f3 | grep loopback
> > loopback: on
> > [root@...alhost 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@...alhost 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@...alhost admin]# ethtool -k ens801f0np0 | grep loopback
> > loopback: off
> > [root@...alhost admin]# ethtool -K ens801f0np0 loopback on
> > [root@...alhost admin]# ethtool -k ens801f0np0 | grep loopback
> > loopback: on
> > [root@...alhost 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@...alhost 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@...osl.org> On Behalf Of Lobakin, Alexandr
> > Sent: 22 November 2022 21:28
> > To: Leon Romanovsky <leon@...nel.org>
> > Cc: Sarkar, Tirthendu <tirthendu.sarkar@...el.com>; tirtha@...il.com; intel-wired-lan@...ts.osuosl.org; netdev@...r.kernel.org; Karlsson, Magnus <magnus.karlsson@...el.com>
> > Subject: Re: [Intel-wired-lan] [PATCH intel-next v4] i40e: allow toggling loopback mode via ndo_set_features callback
> >
> > From: Leon Romanovsky <leon@...nel.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@...el.com>
> >
> > Thanks,
> > Olek
> > _______________________________________________
> > Intel-wired-lan mailing list
> > Intel-wired-lan@...osl.org
> > https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

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