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: <20101219005404.GB12005@rere.qmqm.pl> Date: Sun, 19 Dec 2010 01:54:04 +0100 From: Michał Mirosław <mirq-linux@...e.qmqm.pl> To: Ben Hutchings <bhutchings@...arflare.com> Cc: netdev@...r.kernel.org Subject: Re: [RFC PATCH 03/12] net: ethtool: use ndo_fix_features for offload setting On Thu, Dec 16, 2010 at 11:23:49PM +0000, Ben Hutchings wrote: > On Wed, 2010-12-15 at 23:24 +0100, Michał Mirosław wrote: > > Signed-off-by: Michał Mirosław <mirq-linux@...e.qmqm.pl> > > --- > > include/linux/netdevice.h | 2 + > > net/core/dev.c | 8 ++ > > net/core/ethtool.c | 252 +++++++++++++++++++------------------------- > > 3 files changed, 119 insertions(+), 143 deletions(-) > > > > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h > > index 4b20944..7634cab 100644 > > --- a/include/linux/netdevice.h > > +++ b/include/linux/netdevice.h > > @@ -941,6 +941,8 @@ struct net_device { > > #define NETIF_F_V6_CSUM (NETIF_F_GEN_CSUM | NETIF_F_IPV6_CSUM) > > #define NETIF_F_ALL_CSUM (NETIF_F_V4_CSUM | NETIF_F_V6_CSUM) > > > > +#define NETIF_F_ALL_TSO (NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_TSO_ECN) > > + > > /* > > * If one device supports one of these features, then enable them > > * for all in netdev_increment_features. > > diff --git a/net/core/dev.c b/net/core/dev.c > > index 1e616bb..95d0a49 100644 > > --- a/net/core/dev.c > > +++ b/net/core/dev.c > > @@ -5054,6 +5054,14 @@ unsigned long netdev_fix_features(unsigned long features, const char *name) > > features &= ~NETIF_F_TSO; > > } > > > > + /* Software GSO depends on SG. */ > > + if ((features & NETIF_F_GSO) && !(features & NETIF_F_SG)) { > > + if (name) > > + printk(KERN_NOTICE "%s: Dropping NETIF_F_GSO since no " > > + "SG feature.\n", name); > > + features &= ~NETIF_F_GSO; > > + } > > + > > if (features & NETIF_F_UFO) { > > /* maybe split UFO into V4 and V6? */ > > if (!((features & NETIF_F_GEN_CSUM) || > The severity of these messages will need to be reduced if ethtool relies > on this function to propagate feature changes. (And I wonder why some > of them are ERR and some NOTICE.) Patch is ready (all converted to netdev_info). > > diff --git a/net/core/ethtool.c b/net/core/ethtool.c > > index f08e7f1..b068738 100644 > > --- a/net/core/ethtool.c > > +++ b/net/core/ethtool.c > > @@ -55,6 +55,7 @@ int ethtool_op_set_tx_csum(struct net_device *dev, u32 data) > > > > return 0; > > } > > +EXPORT_SYMBOL(ethtool_op_set_tx_csum); > > > > int ethtool_op_set_tx_hw_csum(struct net_device *dev, u32 data) > > { > > @@ -220,6 +221,85 @@ static int ethtool_set_features(struct net_device *dev, void __user *useraddr) > > return 0; > > } > > > > +static unsigned long ethtool_get_feature_mask(u32 eth_cmd) > > +{ > > + switch (eth_cmd) { > > + case ETHTOOL_GTXCSUM: > > + case ETHTOOL_STXCSUM: > > + return NETIF_F_ALL_CSUM|NETIF_F_SCTP_CSUM; > I wonder whether this should cover NETIF_F_FCOE_CRC as well (ixgbe > currently doesn't seem to allow toggling it). This mask is only for 'legacy' set_tx_csum ethtool commands. Other checksumming options - or whatever bits are included in hw_features - can be toggled by ETHTOOL_SFEATURES. > There should be spaces around the '|' operator. Fixed. > > +static int __ethtool_set_tx_csum(struct net_device *dev, u32 data); > > +static int __ethtool_set_sg(struct net_device *dev, u32 data); > > +static int __ethtool_set_tso(struct net_device *dev, u32 data); > > +static int __ethtool_set_ufo(struct net_device *dev, u32 data); > > + > > +static int ethtool_set_one_feature(struct net_device *dev, > > + void __user *useraddr, u32 ethcmd) > > +{ > > + struct ethtool_value edata; > > + unsigned long mask; > > + > > + if (copy_from_user(&edata, useraddr, sizeof(edata))) > > + return -EFAULT; > > + > > + mask = ethtool_get_feature_mask(ethcmd); > > + mask &= dev->hw_features | NETIF_F_SOFT_FEATURES; > > + if (mask) { > > + if (edata.data) > > + dev->wanted_features |= mask; > > + else > > + dev->wanted_features &= ~mask; > > + > > + netdev_update_features(dev); > > + return 0; > > + } > > + > > + switch (ethcmd) { > > + case ETHTOOL_STXCSUM: > > + return __ethtool_set_tx_csum(dev, edata.data); > > + case ETHTOOL_SSG: > > + return __ethtool_set_sg(dev, edata.data); > > + case ETHTOOL_STSO: > > + return __ethtool_set_tso(dev, edata.data); > > + case ETHTOOL_SUFO: > > + return __ethtool_set_ufo(dev, edata.data); > > + default: > > + return -EOPNOTSUPP; > > + } > > +} > [...] > > This deserves some comments. Sure. This is compatibility layer for current ethtool tools, and drivers not converted to ndo_fix_features() and friends. Best Regards, Michał Mirosław -- 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