| Message ID | E1LB0d2-0000rq-Dr@gondolin.me.apana.org.au |
|---|---|
| State | Superseded, archived |
| Delegated to: | David Miller |
| Headers | show |
On Fri, 2008-12-12 at 16:31 +1100, Herbert Xu wrote: > ethtool: Add GGRO and SGRO ops > > This patch adds the ethtool ops to enable and disable GRO. It also > makes GRO depend on RX checksum offload much the same as how TSO > depends on SG support. [...] Why don't you add NETIF_F_GRO to the flags handled by get_flags() and set_flags()? Ben.
On Fri, Dec 12, 2008 at 08:11:17PM +0000, Ben Hutchings wrote: > On Fri, 2008-12-12 at 16:31 +1100, Herbert Xu wrote: > > ethtool: Add GGRO and SGRO ops > > > > This patch adds the ethtool ops to enable and disable GRO. It also > > makes GRO depend on RX checksum offload much the same as how TSO > > depends on SG support. > [...] > > Why don't you add NETIF_F_GRO to the flags handled by get_flags() and > set_flags()? Surely the patch itself has answered this :) It's because of the dependency on RX checksum offload. We want GRO to be off unless RX checksum offload is on. Cheers,
On Sat, 2008-12-13 at 08:48 +1100, Herbert Xu wrote: > On Fri, Dec 12, 2008 at 08:11:17PM +0000, Ben Hutchings wrote: > > On Fri, 2008-12-12 at 16:31 +1100, Herbert Xu wrote: > > > ethtool: Add GGRO and SGRO ops > > > > > > This patch adds the ethtool ops to enable and disable GRO. It also > > > makes GRO depend on RX checksum offload much the same as how TSO > > > depends on SG support. > > [...] > > > > Why don't you add NETIF_F_GRO to the flags handled by get_flags() and > > set_flags()? > > Surely the patch itself has answered this :) It's because of the > dependency on RX checksum offload. We want GRO to be off unless > RX checksum offload is on. If I'm not mistaken, the whole point of set_flags() is to end the continued expansion of struct ethtool_ops by another 2 functions for every new offload feature. The comments make it fairly clear that Jeff anticipated that it might be necessary to include such checks for some flags. Ben.
On Fri, Dec 12, 2008 at 10:35:27PM +0000, Ben Hutchings wrote: > > If I'm not mistaken, the whole point of set_flags() is to end the > continued expansion of struct ethtool_ops by another 2 functions for > every new offload feature. The comments make it fairly clear that Jeff > anticipated that it might be necessary to include such checks for some > flags. GRO is purely a stack feature so it doesn't need anything in ethtool_ops at all. Cheers,
On Fri, 12 Dec 2008, Herbert Xu wrote: > On Fri, Dec 12, 2008 at 10:35:27PM +0000, Ben Hutchings wrote: > > > > If I'm not mistaken, the whole point of set_flags() is to end the > > continued expansion of struct ethtool_ops by another 2 functions for > > every new offload feature. The comments make it fairly clear that Jeff > > anticipated that it might be necessary to include such checks for some > > flags. > > GRO is purely a stack feature so it doesn't need anything in > ethtool_ops at all. I'm confused then. You're adding two ethtool entry points with ETHTOOL_GGRO and ETHTOOL_SGRO, adding the callpoints in dev_ethtool with set_gro and get_gro, but how do you manipulate this without adding to the userspace application? Adding this functionality to the set/get_flags will keep the userspace app from needing a patch to support the new callbacks. Cheers, -PJ Waskiewicz -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h index b4b038b..27c67a5 100644 --- a/include/linux/ethtool.h +++ b/include/linux/ethtool.h @@ -467,6 +467,8 @@ struct ethtool_ops { #define ETHTOOL_GRXFH 0x00000029 /* Get RX flow hash configuration */ #define ETHTOOL_SRXFH 0x0000002a /* Set RX flow hash configuration */ +#define ETHTOOL_GGRO 0x0000002b /* Get GRO enable (ethtool_value) */ +#define ETHTOOL_SGRO 0x0000002c /* Set GRO enable (ethtool_value) */ /* compatibility with older code */ #define SPARC_ETH_GSET ETHTOOL_GSET diff --git a/net/core/ethtool.c b/net/core/ethtool.c index 14ada53..947710a 100644 --- a/net/core/ethtool.c +++ b/net/core/ethtool.c @@ -528,6 +528,22 @@ static int ethtool_set_tx_csum(struct net_device *dev, char __user *useraddr) return dev->ethtool_ops->set_tx_csum(dev, edata.data); } +static int ethtool_set_rx_csum(struct net_device *dev, char __user *useraddr) +{ + struct ethtool_value edata; + + if (!dev->ethtool_ops->set_rx_csum) + return -EOPNOTSUPP; + + if (copy_from_user(&edata, useraddr, sizeof(edata))) + return -EFAULT; + + if (!edata.data && dev->ethtool_ops->set_sg) + dev->features &= ~NETIF_F_GRO; + + return dev->ethtool_ops->set_rx_csum(dev, edata.data); +} + static int ethtool_set_sg(struct net_device *dev, char __user *useraddr) { struct ethtool_value edata; @@ -599,6 +615,34 @@ static int ethtool_set_gso(struct net_device *dev, char __user *useraddr) return 0; } +static int ethtool_get_gro(struct net_device *dev, char __user *useraddr) +{ + struct ethtool_value edata = { ETHTOOL_GGRO }; + + edata.data = dev->features & NETIF_F_GRO; + if (copy_to_user(useraddr, &edata, sizeof(edata))) + return -EFAULT; + return 0; +} + +static int ethtool_set_gro(struct net_device *dev, char __user *useraddr) +{ + struct ethtool_value edata; + + if (copy_from_user(&edata, useraddr, sizeof(edata))) + return -EFAULT; + + if (edata.data) { + if (!dev->ethtool_ops->get_rx_csum || + !dev->ethtool_ops->get_rx_csum(dev)) + return -EINVAL; + dev->features |= NETIF_F_GRO; + } else + dev->features &= ~NETIF_F_GRO; + + return 0; +} + static int ethtool_self_test(struct net_device *dev, char __user *useraddr) { struct ethtool_test test; @@ -932,8 +976,7 @@ int dev_ethtool(struct net *net, struct ifreq *ifr) dev->ethtool_ops->get_rx_csum); break; case ETHTOOL_SRXCSUM: - rc = ethtool_set_value(dev, useraddr, - dev->ethtool_ops->set_rx_csum); + rc = ethtool_set_rx_csum(dev, useraddr); break; case ETHTOOL_GTXCSUM: rc = ethtool_get_value(dev, useraddr, ethcmd, @@ -1014,6 +1057,12 @@ int dev_ethtool(struct net *net, struct ifreq *ifr) case ETHTOOL_SRXFH: rc = ethtool_set_rxhash(dev, useraddr); break; + case ETHTOOL_GGRO: + rc = ethtool_get_gro(dev, useraddr); + break; + case ETHTOOL_SGRO: + rc = ethtool_set_gro(dev, useraddr); + break; default: rc = -EOPNOTSUPP; }
ethtool: Add GGRO and SGRO ops This patch adds the ethtool ops to enable and disable GRO. It also makes GRO depend on RX checksum offload much the same as how TSO depends on SG support. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> --- include/linux/ethtool.h | 2 + net/core/ethtool.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 53 insertions(+), 2 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html