Commit f9dc3e52 authored by Jakub Kicinski's avatar Jakub Kicinski
Browse files

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.

Reviewed-by: default avatarMaxime Chevallier <maxime.chevallier@bootlin.com>
Tested-by: default avatarMaxime Chevallier <maxime.chevallier@bootlin.com>
Link: https://patch.msgid.link/20250623231720.3124717-5-kuba@kernel.org


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent 963781bd

include/linux/netdevice.h

+2 −3
Original line number Diff line number Diff line
@@ -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

net/ethtool/ioctl.c

+12 −12
Original line number Diff line number Diff line
@@ -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,

net/ethtool/netlink.c

+8 −3
Original line number Diff line number Diff line
@@ -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,

net/ethtool/netlink.h

+1 −0
Original line number Diff line number Diff line
@@ -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