* net: enable dynamic lro disabling for vlans
@ 2011-05-24 17:15 Neil Horman
2011-05-24 17:15 ` [PATCH 1/2] net: add dev_ethtool_set_flags helper Neil Horman
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: Neil Horman @ 2011-05-24 17:15 UTC (permalink / raw)
To: netdev; +Cc: Neil Horman, davem
Hey there-
Noted recently that, while physical devices have lro disabled when
attached to a bridge, vlan devices do not. This is because the vlan netdev has
no get/set flags method in its ethtool_ops struct. This series adds those
methods as passthrough calls to the underlying physical devices, so that whe
dev_disable_lro is called on a vlan device, the physical device underneath has
lro properly disabled.
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: davem@davemloft.net
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/2] net: add dev_ethtool_set_flags helper
2011-05-24 17:15 net: enable dynamic lro disabling for vlans Neil Horman
@ 2011-05-24 17:15 ` Neil Horman
2011-05-24 17:15 ` [PATCH 2/2] net: add passthrough ethtool commands for get/set flags to vlan dev Neil Horman
` (2 subsequent siblings)
3 siblings, 0 replies; 14+ messages in thread
From: Neil Horman @ 2011-05-24 17:15 UTC (permalink / raw)
To: netdev; +Cc: Neil Horman, davem
Add a helper to check for presence of a set_flags method in an ethtool_ops
structure and conditionally call it
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: davem@davemloft.net
---
include/linux/netdevice.h | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index ca333e7..243781c 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2624,6 +2624,13 @@ static inline u32 dev_ethtool_get_flags(struct net_device *dev)
return dev->ethtool_ops->get_flags(dev);
}
+static inline int dev_ethtool_set_flags(struct net_device *dev, u32 flags)
+{
+ if (!dev->ethtool_ops || !dev->ethtool_ops->set_flags)
+ return 0;
+ return dev->ethtool_ops->set_flags(dev, flags);
+}
+
/* Logging, debugging and troubleshooting/diagnostic helpers. */
/* netdev_printk helpers, similar to dev_printk */
--
1.7.5.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/2] net: add passthrough ethtool commands for get/set flags to vlan dev
2011-05-24 17:15 net: enable dynamic lro disabling for vlans Neil Horman
2011-05-24 17:15 ` [PATCH 1/2] net: add dev_ethtool_set_flags helper Neil Horman
@ 2011-05-24 17:15 ` Neil Horman
2011-05-24 17:22 ` net: enable dynamic lro disabling for vlans Ben Hutchings
2011-05-24 18:31 ` net: enable dynamic lro disabling for vlans (v2) Neil Horman
3 siblings, 0 replies; 14+ messages in thread
From: Neil Horman @ 2011-05-24 17:15 UTC (permalink / raw)
To: netdev; +Cc: Neil Horman, davem
Add ethtool command to the vlan driver so that when dev_disable_lro is called on
a vlan device, the disablement can be correctly passed down to the underlying
hardware
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: davem@davemloft.net
---
net/8021q/vlan_dev.c | 14 ++++++++++++++
1 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index f247f5b..611839b 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -648,10 +648,24 @@ static struct rtnl_link_stats64 *vlan_dev_get_stats64(struct net_device *dev, st
return stats;
}
+static u32 vlan_ethtool_get_flags (struct net_device *vdev)
+{
+ const struct vlan_dev_info *vlan = vlan_dev_info(vdev);
+ return dev_ethtool_get_flags(vlan->real_dev);
+}
+
+static int vlan_ethtool_set_flags(struct net_device *vdev, u32 flags)
+{
+ const struct vlan_dev_info *vlan = vlan_dev_info(vdev);
+ return dev_ethtool_set_flags(vlan->real_dev, flags);
+}
+
static const struct ethtool_ops vlan_ethtool_ops = {
.get_settings = vlan_ethtool_get_settings,
.get_drvinfo = vlan_ethtool_get_drvinfo,
.get_link = ethtool_op_get_link,
+ .get_flags = vlan_ethtool_get_flags,
+ .set_flags = vlan_ethtool_set_flags,
};
static const struct net_device_ops vlan_netdev_ops = {
--
1.7.5.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: net: enable dynamic lro disabling for vlans
2011-05-24 17:15 net: enable dynamic lro disabling for vlans Neil Horman
2011-05-24 17:15 ` [PATCH 1/2] net: add dev_ethtool_set_flags helper Neil Horman
2011-05-24 17:15 ` [PATCH 2/2] net: add passthrough ethtool commands for get/set flags to vlan dev Neil Horman
@ 2011-05-24 17:22 ` Ben Hutchings
2011-05-24 17:54 ` Neil Horman
2011-05-24 18:31 ` net: enable dynamic lro disabling for vlans (v2) Neil Horman
3 siblings, 1 reply; 14+ messages in thread
From: Ben Hutchings @ 2011-05-24 17:22 UTC (permalink / raw)
To: Neil Horman; +Cc: netdev, davem
On Tue, 2011-05-24 at 13:15 -0400, Neil Horman wrote:
> Hey there-
> Noted recently that, while physical devices have lro disabled when
> attached to a bridge, vlan devices do not.
Good point.
> This is because the vlan netdev has
> no get/set flags method in its ethtool_ops struct. This series adds those
> methods as passthrough calls to the underlying physical devices, so that whe
> dev_disable_lro is called on a vlan device, the physical device underneath has
> lro properly disabled.
But I don't think this is correct.
The get_flags() result should be masked with vlan_features. And
set_flags() shouldn't be allowed; the VLAN device should normally follow
the parent device and not the other way round. I think
dev_disable_lro() needs to handle VLAN devices explicitly, instead.
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: net: enable dynamic lro disabling for vlans
2011-05-24 17:22 ` net: enable dynamic lro disabling for vlans Ben Hutchings
@ 2011-05-24 17:54 ` Neil Horman
0 siblings, 0 replies; 14+ messages in thread
From: Neil Horman @ 2011-05-24 17:54 UTC (permalink / raw)
To: Ben Hutchings; +Cc: netdev, davem
On Tue, May 24, 2011 at 10:22:49AM -0700, Ben Hutchings wrote:
> On Tue, 2011-05-24 at 13:15 -0400, Neil Horman wrote:
> > Hey there-
> > Noted recently that, while physical devices have lro disabled when
> > attached to a bridge, vlan devices do not.
>
> Good point.
>
> > This is because the vlan netdev has
> > no get/set flags method in its ethtool_ops struct. This series adds those
> > methods as passthrough calls to the underlying physical devices, so that whe
> > dev_disable_lro is called on a vlan device, the physical device underneath has
> > lro properly disabled.
>
> But I don't think this is correct.
>
> The get_flags() result should be masked with vlan_features. And
> set_flags() shouldn't be allowed; the VLAN device should normally follow
> the parent device and not the other way round. I think
I'm not sure thats entirely true (see vlan_ethtool_get_settings, it does a
passthrough to the physical device just like I'm doing, although you're correct
we don't do any passthrough of the set methods).
> dev_disable_lro() needs to handle VLAN devices explicitly, instead.
>
I suppose that will work just as well as this, and likely will be more concise.
I'll respin shortly.
Best
Neil
> Ben.
>
> --
> Ben Hutchings, Senior Software Engineer, Solarflare
> Not speaking for my employer; that's the marketing department's job.
> They asked us to note that Solarflare product names are trademarked.
>
> --
> 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
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* net: enable dynamic lro disabling for vlans (v2)
2011-05-24 17:15 net: enable dynamic lro disabling for vlans Neil Horman
` (2 preceding siblings ...)
2011-05-24 17:22 ` net: enable dynamic lro disabling for vlans Ben Hutchings
@ 2011-05-24 18:31 ` Neil Horman
2011-05-24 18:31 ` [PATCH 1/2] net: move is_vlan_dev into public header file (v2) Neil Horman
2011-05-24 18:31 ` [PATCH 2/2] net: make dev_disable_lro use physical device if passed a vlan dev (v2) Neil Horman
3 siblings, 2 replies; 14+ messages in thread
From: Neil Horman @ 2011-05-24 18:31 UTC (permalink / raw)
To: netdev; +Cc: Neil Horman, davem, bhutchings
Noted recently that, while physical devices have lro disabled when
attached to a bridge, vlan devices do not. This is because the vlan netdev has
no get/set flags method in its ethtool_ops struct. This series teaches
dev_disable_lro to recognize vlan interfaces and operate on the underlying
physical device instead.
Change notes:
v2) Rewrite of my origional solution As per Ben H.- instead of passing ethtool
command from the vlan device to the physical device, instead teach
dev_disable_lro to recognize vlans and operate on the underlying physical device
directly. This lets us avoid the various oddities of operating with ethtool on
a vlan device, and is more consice to boot.
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: davem@davemloft.net
CC: bhutchings@solarflare.com
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/2] net: move is_vlan_dev into public header file (v2)
2011-05-24 18:31 ` net: enable dynamic lro disabling for vlans (v2) Neil Horman
@ 2011-05-24 18:31 ` Neil Horman
2011-05-24 18:36 ` Joe Perches
2011-05-25 21:56 ` David Miller
2011-05-24 18:31 ` [PATCH 2/2] net: make dev_disable_lro use physical device if passed a vlan dev (v2) Neil Horman
1 sibling, 2 replies; 14+ messages in thread
From: Neil Horman @ 2011-05-24 18:31 UTC (permalink / raw)
To: netdev; +Cc: Neil Horman, davem, bhutchings
Migrate is_vlan_dev() to if_vlan.h so that core networkig can use it
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: davem@davemloft.net
CC: bhutchings@solarflare.com
---
include/linux/if_vlan.h | 5 +++++
net/8021q/vlan.h | 5 -----
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index 290bd8a..dc01681 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -110,6 +110,11 @@ static inline void vlan_group_set_device(struct vlan_group *vg,
array[vlan_id % VLAN_GROUP_ARRAY_PART_LEN] = dev;
}
+static inline int is_vlan_dev(struct net_device *dev)
+{
+ return dev->priv_flags & IFF_802_1Q_VLAN;
+}
+
#define vlan_tx_tag_present(__skb) ((__skb)->vlan_tci & VLAN_TAG_PRESENT)
#define vlan_tx_tag_get(__skb) ((__skb)->vlan_tci & ~VLAN_TAG_PRESENT)
diff --git a/net/8021q/vlan.h b/net/8021q/vlan.h
index c3408de..9da07e3 100644
--- a/net/8021q/vlan.h
+++ b/net/8021q/vlan.h
@@ -118,11 +118,6 @@ extern void vlan_netlink_fini(void);
extern struct rtnl_link_ops vlan_link_ops;
-static inline int is_vlan_dev(struct net_device *dev)
-{
- return dev->priv_flags & IFF_802_1Q_VLAN;
-}
-
extern int vlan_net_id;
struct proc_dir_entry;
--
1.7.5.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/2] net: make dev_disable_lro use physical device if passed a vlan dev (v2)
2011-05-24 18:31 ` net: enable dynamic lro disabling for vlans (v2) Neil Horman
2011-05-24 18:31 ` [PATCH 1/2] net: move is_vlan_dev into public header file (v2) Neil Horman
@ 2011-05-24 18:31 ` Neil Horman
2011-05-25 21:56 ` David Miller
2011-05-28 3:11 ` Ben Hutchings
1 sibling, 2 replies; 14+ messages in thread
From: Neil Horman @ 2011-05-24 18:31 UTC (permalink / raw)
To: netdev; +Cc: Neil Horman, davem, bhutchings
If the device passed into dev_disable_lro is a vlan, then repoint the dev
poniter so that we actually modify the underlying physical device.
Signed-of-by: Neil Horman <nhorman@tuxdriver.com>
CC: davem@davemloft.net
CC: bhutchings@solarflare.com
---
net/core/dev.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index d945379..da16c6a 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1308,6 +1308,13 @@ void dev_disable_lro(struct net_device *dev)
{
u32 flags;
+ /*
+ * If we're trying to disable lro on a vlan device
+ * use the underlying physical device instead
+ */
+ if (is_vlan_dev(dev))
+ dev = vlan_dev_real_dev(dev);
+
if (dev->ethtool_ops && dev->ethtool_ops->get_flags)
flags = dev->ethtool_ops->get_flags(dev);
else
--
1.7.5.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] net: move is_vlan_dev into public header file (v2)
2011-05-24 18:31 ` [PATCH 1/2] net: move is_vlan_dev into public header file (v2) Neil Horman
@ 2011-05-24 18:36 ` Joe Perches
2011-05-24 19:03 ` Neil Horman
2011-05-25 21:56 ` David Miller
1 sibling, 1 reply; 14+ messages in thread
From: Joe Perches @ 2011-05-24 18:36 UTC (permalink / raw)
To: Neil Horman; +Cc: netdev, davem, bhutchings
On Tue, 2011-05-24 at 14:31 -0400, Neil Horman wrote:
> Migrate is_vlan_dev() to if_vlan.h so that core networkig can use it
[]
> diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
[]
> @@ -110,6 +110,11 @@ static inline void vlan_group_set_device(struct vlan_group *vg,
> array[vlan_id % VLAN_GROUP_ARRAY_PART_LEN] = dev;
> }
>
> +static inline int is_vlan_dev(struct net_device *dev)
> +{
> + return dev->priv_flags & IFF_802_1Q_VLAN;
> +}
perhaps:
static bool is_vlan_dev(const struct net_device *dev)
{
return !!(dev->priv_flags & IFF_802_1Q_VLAN);
}
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] net: move is_vlan_dev into public header file (v2)
2011-05-24 18:36 ` Joe Perches
@ 2011-05-24 19:03 ` Neil Horman
0 siblings, 0 replies; 14+ messages in thread
From: Neil Horman @ 2011-05-24 19:03 UTC (permalink / raw)
To: Joe Perches; +Cc: netdev, davem, bhutchings
On Tue, May 24, 2011 at 11:36:35AM -0700, Joe Perches wrote:
> On Tue, 2011-05-24 at 14:31 -0400, Neil Horman wrote:
> > Migrate is_vlan_dev() to if_vlan.h so that core networkig can use it
> []
> > diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
> []
> > @@ -110,6 +110,11 @@ static inline void vlan_group_set_device(struct vlan_group *vg,
> > array[vlan_id % VLAN_GROUP_ARRAY_PART_LEN] = dev;
> > }
> >
> > +static inline int is_vlan_dev(struct net_device *dev)
> > +{
> > + return dev->priv_flags & IFF_802_1Q_VLAN;
> > +}
>
> perhaps:
>
> static bool is_vlan_dev(const struct net_device *dev)
> {
> return !!(dev->priv_flags & IFF_802_1Q_VLAN);
> }
>
I migrated this directly out of vlan.h as is. I suppose we could change it, but
I'm not sure I see a need to do so immediately. All callers of this function
already work with it properly as defined currently (not that they wouldn't
otherwise).
Perhaps we could do a separate patch to fix this up as well as other common test
functions (br_port_exists, is_valid_iface, macvlan_port_exists,
netif_is_bond_slave, etc), all just return the result of a bitwise and
currently)
Neil
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] net: move is_vlan_dev into public header file (v2)
2011-05-24 18:31 ` [PATCH 1/2] net: move is_vlan_dev into public header file (v2) Neil Horman
2011-05-24 18:36 ` Joe Perches
@ 2011-05-25 21:56 ` David Miller
1 sibling, 0 replies; 14+ messages in thread
From: David Miller @ 2011-05-25 21:56 UTC (permalink / raw)
To: nhorman; +Cc: netdev, bhutchings
From: Neil Horman <nhorman@tuxdriver.com>
Date: Tue, 24 May 2011 14:31:08 -0400
> Migrate is_vlan_dev() to if_vlan.h so that core networkig can use it
>
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
Applied.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] net: make dev_disable_lro use physical device if passed a vlan dev (v2)
2011-05-24 18:31 ` [PATCH 2/2] net: make dev_disable_lro use physical device if passed a vlan dev (v2) Neil Horman
@ 2011-05-25 21:56 ` David Miller
2011-05-28 3:11 ` Ben Hutchings
1 sibling, 0 replies; 14+ messages in thread
From: David Miller @ 2011-05-25 21:56 UTC (permalink / raw)
To: nhorman; +Cc: netdev, bhutchings
From: Neil Horman <nhorman@tuxdriver.com>
Date: Tue, 24 May 2011 14:31:09 -0400
> If the device passed into dev_disable_lro is a vlan, then repoint the dev
> poniter so that we actually modify the underlying physical device.
>
> Signed-of-by: Neil Horman <nhorman@tuxdriver.com>
Applied.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] net: make dev_disable_lro use physical device if passed a vlan dev (v2)
2011-05-24 18:31 ` [PATCH 2/2] net: make dev_disable_lro use physical device if passed a vlan dev (v2) Neil Horman
2011-05-25 21:56 ` David Miller
@ 2011-05-28 3:11 ` Ben Hutchings
2011-05-31 0:06 ` Neil Horman
1 sibling, 1 reply; 14+ messages in thread
From: Ben Hutchings @ 2011-05-28 3:11 UTC (permalink / raw)
To: Neil Horman; +Cc: netdev, davem
On Tue, 2011-05-24 at 14:31 -0400, Neil Horman wrote:
> If the device passed into dev_disable_lro is a vlan, then repoint the dev
> poniter so that we actually modify the underlying physical device.
[...]
Thanks Neil, this looks good.
There seems to be a slightly weird bug remaining, in that NETIF_F_LRO
may remain set on the VLAN device. But that's really cosmetic and
should go away in 2.6.40 along with the old ethtool operations for
offload setting.
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] net: make dev_disable_lro use physical device if passed a vlan dev (v2)
2011-05-28 3:11 ` Ben Hutchings
@ 2011-05-31 0:06 ` Neil Horman
0 siblings, 0 replies; 14+ messages in thread
From: Neil Horman @ 2011-05-31 0:06 UTC (permalink / raw)
To: Ben Hutchings; +Cc: netdev, davem
On Sat, May 28, 2011 at 04:11:03AM +0100, Ben Hutchings wrote:
> On Tue, 2011-05-24 at 14:31 -0400, Neil Horman wrote:
> > If the device passed into dev_disable_lro is a vlan, then repoint the dev
> > poniter so that we actually modify the underlying physical device.
> [...]
>
> Thanks Neil, this looks good.
>
> There seems to be a slightly weird bug remaining, in that NETIF_F_LRO
> may remain set on the VLAN device. But that's really cosmetic and
> should go away in 2.6.40 along with the old ethtool operations for
> offload setting.
>
> Ben.
>
Copy that, thanks Ben.
Neil
> --
> Ben Hutchings, Senior Software Engineer, Solarflare
> Not speaking for my employer; that's the marketing department's job.
> They asked us to note that Solarflare product names are trademarked.
>
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2011-05-31 0:06 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-24 17:15 net: enable dynamic lro disabling for vlans Neil Horman
2011-05-24 17:15 ` [PATCH 1/2] net: add dev_ethtool_set_flags helper Neil Horman
2011-05-24 17:15 ` [PATCH 2/2] net: add passthrough ethtool commands for get/set flags to vlan dev Neil Horman
2011-05-24 17:22 ` net: enable dynamic lro disabling for vlans Ben Hutchings
2011-05-24 17:54 ` Neil Horman
2011-05-24 18:31 ` net: enable dynamic lro disabling for vlans (v2) Neil Horman
2011-05-24 18:31 ` [PATCH 1/2] net: move is_vlan_dev into public header file (v2) Neil Horman
2011-05-24 18:36 ` Joe Perches
2011-05-24 19:03 ` Neil Horman
2011-05-25 21:56 ` David Miller
2011-05-24 18:31 ` [PATCH 2/2] net: make dev_disable_lro use physical device if passed a vlan dev (v2) Neil Horman
2011-05-25 21:56 ` David Miller
2011-05-28 3:11 ` Ben Hutchings
2011-05-31 0:06 ` Neil Horman
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.