| Message ID | 37d3c96bc52c87fcd4cdd39c9c852b1d2bbc249d.1300466356.git.mirq-linux@rere.qmqm.pl |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Fri, 2011-03-18 at 18:42 +0100, Michał Mirosław wrote: > Implement compatibility with new hw_features for dev_disable_lro(). Good point, but... > This is a transition path - dev_disable_lro() should be later > integrated into netdev_fix_features() after all drivers are converted. > > There's no point in exporting __ethtool_set_flags() as it and dev_disable_lro() > are always built-in. > > Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl> > --- > > Patch is build-tested only, as I don't have any LRO-capable hardware. > It might be prettier to move dev_disable_lro() to ethtool.c. It *looks* right. > net/core/dev.c | 20 ++++++++++++-------- > net/core/ethtool.c | 2 +- > 2 files changed, 13 insertions(+), 9 deletions(-) > > diff --git a/net/core/dev.c b/net/core/dev.c > index 0b88eba..105e082 100644 > --- a/net/core/dev.c > +++ b/net/core/dev.c > @@ -1353,14 +1353,18 @@ EXPORT_SYMBOL(dev_close); > */ > void dev_disable_lro(struct net_device *dev) > { > - if (dev->ethtool_ops && dev->ethtool_ops->get_flags && > - dev->ethtool_ops->set_flags) { > - u32 flags = dev->ethtool_ops->get_flags(dev); > - if (flags & ETH_FLAG_LRO) { > - flags &= ~ETH_FLAG_LRO; > - dev->ethtool_ops->set_flags(dev, flags); > - } > - } > + extern int __ethtool_set_flags(struct net_device *, u32); [...] Local function declarations are a very bad idea, as they are not checked against the function definition. Please declare the function in <linux/ethtool.h>; that does not oblige us to export it. Ben.
From: Ben Hutchings <bhutchings@solarflare.com> Date: Fri, 18 Mar 2011 18:13:06 +0000 > Local function declarations are a very bad idea, as they are not checked > against the function definition. Please declare the function in > <linux/ethtool.h>; that does not oblige us to export it. Agreed. -- 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/net/core/dev.c b/net/core/dev.c index 0b88eba..105e082 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -1353,14 +1353,18 @@ EXPORT_SYMBOL(dev_close); */ void dev_disable_lro(struct net_device *dev) { - if (dev->ethtool_ops && dev->ethtool_ops->get_flags && - dev->ethtool_ops->set_flags) { - u32 flags = dev->ethtool_ops->get_flags(dev); - if (flags & ETH_FLAG_LRO) { - flags &= ~ETH_FLAG_LRO; - dev->ethtool_ops->set_flags(dev, flags); - } - } + extern int __ethtool_set_flags(struct net_device *, u32); + u32 flags; + + if (dev->ethtool_ops && dev->ethtool_ops->get_flags) + flags = dev->ethtool_ops->get_flags(dev); + else + flags = ethtool_op_get_flags(dev); + + if (!(flags & ETH_FLAG_LRO)) + return; + + __ethtool_set_flags(dev, flags & ~ETH_FLAG_LRO); WARN_ON(dev->features & NETIF_F_LRO); } EXPORT_SYMBOL(dev_disable_lro); diff --git a/net/core/ethtool.c b/net/core/ethtool.c index c1a71bb..7371177 100644 --- a/net/core/ethtool.c +++ b/net/core/ethtool.c @@ -513,7 +513,7 @@ static int ethtool_set_one_feature(struct net_device *dev, } } -static int __ethtool_set_flags(struct net_device *dev, u32 data) +int __ethtool_set_flags(struct net_device *dev, u32 data) { u32 changed;
Implement compatibility with new hw_features for dev_disable_lro(). This is a transition path - dev_disable_lro() should be later integrated into netdev_fix_features() after all drivers are converted. There's no point in exporting __ethtool_set_flags() as it and dev_disable_lro() are always built-in. Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl> --- Patch is build-tested only, as I don't have any LRO-capable hardware. It might be prettier to move dev_disable_lro() to ethtool.c. net/core/dev.c | 20 ++++++++++++-------- net/core/ethtool.c | 2 +- 2 files changed, 13 insertions(+), 9 deletions(-)