[PATCH net-next 0/2] netdevsim: add link speed support

[PATCH net-next 0/2] netdevsim: add link speed support

[Hangbin Liu]

Add link speed support for netdevsim, so users could test different‑speed
NICs. Currently, I am still using max speed 5000. Maybe we should increase
the max speed in the future.

The 2nd patch adds a simple selftest for the speed feature.

Signed-off-by: Hangbin Liu <liuhangbin@kylinos.cn>
---
Hangbin Liu (2):
      netdevsim: add link speed support
      selftests: netdevsim: add speed testing

 drivers/net/netdevsim/dev.c                        |  6 ----
 drivers/net/netdevsim/ethtool.c                    | 30 ++++++++++++++++++++
 drivers/net/netdevsim/netdevsim.h                  |  8 ++++++
 .../selftests/drivers/net/netdevsim/Makefile       |  1 +
 .../drivers/net/netdevsim/ethtool-speed.sh         | 33 ++++++++++++++++++++++
 5 files changed, 72 insertions(+), 6 deletions(-)
---
base-commit: 1bb784eb6e38fd73143f021608e4ef3095d0c0d7
change-id: 20260821-nsim_speed-d61e5c415b02

Best regards,
-- 
Hangbin Liu <liuhangbin@kylinos.cn>

[PATCH net-next 1/2] netdevsim: add link speed support

[Hangbin Liu]

From: Hangbin Liu <liuhangbin@kylinos.cn>

Add ethtool get/set_link_ksettings callbacks to netdevsim so the simulated
link speed and duplex can be queried and configured from userspace.

Move NSIM_LINK_SPEED_MAX and NSIM_LINK_SPEED_UNIT from dev.c to netdevsim.h
so they are available to both the devlink rate path and the new ethtool
code. The set callback rejects speeds exceeding NSIM_LINK_SPEED_MAX.

The default link speed is set to SPEED_5000 with DUPLEX_FULL, matching the
existing NSIM_LINK_SPEED_MAX definition.

Signed-off-by: Hangbin Liu <liuhangbin@kylinos.cn>
---
 drivers/net/netdevsim/dev.c       |  6 ------
 drivers/net/netdevsim/ethtool.c   | 30 ++++++++++++++++++++++++++++++
 drivers/net/netdevsim/netdevsim.h |  8 ++++++++
 3 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index feb88ccbbced..729ce80f6c83 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -1138,12 +1138,6 @@ nsim_dev_devlink_trap_policer_counter_get(struct devlink *devlink,
 	return 0;
 }
 
