[PATCH net-next 1/2] selftests: drv-net: gro: improve feature config

[PATCH net-next 1/2] selftests: drv-net: gro: improve feature config

[Jakub Kicinski]

We'll need to do a lot more feature handling to test HW-GRO and LRO.
Clean up the feature handling for SW GRO a bit to let the next commit
focus on the new test cases, only.

Make sure HW GRO-like features are not enabled for the SW tests.
Be more careful about changing features as "nothing changed"
situations may result in non-zero error code from ethtool.

Don't disable TSO on the local interface (receiver) when running over
netdevsim, we just want GSO to break up the segments on the sender.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
CC: shuah@kernel.org
CC: linux-kselftest@vger.kernel.org
---
 tools/testing/selftests/drivers/net/gro.py | 38 ++++++++++++++++++++--
 1 file changed, 35 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/gro.py b/tools/testing/selftests/drivers/net/gro.py
index ba83713bf7b5..6d633bdc7e67 100755
--- a/tools/testing/selftests/drivers/net/gro.py
+++ b/tools/testing/selftests/drivers/net/gro.py
@@ -20,7 +20,7 @@ coalescing behavior.
 import os
 from lib.py import ksft_run, ksft_exit, ksft_pr
 from lib.py import NetDrvEpEnv, KsftXfailEx
-from lib.py import cmd, defer, bkg, ip
+from lib.py import cmd, defer, bkg, ethtool, ip
 from lib.py import ksft_variants
 
 
@@ -70,6 +70,27 @@ from lib.py import ksft_variants
         defer(ip, f"link set dev {dev['ifname']} mtu {dev['mtu']}", host=host)
 
 
+def _set_ethtool_feat(dev, current, feats, host=None):
+    s2n = {True: "on", False: "off"}
+
+    new = ["-K", dev]
+    old = ["-K", dev]
+    no_change = True
+    for name, state in feats.items():
+        new += [name, s2n[state]]
+        old += [name, s2n[not state]]
+
+        if current[name]["active"] != state:
+            no_change = False
+            if current[name]["fixed"]:
+                raise KsftXfailEx(f"Device does not support {name}")
+    if no_change:
+        return
+
+    ethtool(" ".join(new), host=host)
+    defer(ethtool, " ".join(old), host=host)
+
+
 def _setup(cfg, test_name):
     """ Setup hardware loopback mode for GRO testing. """
 
@@ -77,6 +98,11 @@ from lib.py import ksft_variants
         cfg.bin_local = cfg.test_dir / "gro"
         cfg.bin_remote = cfg.remote.deploy(cfg.bin_local)
 
+    if not hasattr(cfg, "feat"):
+        cfg.feat = ethtool(f"-k {cfg.ifname}", json=True)[0]
+        cfg.remote_feat = ethtool(f"-k {cfg.remote_ifname}",
+                                  host=cfg.remote, json=True)[0]
+
     # "large" test needs at least 4k MTU
     if test_name == "large":
         _set_mtu_restore(cfg.dev, 4096, None)
@@ -88,15 +114,21 @@ from lib.py import ksft_variants
     _write_defer_restore(cfg, flush_path, "200000", defer_undo=True)
     _write_defer_restore(cfg, irq_path, "10", defer_undo=True)
 
+    _set_ethtool_feat(cfg.ifname, cfg.feat,
+                      {"generic-receive-offload": True,
+                       "rx-gro-hw": False,
+                       "large-receive-offload": False})
+
     try:
         # Disable TSO for local tests
         cfg.require_nsim()  # will raise KsftXfailEx if not running on nsim
 
-        cmd(f"ethtool -K {cfg.ifname} gro on tso off")
-        cmd(f"ethtool -K {cfg.remote_ifname} gro on tso off", host=cfg.remote)
+        _set_ethtool_feat(cfg.remote_ifname, cfg.remote_feat, {"tso": False},
+                          host=cfg.remote)
     except KsftXfailEx:
         pass
 
+
 def _gro_variants():
     """Generator that yields all combinations of protocol and test types."""
 
-- 
2.51.1

[PATCH net-next 2/2] selftests: drv-net: gro: run the test against HW GRO and LRO

[Jakub Kicinski]

Run the test against HW GRO and LRO. NICs I have pass the base cases.
Interestingly all are happy to build GROs larger than 64k.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
CC: shuah@kernel.org
CC: sdf@fomichev.me
CC: linux-kselftest@vger.kernel.org
---
 tools/testing/selftests/drivers/net/gro.py | 50 ++++++++++++++++------
 1 file changed, 36 insertions(+), 14 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/gro.py b/tools/testing/selftests/drivers/net/gro.py
index 6d633bdc7e67..ea7070b033d4 100755
--- a/tools/testing/selftests/drivers/net/gro.py
+++ b/tools/testing/selftests/drivers/net/gro.py
@@ -91,7 +91,7 @@ from lib.py import ksft_variants
     defer(ethtool, " ".join(old), host=host)
 
 
-def _setup(cfg, test_name):
+def _setup(cfg, mode, test_name):
     """ Setup hardware loopback mode for GRO testing. """
 
     if not hasattr(cfg, "bin_remote"):
@@ -108,16 +108,37 @@ from lib.py import ksft_variants
         _set_mtu_restore(cfg.dev, 4096, None)
         _set_mtu_restore(cfg.remote_dev, 4096, cfg.remote)
 
-    flush_path = f"/sys/class/net/{cfg.ifname}/gro_flush_timeout"
-    irq_path = f"/sys/class/net/{cfg.ifname}/napi_defer_hard_irqs"
+    if mode == "sw":
+        flush_path = f"/sys/class/net/{cfg.ifname}/gro_flush_timeout"
+        irq_path = f"/sys/class/net/{cfg.ifname}/napi_defer_hard_irqs"
 
-    _write_defer_restore(cfg, flush_path, "200000", defer_undo=True)
-    _write_defer_restore(cfg, irq_path, "10", defer_undo=True)
+        _write_defer_restore(cfg, flush_path, "200000", defer_undo=True)
+        _write_defer_restore(cfg, irq_path, "10", defer_undo=True)
 
-    _set_ethtool_feat(cfg.ifname, cfg.feat,
-                      {"generic-receive-offload": True,
-                       "rx-gro-hw": False,
-                       "large-receive-offload": False})
+        _set_ethtool_feat(cfg.ifname, cfg.feat,
+                          {"generic-receive-offload": True,
+                           "rx-gro-hw": False,
+                           "large-receive-offload": False})
+    elif mode == "hw":
+        # The only way to get HW GRO but elide SW GRO is to install
+        # a dummy XDP generic program. Disabling SW GRO as a feature
+        # would also disable HW GRO.
+        prog = cfg.net_lib_dir / "xdp_dummy.bpf.o"
+        ip(f"link set dev {cfg.ifname} xdpgeneric obj {prog} sec xdp")
+        defer(ip, f"link set dev {cfg.ifname} xdpgeneric off")
+
+        # Attaching XDP may change features, fetch the latest state
+        feat = ethtool(f"-k {cfg.ifname}", json=True)[0]
+
+        _set_ethtool_feat(cfg.ifname, feat,
+                          {"generic-receive-offload": True,
+                           "rx-gro-hw": True,
+                           "large-receive-offload": False})
+    elif mode == "lro":
+        _set_ethtool_feat(cfg.ifname, cfg.feat,
+                          {"generic-receive-offload": False,
+                           "rx-gro-hw": False,
+                           "large-receive-offload": True})
 
     try:
         # Disable TSO for local tests
@@ -132,19 +153,20 @@ from lib.py import ksft_variants
 def _gro_variants():
     """Generator that yields all combinations of protocol and test types."""
 
-    for protocol in ["ipv4", "ipv6", "ipip"]:
-        for test_name in ["data", "ack", "flags", "tcp", "ip", "large"]:
-            yield protocol, test_name
+    for mode in ["sw", "hw", "lro"]:
+        for protocol in ["ipv4", "ipv6", "ipip"]:
+            for test_name in ["data", "ack", "flags", "tcp", "ip", "large"]:
+                yield mode, protocol, test_name
 
 
 @ksft_variants(_gro_variants())
-def test(cfg, protocol, test_name):
+def test(cfg, mode, protocol, test_name):
     """Run a single GRO test with retries."""
 
     ipver = "6" if protocol[-1] == "6" else "4"
     cfg.require_ipver(ipver)
 
-    _setup(cfg, test_name)
+    _setup(cfg, mode, test_name)
 
     base_cmd_args = [
         f"--{protocol}",
-- 
2.51.1

Re: [PATCH net-next 2/2] selftests: drv-net: gro: run the test against HW GRO and LRO

[Willem de Bruijn]

Jakub Kicinski wrote:
> Run the test against HW GRO and LRO. NICs I have pass the base cases.
> Interestingly all are happy to build GROs larger than 64k.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

Reviewed-by: Willem de Bruijn <willemb@google.com>

> ---
> CC: shuah@kernel.org
> CC: sdf@fomichev.me
> CC: linux-kselftest@vger.kernel.org
> ---
>  tools/testing/selftests/drivers/net/gro.py | 50 ++++++++++++++++------
>  1 file changed, 36 insertions(+), 14 deletions(-)
> 
> diff --git a/tools/testing/selftests/drivers/net/gro.py b/tools/testing/selftests/drivers/net/gro.py
> index 6d633bdc7e67..ea7070b033d4 100755
> --- a/tools/testing/selftests/drivers/net/gro.py
> +++ b/tools/testing/selftests/drivers/net/gro.py
> @@ -91,7 +91,7 @@ from lib.py import ksft_variants
>      defer(ethtool, " ".join(old), host=host)
>  
>  
> -def _setup(cfg, test_name):
> +def _setup(cfg, mode, test_name):
>      """ Setup hardware loopback mode for GRO testing. """
>  
>      if not hasattr(cfg, "bin_remote"):
> @@ -108,16 +108,37 @@ from lib.py import ksft_variants
>          _set_mtu_restore(cfg.dev, 4096, None)
>          _set_mtu_restore(cfg.remote_dev, 4096, cfg.remote)
>  
> -    flush_path = f"/sys/class/net/{cfg.ifname}/gro_flush_timeout"
> -    irq_path = f"/sys/class/net/{cfg.ifname}/napi_defer_hard_irqs"
> +    if mode == "sw":
> +        flush_path = f"/sys/class/net/{cfg.ifname}/gro_flush_timeout"
> +        irq_path = f"/sys/class/net/{cfg.ifname}/napi_defer_hard_irqs"
>  
> -    _write_defer_restore(cfg, flush_path, "200000", defer_undo=True)
> -    _write_defer_restore(cfg, irq_path, "10", defer_undo=True)
> +        _write_defer_restore(cfg, flush_path, "200000", defer_undo=True)
> +        _write_defer_restore(cfg, irq_path, "10", defer_undo=True)
>  
> -    _set_ethtool_feat(cfg.ifname, cfg.feat,
> -                      {"generic-receive-offload": True,
> -                       "rx-gro-hw": False,
> -                       "large-receive-offload": False})
> +        _set_ethtool_feat(cfg.ifname, cfg.feat,
> +                          {"generic-receive-offload": True,
> +                           "rx-gro-hw": False,
> +                           "large-receive-offload": False})
> +    elif mode == "hw":
> +        # The only way to get HW GRO but elide SW GRO is to install
> +        # a dummy XDP generic program. Disabling SW GRO as a feature
> +        # would also disable HW GRO.
> +        prog = cfg.net_lib_dir / "xdp_dummy.bpf.o"
> +        ip(f"link set dev {cfg.ifname} xdpgeneric obj {prog} sec xdp")
> +        defer(ip, f"link set dev {cfg.ifname} xdpgeneric off")
> +
> +        # Attaching XDP may change features, fetch the latest state
> +        feat = ethtool(f"-k {cfg.ifname}", json=True)[0]
> +
> +        _set_ethtool_feat(cfg.ifname, feat,
> +                          {"generic-receive-offload": True,
> +                           "rx-gro-hw": True,
> +                           "large-receive-offload": False})
> +    elif mode == "lro":
> +        _set_ethtool_feat(cfg.ifname, cfg.feat,
> +                          {"generic-receive-offload": False,

So GRO off disables HW_GRO, but not LRO? That difference is behavior
is confusing. Could we still see this as a regression and make the
ethtool HW_GRO feature equally independent from SW_GRO?

> +                           "rx-gro-hw": False,
> +                           "large-receive-offload": True})
>  
>      try:
>          # Disable TSO for local tests
> @@ -132,19 +153,20 @@ from lib.py import ksft_variants
>  def _gro_variants():
>      """Generator that yields all combinations of protocol and test types."""
>  
> -    for protocol in ["ipv4", "ipv6", "ipip"]:
> -        for test_name in ["data", "ack", "flags", "tcp", "ip", "large"]:
> -            yield protocol, test_name
> +    for mode in ["sw", "hw", "lro"]:
> +        for protocol in ["ipv4", "ipv6", "ipip"]:
> +            for test_name in ["data", "ack", "flags", "tcp", "ip", "large"]:
> +                yield mode, protocol, test_name
>  
>  
>  @ksft_variants(_gro_variants())
> -def test(cfg, protocol, test_name):
> +def test(cfg, mode, protocol, test_name):
>      """Run a single GRO test with retries."""
>  
>      ipver = "6" if protocol[-1] == "6" else "4"
>      cfg.require_ipver(ipver)
>  
> -    _setup(cfg, test_name)
> +    _setup(cfg, mode, test_name)
>  
>      base_cmd_args = [
>          f"--{protocol}",
> -- 
> 2.51.1
> 

Re: [PATCH net-next 2/2] selftests: drv-net: gro: run the test against HW GRO and LRO

[Jakub Kicinski]

On Fri, 28 Nov 2025 15:42:40 -0500 Willem de Bruijn wrote:
> > +    elif mode == "lro":
> > +        _set_ethtool_feat(cfg.ifname, cfg.feat,
> > +                          {"generic-receive-offload": False,  
> 
> So GRO off disables HW_GRO, but not LRO? That difference is behavior
> is confusing. Could we still see this as a regression and make the
> ethtool HW_GRO feature equally independent from SW_GRO?

I couldn't convince myself that it's justified. Of course it would have
made testing a lot easier. But apart from that - what's your reading of
the status quo? Working backwards from were we ended up (and I
haven't dug into the git history) I'm guessing that LRO disable is used
to prevent changing geometry of the packets. GRO would presumably be
disabled when user knows that it will be ineffective, to save the cost.
Or when some portion of the stack (XDP?) can't deal with super frames.

If those are the reasons, practically, I don't see why user would want
HW GRO without SW. Ever since we allowed SW GRO to re-GRO HW GRO'ed
frames it's always better to leave SW enabled. HW leaves a lot of
aggregation opportunities on the table.

I concluded that changing the current behavior would not help any real
life scenario, just testing. LMK if you see one or the inconsistency
is a big enough reason.

Re: [PATCH net-next 2/2] selftests: drv-net: gro: run the test against HW GRO and LRO

[Willem de Bruijn]

Jakub Kicinski wrote:
> On Fri, 28 Nov 2025 15:42:40 -0500 Willem de Bruijn wrote:
> > > +    elif mode == "lro":
> > > +        _set_ethtool_feat(cfg.ifname, cfg.feat,
> > > +                          {"generic-receive-offload": False,  
> > 
> > So GRO off disables HW_GRO, but not LRO? That difference is behavior
> > is confusing. Could we still see this as a regression and make the
> > ethtool HW_GRO feature equally independent from SW_GRO?
> 
> I couldn't convince myself that it's justified. Of course it would have
> made testing a lot easier. But apart from that - what's your reading of
> the status quo? Working backwards from were we ended up (and I
> haven't dug into the git history) I'm guessing that LRO disable is used
> to prevent changing geometry of the packets. GRO would presumably be
> disabled when user knows that it will be ineffective, to save the cost.
> Or when some portion of the stack (XDP?) can't deal with super frames.
> 
> If those are the reasons, practically, I don't see why user would want
> HW GRO without SW. Ever since we allowed SW GRO to re-GRO HW GRO'ed
> frames it's always better to leave SW enabled. HW leaves a lot of
> aggregation opportunities on the table.
> 
> I concluded that changing the current behavior would not help any real
> life scenario, just testing. LMK if you see one or the inconsistency
> is a big enough reason.

I think that's fair.

But from reading the code I don't see how disabling NETIF_F_GRO also
disables NETIF_F_GRO_HW. And indeed I just tested on one (admittedly
not latest upstream) IDPF driver and it does not.

Also, the XDP limitation is perhaps vestigial and could go away, since
generic XDP appears to support XDP frags (AKA multibuffer XDP), as of
commit e6d5dbdd20aa ("xdp: add multi-buff support for xdp running in
generic mode").

Re: [PATCH net-next 2/2] selftests: drv-net: gro: run the test against HW GRO and LRO

[Jakub Kicinski]

On Sun, 30 Nov 2025 09:56:24 -0500 Willem de Bruijn wrote:
> Jakub Kicinski wrote:
> > On Fri, 28 Nov 2025 15:42:40 -0500 Willem de Bruijn wrote:  
> > > So GRO off disables HW_GRO, but not LRO? That difference is behavior
> > > is confusing. Could we still see this as a regression and make the
> > > ethtool HW_GRO feature equally independent from SW_GRO?  
> > 
> > I couldn't convince myself that it's justified. Of course it would have
> > made testing a lot easier. But apart from that - what's your reading of
> > the status quo? Working backwards from were we ended up (and I
> > haven't dug into the git history) I'm guessing that LRO disable is used
> > to prevent changing geometry of the packets. GRO would presumably be
> > disabled when user knows that it will be ineffective, to save the cost.
> > Or when some portion of the stack (XDP?) can't deal with super frames.
> > 
> > If those are the reasons, practically, I don't see why user would want
> > HW GRO without SW. Ever since we allowed SW GRO to re-GRO HW GRO'ed
> > frames it's always better to leave SW enabled. HW leaves a lot of
> > aggregation opportunities on the table.
> > 
> > I concluded that changing the current behavior would not help any real
> > life scenario, just testing. LMK if you see one or the inconsistency
> > is a big enough reason.  
> 
> I think that's fair.
> 
> But from reading the code I don't see how disabling NETIF_F_GRO also
> disables NETIF_F_GRO_HW. And indeed I just tested on one (admittedly
> not latest upstream) IDPF driver and it does not.

Looks like you're right. Broadcom drivers where GRO_HW originates do it
locally, so does qede. I guess somewhere along the way drives started
treating GRO_HW as a separate feature rather than a GRO offload. 

I don't think it changes the reasoning in any major way? 

Re: [PATCH net-next 2/2] selftests: drv-net: gro: run the test against HW GRO and LRO

[Willem de Bruijn]

Jakub Kicinski wrote:
> On Sun, 30 Nov 2025 09:56:24 -0500 Willem de Bruijn wrote:
> > Jakub Kicinski wrote:
> > > On Fri, 28 Nov 2025 15:42:40 -0500 Willem de Bruijn wrote:  
> > > > So GRO off disables HW_GRO, but not LRO? That difference is behavior
> > > > is confusing. Could we still see this as a regression and make the
> > > > ethtool HW_GRO feature equally independent from SW_GRO?  
> > > 
> > > I couldn't convince myself that it's justified. Of course it would have
> > > made testing a lot easier. But apart from that - what's your reading of
> > > the status quo? Working backwards from were we ended up (and I
> > > haven't dug into the git history) I'm guessing that LRO disable is used
> > > to prevent changing geometry of the packets. GRO would presumably be
> > > disabled when user knows that it will be ineffective, to save the cost.
> > > Or when some portion of the stack (XDP?) can't deal with super frames.
> > > 
> > > If those are the reasons, practically, I don't see why user would want
> > > HW GRO without SW. Ever since we allowed SW GRO to re-GRO HW GRO'ed
> > > frames it's always better to leave SW enabled. HW leaves a lot of
> > > aggregation opportunities on the table.
> > > 
> > > I concluded that changing the current behavior would not help any real
> > > life scenario, just testing. LMK if you see one or the inconsistency
> > > is a big enough reason.  
> > 
> > I think that's fair.
> > 
> > But from reading the code I don't see how disabling NETIF_F_GRO also
> > disables NETIF_F_GRO_HW. And indeed I just tested on one (admittedly
> > not latest upstream) IDPF driver and it does not.
> 
> Looks like you're right. Broadcom drivers where GRO_HW originates do it
> locally, so does qede. I guess somewhere along the way drives started
> treating GRO_HW as a separate feature rather than a GRO offload. 
> 
> I don't think it changes the reasoning in any major way? 

Agreed. If respinning, maybe change the wording a bit:

+        # a dummy XDP generic program. Disabling SW GRO as a feature
-+        # would also disable HW GRO.
++	  # may also disable HW GRO on some devices.

Re: [PATCH net-next 1/2] selftests: drv-net: gro: improve feature config

[Willem de Bruijn]

Jakub Kicinski wrote:
> We'll need to do a lot more feature handling to test HW-GRO and LRO.
> Clean up the feature handling for SW GRO a bit to let the next commit
> focus on the new test cases, only.
> 
> Make sure HW GRO-like features are not enabled for the SW tests.
> Be more careful about changing features as "nothing changed"
> situations may result in non-zero error code from ethtool.
> 
> Don't disable TSO on the local interface (receiver) when running over
> netdevsim, we just want GSO to break up the segments on the sender.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> CC: shuah@kernel.org
> CC: linux-kselftest@vger.kernel.org
> ---
>  tools/testing/selftests/drivers/net/gro.py | 38 ++++++++++++++++++++--
>  1 file changed, 35 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/drivers/net/gro.py b/tools/testing/selftests/drivers/net/gro.py
> index ba83713bf7b5..6d633bdc7e67 100755
> --- a/tools/testing/selftests/drivers/net/gro.py
> +++ b/tools/testing/selftests/drivers/net/gro.py
> @@ -20,7 +20,7 @@ coalescing behavior.
>  import os
>  from lib.py import ksft_run, ksft_exit, ksft_pr
>  from lib.py import NetDrvEpEnv, KsftXfailEx
> -from lib.py import cmd, defer, bkg, ip
> +from lib.py import cmd, defer, bkg, ethtool, ip

Is there a pattern behind this order. Since inserted rather than
appended. Intended to be alphabetical?

>  from lib.py import ksft_variants
>  
>  
> @@ -70,6 +70,27 @@ from lib.py import ksft_variants
>          defer(ip, f"link set dev {dev['ifname']} mtu {dev['mtu']}", host=host)
>  
>  
> +def _set_ethtool_feat(dev, current, feats, host=None):
> +    s2n = {True: "on", False: "off"}
> +
> +    new = ["-K", dev]
> +    old = ["-K", dev]
> +    no_change = True
> +    for name, state in feats.items():
> +        new += [name, s2n[state]]
> +        old += [name, s2n[not state]]

Should the change set not only include items for which
current != state?

Now old assumes not state, but that is not necessarily true?
> +
> +        if current[name]["active"] != state:
> +            no_change = False
> +            if current[name]["fixed"]:
> +                raise KsftXfailEx(f"Device does not support {name}")
> +    if no_change:
> +        return
> +
> +    ethtool(" ".join(new), host=host)
> +    defer(ethtool, " ".join(old), host=host)
> +
> +
>  def _setup(cfg, test_name):
>      """ Setup hardware loopback mode for GRO testing. """
>  
> @@ -77,6 +98,11 @@ from lib.py import ksft_variants
>          cfg.bin_local = cfg.test_dir / "gro"
>          cfg.bin_remote = cfg.remote.deploy(cfg.bin_local)
>  
> +    if not hasattr(cfg, "feat"):
> +        cfg.feat = ethtool(f"-k {cfg.ifname}", json=True)[0]
> +        cfg.remote_feat = ethtool(f"-k {cfg.remote_ifname}",
> +                                  host=cfg.remote, json=True)[0]
> +
>      # "large" test needs at least 4k MTU
>      if test_name == "large":
>          _set_mtu_restore(cfg.dev, 4096, None)
> @@ -88,15 +114,21 @@ from lib.py import ksft_variants
>      _write_defer_restore(cfg, flush_path, "200000", defer_undo=True)
>      _write_defer_restore(cfg, irq_path, "10", defer_undo=True)
>  
> +    _set_ethtool_feat(cfg.ifname, cfg.feat,
> +                      {"generic-receive-offload": True,
> +                       "rx-gro-hw": False,
> +                       "large-receive-offload": False})
> +
>      try:
>          # Disable TSO for local tests
>          cfg.require_nsim()  # will raise KsftXfailEx if not running on nsim
>  
> -        cmd(f"ethtool -K {cfg.ifname} gro on tso off")
> -        cmd(f"ethtool -K {cfg.remote_ifname} gro on tso off", host=cfg.remote)
> +        _set_ethtool_feat(cfg.remote_ifname, cfg.remote_feat, {"tso": False},
> +                          host=cfg.remote)
>      except KsftXfailEx:
>          pass
>  
> +
>  def _gro_variants():
>      """Generator that yields all combinations of protocol and test types."""
>  
> -- 
> 2.51.1
> 

Re: [PATCH net-next 1/2] selftests: drv-net: gro: improve feature config

[Jakub Kicinski]

On Fri, 28 Nov 2025 15:44:06 -0500 Willem de Bruijn wrote:
> >  import os
> >  from lib.py import ksft_run, ksft_exit, ksft_pr
> >  from lib.py import NetDrvEpEnv, KsftXfailEx
> > -from lib.py import cmd, defer, bkg, ip
> > +from lib.py import cmd, defer, bkg, ethtool, ip  
> 
> Is there a pattern behind this order. Since inserted rather than
> appended. Intended to be alphabetical?

"alphabetical if I remember"? :(

> >  from lib.py import ksft_variants
> >  
> >  
> > @@ -70,6 +70,27 @@ from lib.py import ksft_variants
> >          defer(ip, f"link set dev {dev['ifname']} mtu {dev['mtu']}", host=host)
> >  
> >  
> > +def _set_ethtool_feat(dev, current, feats, host=None):
> > +    s2n = {True: "on", False: "off"}
> > +
> > +    new = ["-K", dev]
> > +    old = ["-K", dev]
> > +    no_change = True
> > +    for name, state in feats.items():
> > +        new += [name, s2n[state]]
> > +        old += [name, s2n[not state]]  
> 
> Should the change set not only include items for which
> current != state?
> 
> Now old assumes not state, but that is not necessarily true?

Good catch