* [PATCH net-next v2 1/8] netlink: specs: add the multicast group name to spec
2025-06-23 23:17 [PATCH net-next v2 0/8] net: ethtool: rss: add notifications Jakub Kicinski
@ 2025-06-23 23:17 ` Jakub Kicinski
2025-06-23 23:17 ` [PATCH net-next v2 2/8] net: ethtool: dynamically allocate full req size req Jakub Kicinski
` (8 subsequent siblings)
9 siblings, 0 replies; 20+ messages in thread
From: Jakub Kicinski @ 2025-06-23 23:17 UTC (permalink / raw)
To: davem
Cc: netdev, edumazet, pabeni, andrew+netdev, horms, donald.hunter,
maxime.chevallier, sdf, jdamato, ecree.xilinx, Jakub Kicinski
Add the multicast group's name to the YAML spec.
Without it YNL doesn't know how to subscribe to notifications.
Reviewed-by: Donald Hunter <donald.hunter@gmail.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
Documentation/netlink/specs/ethtool.yaml | 6 ++++++
include/uapi/linux/ethtool_netlink.h | 2 --
include/uapi/linux/ethtool_netlink_generated.h | 2 ++
3 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/Documentation/netlink/specs/ethtool.yaml b/Documentation/netlink/specs/ethtool.yaml
index c1651e175e8b..cfe84f84ba29 100644
--- a/Documentation/netlink/specs/ethtool.yaml
+++ b/Documentation/netlink/specs/ethtool.yaml
@@ -2492,3 +2492,9 @@ c-version-name: ethtool-genl-version
attributes:
- header
- events
+
+mcast-groups:
+ list:
+ -
+ name: monitor
+ c-define-name: ethtool-mcgrp-monitor-name
diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
index 09a75bdb6560..fa5d645140a4 100644
--- a/include/uapi/linux/ethtool_netlink.h
+++ b/include/uapi/linux/ethtool_netlink.h
@@ -208,6 +208,4 @@ enum {
ETHTOOL_A_STATS_PHY_MAX = (__ETHTOOL_A_STATS_PHY_CNT - 1)
};
-#define ETHTOOL_MCGRP_MONITOR_NAME "monitor"
-
#endif /* _UAPI_LINUX_ETHTOOL_NETLINK_H_ */
diff --git a/include/uapi/linux/ethtool_netlink_generated.h b/include/uapi/linux/ethtool_netlink_generated.h
index 4944badf9fba..859e28c8a91a 100644
--- a/include/uapi/linux/ethtool_netlink_generated.h
+++ b/include/uapi/linux/ethtool_netlink_generated.h
@@ -867,4 +867,6 @@ enum {
ETHTOOL_MSG_KERNEL_MAX = (__ETHTOOL_MSG_KERNEL_CNT - 1)
};
+#define ETHTOOL_MCGRP_MONITOR_NAME "monitor"
+
#endif /* _UAPI_LINUX_ETHTOOL_NETLINK_GENERATED_H */
--
2.49.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH net-next v2 2/8] net: ethtool: dynamically allocate full req size req
2025-06-23 23:17 [PATCH net-next v2 0/8] net: ethtool: rss: add notifications Jakub Kicinski
2025-06-23 23:17 ` [PATCH net-next v2 1/8] netlink: specs: add the multicast group name to spec Jakub Kicinski
@ 2025-06-23 23:17 ` Jakub Kicinski
2025-06-24 5:57 ` Maxime Chevallier
2025-06-23 23:17 ` [PATCH net-next v2 3/8] net: ethtool: call .parse_request for SET handlers Jakub Kicinski
` (7 subsequent siblings)
9 siblings, 1 reply; 20+ messages in thread
From: Jakub Kicinski @ 2025-06-23 23:17 UTC (permalink / raw)
To: davem
Cc: netdev, edumazet, pabeni, andrew+netdev, horms, donald.hunter,
maxime.chevallier, sdf, jdamato, ecree.xilinx, Jakub Kicinski
In preparation for using req_info to carry parameters between
SET and NTF allocate a full request info struct. Since the size
depends on the subcommand we need to allocate it on the heap.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
v2:
- s/into/info/
---
net/ethtool/netlink.c | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index 9de828df46cd..a9467b96f00c 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -863,8 +863,8 @@ static int ethnl_default_done(struct netlink_callback *cb)
static int ethnl_default_set_doit(struct sk_buff *skb, struct genl_info *info)
{
const struct ethnl_request_ops *ops;
- struct ethnl_req_info req_info = {};
const u8 cmd = info->genlhdr->cmd;
+ struct ethnl_req_info *req_info;
struct net_device *dev;
int ret;
@@ -874,20 +874,24 @@ static int ethnl_default_set_doit(struct sk_buff *skb, struct genl_info *info)
if (GENL_REQ_ATTR_CHECK(info, ops->hdr_attr))
return -EINVAL;
- ret = ethnl_parse_header_dev_get(&req_info, info->attrs[ops->hdr_attr],
+ req_info = kzalloc(ops->req_info_size, GFP_KERNEL);
+ if (!req_info)
+ return -ENOMEM;
+
+ ret = ethnl_parse_header_dev_get(req_info, info->attrs[ops->hdr_attr],
genl_info_net(info), info->extack,
true);
if (ret < 0)
- return ret;
+ goto out_free_req;
if (ops->set_validate) {
- ret = ops->set_validate(&req_info, info);
+ ret = ops->set_validate(req_info, info);
/* 0 means nothing to do */
if (ret <= 0)
goto out_dev;
}
- dev = req_info.dev;
+ dev = req_info->dev;
rtnl_lock();
netdev_lock_ops(dev);
@@ -902,7 +906,7 @@ static int ethnl_default_set_doit(struct sk_buff *skb, struct genl_info *info)
if (ret < 0)
goto out_free_cfg;
- ret = ops->set(&req_info, info);
+ ret = ops->set(req_info, info);
if (ret < 0)
goto out_ops;
@@ -921,7 +925,9 @@ static int ethnl_default_set_doit(struct sk_buff *skb, struct genl_info *info)
netdev_unlock_ops(dev);
rtnl_unlock();
out_dev:
- ethnl_parse_header_dev_put(&req_info);
+ ethnl_parse_header_dev_put(req_info);
+out_free_req:
+ kfree(req_info);
return ret;
}
--
2.49.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH net-next v2 2/8] net: ethtool: dynamically allocate full req size req
2025-06-23 23:17 ` [PATCH net-next v2 2/8] net: ethtool: dynamically allocate full req size req Jakub Kicinski
@ 2025-06-24 5:57 ` Maxime Chevallier
0 siblings, 0 replies; 20+ messages in thread
From: Maxime Chevallier @ 2025-06-24 5:57 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms,
donald.hunter, sdf, jdamato, ecree.xilinx
On Mon, 23 Jun 2025 16:17:14 -0700
Jakub Kicinski <kuba@kernel.org> wrote:
> In preparation for using req_info to carry parameters between
> SET and NTF allocate a full request info struct. Since the size
> depends on the subcommand we need to allocate it on the heap.
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Tested-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Maxime
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH net-next v2 3/8] net: ethtool: call .parse_request for SET handlers
2025-06-23 23:17 [PATCH net-next v2 0/8] net: ethtool: rss: add notifications Jakub Kicinski
2025-06-23 23:17 ` [PATCH net-next v2 1/8] netlink: specs: add the multicast group name to spec Jakub Kicinski
2025-06-23 23:17 ` [PATCH net-next v2 2/8] net: ethtool: dynamically allocate full req size req Jakub Kicinski
@ 2025-06-23 23:17 ` Jakub Kicinski
2025-06-24 5:57 ` Maxime Chevallier
2025-06-23 23:17 ` [PATCH net-next v2 4/8] net: ethtool: remove the data argument from ethtool_notify() Jakub Kicinski
` (6 subsequent siblings)
9 siblings, 1 reply; 20+ messages in thread
From: Jakub Kicinski @ 2025-06-23 23:17 UTC (permalink / raw)
To: davem
Cc: netdev, edumazet, pabeni, andrew+netdev, horms, donald.hunter,
maxime.chevallier, sdf, jdamato, ecree.xilinx, Jakub Kicinski
In preparation for using req_info to carry parameters between SET
and NTF - call .parse_request during ethnl_default_set_doit().
The main question here is whether .parse_request is intended to be
GET-specific. Originally the SET handling was delegated to each subcommand
directly - ethnl_default_set_doit() and .set callbacks in ethnl_request_ops
did not exist. Looking at existing users does not shed much light, all
of the following subcommands use .parse_request but have no SET handler
(and no NTF):
net/ethtool/eeprom.c
net/ethtool/rss.c
net/ethtool/stats.c
net/ethtool/strset.c
net/ethtool/tsinfo.c
There's only one which does have a SET:
net/ethtool/pause.c
where .parse_request handling is used to select which statistics to query.
Not relevant for SET but also harmless.
Going back to RSS (which doesn't have SET today) .parse_request parses
the rss_context ID. Using the req_info struct to pass the context ID
from SET to NTF will be very useful.
Switch to ethnl_default_parse(), effectively adding the .parse_request
for SET handlers.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
net/ethtool/netlink.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index a9467b96f00c..c5ec3c82ab2e 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -878,9 +878,7 @@ static int ethnl_default_set_doit(struct sk_buff *skb, struct genl_info *info)
if (!req_info)
return -ENOMEM;
- ret = ethnl_parse_header_dev_get(req_info, info->attrs[ops->hdr_attr],
- genl_info_net(info), info->extack,
- true);
+ ret = ethnl_default_parse(req_info, info, ops, true);
if (ret < 0)
goto out_free_req;
--
2.49.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH net-next v2 3/8] net: ethtool: call .parse_request for SET handlers
2025-06-23 23:17 ` [PATCH net-next v2 3/8] net: ethtool: call .parse_request for SET handlers Jakub Kicinski
@ 2025-06-24 5:57 ` Maxime Chevallier
0 siblings, 0 replies; 20+ messages in thread
From: Maxime Chevallier @ 2025-06-24 5:57 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms,
donald.hunter, sdf, jdamato, ecree.xilinx
On Mon, 23 Jun 2025 16:17:15 -0700
Jakub Kicinski <kuba@kernel.org> wrote:
> In preparation for using req_info to carry parameters between SET
> and NTF - call .parse_request during ethnl_default_set_doit().
>
> The main question here is whether .parse_request is intended to be
> GET-specific. Originally the SET handling was delegated to each subcommand
> directly - ethnl_default_set_doit() and .set callbacks in ethnl_request_ops
> did not exist. Looking at existing users does not shed much light, all
> of the following subcommands use .parse_request but have no SET handler
> (and no NTF):
>
> net/ethtool/eeprom.c
> net/ethtool/rss.c
> net/ethtool/stats.c
> net/ethtool/strset.c
> net/ethtool/tsinfo.c
>
> There's only one which does have a SET:
>
> net/ethtool/pause.c
>
> where .parse_request handling is used to select which statistics to query.
> Not relevant for SET but also harmless.
>
> Going back to RSS (which doesn't have SET today) .parse_request parses
> the rss_context ID. Using the req_info struct to pass the context ID
> from SET to NTF will be very useful.
>
> Switch to ethnl_default_parse(), effectively adding the .parse_request
> for SET handlers.
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Tested-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Maxime
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH net-next v2 4/8] net: ethtool: remove the data argument from ethtool_notify()
2025-06-23 23:17 [PATCH net-next v2 0/8] net: ethtool: rss: add notifications Jakub Kicinski
` (2 preceding siblings ...)
2025-06-23 23:17 ` [PATCH net-next v2 3/8] net: ethtool: call .parse_request for SET handlers Jakub Kicinski
@ 2025-06-23 23:17 ` Jakub Kicinski
2025-06-24 5:56 ` Maxime Chevallier
2025-06-23 23:17 ` [PATCH net-next v2 5/8] net: ethtool: copy req_info from SET to NTF Jakub Kicinski
` (5 subsequent siblings)
9 siblings, 1 reply; 20+ messages in thread
From: Jakub Kicinski @ 2025-06-23 23:17 UTC (permalink / raw)
To: davem
Cc: netdev, edumazet, pabeni, andrew+netdev, horms, donald.hunter,
maxime.chevallier, sdf, jdamato, ecree.xilinx, Jakub Kicinski
ethtool_notify() takes a const void *data argument, which presumably
was intended to pass information from the call site to the subcommand
handler. This argument currently has no users.
Expecting the data to be subcommand-specific has two complications.
Complication #1 is that its not plumbed thru any of the standardized
callbacks. It gets propagated to ethnl_default_notify() where it
remains unused. Coming from the ethnl_default_set_doit() side we pass
in NULL, because how could we have a command specific attribute in
a generic handler.
Complication #2 is that we expect the ethtool_notify() callers to
know what attribute type to pass in. Again, the data pointer is
untyped.
RSS will need to pass the context ID to the notifications.
I think it's a better design if the "subcommand" exports its own
typed interface and constructs the appropriate argument struct
(which will be req_info). Remove the unused data argument from
ethtool_notify() but retain it in a new internal helper which
subcommands can use to build a typed interface.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
include/linux/netdevice.h | 5 ++---
net/ethtool/netlink.h | 1 +
net/ethtool/ioctl.c | 24 ++++++++++++------------
net/ethtool/netlink.c | 11 ++++++++---
4 files changed, 23 insertions(+), 18 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 03c26bb0fbbe..db5bfd4e7ec8 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -5138,10 +5138,9 @@ void netdev_bonding_info_change(struct net_device *dev,
struct netdev_bonding_info *bonding_info);
#if IS_ENABLED(CONFIG_ETHTOOL_NETLINK)
-void ethtool_notify(struct net_device *dev, unsigned int cmd, const void *data);
+void ethtool_notify(struct net_device *dev, unsigned int cmd);
#else
-static inline void ethtool_notify(struct net_device *dev, unsigned int cmd,
- const void *data)
+static inline void ethtool_notify(struct net_device *dev, unsigned int cmd)
{
}
#endif
diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
index 91b953924af3..4a061944a3aa 100644
--- a/net/ethtool/netlink.h
+++ b/net/ethtool/netlink.h
@@ -23,6 +23,7 @@ void *ethnl_dump_put(struct sk_buff *skb, struct netlink_callback *cb, u8 cmd);
void *ethnl_bcastmsg_put(struct sk_buff *skb, u8 cmd);
void *ethnl_unicast_put(struct sk_buff *skb, u32 portid, u32 seq, u8 cmd);
int ethnl_multicast(struct sk_buff *skb, struct net_device *dev);
+void ethnl_notify(struct net_device *dev, unsigned int cmd, const void *data);
/**
* ethnl_strz_size() - calculate attribute length for fixed size string
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index 82cde640aa87..96da9d18789b 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -617,8 +617,8 @@ static int ethtool_set_link_ksettings(struct net_device *dev,
err = dev->ethtool_ops->set_link_ksettings(dev, &link_ksettings);
if (err >= 0) {
- ethtool_notify(dev, ETHTOOL_MSG_LINKINFO_NTF, NULL);
- ethtool_notify(dev, ETHTOOL_MSG_LINKMODES_NTF, NULL);
+ ethtool_notify(dev, ETHTOOL_MSG_LINKINFO_NTF);
+ ethtool_notify(dev, ETHTOOL_MSG_LINKMODES_NTF);
}
return err;
}
@@ -708,8 +708,8 @@ static int ethtool_set_settings(struct net_device *dev, void __user *useraddr)
__ETHTOOL_LINK_MODE_MASK_NU32;
ret = dev->ethtool_ops->set_link_ksettings(dev, &link_ksettings);
if (ret >= 0) {
- ethtool_notify(dev, ETHTOOL_MSG_LINKINFO_NTF, NULL);
- ethtool_notify(dev, ETHTOOL_MSG_LINKMODES_NTF, NULL);
+ ethtool_notify(dev, ETHTOOL_MSG_LINKINFO_NTF);
+ ethtool_notify(dev, ETHTOOL_MSG_LINKMODES_NTF);
}
return ret;
}
@@ -1868,7 +1868,7 @@ static int ethtool_set_wol(struct net_device *dev, char __user *useraddr)
return ret;
dev->ethtool->wol_enabled = !!wol.wolopts;
- ethtool_notify(dev, ETHTOOL_MSG_WOL_NTF, NULL);
+ ethtool_notify(dev, ETHTOOL_MSG_WOL_NTF);
return 0;
}
@@ -1944,7 +1944,7 @@ static int ethtool_set_eee(struct net_device *dev, char __user *useraddr)
eee_to_keee(&keee, &eee);
ret = dev->ethtool_ops->set_eee(dev, &keee);
if (!ret)
- ethtool_notify(dev, ETHTOOL_MSG_EEE_NTF, NULL);
+ ethtool_notify(dev, ETHTOOL_MSG_EEE_NTF);
return ret;
}
@@ -2184,7 +2184,7 @@ static noinline_for_stack int ethtool_set_coalesce(struct net_device *dev,
ret = dev->ethtool_ops->set_coalesce(dev, &coalesce, &kernel_coalesce,
NULL);
if (!ret)
- ethtool_notify(dev, ETHTOOL_MSG_COALESCE_NTF, NULL);
+ ethtool_notify(dev, ETHTOOL_MSG_COALESCE_NTF);
return ret;
}
@@ -2228,7 +2228,7 @@ static int ethtool_set_ringparam(struct net_device *dev, void __user *useraddr)
ret = dev->ethtool_ops->set_ringparam(dev, &ringparam,
&kernel_ringparam, NULL);
if (!ret)
- ethtool_notify(dev, ETHTOOL_MSG_RINGS_NTF, NULL);
+ ethtool_notify(dev, ETHTOOL_MSG_RINGS_NTF);
return ret;
}
@@ -2295,7 +2295,7 @@ static noinline_for_stack int ethtool_set_channels(struct net_device *dev,
ret = dev->ethtool_ops->set_channels(dev, &channels);
if (!ret)
- ethtool_notify(dev, ETHTOOL_MSG_CHANNELS_NTF, NULL);
+ ethtool_notify(dev, ETHTOOL_MSG_CHANNELS_NTF);
return ret;
}
@@ -2326,7 +2326,7 @@ static int ethtool_set_pauseparam(struct net_device *dev, void __user *useraddr)
ret = dev->ethtool_ops->set_pauseparam(dev, &pauseparam);
if (!ret)
- ethtool_notify(dev, ETHTOOL_MSG_PAUSE_NTF, NULL);
+ ethtool_notify(dev, ETHTOOL_MSG_PAUSE_NTF);
return ret;
}
@@ -3328,7 +3328,7 @@ __dev_ethtool(struct net *net, struct ifreq *ifr, void __user *useraddr,
rc = ethtool_set_value_void(dev, useraddr,
dev->ethtool_ops->set_msglevel);
if (!rc)
- ethtool_notify(dev, ETHTOOL_MSG_DEBUG_NTF, NULL);
+ ethtool_notify(dev, ETHTOOL_MSG_DEBUG_NTF);
break;
case ETHTOOL_GEEE:
rc = ethtool_get_eee(dev, useraddr);
@@ -3392,7 +3392,7 @@ __dev_ethtool(struct net *net, struct ifreq *ifr, void __user *useraddr,
rc = ethtool_get_value(dev, useraddr, ethcmd,
dev->ethtool_ops->get_priv_flags);
if (!rc)
- ethtool_notify(dev, ETHTOOL_MSG_PRIVFLAGS_NTF, NULL);
+ ethtool_notify(dev, ETHTOOL_MSG_PRIVFLAGS_NTF);
break;
case ETHTOOL_SPFLAGS:
rc = ethtool_set_value(dev, useraddr,
diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index c5ec3c82ab2e..129f9d56ac65 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -911,7 +911,7 @@ static int ethnl_default_set_doit(struct sk_buff *skb, struct genl_info *info)
swap(dev->cfg, dev->cfg_pending);
if (!ret)
goto out_ops;
- ethtool_notify(dev, ops->set_ntf_cmd, NULL);
+ ethtool_notify(dev, ops->set_ntf_cmd);
ret = 0;
out_ops:
@@ -1049,7 +1049,7 @@ static const ethnl_notify_handler_t ethnl_notify_handlers[] = {
[ETHTOOL_MSG_MM_NTF] = ethnl_default_notify,
};
-void ethtool_notify(struct net_device *dev, unsigned int cmd, const void *data)
+void ethnl_notify(struct net_device *dev, unsigned int cmd, const void *data)
{
if (unlikely(!ethnl_ok))
return;
@@ -1062,13 +1062,18 @@ void ethtool_notify(struct net_device *dev, unsigned int cmd, const void *data)
WARN_ONCE(1, "notification %u not implemented (dev=%s)\n",
cmd, netdev_name(dev));
}
+
+void ethtool_notify(struct net_device *dev, unsigned int cmd)
+{
+ ethnl_notify(dev, cmd, NULL);
+}
EXPORT_SYMBOL(ethtool_notify);
static void ethnl_notify_features(struct netdev_notifier_info *info)
{
struct net_device *dev = netdev_notifier_info_to_dev(info);
- ethtool_notify(dev, ETHTOOL_MSG_FEATURES_NTF, NULL);
+ ethtool_notify(dev, ETHTOOL_MSG_FEATURES_NTF);
}
static int ethnl_netdev_event(struct notifier_block *this, unsigned long event,
--
2.49.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH net-next v2 4/8] net: ethtool: remove the data argument from ethtool_notify()
2025-06-23 23:17 ` [PATCH net-next v2 4/8] net: ethtool: remove the data argument from ethtool_notify() Jakub Kicinski
@ 2025-06-24 5:56 ` Maxime Chevallier
0 siblings, 0 replies; 20+ messages in thread
From: Maxime Chevallier @ 2025-06-24 5:56 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms,
donald.hunter, sdf, jdamato, ecree.xilinx
On Mon, 23 Jun 2025 16:17:16 -0700
Jakub Kicinski <kuba@kernel.org> wrote:
> ethtool_notify() takes a const void *data argument, which presumably
> was intended to pass information from the call site to the subcommand
> handler. This argument currently has no users.
>
> Expecting the data to be subcommand-specific has two complications.
>
> Complication #1 is that its not plumbed thru any of the standardized
> callbacks. It gets propagated to ethnl_default_notify() where it
> remains unused. Coming from the ethnl_default_set_doit() side we pass
> in NULL, because how could we have a command specific attribute in
> a generic handler.
>
> Complication #2 is that we expect the ethtool_notify() callers to
> know what attribute type to pass in. Again, the data pointer is
> untyped.
>
> RSS will need to pass the context ID to the notifications.
> I think it's a better design if the "subcommand" exports its own
> typed interface and constructs the appropriate argument struct
> (which will be req_info). Remove the unused data argument from
> ethtool_notify() but retain it in a new internal helper which
> subcommands can use to build a typed interface.
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Tested-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Maxime
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH net-next v2 5/8] net: ethtool: copy req_info from SET to NTF
2025-06-23 23:17 [PATCH net-next v2 0/8] net: ethtool: rss: add notifications Jakub Kicinski
` (3 preceding siblings ...)
2025-06-23 23:17 ` [PATCH net-next v2 4/8] net: ethtool: remove the data argument from ethtool_notify() Jakub Kicinski
@ 2025-06-23 23:17 ` Jakub Kicinski
2025-06-24 5:55 ` Maxime Chevallier
2025-06-23 23:17 ` [PATCH net-next v2 6/8] net: ethtool: rss: add notifications Jakub Kicinski
` (4 subsequent siblings)
9 siblings, 1 reply; 20+ messages in thread
From: Jakub Kicinski @ 2025-06-23 23:17 UTC (permalink / raw)
To: davem
Cc: netdev, edumazet, pabeni, andrew+netdev, horms, donald.hunter,
maxime.chevallier, sdf, jdamato, ecree.xilinx, Jakub Kicinski
Copy information parsed for SET with .req_parse to NTF handling
and therefore the GET-equivalent that it ends up executing.
This way if the SET was on a sub-object (like RSS context)
the notification will also be appropriately scoped.
Also copy the phy_index, Maxime suggests this will help PLCA
commands generate accurate notifications as well.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
v2:
- populate the phy_index
v1: https://lore.kernel.org/20250621171944.2619249-6-kuba@kernel.org
---
net/ethtool/netlink.h | 5 ++++-
net/ethtool/netlink.c | 16 +++++++++++-----
2 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
index 4a061944a3aa..373a8d5e86ae 100644
--- a/net/ethtool/netlink.h
+++ b/net/ethtool/netlink.h
@@ -23,7 +23,8 @@ void *ethnl_dump_put(struct sk_buff *skb, struct netlink_callback *cb, u8 cmd);
void *ethnl_bcastmsg_put(struct sk_buff *skb, u8 cmd);
void *ethnl_unicast_put(struct sk_buff *skb, u32 portid, u32 seq, u8 cmd);
int ethnl_multicast(struct sk_buff *skb, struct net_device *dev);
-void ethnl_notify(struct net_device *dev, unsigned int cmd, const void *data);
+void ethnl_notify(struct net_device *dev, unsigned int cmd,
+ const struct ethnl_req_info *req_info);
/**
* ethnl_strz_size() - calculate attribute length for fixed size string
@@ -338,6 +339,8 @@ int ethnl_sock_priv_set(struct sk_buff *skb, struct net_device *dev, u32 portid,
* header is already filled on entry, the rest up to @repdata_offset
* is zero initialized. This callback should only modify type specific
* request info by parsed attributes from request message.
+ * Called for both GET and SET. Information parsed for SET will
+ * be conveyed to the req_info used during NTF generation.
* @prepare_data:
* Retrieve and prepare data needed to compose a reply message. Calls to
* ethtool_ops handlers are limited to this callback. Common reply data
diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index 129f9d56ac65..60b3c07507d2 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -911,7 +911,7 @@ static int ethnl_default_set_doit(struct sk_buff *skb, struct genl_info *info)
swap(dev->cfg, dev->cfg_pending);
if (!ret)
goto out_ops;
- ethtool_notify(dev, ops->set_ntf_cmd);
+ ethnl_notify(dev, ops->set_ntf_cmd, req_info);
ret = 0;
out_ops:
@@ -950,7 +950,7 @@ ethnl_default_notify_ops[ETHTOOL_MSG_KERNEL_MAX + 1] = {
/* default notification handler */
static void ethnl_default_notify(struct net_device *dev, unsigned int cmd,
- const void *data)
+ const struct ethnl_req_info *orig_req_info)
{
struct ethnl_reply_data *reply_data;
const struct ethnl_request_ops *ops;
@@ -979,6 +979,11 @@ static void ethnl_default_notify(struct net_device *dev, unsigned int cmd,
req_info->dev = dev;
req_info->flags |= ETHTOOL_FLAG_COMPACT_BITSETS;
+ if (orig_req_info) {
+ req_info->phy_index = orig_req_info->phy_index;
+ memcpy(&req_info[1], &orig_req_info[1],
+ ops->req_info_size - sizeof(*req_info));
+ }
netdev_ops_assert_locked(dev);
@@ -1029,7 +1034,7 @@ static void ethnl_default_notify(struct net_device *dev, unsigned int cmd,
/* notifications */
typedef void (*ethnl_notify_handler_t)(struct net_device *dev, unsigned int cmd,
- const void *data);
+ const struct ethnl_req_info *req_info);
static const ethnl_notify_handler_t ethnl_notify_handlers[] = {
[ETHTOOL_MSG_LINKINFO_NTF] = ethnl_default_notify,
@@ -1049,7 +1054,8 @@ static const ethnl_notify_handler_t ethnl_notify_handlers[] = {
[ETHTOOL_MSG_MM_NTF] = ethnl_default_notify,
};
-void ethnl_notify(struct net_device *dev, unsigned int cmd, const void *data)
+void ethnl_notify(struct net_device *dev, unsigned int cmd,
+ const struct ethnl_req_info *req_info)
{
if (unlikely(!ethnl_ok))
return;
@@ -1057,7 +1063,7 @@ void ethnl_notify(struct net_device *dev, unsigned int cmd, const void *data)
if (likely(cmd < ARRAY_SIZE(ethnl_notify_handlers) &&
ethnl_notify_handlers[cmd]))
- ethnl_notify_handlers[cmd](dev, cmd, data);
+ ethnl_notify_handlers[cmd](dev, cmd, req_info);
else
WARN_ONCE(1, "notification %u not implemented (dev=%s)\n",
cmd, netdev_name(dev));
--
2.49.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH net-next v2 5/8] net: ethtool: copy req_info from SET to NTF
2025-06-23 23:17 ` [PATCH net-next v2 5/8] net: ethtool: copy req_info from SET to NTF Jakub Kicinski
@ 2025-06-24 5:55 ` Maxime Chevallier
0 siblings, 0 replies; 20+ messages in thread
From: Maxime Chevallier @ 2025-06-24 5:55 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms,
donald.hunter, sdf, jdamato, ecree.xilinx
Hi Jakub,
On Mon, 23 Jun 2025 16:17:17 -0700
Jakub Kicinski <kuba@kernel.org> wrote:
> Copy information parsed for SET with .req_parse to NTF handling
> and therefore the GET-equivalent that it ends up executing.
> This way if the SET was on a sub-object (like RSS context)
> the notification will also be appropriately scoped.
>
> Also copy the phy_index, Maxime suggests this will help PLCA
> commands generate accurate notifications as well.
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
It works perfectly for PLCA, now the notif is generated, and for the
right PHY :) thanks !
Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Tested-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Maxime
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH net-next v2 6/8] net: ethtool: rss: add notifications
2025-06-23 23:17 [PATCH net-next v2 0/8] net: ethtool: rss: add notifications Jakub Kicinski
` (4 preceding siblings ...)
2025-06-23 23:17 ` [PATCH net-next v2 5/8] net: ethtool: copy req_info from SET to NTF Jakub Kicinski
@ 2025-06-23 23:17 ` Jakub Kicinski
2025-06-23 23:17 ` [PATCH net-next v2 7/8] doc: ethtool: mark ETHTOOL_GRXFHINDIR as reimplemented Jakub Kicinski
` (3 subsequent siblings)
9 siblings, 0 replies; 20+ messages in thread
From: Jakub Kicinski @ 2025-06-23 23:17 UTC (permalink / raw)
To: davem
Cc: netdev, edumazet, pabeni, andrew+netdev, horms, donald.hunter,
maxime.chevallier, sdf, jdamato, ecree.xilinx, Jakub Kicinski
In preparation for RSS_SET handling in ethnl introduce Netlink
notifications for RSS. Only cover modifications, not creation
and not removal of a context, because the latter may deserve
a different notification type. We should cross that bridge
when we add the support for context add / remove via Netlink.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
v2:
- fix compilation with ETHTOOL_NETLINK=n
v1: https://lore.kernel.org/20250621171944.2619249-7-kuba@kernel.org
---
Documentation/netlink/specs/ethtool.yaml | 7 +++++++
Documentation/networking/ethtool-netlink.rst | 1 +
include/uapi/linux/ethtool_netlink_generated.h | 1 +
net/ethtool/common.h | 8 ++++++++
net/ethtool/ioctl.c | 4 ++++
net/ethtool/netlink.c | 2 ++
| 11 +++++++++++
7 files changed, 34 insertions(+)
diff --git a/Documentation/netlink/specs/ethtool.yaml b/Documentation/netlink/specs/ethtool.yaml
index cfe84f84ba29..19a32229772a 100644
--- a/Documentation/netlink/specs/ethtool.yaml
+++ b/Documentation/netlink/specs/ethtool.yaml
@@ -2492,6 +2492,13 @@ c-version-name: ethtool-genl-version
attributes:
- header
- events
+ -
+ name: rss-ntf
+ doc: |
+ Notification for change in RSS configuration.
+ For additional contexts only modifications are modified, not creation
+ or removal of the contexts.
+ notify: rss-get
mcast-groups:
list:
diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
index e45bb555e909..08abca99a6dc 100644
--- a/Documentation/networking/ethtool-netlink.rst
+++ b/Documentation/networking/ethtool-netlink.rst
@@ -281,6 +281,7 @@ All constants identifying message types use ``ETHTOOL_CMD_`` prefix and suffix
``ETHTOOL_MSG_MODULE_GET_REPLY`` transceiver module parameters
``ETHTOOL_MSG_PSE_GET_REPLY`` PSE parameters
``ETHTOOL_MSG_RSS_GET_REPLY`` RSS settings
+ ``ETHTOOL_MSG_RSS_NTF`` RSS settings
``ETHTOOL_MSG_PLCA_GET_CFG_REPLY`` PLCA RS parameters
``ETHTOOL_MSG_PLCA_GET_STATUS_REPLY`` PLCA RS status
``ETHTOOL_MSG_PLCA_NTF`` PLCA RS parameters
diff --git a/include/uapi/linux/ethtool_netlink_generated.h b/include/uapi/linux/ethtool_netlink_generated.h
index 859e28c8a91a..8f30ffa1cd14 100644
--- a/include/uapi/linux/ethtool_netlink_generated.h
+++ b/include/uapi/linux/ethtool_netlink_generated.h
@@ -862,6 +862,7 @@ enum {
ETHTOOL_MSG_TSCONFIG_GET_REPLY,
ETHTOOL_MSG_TSCONFIG_SET_REPLY,
ETHTOOL_MSG_PSE_NTF,
+ ETHTOOL_MSG_RSS_NTF,
__ETHTOOL_MSG_KERNEL_CNT,
ETHTOOL_MSG_KERNEL_MAX = (__ETHTOOL_MSG_KERNEL_CNT - 1)
diff --git a/net/ethtool/common.h b/net/ethtool/common.h
index b4683d286a5a..c41db1595621 100644
--- a/net/ethtool/common.h
+++ b/net/ethtool/common.h
@@ -74,4 +74,12 @@ int ethtool_get_module_eeprom_call(struct net_device *dev,
bool __ethtool_dev_mm_supported(struct net_device *dev);
+#if IS_ENABLED(CONFIG_ETHTOOL_NETLINK)
+void ethtool_rss_notify(struct net_device *dev, u32 rss_context);
+#else
+static inline void ethtool_rss_notify(struct net_device *dev, u32 rss_context)
+{
+}
+#endif
+
#endif /* _ETHTOOL_COMMON_H */
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index 96da9d18789b..c34bac7bffd8 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -1502,6 +1502,7 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
struct ethtool_rxfh rxfh;
bool locked = false; /* dev->ethtool->rss_lock taken */
bool create = false;
+ bool mod = false;
u8 *rss_config;
int ret;
@@ -1688,6 +1689,7 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
}
goto out;
}
+ mod = !create && !rxfh_dev.rss_delete;
if (copy_to_user(useraddr + offsetof(struct ethtool_rxfh, rss_context),
&rxfh_dev.rss_context, sizeof(rxfh_dev.rss_context)))
@@ -1757,6 +1759,8 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
if (locked)
mutex_unlock(&dev->ethtool->rss_lock);
kfree(rss_config);
+ if (mod)
+ ethtool_rss_notify(dev, rxfh.rss_context);
return ret;
}
diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index 60b3c07507d2..09c81cc9a08f 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -946,6 +946,7 @@ ethnl_default_notify_ops[ETHTOOL_MSG_KERNEL_MAX + 1] = {
[ETHTOOL_MSG_MODULE_NTF] = ðnl_module_request_ops,
[ETHTOOL_MSG_PLCA_NTF] = ðnl_plca_cfg_request_ops,
[ETHTOOL_MSG_MM_NTF] = ðnl_mm_request_ops,
+ [ETHTOOL_MSG_RSS_NTF] = ðnl_rss_request_ops,
};
/* default notification handler */
@@ -1052,6 +1053,7 @@ static const ethnl_notify_handler_t ethnl_notify_handlers[] = {
[ETHTOOL_MSG_MODULE_NTF] = ethnl_default_notify,
[ETHTOOL_MSG_PLCA_NTF] = ethnl_default_notify,
[ETHTOOL_MSG_MM_NTF] = ethnl_default_notify,
+ [ETHTOOL_MSG_RSS_NTF] = ethnl_default_notify,
};
void ethnl_notify(struct net_device *dev, unsigned int cmd,
--git a/net/ethtool/rss.c b/net/ethtool/rss.c
index 6d9b1769896b..3adddca7e215 100644
--- a/net/ethtool/rss.c
+++ b/net/ethtool/rss.c
@@ -358,6 +358,17 @@ int ethnl_rss_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
return ret;
}
+/* RSS_NTF */
+
+void ethtool_rss_notify(struct net_device *dev, u32 rss_context)
+{
+ struct rss_req_info req_info = {
+ .rss_context = rss_context,
+ };
+
+ ethnl_notify(dev, ETHTOOL_MSG_RSS_NTF, &req_info.base);
+}
+
const struct ethnl_request_ops ethnl_rss_request_ops = {
.request_cmd = ETHTOOL_MSG_RSS_GET,
.reply_cmd = ETHTOOL_MSG_RSS_GET_REPLY,
--
2.49.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH net-next v2 7/8] doc: ethtool: mark ETHTOOL_GRXFHINDIR as reimplemented
2025-06-23 23:17 [PATCH net-next v2 0/8] net: ethtool: rss: add notifications Jakub Kicinski
` (5 preceding siblings ...)
2025-06-23 23:17 ` [PATCH net-next v2 6/8] net: ethtool: rss: add notifications Jakub Kicinski
@ 2025-06-23 23:17 ` Jakub Kicinski
2025-06-25 8:55 ` Donald Hunter
2025-06-23 23:17 ` [PATCH net-next v2 8/8] selftests: drv-net: test RSS Netlink notifications Jakub Kicinski
` (2 subsequent siblings)
9 siblings, 1 reply; 20+ messages in thread
From: Jakub Kicinski @ 2025-06-23 23:17 UTC (permalink / raw)
To: davem
Cc: netdev, edumazet, pabeni, andrew+netdev, horms, donald.hunter,
maxime.chevallier, sdf, jdamato, ecree.xilinx, Jakub Kicinski
The ETHTOOL_GRXFHINDIR reimplementation has been completed around
a year ago. We have been tweaking it so a bit hard to point
to a single commit that completed it, but all the fields available
in IOCTL are reported via Netlink.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
Documentation/networking/ethtool-netlink.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
index 08abca99a6dc..07e9808ebd2c 100644
--- a/Documentation/networking/ethtool-netlink.rst
+++ b/Documentation/networking/ethtool-netlink.rst
@@ -2451,7 +2451,7 @@ are netlink only.
``ETHTOOL_SRXNTUPLE`` n/a
``ETHTOOL_GRXNTUPLE`` n/a
``ETHTOOL_GSSET_INFO`` ``ETHTOOL_MSG_STRSET_GET``
- ``ETHTOOL_GRXFHINDIR`` n/a
+ ``ETHTOOL_GRXFHINDIR`` ``ETHTOOL_MSG_RSS_GET``
``ETHTOOL_SRXFHINDIR`` n/a
``ETHTOOL_GFEATURES`` ``ETHTOOL_MSG_FEATURES_GET``
``ETHTOOL_SFEATURES`` ``ETHTOOL_MSG_FEATURES_SET``
--
2.49.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH net-next v2 7/8] doc: ethtool: mark ETHTOOL_GRXFHINDIR as reimplemented
2025-06-23 23:17 ` [PATCH net-next v2 7/8] doc: ethtool: mark ETHTOOL_GRXFHINDIR as reimplemented Jakub Kicinski
@ 2025-06-25 8:55 ` Donald Hunter
0 siblings, 0 replies; 20+ messages in thread
From: Donald Hunter @ 2025-06-25 8:55 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms,
maxime.chevallier, sdf, jdamato, ecree.xilinx
Jakub Kicinski <kuba@kernel.org> writes:
> The ETHTOOL_GRXFHINDIR reimplementation has been completed around
> a year ago. We have been tweaking it so a bit hard to point
> to a single commit that completed it, but all the fields available
> in IOCTL are reported via Netlink.
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Reviewed-by: Donald Hunter <donald.hunter@gmail.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH net-next v2 8/8] selftests: drv-net: test RSS Netlink notifications
2025-06-23 23:17 [PATCH net-next v2 0/8] net: ethtool: rss: add notifications Jakub Kicinski
` (6 preceding siblings ...)
2025-06-23 23:17 ` [PATCH net-next v2 7/8] doc: ethtool: mark ETHTOOL_GRXFHINDIR as reimplemented Jakub Kicinski
@ 2025-06-23 23:17 ` Jakub Kicinski
2025-06-25 9:46 ` Donald Hunter
2025-06-24 6:00 ` [PATCH net-next v2 0/8] net: ethtool: rss: add notifications Maxime Chevallier
2025-06-25 22:50 ` patchwork-bot+netdevbpf
9 siblings, 1 reply; 20+ messages in thread
From: Jakub Kicinski @ 2025-06-23 23:17 UTC (permalink / raw)
To: davem
Cc: netdev, edumazet, pabeni, andrew+netdev, horms, donald.hunter,
maxime.chevallier, sdf, jdamato, ecree.xilinx, Jakub Kicinski
Test that changing the RSS config generates Netlink notifications.
# ./tools/testing/selftests/drivers/net/hw/rss_api.py
TAP version 13
1..2
ok 1 rss_api.test_rxfh_indir_ntf
ok 2 rss_api.test_rxfh_indir_ctx_ntf
# Totals: pass:2 fail:0 xfail:0 xpass:0 skip:0 error:0
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
| 89 +++++++++++++++++++
1 file changed, 89 insertions(+)
create mode 100755 tools/testing/selftests/drivers/net/hw/rss_api.py
--git a/tools/testing/selftests/drivers/net/hw/rss_api.py b/tools/testing/selftests/drivers/net/hw/rss_api.py
new file mode 100755
index 000000000000..db0f723a674b
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/hw/rss_api.py
@@ -0,0 +1,89 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0
+
+"""
+API level tests for RSS (mostly Netlink vs IOCTL).
+"""
+
+import glob
+from lib.py import ksft_run, ksft_exit, ksft_eq, ksft_is, ksft_ne
+from lib.py import KsftSkipEx, KsftFailEx
+from lib.py import defer, ethtool
+from lib.py import EthtoolFamily
+from lib.py import NetDrvEnv
+
+
+def _ethtool_create(cfg, act, opts):
+ output = ethtool(f"{act} {cfg.ifname} {opts}").stdout
+ # Output will be something like: "New RSS context is 1" or
+ # "Added rule with ID 7", we want the integer from the end
+ return int(output.split()[-1])
+
+
+def test_rxfh_indir_ntf(cfg):
+ """
+ Check that Netlink notifications are generated when RSS indirection
+ table was modified.
+ """
+
+ qcnt = len(glob.glob(f"/sys/class/net/{cfg.ifname}/queues/rx-*"))
+ if qcnt < 2:
+ raise KsftSkipEx(f"Local has only {qcnt} queues")
+
+ ethnl = EthtoolFamily()
+ ethnl.ntf_subscribe("monitor")
+
+ ethtool(f"--disable-netlink -X {cfg.ifname} weight 0 1")
+ reset = defer(ethtool, f"-X {cfg.ifname} default")
+
+ ntf = next(ethnl.poll_ntf(duration=0.2), None)
+ if ntf is None:
+ raise KsftFailEx("No notification received")
+ ksft_eq(ntf["name"], "rss-ntf")
+ ksft_eq(set(ntf["msg"]["indir"]), {1})
+
+ reset.exec()
+ ntf = next(ethnl.poll_ntf(duration=0.2), None)
+ if ntf is None:
+ raise KsftFailEx("No notification received after reset")
+ ksft_eq(ntf["name"], "rss-ntf")
+ ksft_is(ntf["msg"].get("context"), None)
+ ksft_ne(set(ntf["msg"]["indir"]), {1})
+
+
+def test_rxfh_indir_ctx_ntf(cfg):
+ """
+ Check that Netlink notifications are generated when RSS indirection
+ table was modified on an additional RSS context.
+ """
+
+ qcnt = len(glob.glob(f"/sys/class/net/{cfg.ifname}/queues/rx-*"))
+ if qcnt < 2:
+ raise KsftSkipEx(f"Local has only {qcnt} queues")
+
+ ctx_id = _ethtool_create(cfg, "-X", "context new")
+ defer(ethtool, f"-X {cfg.ifname} context {ctx_id} delete")
+
+ ethnl = EthtoolFamily()
+ ethnl.ntf_subscribe("monitor")
+
+ ethtool(f"--disable-netlink -X {cfg.ifname} context {ctx_id} weight 0 1")
+
+ ntf = next(ethnl.poll_ntf(duration=0.2), None)
+ if ntf is None:
+ raise KsftFailEx("No notification received")
+ ksft_eq(ntf["name"], "rss-ntf")
+ ksft_eq(ntf["msg"].get("context"), ctx_id)
+ ksft_eq(set(ntf["msg"]["indir"]), {1})
+
+
+def main() -> None:
+ """ Ksft boiler plate main """
+
+ with NetDrvEnv(__file__, nsim_test=False) as cfg:
+ ksft_run(globs=globals(), case_pfx={"test_"}, args=(cfg, ))
+ ksft_exit()
+
+
+if __name__ == "__main__":
+ main()
--
2.49.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH net-next v2 8/8] selftests: drv-net: test RSS Netlink notifications
2025-06-23 23:17 ` [PATCH net-next v2 8/8] selftests: drv-net: test RSS Netlink notifications Jakub Kicinski
@ 2025-06-25 9:46 ` Donald Hunter
2025-06-25 20:04 ` Jakub Kicinski
0 siblings, 1 reply; 20+ messages in thread
From: Donald Hunter @ 2025-06-25 9:46 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms,
maxime.chevallier, sdf, jdamato, ecree.xilinx
Jakub Kicinski <kuba@kernel.org> writes:
> Test that changing the RSS config generates Netlink notifications.
>
> # ./tools/testing/selftests/drivers/net/hw/rss_api.py
> TAP version 13
> 1..2
> ok 1 rss_api.test_rxfh_indir_ntf
> ok 2 rss_api.test_rxfh_indir_ctx_ntf
> # Totals: pass:2 fail:0 xfail:0 xpass:0 skip:0 error:0
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> .../selftests/drivers/net/hw/rss_api.py | 89 +++++++++++++++++++
> 1 file changed, 89 insertions(+)
> create mode 100755 tools/testing/selftests/drivers/net/hw/rss_api.py
>
> diff --git a/tools/testing/selftests/drivers/net/hw/rss_api.py b/tools/testing/selftests/drivers/net/hw/rss_api.py
> new file mode 100755
> index 000000000000..db0f723a674b
> --- /dev/null
> +++ b/tools/testing/selftests/drivers/net/hw/rss_api.py
> @@ -0,0 +1,89 @@
> +#!/usr/bin/env python3
> +# SPDX-License-Identifier: GPL-2.0
> +
> +"""
> +API level tests for RSS (mostly Netlink vs IOCTL).
> +"""
> +
> +import glob
> +from lib.py import ksft_run, ksft_exit, ksft_eq, ksft_is, ksft_ne
> +from lib.py import KsftSkipEx, KsftFailEx
> +from lib.py import defer, ethtool
> +from lib.py import EthtoolFamily
> +from lib.py import NetDrvEnv
> +
> +
> +def _ethtool_create(cfg, act, opts):
> + output = ethtool(f"{act} {cfg.ifname} {opts}").stdout
> + # Output will be something like: "New RSS context is 1" or
> + # "Added rule with ID 7", we want the integer from the end
> + return int(output.split()[-1])
I think .split() is not required because you can access strings as
arrays.
Will this only ever need to handle single digit values?
> +def test_rxfh_indir_ntf(cfg):
> + """
> + Check that Netlink notifications are generated when RSS indirection
> + table was modified.
> + """
> +
> + qcnt = len(glob.glob(f"/sys/class/net/{cfg.ifname}/queues/rx-*"))
> + if qcnt < 2:
> + raise KsftSkipEx(f"Local has only {qcnt} queues")
> +
> + ethnl = EthtoolFamily()
> + ethnl.ntf_subscribe("monitor")
> +
> + ethtool(f"--disable-netlink -X {cfg.ifname} weight 0 1")
> + reset = defer(ethtool, f"-X {cfg.ifname} default")
> +
> + ntf = next(ethnl.poll_ntf(duration=0.2), None)
> + if ntf is None:
> + raise KsftFailEx("No notification received")
> + ksft_eq(ntf["name"], "rss-ntf")
> + ksft_eq(set(ntf["msg"]["indir"]), {1})
> +
> + reset.exec()
> + ntf = next(ethnl.poll_ntf(duration=0.2), None)
> + if ntf is None:
> + raise KsftFailEx("No notification received after reset")
> + ksft_eq(ntf["name"], "rss-ntf")
> + ksft_is(ntf["msg"].get("context"), None)
> + ksft_ne(set(ntf["msg"]["indir"]), {1})
> +
> +
> +def test_rxfh_indir_ctx_ntf(cfg):
> + """
> + Check that Netlink notifications are generated when RSS indirection
> + table was modified on an additional RSS context.
> + """
> +
> + qcnt = len(glob.glob(f"/sys/class/net/{cfg.ifname}/queues/rx-*"))
> + if qcnt < 2:
> + raise KsftSkipEx(f"Local has only {qcnt} queues")
> +
> + ctx_id = _ethtool_create(cfg, "-X", "context new")
> + defer(ethtool, f"-X {cfg.ifname} context {ctx_id} delete")
> +
> + ethnl = EthtoolFamily()
> + ethnl.ntf_subscribe("monitor")
> +
> + ethtool(f"--disable-netlink -X {cfg.ifname} context {ctx_id} weight 0 1")
> +
> + ntf = next(ethnl.poll_ntf(duration=0.2), None)
> + if ntf is None:
> + raise KsftFailEx("No notification received")
> + ksft_eq(ntf["name"], "rss-ntf")
> + ksft_eq(ntf["msg"].get("context"), ctx_id)
> + ksft_eq(set(ntf["msg"]["indir"]), {1})
> +
> +
> +def main() -> None:
> + """ Ksft boiler plate main """
> +
> + with NetDrvEnv(__file__, nsim_test=False) as cfg:
> + ksft_run(globs=globals(), case_pfx={"test_"}, args=(cfg, ))
> + ksft_exit()
> +
> +
> +if __name__ == "__main__":
> + main()
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next v2 8/8] selftests: drv-net: test RSS Netlink notifications
2025-06-25 9:46 ` Donald Hunter
@ 2025-06-25 20:04 ` Jakub Kicinski
2025-06-25 21:55 ` Donald Hunter
0 siblings, 1 reply; 20+ messages in thread
From: Jakub Kicinski @ 2025-06-25 20:04 UTC (permalink / raw)
To: Donald Hunter
Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms,
maxime.chevallier, sdf, jdamato, ecree.xilinx
On Wed, 25 Jun 2025 10:46:53 +0100 Donald Hunter wrote:
> > +def _ethtool_create(cfg, act, opts):
> > + output = ethtool(f"{act} {cfg.ifname} {opts}").stdout
> > + # Output will be something like: "New RSS context is 1" or
> > + # "Added rule with ID 7", we want the integer from the end
> > + return int(output.split()[-1])
>
> I think .split() is not required because you can access strings as
> arrays.
>
> Will this only ever need to handle single digit values?
nosir, IIUC split splits on whitespace, so from:
"Added rule with ID 7" -> [.."ID", "7"]
"Added rule with ID 71" -> [.."ID", "71"]
and we take the last elem, the ID. We use similar code in the rss_ctx
test, I think it works..
Unfortunately there is no plan to migrate the flow steering to netlink.
And ethtool only supports JSON output in the netlink code :S
Mountains of technical debt :)
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next v2 8/8] selftests: drv-net: test RSS Netlink notifications
2025-06-25 20:04 ` Jakub Kicinski
@ 2025-06-25 21:55 ` Donald Hunter
0 siblings, 0 replies; 20+ messages in thread
From: Donald Hunter @ 2025-06-25 21:55 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms,
maxime.chevallier, sdf, jdamato, ecree.xilinx
On Wed, 25 Jun 2025 at 21:04, Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Wed, 25 Jun 2025 10:46:53 +0100 Donald Hunter wrote:
> > > +def _ethtool_create(cfg, act, opts):
> > > + output = ethtool(f"{act} {cfg.ifname} {opts}").stdout
> > > + # Output will be something like: "New RSS context is 1" or
> > > + # "Added rule with ID 7", we want the integer from the end
> > > + return int(output.split()[-1])
> >
> > I think .split() is not required because you can access strings as
> > arrays.
> >
> > Will this only ever need to handle single digit values?
>
> nosir, IIUC split splits on whitespace, so from:
>
> "Added rule with ID 7" -> [.."ID", "7"]
> "Added rule with ID 71" -> [.."ID", "71"]
>
> and we take the last elem, the ID. We use similar code in the rss_ctx
> test, I think it works..
Ah, my bad. Ignore my rambling and sorry for the noise.
> Unfortunately there is no plan to migrate the flow steering to netlink.
> And ethtool only supports JSON output in the netlink code :S
> Mountains of technical debt :)
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next v2 0/8] net: ethtool: rss: add notifications
2025-06-23 23:17 [PATCH net-next v2 0/8] net: ethtool: rss: add notifications Jakub Kicinski
` (7 preceding siblings ...)
2025-06-23 23:17 ` [PATCH net-next v2 8/8] selftests: drv-net: test RSS Netlink notifications Jakub Kicinski
@ 2025-06-24 6:00 ` Maxime Chevallier
2025-06-24 14:13 ` Jakub Kicinski
2025-06-25 22:50 ` patchwork-bot+netdevbpf
9 siblings, 1 reply; 20+ messages in thread
From: Maxime Chevallier @ 2025-06-24 6:00 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms,
donald.hunter, sdf, jdamato, ecree.xilinx
Hi Jakub,
On Mon, 23 Jun 2025 16:17:12 -0700
Jakub Kicinski <kuba@kernel.org> wrote:
> Next step on the path to moving RSS config to Netlink. With the
> refactoring of the driver-facing API for ETHTOOL_GRXFH/ETHTOOL_SRXFH
> out of the way we can move on to more interesting work.
>
> Add Netlink notifications for changes in RSS configuration.
>
> As a reminder (part) of rss-get was introduced in previous releases
> when input-xfrm (symmetric hashing) was added. rss-set isn't
> implemented, yet, but we can implement rss-ntf and hook it into
> the changes done via the IOCTL path (same as other ethtool-nl
> notifications do).
>
> Most of the series is concerned with passing arguments to notifications.
> So far none of the notifications needed to be parametrized, but RSS can
> have multiple contexts per device, and since GET operates on a single
> context at a time, the notification needs to also be scoped to a context.
> Patches 2-5 add support for passing arguments to notifications thru
> ethtool-nl generic infra.
>
> The notification handling itself is pretty trivial, it's mostly
> hooking in the right entries into the ethool-nl op tables.
I was able to test the PLCA path, which works fine :)
So, I reviewed and tested some of the ethnl patches in this series,
though I wasn't able to test the RSS-specific aspects.
Thanks,
Maxime
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next v2 0/8] net: ethtool: rss: add notifications
2025-06-24 6:00 ` [PATCH net-next v2 0/8] net: ethtool: rss: add notifications Maxime Chevallier
@ 2025-06-24 14:13 ` Jakub Kicinski
0 siblings, 0 replies; 20+ messages in thread
From: Jakub Kicinski @ 2025-06-24 14:13 UTC (permalink / raw)
To: Maxime Chevallier
Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms,
donald.hunter, sdf, jdamato, ecree.xilinx
On Tue, 24 Jun 2025 08:00:04 +0200 Maxime Chevallier wrote:
> I was able to test the PLCA path, which works fine :)
>
> So, I reviewed and tested some of the ethnl patches in this series,
> though I wasn't able to test the RSS-specific aspects.
Excellent, thank you!
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next v2 0/8] net: ethtool: rss: add notifications
2025-06-23 23:17 [PATCH net-next v2 0/8] net: ethtool: rss: add notifications Jakub Kicinski
` (8 preceding siblings ...)
2025-06-24 6:00 ` [PATCH net-next v2 0/8] net: ethtool: rss: add notifications Maxime Chevallier
@ 2025-06-25 22:50 ` patchwork-bot+netdevbpf
9 siblings, 0 replies; 20+ messages in thread
From: patchwork-bot+netdevbpf @ 2025-06-25 22:50 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms,
donald.hunter, maxime.chevallier, sdf, jdamato, ecree.xilinx
Hello:
This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Mon, 23 Jun 2025 16:17:12 -0700 you wrote:
> Next step on the path to moving RSS config to Netlink. With the
> refactoring of the driver-facing API for ETHTOOL_GRXFH/ETHTOOL_SRXFH
> out of the way we can move on to more interesting work.
>
> Add Netlink notifications for changes in RSS configuration.
>
> As a reminder (part) of rss-get was introduced in previous releases
> when input-xfrm (symmetric hashing) was added. rss-set isn't
> implemented, yet, but we can implement rss-ntf and hook it into
> the changes done via the IOCTL path (same as other ethtool-nl
> notifications do).
>
> [...]
Here is the summary with links:
- [net-next,v2,1/8] netlink: specs: add the multicast group name to spec
https://git.kernel.org/netdev/net-next/c/826334359eac
- [net-next,v2,2/8] net: ethtool: dynamically allocate full req size req
https://git.kernel.org/netdev/net-next/c/ceca0769e87f
- [net-next,v2,3/8] net: ethtool: call .parse_request for SET handlers
https://git.kernel.org/netdev/net-next/c/963781bdfe20
- [net-next,v2,4/8] net: ethtool: remove the data argument from ethtool_notify()
https://git.kernel.org/netdev/net-next/c/f9dc3e52d821
- [net-next,v2,5/8] net: ethtool: copy req_info from SET to NTF
https://git.kernel.org/netdev/net-next/c/3073947de382
- [net-next,v2,6/8] net: ethtool: rss: add notifications
https://git.kernel.org/netdev/net-next/c/46837be5afc6
- [net-next,v2,7/8] doc: ethtool: mark ETHTOOL_GRXFHINDIR as reimplemented
https://git.kernel.org/netdev/net-next/c/47c3ed01af43
- [net-next,v2,8/8] selftests: drv-net: test RSS Netlink notifications
https://git.kernel.org/netdev/net-next/c/4d13c6c449af
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 20+ messages in thread