-#define NSIM_LINK_SPEED_MAX     5000 /* Mbps */
-#define NSIM_LINK_SPEED_UNIT    125000 /* 1 Mbps given in bytes/sec to avoid
-					* u64 overflow during conversion from
-					* bytes to bits.
-					*/
-
 static int nsim_rate_bytes_to_units(char *name, u64 *rate, struct netlink_ext_ack *extack)
 {
 	u64 val;
diff --git a/drivers/net/netdevsim/ethtool.c b/drivers/net/netdevsim/ethtool.c
index 025ea79879f3..59768bd7b620 100644
--- a/drivers/net/netdevsim/ethtool.c
+++ b/drivers/net/netdevsim/ethtool.c
@@ -205,6 +205,30 @@ static int nsim_get_ts_info(struct net_device *dev,
 	return 0;
 }
 
+static int nsim_get_link_ksettings(struct net_device *dev,
+				   struct ethtool_link_ksettings *cmd)
+{
+	struct netdevsim *ns = netdev_priv(dev);
+
+	cmd->base.speed		= ns->ethtool.speed;
+	cmd->base.duplex	= ns->ethtool.duplex;
+	cmd->base.port		= PORT_OTHER;
+	cmd->base.autoneg	= AUTONEG_DISABLE;
+	return 0;
+}
+
+static int nsim_set_link_ksettings(struct net_device *dev,
+				   const struct ethtool_link_ksettings *cmd)
+{
+	struct netdevsim *ns = netdev_priv(dev);
+
+	if (cmd->base.speed > NSIM_LINK_SPEED_MAX)
+		return -EINVAL;
+
+	return ethtool_virtdev_set_link_ksettings(dev, cmd, &ns->ethtool.speed,
+						  &ns->ethtool.duplex);
+}
+
 static const struct ethtool_ops nsim_ethtool_ops = {
 	.supported_coalesce_params	= ETHTOOL_COALESCE_ALL_PARAMS,
 	.supported_ring_params		= ETHTOOL_RING_USE_TCP_DATA_SPLIT |
@@ -223,6 +247,8 @@ static const struct ethtool_ops nsim_ethtool_ops = {
 	.set_fecparam			= nsim_set_fecparam,
 	.get_fec_stats			= nsim_get_fec_stats,
 	.get_ts_info			= nsim_get_ts_info,
+	.get_link_ksettings		= nsim_get_link_ksettings,
+	.set_link_ksettings		= nsim_set_link_ksettings,
 };
 
 static void nsim_ethtool_ring_init(struct netdevsim *ns)
@@ -250,12 +276,16 @@ void nsim_ethtool_init(struct netdevsim *ns)
 	ns->ethtool.fec.active_fec = ETHTOOL_FEC_NONE;
 
 	ns->ethtool.channels = ns->nsim_bus_dev->num_queues;
+	ns->ethtool.duplex = DUPLEX_FULL;
+	ns->ethtool.speed = SPEED_5000;
 
 	ethtool = debugfs_create_dir("ethtool", ns->nsim_dev_port->ddir);
 	ns->ethtool_ddir = ethtool;
 
 	debugfs_create_u32("get_err", 0600, ethtool, &ns->ethtool.get_err);
 	debugfs_create_u32("set_err", 0600, ethtool, &ns->ethtool.set_err);
+	debugfs_create_u32("speed", 0600, ethtool, &ns->ethtool.speed);
+	debugfs_create_u8("duplex", 0600, ethtool, &ns->ethtool.duplex);
 
 	dir = debugfs_create_dir("pause", ethtool);
 	debugfs_create_bool("report_stats_rx", 0600, dir,
diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
index 55aec41237b9..ce0d0dea00fb 100644
--- a/drivers/net/netdevsim/netdevsim.h
+++ b/drivers/net/netdevsim/netdevsim.h
@@ -39,6 +39,11 @@
 
 #define NSIM_HDS_THRESHOLD_MAX		1024
 
+#define NSIM_LINK_SPEED_MAX     5000 /* Mbps */
+#define NSIM_LINK_SPEED_UNIT    125000 /* 1 Mbps given in bytes/sec to avoid
+					* u64 overflow during conversion from
+					* bytes to bits.
+					*/
 struct nsim_sa {
 	struct xfrm_state *xs;
 	__be32 ipaddr[4];
@@ -95,6 +100,9 @@ struct nsim_ethtool {
 	struct ethtool_coalesce coalesce;
 	struct ethtool_ringparam ring;
 	struct ethtool_fecparam fec;
+
+	u32 speed;
+	u8 duplex;
 };
 
 struct nsim_rq {

-- 
2.55.0

[PATCH net-next 2/2] selftests: netdevsim: add speed testing

[Hangbin Liu]

From: Hangbin Liu <liuhangbin@kylinos.cn>

Add a script for netdevsim speed test. Make sure the speed and duplex
could be changed. And the speed can't be larger than the max speed.

Signed-off-by: Hangbin Liu <liuhangbin@kylinos.cn>
---
 .../selftests/drivers/net/netdevsim/Makefile       |  1 +
 .../drivers/net/netdevsim/ethtool-speed.sh         | 33 ++++++++++++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/tools/testing/selftests/drivers/net/netdevsim/Makefile b/tools/testing/selftests/drivers/net/netdevsim/Makefile
index 9808c2fbae9e..bd3d0f5ee07a 100644
--- a/tools/testing/selftests/drivers/net/netdevsim/Makefile
+++ b/tools/testing/selftests/drivers/net/netdevsim/Makefile
@@ -8,6 +8,7 @@ TEST_PROGS := \
 	ethtool-features.sh \
 	ethtool-fec.sh \
 	ethtool-pause.sh \
+	ethtool-speed.sh \
 	fib.sh \
 	fib_notifications.sh \
 	hw_stats_l3.sh \
diff --git a/tools/testing/selftests/drivers/net/netdevsim/ethtool-speed.sh b/tools/testing/selftests/drivers/net/netdevsim/ethtool-speed.sh
new file mode 100755
index 000000000000..c8aefa9c8a1f
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/netdevsim/ethtool-speed.sh
@@ -0,0 +1,33 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0-only
+
+source ethtool-common.sh
+
+NSIM_NETDEV=$(make_netdev)
+
+set -o pipefail
+
+s=$(ethtool --json "$NSIM_NETDEV" | jq '.[].speed')
+check $? "$s" "5000"
+
+s=$(ethtool --json "$NSIM_NETDEV" | jq -r '.[].duplex')
+check $? "$s" "Full"
+
+ethtool -s "$NSIM_NETDEV" speed 1000 duplex half
+
+s=$(ethtool --json "$NSIM_NETDEV" | jq '.[].speed')
+check $? "$s" "1000"
+
+s=$(ethtool --json "$NSIM_NETDEV" | jq -r '.[].duplex')
+check $? "$s" "Half"
+
+ethtool -s "$NSIM_NETDEV" speed 10000 2>/dev/null
+check $? "" "" 1
+
+if [ "$num_errors" -eq 0 ]; then
+    echo "PASSED all $((num_passes)) checks"
+    exit 0
+else
+    echo "FAILED $num_errors/$((num_errors+num_passes)) checks"
+    exit 1
+fi

-- 
2.55.0

Re: [PATCH net-next 1/2] netdevsim: add link speed support

[Andrew Lunn]

>  static void nsim_ethtool_ring_init(struct netdevsim *ns)
> @@ -250,12 +276,16 @@ void nsim_ethtool_init(struct netdevsim *ns)
>  	ns->ethtool.fec.active_fec = ETHTOOL_FEC_NONE;
>  
>  	ns->ethtool.channels = ns->nsim_bus_dev->num_queues;
> +	ns->ethtool.duplex = DUPLEX_FULL;
> +	ns->ethtool.speed = SPEED_5000;
>  
>  	ethtool = debugfs_create_dir("ethtool", ns->nsim_dev_port->ddir);
>  	ns->ethtool_ddir = ethtool;
>  
>  	debugfs_create_u32("get_err", 0600, ethtool, &ns->ethtool.get_err);
>  	debugfs_create_u32("set_err", 0600, ethtool, &ns->ethtool.set_err);
> +	debugfs_create_u32("speed", 0600, ethtool, &ns->ethtool.speed);
> +	debugfs_create_u8("duplex", 0600, ethtool, &ns->ethtool.duplex);

debugfs_create_bool() ?

	Andrew

Re: [PATCH net-next 1/2] netdevsim: add link speed support

[Andrew Lunn]

> +static int nsim_get_link_ksettings(struct net_device *dev,
> +				   struct ethtool_link_ksettings *cmd)
> +{
> +	struct netdevsim *ns = netdev_priv(dev);
> +
> +	cmd->base.speed		= ns->ethtool.speed;
> +	cmd->base.duplex	= ns->ethtool.duplex;
> +	cmd->base.port		= PORT_OTHER;
> +	cmd->base.autoneg	= AUTONEG_DISABLE;

Here you hard code autoneg as disabled.

> +static int nsim_set_link_ksettings(struct net_device *dev,
> +				   const struct ethtool_link_ksettings *cmd)
> +{
> +	struct netdevsim *ns = netdev_priv(dev);
> +
> +	if (cmd->base.speed > NSIM_LINK_SPEED_MAX)
> +		return -EINVAL;
> +
> +	return ethtool_virtdev_set_link_ksettings(dev, cmd, &ns->ethtool.speed,
> +						  &ns->ethtool.duplex);

but here you don't appear to block enabling autoneg. I would expect a
test and EOPNOTSUPP.

     Andrew

Re: [PATCH net-next 2/2] selftests: netdevsim: add speed testing

[Jakub Kicinski]

On Mon, 31 Aug 2026 11:28:11 +0800 Hangbin Liu wrote:
> Add a script for netdevsim speed test. Make sure the speed and duplex
> could be changed. And the speed can't be larger than the max speed.

nak, this is not a meaningful test

Re: [PATCH net-next 1/2] netdevsim: add link speed support

[Jakub Kicinski]

On Mon, 31 Aug 2026 11:28:10 +0800 Hangbin Liu wrote:
> Add ethtool get/set_link_ksettings callbacks to netdevsim so the simulated
> link speed and duplex can be queried and configured from userspace.
> 
> Move NSIM_LINK_SPEED_MAX and NSIM_LINK_SPEED_UNIT from dev.c to netdevsim.h
> so they are available to both the devlink rate path and the new ethtool
> code. The set callback rejects speeds exceeding NSIM_LINK_SPEED_MAX.
> 
> The default link speed is set to SPEED_5000 with DUPLEX_FULL, matching the
> existing NSIM_LINK_SPEED_MAX definition.

This breaks TDC which uses netdevsim for taprio testing.
You posted 2 series for net-next and both broke tests.
Please try harder to test stuff locally.
Matt added support for Docker to NIPA so you should be able
to repro all ksfts locally. If something doesn't work please 
report back.

Re: [PATCH net-next 1/2] netdevsim: add link speed support

[Hangbin Liu]

Hi Andrew,
On Mon, Aug 31, 2026 at 02:24:54PM +0200, Andrew Lunn wrote:
> > +static int nsim_get_link_ksettings(struct net_device *dev,
> > +				   struct ethtool_link_ksettings *cmd)
> > +{
> > +	struct netdevsim *ns = netdev_priv(dev);
> > +
> > +	cmd->base.speed		= ns->ethtool.speed;
> > +	cmd->base.duplex	= ns->ethtool.duplex;
> > +	cmd->base.port		= PORT_OTHER;
> > +	cmd->base.autoneg	= AUTONEG_DISABLE;
> 
> Here you hard code autoneg as disabled.
> 
> > +static int nsim_set_link_ksettings(struct net_device *dev,
> > +				   const struct ethtool_link_ksettings *cmd)
> > +{
> > +	struct netdevsim *ns = netdev_priv(dev);
> > +
> > +	if (cmd->base.speed > NSIM_LINK_SPEED_MAX)
> > +		return -EINVAL;
> > +
> > +	return ethtool_virtdev_set_link_ksettings(dev, cmd, &ns->ethtool.speed,
> > +						  &ns->ethtool.duplex);
> 
> but here you don't appear to block enabling autoneg. I would expect a
> test and EOPNOTSUPP.

ethtool_virtdev_set_link_ksettings will check the cmds and only allow user
to set speed and duplex. If a user tries to set autoneg it will return
-EINVAL. Do you want nsim to return -EOPNOTSUPP specifically?

Thanks
Hangbin

Re: [PATCH net-next 1/2] netdevsim: add link speed support

[Hangbin Liu]

On Mon, Aug 31, 2026 at 02:16:46PM +0200, Andrew Lunn wrote:
> >  static void nsim_ethtool_ring_init(struct netdevsim *ns)
> > @@ -250,12 +276,16 @@ void nsim_ethtool_init(struct netdevsim *ns)
> >  	ns->ethtool.fec.active_fec = ETHTOOL_FEC_NONE;
> >  
> >  	ns->ethtool.channels = ns->nsim_bus_dev->num_queues;
> > +	ns->ethtool.duplex = DUPLEX_FULL;
> > +	ns->ethtool.speed = SPEED_5000;
> >  
> >  	ethtool = debugfs_create_dir("ethtool", ns->nsim_dev_port->ddir);
> >  	ns->ethtool_ddir = ethtool;
> >  
> >  	debugfs_create_u32("get_err", 0600, ethtool, &ns->ethtool.get_err);
> >  	debugfs_create_u32("set_err", 0600, ethtool, &ns->ethtool.set_err);
> > +	debugfs_create_u32("speed", 0600, ethtool, &ns->ethtool.speed);
> > +	debugfs_create_u8("duplex", 0600, ethtool, &ns->ethtool.duplex);
> 
> debugfs_create_bool() ?

I'm not sure if anyone want to set nsim duplex to unknown for testing.
Do you think that we should disable this behavior?

Thanks
Hangbin

Re: [PATCH net-next 2/2] selftests: netdevsim: add speed testing

[Hangbin Liu]

On Mon, Aug 31, 2026 at 03:06:42PM -0700, Jakub Kicinski wrote:
> On Mon, 31 Aug 2026 11:28:11 +0800 Hangbin Liu wrote:
> > Add a script for netdevsim speed test. Make sure the speed and duplex
> > could be changed. And the speed can't be larger than the max speed.
> 
> nak, this is not a meaningful test

OK, I thought we should add selftests for new features. And there are
netdevsim ethtool tests like ethtool-pause.sh. Since you think this test
is meaningless. I will drop it in next version.

Hangbin

Re: [PATCH net-next 1/2] netdevsim: add link speed support

[Hangbin Liu]

On Mon, Aug 31, 2026 at 03:08:18PM -0700, Jakub Kicinski wrote:
> On Mon, 31 Aug 2026 11:28:10 +0800 Hangbin Liu wrote:
> > Add ethtool get/set_link_ksettings callbacks to netdevsim so the simulated
> > link speed and duplex can be queried and configured from userspace.
> > 
> > Move NSIM_LINK_SPEED_MAX and NSIM_LINK_SPEED_UNIT from dev.c to netdevsim.h
> > so they are available to both the devlink rate path and the new ethtool
> > code. The set callback rejects speeds exceeding NSIM_LINK_SPEED_MAX.
> > 
> > The default link speed is set to SPEED_5000 with DUPLEX_FULL, matching the
> > existing NSIM_LINK_SPEED_MAX definition.
> 
> This breaks TDC which uses netdevsim for taprio testing.

Sigh, I really didn't expect a speed feature could break the tc qdisc
testing... I will check the reason.

> You posted 2 series for net-next and both broke tests.
> Please try harder to test stuff locally.

Sorry for the extra work this caused you. You know I did not mean for this
to happen.

Every time I prepare a patch, I build it locally, run the relevant selftests,
and get an AI review before posting.

The selftest lib patch touches too many files. The fib_test failed locally
without producing any useful error output: it returns 1, but I cannot spot
the it by human viewing. The netdevsim test failed during tc testing, which
I did not expect at all.

> Matt added support for Docker to NIPA so you should be able
> to repro all ksfts locally. If something doesn't work please 
> report back.

I will give this a try. NIPA could help capture this return‑1 failure.

Regarding the tc‑related failures: do you run the full set of network
selftests for every single patch, or are there rules to select tests based
on the changes? As you know, not all companies have testing resources
comparable to RedHat. I have to run all these tests on my personal laptop.

Thanks
Hangbin

Re: [PATCH net-next 1/2] netdevsim: add link speed support

[Hangbin Liu]

On Tue, Sep 01, 2026 at 02:58:17PM +0800, Hangbin Liu wrote:
> On Mon, Aug 31, 2026 at 03:08:18PM -0700, Jakub Kicinski wrote:
> > On Mon, 31 Aug 2026 11:28:10 +0800 Hangbin Liu wrote:
> > > Add ethtool get/set_link_ksettings callbacks to netdevsim so the simulated
> > > link speed and duplex can be queried and configured from userspace.
> > > 
> > > Move NSIM_LINK_SPEED_MAX and NSIM_LINK_SPEED_UNIT from dev.c to netdevsim.h
> > > so they are available to both the devlink rate path and the new ethtool
> > > code. The set callback rejects speeds exceeding NSIM_LINK_SPEED_MAX.
> > > 
> > > The default link speed is set to SPEED_5000 with DUPLEX_FULL, matching the
> > > existing NSIM_LINK_SPEED_MAX definition.
> > 
> > This breaks TDC which uses netdevsim for taprio testing.
> 
> Sigh, I really didn't expect a speed feature could break the tc qdisc
> testing... I will check the reason.

OK, here is the reason. In taprio_set_picos_per_byte(), it init NIC speed to
SPEED_10. If the NIC doesn't support get_link_ksettings, the final calculated
picos_per_byte would be a extremely large number 800000. The later tc taprio
testing `tc qdisc ... sched-entry S 02 300` will always failed in
fill_sched_entry(), as min_duration is also a large number 48000.

After we supports get_link_ksettings for netdevsim, the speed is 5000 and
picos_per_byte will be init to 1600. The later min_duration checking in
fill_sched_entry() is 300 vs 96. So the tc qdisc command always return 0.

To fix the test, I think we should reduce the interval number. e.g.

diff --git a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/taprio.json b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/taprio.json
index cd19d05925e4..c3418bde9ad6 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/taprio.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/taprio.json
@@ -145,7 +145,7 @@
         "setup": [
             "echo \"1 1 8\" > /sys/bus/netdevsim/new_device"
         ],
-        "cmdUnderTest": "$TC qdisc add dev $ETH root handle 1: taprio num_tc 2 queues 1@0 1@1 sched-entry S 01 300 sched-entry S 02 1700 clockid CLOCK_TAI",
+        "cmdUnderTest": "$TC qdisc add dev $ETH root handle 1: taprio num_tc 2 queues 1@0 1@1 sched-entry S 01 30 sched-entry S 02 70 clockid CLOCK_TAI",
         "expExitCode": "2",
         "verifyCmd": "$TC qdisc show dev $ETH",
         "matchPattern": "qdisc taprio 1: root refcnt",

Thanks
Hangbin

Re: [PATCH net-next 1/2] netdevsim: add link speed support

[Andrew Lunn]

On Tue, Sep 01, 2026 at 02:39:32PM +0800, Hangbin Liu wrote:
> On Mon, Aug 31, 2026 at 02:16:46PM +0200, Andrew Lunn wrote:
> > >  static void nsim_ethtool_ring_init(struct netdevsim *ns)
> > > @@ -250,12 +276,16 @@ void nsim_ethtool_init(struct netdevsim *ns)
> > >  	ns->ethtool.fec.active_fec = ETHTOOL_FEC_NONE;
> > >  
> > >  	ns->ethtool.channels = ns->nsim_bus_dev->num_queues;
> > > +	ns->ethtool.duplex = DUPLEX_FULL;
> > > +	ns->ethtool.speed = SPEED_5000;
> > >  
> > >  	ethtool = debugfs_create_dir("ethtool", ns->nsim_dev_port->ddir);
> > >  	ns->ethtool_ddir = ethtool;
> > >  
> > >  	debugfs_create_u32("get_err", 0600, ethtool, &ns->ethtool.get_err);
> > >  	debugfs_create_u32("set_err", 0600, ethtool, &ns->ethtool.set_err);
> > > +	debugfs_create_u32("speed", 0600, ethtool, &ns->ethtool.speed);
> > > +	debugfs_create_u8("duplex", 0600, ethtool, &ns->ethtool.duplex);
> > 
> > debugfs_create_bool() ?
> 
> I'm not sure if anyone want to set nsim duplex to unknown for testing.
> Do you think that we should disable this behavior?

Ah, i did not think of DUPLEX_UNKNOWN.

We need to consider Jakubs reply, what are you actually testing here,
do such tests make any sense?

If they do, i think some validation would be good here. We already
have:

static inline int ethtool_validate_duplex(__u8 duplex)
{
	switch (duplex) {
	case DUPLEX_HALF:
	case DUPLEX_FULL:
	case DUPLEX_UNKNOWN:
		return 1;
	}

	return 0;
}

We don't expect a driver to set duplex to 42, so why should the user
be allowed to do that?

Also, SPEED_UNKNOWN is -1, so you need debugfs_create_s32().

	Andrew

Re: [PATCH net-next 1/2] netdevsim: add link speed support

[Andrew Lunn]

> Regarding the tc‑related failures: do you run the full set of network
> selftests for every single patch, or are there rules to select tests based
> on the changes?

Not every single patch. Patchsets get collected into a bundle, and
then all tests are run on a bundle.

     Andrew

Re: [PATCH net-next 1/2] netdevsim: add link speed support

[Andrew Lunn]

On Tue, Sep 01, 2026 at 04:23:51PM +0800, Hangbin Liu wrote:
> On Tue, Sep 01, 2026 at 02:58:17PM +0800, Hangbin Liu wrote:
> > On Mon, Aug 31, 2026 at 03:08:18PM -0700, Jakub Kicinski wrote:
> > > On Mon, 31 Aug 2026 11:28:10 +0800 Hangbin Liu wrote:
> > > > Add ethtool get/set_link_ksettings callbacks to netdevsim so the simulated
> > > > link speed and duplex can be queried and configured from userspace.
> > > > 
> > > > Move NSIM_LINK_SPEED_MAX and NSIM_LINK_SPEED_UNIT from dev.c to netdevsim.h
> > > > so they are available to both the devlink rate path and the new ethtool
> > > > code. The set callback rejects speeds exceeding NSIM_LINK_SPEED_MAX.
> > > > 
> > > > The default link speed is set to SPEED_5000 with DUPLEX_FULL, matching the
> > > > existing NSIM_LINK_SPEED_MAX definition.
> > > 
> > > This breaks TDC which uses netdevsim for taprio testing.
> > 
> > Sigh, I really didn't expect a speed feature could break the tc qdisc
> > testing... I will check the reason.
> 
> OK, here is the reason. In taprio_set_picos_per_byte(), it init NIC speed to
> SPEED_10. If the NIC doesn't support get_link_ksettings, the final calculated
> picos_per_byte would be a extremely large number 800000. The later tc taprio
> testing `tc qdisc ... sched-entry S 02 300` will always failed in
> fill_sched_entry(), as min_duration is also a large number 48000.
> 
> After we supports get_link_ksettings for netdevsim, the speed is 5000 and
> picos_per_byte will be init to 1600. The later min_duration checking in
> fill_sched_entry() is 300 vs 96. So the tc qdisc command always return 0.
> 
> To fix the test, I think we should reduce the interval number. e.g.

Why not just set the initial speed to SPEED_10, so nothing changes?

    Andrew

Re: [PATCH net-next 2/2] selftests: netdevsim: add speed testing

[Jakub Kicinski]

On Tue, 1 Sep 2026 14:42:06 +0800 Hangbin Liu wrote:
> On Mon, Aug 31, 2026 at 03:06:42PM -0700, Jakub Kicinski wrote:
> > On Mon, 31 Aug 2026 11:28:11 +0800 Hangbin Liu wrote:  
> > > Add a script for netdevsim speed test. Make sure the speed and duplex
> > > could be changed. And the speed can't be larger than the max speed.  
> > 
> > nak, this is not a meaningful test  
> 
> OK, I thought we should add selftests for new features. And there are
> netdevsim ethtool tests like ethtool-pause.sh. Since you think this test
> is meaningless. I will drop it in next version.

to be clear - the entire patch set, not just the test.

Re: [PATCH net-next 1/2] netdevsim: add link speed support

[Hangbin Liu]

On Tue, Sep 01, 2026 at 02:43:39PM +0200, Andrew Lunn wrote:
> On Tue, Sep 01, 2026 at 02:39:32PM +0800, Hangbin Liu wrote:
> > On Mon, Aug 31, 2026 at 02:16:46PM +0200, Andrew Lunn wrote:
> > > >  static void nsim_ethtool_ring_init(struct netdevsim *ns)
> > > > @@ -250,12 +276,16 @@ void nsim_ethtool_init(struct netdevsim *ns)
> > > >  	ns->ethtool.fec.active_fec = ETHTOOL_FEC_NONE;
> > > >  
> > > >  	ns->ethtool.channels = ns->nsim_bus_dev->num_queues;
> > > > +	ns->ethtool.duplex = DUPLEX_FULL;
> > > > +	ns->ethtool.speed = SPEED_5000;
> > > >  
> > > >  	ethtool = debugfs_create_dir("ethtool", ns->nsim_dev_port->ddir);
> > > >  	ns->ethtool_ddir = ethtool;
> > > >  
> > > >  	debugfs_create_u32("get_err", 0600, ethtool, &ns->ethtool.get_err);
> > > >  	debugfs_create_u32("set_err", 0600, ethtool, &ns->ethtool.set_err);
> > > > +	debugfs_create_u32("speed", 0600, ethtool, &ns->ethtool.speed);
> > > > +	debugfs_create_u8("duplex", 0600, ethtool, &ns->ethtool.duplex);
> > > 
> > > debugfs_create_bool() ?
> > 
> > I'm not sure if anyone want to set nsim duplex to unknown for testing.
> > Do you think that we should disable this behavior?
> 
> Ah, i did not think of DUPLEX_UNKNOWN.
> 
> We need to consider Jakubs reply, what are you actually testing here,
> do such tests make any sense?

I'm fixing a bug in bonding that need to test with different speed slaves.
So I add this feature for netdevsim.
> 
> If they do, i think some validation would be good here. We already
> have:
> 
> static inline int ethtool_validate_duplex(__u8 duplex)
> {
> 	switch (duplex) {
> 	case DUPLEX_HALF:
> 	case DUPLEX_FULL:
> 	case DUPLEX_UNKNOWN:
> 		return 1;
> 	}
> 
> 	return 0;
> }
> 
> We don't expect a driver to set duplex to 42, so why should the user
> be allowed to do that?

The user interface (ethtool) setting is already handled by
ethtool_virtdev_set_link_ksettings(). Here is the debugfs issue.
Since no one is asking for setting DUPLEX_UNKNOWN, I can use
debugfs_create_bool first.

> 
> Also, SPEED_UNKNOWN is -1, so you need debugfs_create_s32().

Ah, yes. Thanks

Hangbin

Re: [PATCH net-next 1/2] netdevsim: add link speed support

[Hangbin Liu]

On Tue, Sep 01, 2026 at 04:21:04PM +0200, Andrew Lunn wrote:
> > OK, here is the reason. In taprio_set_picos_per_byte(), it init NIC speed to
> > SPEED_10. If the NIC doesn't support get_link_ksettings, the final calculated
> > picos_per_byte would be a extremely large number 800000. The later tc taprio
> > testing `tc qdisc ... sched-entry S 02 300` will always failed in
> > fill_sched_entry(), as min_duration is also a large number 48000.
> > 
> > After we supports get_link_ksettings for netdevsim, the speed is 5000 and
> > picos_per_byte will be init to 1600. The later min_duration checking in
> > fill_sched_entry() is 300 vs 96. So the tc qdisc command always return 0.
> > 
> > To fix the test, I think we should reduce the interval number. e.g.
> 
> Why not just set the initial speed to SPEED_10, so nothing changes?
> 

`nsim_leaf_tx_share_set` and `nsim_leaf_tx_max_set` compare against
`NSIM_LINK_SPEED_MAX` for TX‑rate setting. Using `SPEED_10` as the value
appears too small. I am concerned this could break other existing tests.

Hangbin

Re: [PATCH net-next 2/2] selftests: netdevsim: add speed testing

[Hangbin Liu]

On Tue, Sep 01, 2026 at 08:06:41AM -0700, Jakub Kicinski wrote:
> On Tue, 1 Sep 2026 14:42:06 +0800 Hangbin Liu wrote:
> > On Mon, Aug 31, 2026 at 03:06:42PM -0700, Jakub Kicinski wrote:
> > > On Mon, 31 Aug 2026 11:28:11 +0800 Hangbin Liu wrote:  
> > > > Add a script for netdevsim speed test. Make sure the speed and duplex
> > > > could be changed. And the speed can't be larger than the max speed.  
> > > 
> > > nak, this is not a meaningful test  
> > 
> > OK, I thought we should add selftests for new features. And there are
> > netdevsim ethtool tests like ethtool-pause.sh. Since you think this test
> > is meaningless. I will drop it in next version.
> 
> to be clear - the entire patch set, not just the test.

OK... Thank you for the clarification. I will drop this patch set.

Regards
Hangbin

Re: [PATCH net-next 1/2] netdevsim: add link speed support

[Andrew Lunn]

On Wed, Sep 02, 2026 at 09:53:39AM +0800, Hangbin Liu wrote:
> On Tue, Sep 01, 2026 at 02:43:39PM +0200, Andrew Lunn wrote:
> > On Tue, Sep 01, 2026 at 02:39:32PM +0800, Hangbin Liu wrote:
> > > On Mon, Aug 31, 2026 at 02:16:46PM +0200, Andrew Lunn wrote:
> > > > >  static void nsim_ethtool_ring_init(struct netdevsim *ns)
> > > > > @@ -250,12 +276,16 @@ void nsim_ethtool_init(struct netdevsim *ns)
> > > > >  	ns->ethtool.fec.active_fec = ETHTOOL_FEC_NONE;
> > > > >  
> > > > >  	ns->ethtool.channels = ns->nsim_bus_dev->num_queues;
> > > > > +	ns->ethtool.duplex = DUPLEX_FULL;
> > > > > +	ns->ethtool.speed = SPEED_5000;
> > > > >  
> > > > >  	ethtool = debugfs_create_dir("ethtool", ns->nsim_dev_port->ddir);
> > > > >  	ns->ethtool_ddir = ethtool;
> > > > >  
> > > > >  	debugfs_create_u32("get_err", 0600, ethtool, &ns->ethtool.get_err);
> > > > >  	debugfs_create_u32("set_err", 0600, ethtool, &ns->ethtool.set_err);
> > > > > +	debugfs_create_u32("speed", 0600, ethtool, &ns->ethtool.speed);
> > > > > +	debugfs_create_u8("duplex", 0600, ethtool, &ns->ethtool.duplex);
> > > > 
> > > > debugfs_create_bool() ?
> > > 
> > > I'm not sure if anyone want to set nsim duplex to unknown for testing.
> > > Do you think that we should disable this behavior?
> > 
> > Ah, i did not think of DUPLEX_UNKNOWN.
> > 
> > We need to consider Jakubs reply, what are you actually testing here,
> > do such tests make any sense?
> 
> I'm fixing a bug in bonding that need to test with different speed slaves.
> So I add this feature for netdevsim.

That would of been useful to put into the commit message, after the

---.

Better still, create a self test for the bonding bug you have found,
show that it fails. Then have the fix, and show that the self test
then passes. That makes it a lot clearer why this change is useful,
plus it gives us a regression test to ensure bonding does not break
again.

	Andrew

Re: [PATCH net-next 1/2] netdevsim: add link speed support

[Andrew Lunn]

On Wed, Sep 02, 2026 at 10:01:00AM +0800, Hangbin Liu wrote:
> On Tue, Sep 01, 2026 at 04:21:04PM +0200, Andrew Lunn wrote:
> > > OK, here is the reason. In taprio_set_picos_per_byte(), it init NIC speed to
> > > SPEED_10. If the NIC doesn't support get_link_ksettings, the final calculated
> > > picos_per_byte would be a extremely large number 800000. The later tc taprio
> > > testing `tc qdisc ... sched-entry S 02 300` will always failed in
> > > fill_sched_entry(), as min_duration is also a large number 48000.
> > > 
> > > After we supports get_link_ksettings for netdevsim, the speed is 5000 and
> > > picos_per_byte will be init to 1600. The later min_duration checking in
> > > fill_sched_entry() is 300 vs 96. So the tc qdisc command always return 0.
> > > 
> > > To fix the test, I think we should reduce the interval number. e.g.
> > 
> > Why not just set the initial speed to SPEED_10, so nothing changes?
> > 
> 
> `nsim_leaf_tx_share_set` and `nsim_leaf_tx_max_set` compare against
> `NSIM_LINK_SPEED_MAX` for TX‑rate setting. Using `SPEED_10` as the value
> appears too small. I am concerned this could break other existing tests.

Well, 10MBps has been around since 1983. I've not counted, but if you
look at all the devices which Linux supports, i expect it is the most
popular speed.

I suggest you give it a try, and run all the tests. See what
happens. But to me, it is a safe choice.

	 Andrew