| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CANP3RGevLSvETJse_XFB6qa6QZoyuaaP7CcfGiw9QPUNnhw_hA@mail.gmail.com>
Date: Fri, 14 Sep 2012 10:10:53 -0700
From: Maciej Żenczykowski <maze@...gle.com>
To: Eric Dumazet <eric.dumazet@...il.com>
Cc: David Miller <davem@...emloft.net>, netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next 1/2] net: provide a default dev->ethtool_ops
Any cleanups like this are always nice to see.
On Fri, Sep 14, 2012 at 12:24 AM, Eric Dumazet <eric.dumazet@...il.com> wrote:
> From: Eric Dumazet <edumazet@...gle.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@...gle.com>
> Cc: Maciej Żenczykowski <maze@...gle.com>
> ---
> net/core/dev.c | 5 +++
> net/core/ethtool.c | 54 +++++++++++++++++--------------------------
> 2 files changed, 27 insertions(+), 32 deletions(-)
>
> 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;
> + }
> return dev;
>
> free_all:
> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
> index cbf033d..aef0162 100644
> --- a/net/core/ethtool.c
> +++ b/net/core/ethtool.c
> @@ -175,7 +175,7 @@ static int __ethtool_get_sset_count(struct net_device *dev, int sset)
> if (sset == ETH_SS_FEATURES)
> return ARRAY_SIZE(netdev_features_strings);
>
> - if (ops && ops->get_sset_count && ops->get_strings)
> + if (ops->get_sset_count && ops->get_strings)
> return ops->get_sset_count(dev, sset);
> else
> return -EOPNOTSUPP;
> @@ -311,7 +311,7 @@ int __ethtool_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
> {
> ASSERT_RTNL();
>
> - if (!dev->ethtool_ops || !dev->ethtool_ops->get_settings)
> + if (!dev->ethtool_ops->get_settings)
> return -EOPNOTSUPP;
>
> memset(cmd, 0, sizeof(struct ethtool_cmd));
> @@ -355,7 +355,7 @@ static noinline_for_stack int ethtool_get_drvinfo(struct net_device *dev,
>
> memset(&info, 0, sizeof(info));
> info.cmd = ETHTOOL_GDRVINFO;
> - if (ops && ops->get_drvinfo) {
> + if (ops->get_drvinfo) {
> ops->get_drvinfo(dev, &info);
> } else if (dev->dev.parent && dev->dev.parent->driver) {
> strlcpy(info.bus_info, dev_name(dev->dev.parent),
> @@ -370,7 +370,7 @@ static noinline_for_stack int ethtool_get_drvinfo(struct net_device *dev,
> * this method of obtaining string set info is deprecated;
> * Use ETHTOOL_GSSET_INFO instead.
> */
> - if (ops && ops->get_sset_count) {
> + if (ops->get_sset_count) {
> int rc;
>
> rc = ops->get_sset_count(dev, ETH_SS_TEST);
> @@ -383,9 +383,9 @@ static noinline_for_stack int ethtool_get_drvinfo(struct net_device *dev,
> if (rc >= 0)
> info.n_priv_flags = rc;
> }
> - if (ops && ops->get_regs_len)
> + if (ops->get_regs_len)
> info.regdump_len = ops->get_regs_len(dev);
> - if (ops && ops->get_eeprom_len)
> + if (ops->get_eeprom_len)
> info.eedump_len = ops->get_eeprom_len(dev);
>
> if (copy_to_user(useraddr, &info, sizeof(info)))
> @@ -1275,7 +1275,7 @@ static int ethtool_get_dump_flag(struct net_device *dev,
> struct ethtool_dump dump;
> const struct ethtool_ops *ops = dev->ethtool_ops;
>
> - if (!dev->ethtool_ops->get_dump_flag)
> + if (!ops->get_dump_flag)
> return -EOPNOTSUPP;
>
> if (copy_from_user(&dump, useraddr, sizeof(dump)))
> @@ -1299,8 +1299,7 @@ static int ethtool_get_dump_data(struct net_device *dev,
> const struct ethtool_ops *ops = dev->ethtool_ops;
> void *data = NULL;
>
> - if (!dev->ethtool_ops->get_dump_data ||
> - !dev->ethtool_ops->get_dump_flag)
> + if (!ops->get_dump_data || !ops->get_dump_flag)
> return -EOPNOTSUPP;
>
> if (copy_from_user(&dump, useraddr, sizeof(dump)))
> @@ -1349,7 +1348,7 @@ static int ethtool_get_ts_info(struct net_device *dev, void __user *useraddr)
>
> err = phydev->drv->ts_info(phydev, &info);
>
> - } else if (dev->ethtool_ops && dev->ethtool_ops->get_ts_info) {
> + } else if (ops->get_ts_info) {
>
> err = ops->get_ts_info(dev, &info);
>
> @@ -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.
> + */
> 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;
> if (copy_from_user(ðcmd, useraddr, sizeof(ethcmd)))
> return -EFAULT;
>
> - if (!dev->ethtool_ops) {
> - /* A few commands do not require any driver support,
> - * are unprivileged, and do not change anything, so we
> - * can take a shortcut to them. */
> - if (ethcmd == ETHTOOL_GDRVINFO)
> - return ethtool_get_drvinfo(dev, useraddr);
> - else if (ethcmd == ETHTOOL_GET_TS_INFO)
> - return ethtool_get_ts_info(dev, useraddr);
> - else
> - return -EOPNOTSUPP;
> - }
> -
> /* Allow some commands to be done by anyone */
> switch (ethcmd) {
> case ETHTOOL_GSET:
> @@ -1476,8 +1466,8 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
> return -EPERM;
> }
>
> - if (dev->ethtool_ops->begin) {
> - rc = dev->ethtool_ops->begin(dev);
> + if (ops->begin) {
> + rc = ops->begin(dev);
> if (rc < 0)
> return rc;
> }
> @@ -1504,11 +1494,11 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
> break;
> case ETHTOOL_GMSGLVL:
> rc = ethtool_get_value(dev, useraddr, ethcmd,
> - dev->ethtool_ops->get_msglevel);
> + ops->get_msglevel);
> break;
> case ETHTOOL_SMSGLVL:
> rc = ethtool_set_value_void(dev, useraddr,
> - dev->ethtool_ops->set_msglevel);
> + ops->set_msglevel);
> break;
> case ETHTOOL_GEEE:
> rc = ethtool_get_eee(dev, useraddr);
> @@ -1570,11 +1560,11 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
> break;
> case ETHTOOL_GPFLAGS:
> rc = ethtool_get_value(dev, useraddr, ethcmd,
> - dev->ethtool_ops->get_priv_flags);
> + ops->get_priv_flags);
> break;
> case ETHTOOL_SPFLAGS:
> rc = ethtool_set_value(dev, useraddr,
> - dev->ethtool_ops->set_priv_flags);
> + ops->set_priv_flags);
> break;
> case ETHTOOL_GRXFH:
> case ETHTOOL_GRXRINGS:
> @@ -1655,8 +1645,8 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
> rc = -EOPNOTSUPP;
> }
>
> - if (dev->ethtool_ops->complete)
> - dev->ethtool_ops->complete(dev);
> + if (ops->complete)
> + ops->complete(dev);
>
> if (old_features != dev->features)
> netdev_features_change(dev);
>
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists