From patchwork Fri Jan 9 05:03:57 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Garzik X-Patchwork-Id: 17468 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by ozlabs.org (Postfix) with ESMTP id 1E677DE89C for ; Fri, 9 Jan 2009 16:04:15 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751219AbZAIFEM (ORCPT ); Fri, 9 Jan 2009 00:04:12 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1750994AbZAIFEJ (ORCPT ); Fri, 9 Jan 2009 00:04:09 -0500 Received: from srv5.dvmed.net ([207.36.208.214]:59323 "EHLO mail.dvmed.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750898AbZAIFEI (ORCPT ); Fri, 9 Jan 2009 00:04:08 -0500 Received: from cpe-069-134-158-197.nc.res.rr.com ([69.134.158.197] helo=core.yyz.us) by mail.dvmed.net with esmtpsa (Exim 4.69 #1 (Red Hat Linux)) id 1LL9XL-0002iW-7N; Fri, 09 Jan 2009 05:04:01 +0000 Message-ID: <4966DABD.1000203@pobox.com> Date: Fri, 09 Jan 2009 00:03:57 -0500 From: Jeff Garzik User-Agent: Thunderbird 2.0.0.18 (X11/20081119) MIME-Version: 1.0 To: Herbert Xu CC: bhutchings@solarflare.com, rick.jones2@hp.com, davem@davemloft.net, netdev@vger.kernel.org Subject: Re: [PATCH] Make possible speeds known to ethtool References: <20090109041952.GA12087@gondor.apana.org.au> In-Reply-To: <20090109041952.GA12087@gondor.apana.org.au> X-Spam-Score: -4.4 (----) X-Spam-Report: SpamAssassin version 3.2.5 on srv5.dvmed.net summary: Content analysis details: (-4.4 points, 5.0 required) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Herbert Xu wrote: > Jeff Garzik wrote: >> For generic net stack flags outside the driver's control, that can >> easily be added to ethtool_{get,set}_flags() overriding or ignoring >> whatever the driver may have done. > > To use the flags interface as is, you'd have to go through every > single driver to get them to call ethtool_op_set_flags. I'm sorry > but I'm sticking with the current interface. I think you misunderstand. You don't have touch any drivers at all... see attached demonstration patch. The more general point is that it is silly to add two ethtool ioctls each time you want to twiddle a single boolean flag (whatever that flag may be, generic or driver-specific or whatnot). If you still desire separation from ->{get,set}_flags() ops, then at least create an ETHTOOL_[GS]STACK_FLAGS. Jeff diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h index 27c67a5..75fab70 100644 --- a/include/linux/ethtool.h +++ b/include/linux/ethtool.h @@ -285,6 +285,7 @@ struct ethtool_perm_addr { */ enum ethtool_flags { ETH_FLAG_LRO = (1 << 15), /* LRO is enabled */ + ETH_FLAG_GRO = (1 << 14), /* GRO is enabled */ }; struct ethtool_rxnfc { diff --git a/net/core/ethtool.c b/net/core/ethtool.c index 947710a..a114afa 100644 --- a/net/core/ethtool.c +++ b/net/core/ethtool.c @@ -864,6 +864,60 @@ static int ethtool_set_value(struct net_device *dev, char __user *useraddr, return actor(dev, edata.data); } +static int ethtool_get_generic_flags(struct net_device *dev, u32 *val_out) +{ + if (dev->features & NETIF_F_GRO) + *val_out |= ETH_FLAG_GRO; + + return 0; +} + +static int ethtool_set_generic_flags(struct net_device *dev, u32 val_in) +{ + if (val_in & ETH_FLAG_GRO) { + 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_get_flags(struct net_device *dev, char __user *useraddr) +{ + struct ethtool_value edata = { ETHTOOL_GFLAGS }; + int rc; + + if (dev->ethtool_ops->get_flags) + edata.data = dev->ethtool_ops->get_flags(dev); + + rc = ethtool_get_generic_flags(dev, &edata.data); + if (rc) + return rc; + + if (copy_to_user(useraddr, &edata, sizeof(edata))) + return -EFAULT; + return 0; +} + +static int ethtool_set_flags(struct net_device *dev, char __user *useraddr) +{ + struct ethtool_value edata; + int rc; + + if (copy_from_user(&edata, useraddr, sizeof(edata))) + return -EFAULT; + + rc = ethtool_set_generic_flags(dev, edata.data); + + if (rc == 0 && dev->ethtool_ops->set_flags) + rc = dev->ethtool_ops->set_flags(dev, edata.data); + + return rc; +} + /* The main entry point in this file. Called from net/core/dev.c */ int dev_ethtool(struct net *net, struct ifreq *ifr) @@ -1036,12 +1090,10 @@ int dev_ethtool(struct net *net, struct ifreq *ifr) rc = ethtool_set_gso(dev, useraddr); break; case ETHTOOL_GFLAGS: - rc = ethtool_get_value(dev, useraddr, ethcmd, - dev->ethtool_ops->get_flags); + rc = ethtool_get_flags(dev, useraddr); break; case ETHTOOL_SFLAGS: - rc = ethtool_set_value(dev, useraddr, - dev->ethtool_ops->set_flags); + rc = ethtool_set_flags(dev, useraddr); break; case ETHTOOL_GPFLAGS: rc = ethtool_get_value(dev, useraddr, ethcmd,