[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250621171944.2619249-5-kuba@kernel.org>
Date: Sat, 21 Jun 2025 10:19:39 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: davem@...emloft.net
Cc: netdev@...r.kernel.org,
edumazet@...gle.com,
pabeni@...hat.com,
andrew+netdev@...n.ch,
horms@...nel.org,
donald.hunter@...il.com,
maxime.chevallier@...tlin.com,
sdf@...ichev.me,
jdamato@...tly.com,
ecree.xilinx@...il.com,
Jakub Kicinski <kuba@...nel.org>
Subject: [PATCH net-next 4/9] net: ethtool: remove the data argument from ethtool_notify()
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@...nel.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
Powered by blists - more mailing lists