[PATCH net 0/2] fix wrong hds-thresh value setting

[PATCH net 0/2] fix wrong hds-thresh value setting

[Taehee Yoo]

A hds-thresh value is not set correctly if input value is 0.
The cause is that ethtool_ringparam_get_cfg(), which is a internal
function that returns ringparameters from both ->get_ringparam() and
dev->cfg can't return a correct hds-thresh value.

The first patch fixes ethtool_ringparam_get_cfg() to set hds-thresh
value correcltly.

The second patch adds random test for hds-thresh value.
So that we can test 0 value for a hds-thresh properly.

Taehee Yoo (2):
  net: ethtool: fix ethtool_ringparam_get_cfg() returns a hds_thresh
    value always as 0.
  selftests: drv-net: test random value for hds-thresh

 net/ethtool/common.c                       |  1 +
 tools/testing/selftests/drivers/net/hds.py | 28 +++++++++++++++++++++-
 2 files changed, 28 insertions(+), 1 deletion(-)

-- 
2.34.1

[PATCH net 1/2] net: ethtool: fix ethtool_ringparam_get_cfg() returns a hds_thresh value always as 0.

[Taehee Yoo]

When hds-thresh is configured, ethnl_set_rings() is called, and it calls
ethtool_ringparam_get_cfg() to get ringparameters from .get_ringparam()
callback and dev->cfg.
Both hds_config and hds_thresh values should be set from dev->cfg, not
from .get_ringparam().
But ethtool_ringparam_get_cfg() sets only hds_config from dev->cfg.
So, ethtool_ringparam_get_cfg() returns always a hds_thresh as 0.

If an input value of hds-thresh is 0, a hds_thresh value from
ethtool_ringparam_get_cfg() are same. So ethnl_set_rings() does
nothing and returns immediately.
It causes a bug that setting a hds-thresh value to 0 is not working.

Reproducer:
    modprobe netdevsim
    echo 1 > /sys/bus/netdevsim/new_device
    ethtool -G eth0 hds-thresh 100
    ethtool -G eth0 hds-thresh 0
    ethtool -g eth0
    #hds-thresh value should be 0, but it shows 100.

The tools/testing/selftests/drivers/net/hds.py can test it too with
applying a following patch for hds.py.

Fixes: 928459bbda19 ("net: ethtool: populate the default HDS params in the core")
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
---

The ethtool command doesn't support hds-thresh option yet.
So, RFC[1] patch is needed to be applied.

 net/ethtool/common.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/ethtool/common.c b/net/ethtool/common.c
index 0cb6da1f692a..49bea6b45bd5 100644
--- a/net/ethtool/common.c
+++ b/net/ethtool/common.c
@@ -830,6 +830,7 @@ void ethtool_ringparam_get_cfg(struct net_device *dev,
 
 	/* Driver gives us current state, we want to return current config */
 	kparam->tcp_data_split = dev->cfg->hds_config;
+	kparam->hds_thresh = dev->cfg->hds_thresh;
 }
 
 static void ethtool_init_tsinfo(struct kernel_ethtool_ts_info *info)
-- 
2.34.1

[PATCH net 2/2] selftests: drv-net: test random value for hds-thresh

[Taehee Yoo]

hds.py has been testing 0(set_hds_thresh_zero()),
MAX(set_hds_thresh_max()), GT(set_hds_thresh_gt()) values for hds-thresh.
However if a hds-thresh value was already 0, set_hds_thresh_zero()
can't test properly.
So, it tests random value first and then tests 0, MAX, GT values.

Testing bnxt:
    TAP version 13
    1..13
    ok 1 hds.get_hds
    ok 2 hds.get_hds_thresh
    ok 3 hds.set_hds_disable # SKIP disabling of HDS not supported by
    the device
    ok 4 hds.set_hds_enable
    ok 5 hds.set_hds_thresh_random
    ok 6 hds.set_hds_thresh_zero
    ok 7 hds.set_hds_thresh_max
    ok 8 hds.set_hds_thresh_gt
    ok 9 hds.set_xdp
    ok 10 hds.enabled_set_xdp
    ok 11 hds.ioctl
    ok 12 hds.ioctl_set_xdp
    ok 13 hds.ioctl_enabled_set_xdp
    # Totals: pass:12 fail:0 xfail:0 xpass:0 skip:1 error:0

Testing lo:
    TAP version 13
    1..13
    ok 1 hds.get_hds # SKIP tcp-data-split not supported by device
    ok 2 hds.get_hds_thresh # SKIP hds-thresh not supported by device
    ok 3 hds.set_hds_disable # SKIP ring-set not supported by the device
    ok 4 hds.set_hds_enable # SKIP ring-set not supported by the device
    ok 5 hds.set_hds_thresh_random # SKIP hds-thresh not supported by
    device
    ok 6 hds.set_hds_thresh_zero # SKIP ring-set not supported by the
    device
    ok 7 hds.set_hds_thresh_max # SKIP hds-thresh not supported by
    device
    ok 8 hds.set_hds_thresh_gt # SKIP hds-thresh not supported by device
    ok 9 hds.set_xdp # SKIP tcp-data-split not supported by device
    ok 10 hds.enabled_set_xdp # SKIP tcp-data-split not supported by
    device
    ok 11 hds.ioctl # SKIP tcp-data-split not supported by device
    ok 12 hds.ioctl_set_xdp # SKIP tcp-data-split not supported by
    device
    ok 13 hds.ioctl_enabled_set_xdp # SKIP tcp-data-split not supported
    by device
    # Totals: pass:0 fail:0 xfail:0 xpass:0 skip:13 error:0

Signed-off-by: Taehee Yoo <ap420073@gmail.com>
---
 tools/testing/selftests/drivers/net/hds.py | 28 +++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/drivers/net/hds.py b/tools/testing/selftests/drivers/net/hds.py
index 8b7f6acad15f..3ba8e4d69c4c 100755
--- a/tools/testing/selftests/drivers/net/hds.py
+++ b/tools/testing/selftests/drivers/net/hds.py
@@ -6,7 +6,7 @@ import os
 from lib.py import ksft_run, ksft_exit, ksft_eq, ksft_raises, KsftSkipEx
 from lib.py import CmdExitFailure, EthtoolFamily, NlError
 from lib.py import NetDrvEnv
-from lib.py import defer, ethtool, ip
+from lib.py import defer, ethtool, ip, random
 
 
 def _get_hds_mode(cfg, netnl) -> str:
@@ -109,6 +109,31 @@ def set_hds_thresh_zero(cfg, netnl) -> None:
 
     ksft_eq(0, rings['hds-thresh'])
 
+def set_hds_thresh_random(cfg, netnl) -> None:
+    try:
+        rings = netnl.rings_get({'header': {'dev-index': cfg.ifindex}})
+    except NlError as e:
+        raise KsftSkipEx('ring-get not supported by device')
+    if 'hds-thresh' not in rings:
+        raise KsftSkipEx('hds-thresh not supported by device')
+    if 'hds-thresh-max' not in rings:
+        raise KsftSkipEx('hds-thresh-max not defined by device')
+
+    while True:
+        hds_thresh = random.randint(1, rings['hds-thresh-max'])
+        if hds_thresh != rings['hds-thresh']:
+            break
+
+    try:
+        netnl.rings_set({'header': {'dev-index': cfg.ifindex}, 'hds-thresh': hds_thresh})
+    except NlError as e:
+        if e.error == errno.EINVAL:
+            raise KsftSkipEx("hds-thresh-set not supported by the device")
+        elif e.error == errno.EOPNOTSUPP:
+            raise KsftSkipEx("ring-set not supported by the device")
+    rings = netnl.rings_get({'header': {'dev-index': cfg.ifindex}})
+    ksft_eq(hds_thresh, rings['hds-thresh'])
+
 def set_hds_thresh_max(cfg, netnl) -> None:
     try:
         rings = netnl.rings_get({'header': {'dev-index': cfg.ifindex}})
@@ -243,6 +268,7 @@ def main() -> None:
                   get_hds_thresh,
                   set_hds_disable,
                   set_hds_enable,
+                  set_hds_thresh_random,
                   set_hds_thresh_zero,
                   set_hds_thresh_max,
                   set_hds_thresh_gt,
-- 
2.34.1

Re: [PATCH net 2/2] selftests: drv-net: test random value for hds-thresh

[Taehee Yoo]

