diff mbox series

[v2,net] ethtool: netlink: add missing netdev_features_change() call

Message ID ahA2YWXYICz5rbUSQqNG4roJ8OlJzzYQX7PTiG80@cp4-web-028.plabs.ch (mailing list archive)
State Not Applicable
Delegated to: Netdev Maintainers
Headers show
Series [v2,net] ethtool: netlink: add missing netdev_features_change() call | expand

Commit Message

Alexander Lobakin Nov. 8, 2020, 12:46 a.m. UTC
After updating userspace Ethtool from 5.7 to 5.9, I noticed that
NETDEV_FEAT_CHANGE is no more raised when changing netdev features
through Ethtool.
That's because the old Ethtool ioctl interface always calls
netdev_features_change() at the end of user request processing to
inform the kernel that our netdevice has some features changed, but
the new Netlink interface does not. Instead, it just notifies itself
with ETHTOOL_MSG_FEATURES_NTF.
Replace this ethtool_notify() call with netdev_features_change(), so
the kernel will be aware of any features changes, just like in case
with the ioctl interface. This does not omit Ethtool notifications,
as Ethtool itself listens to NETDEV_FEAT_CHANGE and drops
ETHTOOL_MSG_FEATURES_NTF on it
(net/ethtool/netlink.c:ethnl_netdev_event()).

From v1 [1]:
- dropped extra new line as advised by Jakub;
- no functional changes.

[1] https://lore.kernel.org/netdev/AlZXQ2o5uuTVHCfNGOiGgJ8vJ3KgO5YIWAnQjH0cDE@cp3-web-009.plabs.ch

Fixes: 0980bfcd6954 ("ethtool: set netdev features with FEATURES_SET request")
Signed-off-by: Alexander Lobakin <alobakin@pm.me>
---
 net/ethtool/features.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Michal Kubecek Nov. 9, 2020, 2 p.m. UTC | #1
On Sun, Nov 08, 2020 at 12:46:15AM +0000, Alexander Lobakin wrote:
> After updating userspace Ethtool from 5.7 to 5.9, I noticed that
> NETDEV_FEAT_CHANGE is no more raised when changing netdev features
> through Ethtool.
> That's because the old Ethtool ioctl interface always calls
> netdev_features_change() at the end of user request processing to
> inform the kernel that our netdevice has some features changed, but
> the new Netlink interface does not. Instead, it just notifies itself
> with ETHTOOL_MSG_FEATURES_NTF.
> Replace this ethtool_notify() call with netdev_features_change(), so
> the kernel will be aware of any features changes, just like in case
> with the ioctl interface. This does not omit Ethtool notifications,
> as Ethtool itself listens to NETDEV_FEAT_CHANGE and drops
> ETHTOOL_MSG_FEATURES_NTF on it
> (net/ethtool/netlink.c:ethnl_netdev_event()).
> 
> From v1 [1]:
> - dropped extra new line as advised by Jakub;
> - no functional changes.
> 
> [1] https://lore.kernel.org/netdev/AlZXQ2o5uuTVHCfNGOiGgJ8vJ3KgO5YIWAnQjH0cDE@cp3-web-009.plabs.ch
> 
> Fixes: 0980bfcd6954 ("ethtool: set netdev features with FEATURES_SET request")
> Signed-off-by: Alexander Lobakin <alobakin@pm.me>

Reviewed-by: Michal Kubecek <mkubecek@suse.cz>

> ---
>  net/ethtool/features.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/ethtool/features.c b/net/ethtool/features.c
> index 8ee4cdbd6b82..1c9f4df273bd 100644
> --- a/net/ethtool/features.c
> +++ b/net/ethtool/features.c
> @@ -280,7 +280,7 @@ int ethnl_set_features(struct sk_buff *skb, struct genl_info *info)
>  					  active_diff_mask, compact);
>  	}
>  	if (mod)
> -		ethtool_notify(dev, ETHTOOL_MSG_FEATURES_NTF, NULL);
> +		netdev_features_change(dev);
>  
>  out_rtnl:
>  	rtnl_unlock();
> -- 
> 2.29.2
> 
>
Jakub Kicinski Nov. 10, 2020, 1:18 a.m. UTC | #2
On Mon, 9 Nov 2020 15:00:02 +0100 Michal Kubecek wrote:
> On Sun, Nov 08, 2020 at 12:46:15AM +0000, Alexander Lobakin wrote:
> > After updating userspace Ethtool from 5.7 to 5.9, I noticed that
> > NETDEV_FEAT_CHANGE is no more raised when changing netdev features
> > through Ethtool.
> > That's because the old Ethtool ioctl interface always calls
> > netdev_features_change() at the end of user request processing to
> > inform the kernel that our netdevice has some features changed, but
> > the new Netlink interface does not. Instead, it just notifies itself
> > with ETHTOOL_MSG_FEATURES_NTF.
> > Replace this ethtool_notify() call with netdev_features_change(), so
> > the kernel will be aware of any features changes, just like in case
> > with the ioctl interface. This does not omit Ethtool notifications,
> > as Ethtool itself listens to NETDEV_FEAT_CHANGE and drops
> > ETHTOOL_MSG_FEATURES_NTF on it
> > (net/ethtool/netlink.c:ethnl_netdev_event()).
> > 
> > From v1 [1]:
> > - dropped extra new line as advised by Jakub;
> > - no functional changes.
> > 
> > [1] https://lore.kernel.org/netdev/AlZXQ2o5uuTVHCfNGOiGgJ8vJ3KgO5YIWAnQjH0cDE@cp3-web-009.plabs.ch
> > 
> > Fixes: 0980bfcd6954 ("ethtool: set netdev features with FEATURES_SET request")
> > Signed-off-by: Alexander Lobakin <alobakin@pm.me>  
> 
> Reviewed-by: Michal Kubecek <mkubecek@suse.cz>

Applied, thanks!
diff mbox series

Patch

diff --git a/net/ethtool/features.c b/net/ethtool/features.c
index 8ee4cdbd6b82..1c9f4df273bd 100644
--- a/net/ethtool/features.c
+++ b/net/ethtool/features.c
@@ -280,7 +280,7 @@  int ethnl_set_features(struct sk_buff *skb, struct genl_info *info)
 					  active_diff_mask, compact);
 	}
 	if (mod)
-		ethtool_notify(dev, ETHTOOL_MSG_FEATURES_NTF, NULL);
+		netdev_features_change(dev);
 
 out_rtnl:
 	rtnl_unlock();