Re: [PATCH net-next 1/2] net: provide a default dev->ethtool_ops
From: Ben Hutchings <hidden>
Date: 2012-09-15 13:40:52
On Fri, 2012-09-14 at 09:24 +0200, Eric Dumazet wrote:
From: Eric Dumazet <edumazet@google.com> Instead of forcing device drivers to provide empty ethtool_ops or tweak net/core/ethtool.c again, we could provide a generic ethtool_ops. This occurred to me when I wanted to add GSO support to GRE tunnels. ethtool -k support should be generic for all drivers. Signed-off-by: Eric Dumazet <edumazet@google.com> Cc: Maciej Żenczykowski <redacted>
Yes, I should have done this ages ago. Just some nit-picking below: [...]
quoted hunk ↗ jump to hunk
diff --git a/net/core/dev.c b/net/core/dev.c index b1e6d63..ff8dcfc 100644 --- a/net/core/dev.c +++ b/net/core/dev.c@@ -6045,6 +6045,11 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name, strcpy(dev->name, name); dev->group = INIT_NETDEV_GROUP; + if (!dev->ethtool_ops) { + static const struct ethtool_ops default_ethtool_ops; + + dev->ethtool_ops = &default_ethtool_ops; + }
This block has a blank line in it, so I think it needs a blank line either side to make the visual grouping of code right. Alternately you could pull the variable out of the block. [...]
quoted hunk ↗ jump to hunk
@@ -1410,8 +1409,9 @@ static int ethtool_get_module_eeprom(struct net_device *dev, modinfo.eeprom_len); } -/* The main entry point in this file. Called from net/core/dev.c */ - +/* The main entry point in this file. Called from net/core/dev.c + * with RTNL held. + */
Good point but an unrelated change.
quoted hunk ↗ jump to hunk
int dev_ethtool(struct net *net, struct ifreq *ifr) { struct net_device *dev = __dev_get_by_name(net, ifr->ifr_name);@@ -1419,25 +1419,15 @@ int dev_ethtool(struct net *net, struct ifreq *ifr) u32 ethcmd; int rc; u32 old_features; + const struct ethtool_ops *ops; if (!dev || !netif_device_present(dev)) return -ENODEV; + ops = dev->ethtool_ops;
[...] Introducing this local variable is a useful cleanup but again should be a separate change. Ben. -- Ben Hutchings, Staff Engineer, Solarflare Not speaking for my employer; that's the marketing department's job. They asked us to note that Solarflare product names are trademarked.