On Thu, Apr 3, 2025 at 3:04 PM Taehee Yoo <ap420073@gmail.com> wrote:
>
> hds.py has been testing 0(set_hds_thresh_zero()),
> MAX(set_hds_thresh_max()), GT(set_hds_thresh_gt()) values for hds-thresh.
> However if a hds-thresh value was already 0, set_hds_thresh_zero()
> can't test properly.
> So, it tests random value first and then tests 0, MAX, GT values.
>
> Testing bnxt:
>     TAP version 13
>     1..13
>     ok 1 hds.get_hds
>     ok 2 hds.get_hds_thresh
>     ok 3 hds.set_hds_disable # SKIP disabling of HDS not supported by
>     the device
>     ok 4 hds.set_hds_enable
>     ok 5 hds.set_hds_thresh_random
>     ok 6 hds.set_hds_thresh_zero
>     ok 7 hds.set_hds_thresh_max
>     ok 8 hds.set_hds_thresh_gt
>     ok 9 hds.set_xdp
>     ok 10 hds.enabled_set_xdp
>     ok 11 hds.ioctl
>     ok 12 hds.ioctl_set_xdp
>     ok 13 hds.ioctl_enabled_set_xdp
>     # Totals: pass:12 fail:0 xfail:0 xpass:0 skip:1 error:0
>
> Testing lo:
>     TAP version 13
>     1..13
>     ok 1 hds.get_hds # SKIP tcp-data-split not supported by device
>     ok 2 hds.get_hds_thresh # SKIP hds-thresh not supported by device
>     ok 3 hds.set_hds_disable # SKIP ring-set not supported by the device
>     ok 4 hds.set_hds_enable # SKIP ring-set not supported by the device
>     ok 5 hds.set_hds_thresh_random # SKIP hds-thresh not supported by
>     device
>     ok 6 hds.set_hds_thresh_zero # SKIP ring-set not supported by the
>     device
>     ok 7 hds.set_hds_thresh_max # SKIP hds-thresh not supported by
>     device
>     ok 8 hds.set_hds_thresh_gt # SKIP hds-thresh not supported by device
>     ok 9 hds.set_xdp # SKIP tcp-data-split not supported by device
>     ok 10 hds.enabled_set_xdp # SKIP tcp-data-split not supported by
>     device
>     ok 11 hds.ioctl # SKIP tcp-data-split not supported by device
>     ok 12 hds.ioctl_set_xdp # SKIP tcp-data-split not supported by
>     device
>     ok 13 hds.ioctl_enabled_set_xdp # SKIP tcp-data-split not supported
>     by device
>     # Totals: pass:0 fail:0 xfail:0 xpass:0 skip:13 error:0
>
> Signed-off-by: Taehee Yoo <ap420073@gmail.com>
> ---
>  tools/testing/selftests/drivers/net/hds.py | 28 +++++++++++++++++++++-
>  1 file changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/drivers/net/hds.py b/tools/testing/selftests/drivers/net/hds.py
> index 8b7f6acad15f..3ba8e4d69c4c 100755
> --- a/tools/testing/selftests/drivers/net/hds.py
> +++ b/tools/testing/selftests/drivers/net/hds.py
> @@ -6,7 +6,7 @@ import os
>  from lib.py import ksft_run, ksft_exit, ksft_eq, ksft_raises, KsftSkipEx
>  from lib.py import CmdExitFailure, EthtoolFamily, NlError
>  from lib.py import NetDrvEnv
> -from lib.py import defer, ethtool, ip
> +from lib.py import defer, ethtool, ip, random
>
>
>  def _get_hds_mode(cfg, netnl) -> str:
> @@ -109,6 +109,31 @@ def set_hds_thresh_zero(cfg, netnl) -> None:
>
>      ksft_eq(0, rings['hds-thresh'])
>
> +def set_hds_thresh_random(cfg, netnl) -> None:
> +    try:
> +        rings = netnl.rings_get({'header': {'dev-index': cfg.ifindex}})
> +    except NlError as e:
> +        raise KsftSkipEx('ring-get not supported by device')
> +    if 'hds-thresh' not in rings:
> +        raise KsftSkipEx('hds-thresh not supported by device')
> +    if 'hds-thresh-max' not in rings:
> +        raise KsftSkipEx('hds-thresh-max not defined by device')
> +
> +    while True:
> +        hds_thresh = random.randint(1, rings['hds-thresh-max'])

If a hds-thresh-max value is too small, it will not work.
As far as I know, the gve's hds-thresh-max would be 0.

I will send a v2 patch to fix this.

Thanks!
Taehee Yoo

> +        if hds_thresh != rings['hds-thresh']:
> +            break
> +
> +    try:
> +        netnl.rings_set({'header': {'dev-index': cfg.ifindex}, 'hds-thresh': hds_thresh})
> +    except NlError as e:
> +        if e.error == errno.EINVAL:
> +            raise KsftSkipEx("hds-thresh-set not supported by the device")
> +        elif e.error == errno.EOPNOTSUPP:
> +            raise KsftSkipEx("ring-set not supported by the device")
> +    rings = netnl.rings_get({'header': {'dev-index': cfg.ifindex}})
> +    ksft_eq(hds_thresh, rings['hds-thresh'])
> +
>  def set_hds_thresh_max(cfg, netnl) -> None:
>      try:
>          rings = netnl.rings_get({'header': {'dev-index': cfg.ifindex}})
> @@ -243,6 +268,7 @@ def main() -> None:
>                    get_hds_thresh,
>                    set_hds_disable,
>                    set_hds_enable,
> +                  set_hds_thresh_random,
>                    set_hds_thresh_zero,
>                    set_hds_thresh_max,
>                    set_hds_thresh_gt,
> --
> 2.34.1
>