* [PATCH] ixgbe: add GRO to vlan_features
@ 2010-09-15 1:50 Brandon Philips
2010-09-15 4:35 ` David Miller
0 siblings, 1 reply; 22+ messages in thread
From: Brandon Philips @ 2010-09-15 1:50 UTC (permalink / raw)
To: jeffrey.t.kirsher, jesse.brandeburg, bruce.w.allan,
alexander.h.duyck, peter.p.waskiewicz.jr, john.ronciak,
donald.c.skidmo
Cc: e1000-devel, netdev, davem
When running a vlan -> bridge -> domU setup I was noticing poor RX
performance. It seems ixgbe and other drivers don't have NETIF_F_GRO set
in vlan_features and thus GRO is off by default for vlans.
I see no reason why the other drivers shouldn't be fixed. If this is OK
I will go off and fix the rest.
Signed-off-by: Brandon Philips <bphilips@suse.de>
---
drivers/net/ixgbe/ixgbe_main.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
index d03eef9..0b39757 100644
--- a/drivers/net/ixgbe/ixgbe_main.c
+++ b/drivers/net/ixgbe/ixgbe_main.c
@@ -6817,6 +6817,7 @@ static int __devinit ixgbe_probe(struct pci_dev *pdev,
netdev->vlan_features |= NETIF_F_IP_CSUM;
netdev->vlan_features |= NETIF_F_IPV6_CSUM;
netdev->vlan_features |= NETIF_F_SG;
+ netdev->vlan_features |= NETIF_F_GRO;
if (adapter->flags & IXGBE_FLAG_SRIOV_ENABLED)
adapter->flags &= ~(IXGBE_FLAG_RSS_ENABLED |
--
1.7.1
------------------------------------------------------------------------------
Start uncovering the many advantages of virtual appliances
and start using them to simplify application deployment and
accelerate your shift to cloud computing.
http://p.sf.net/sfu/novell-sfdev2dev
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH] ixgbe: add GRO to vlan_features
2010-09-15 1:50 [PATCH] ixgbe: add GRO to vlan_features Brandon Philips
@ 2010-09-15 4:35 ` David Miller
2010-09-15 16:37 ` [PATCH] vlan: enable GRO if real_dev supports it Brandon Philips
2010-09-15 16:38 ` [PATCH] ixgbe: add GRO to vlan_features Brandon Philips
0 siblings, 2 replies; 22+ messages in thread
From: David Miller @ 2010-09-15 4:35 UTC (permalink / raw)
To: bphilips
Cc: e1000-devel, bruce.w.allan, jesse.brandeburg, john.ronciak,
jeffrey.t.kirsher, netdev
From: Brandon Philips <bphilips@suse.de>
Date: Tue, 14 Sep 2010 18:50:53 -0700
> When running a vlan -> bridge -> domU setup I was noticing poor RX
> performance. It seems ixgbe and other drivers don't have NETIF_F_GRO set
> in vlan_features and thus GRO is off by default for vlans.
>
> I see no reason why the other drivers shouldn't be fixed. If this is OK
> I will go off and fix the rest.
>
> Signed-off-by: Brandon Philips <bphilips@suse.de>
I think it should be set, but why don't we do this another way?
This feature is a software feature, essentially, so if it is published
in netdev->features we can simply propagate it automatically to
netdev->vlan_features at device registry time in the core networking.
------------------------------------------------------------------------------
Start uncovering the many advantages of virtual appliances
and start using them to simplify application deployment and
accelerate your shift to cloud computing.
http://p.sf.net/sfu/novell-sfdev2dev
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] vlan: enable GRO if real_dev supports it
2010-09-15 4:35 ` David Miller
@ 2010-09-15 16:37 ` Brandon Philips
2010-09-15 17:08 ` Eric Dumazet
2010-09-15 16:38 ` [PATCH] ixgbe: add GRO to vlan_features Brandon Philips
1 sibling, 1 reply; 22+ messages in thread
From: Brandon Philips @ 2010-09-15 16:37 UTC (permalink / raw)
To: David Miller
Cc: jeffrey.t.kirsher, jesse.brandeburg, bruce.w.allan,
alexander.h.duyck, peter.p.waskiewicz.jr, john.ronciak,
donald.c.skidmore, netdev, e1000-devel
Currently vlan devices don't have GRO by default as none of the Ethernet
drivers add NETIF_F_GRO to their vlan_features.
As GRO is a software feature just propogate GRO from the real_dev in the
vlan core. There is no need to have the drivers each add NETIF_F_GRO to
their vlan_features.
Signed-off-by: Brandon Philips <bphilips@suse.de>
---
net/8021q/vlan_dev.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index 3bccdd1..6fbc445 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -735,6 +735,7 @@ static int vlan_dev_init(struct net_device *dev)
(1<<__LINK_STATE_DORMANT))) |
(1<<__LINK_STATE_PRESENT);
+ dev->features |= real_dev->features & NETIF_F_GRO;
dev->features |= real_dev->features & real_dev->vlan_features;
dev->gso_max_size = real_dev->gso_max_size;
--
1.7.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH] vlan: enable GRO if real_dev supports it
2010-09-15 16:37 ` [PATCH] vlan: enable GRO if real_dev supports it Brandon Philips
@ 2010-09-15 17:08 ` Eric Dumazet
2010-09-15 17:14 ` David Miller
2010-09-15 18:59 ` [PATCH] vlan: enable GRO if real_dev supports it Brandon Philips
0 siblings, 2 replies; 22+ messages in thread
From: Eric Dumazet @ 2010-09-15 17:08 UTC (permalink / raw)
To: Brandon Philips
Cc: David Miller, jeffrey.t.kirsher, jesse.brandeburg, bruce.w.allan,
alexander.h.duyck, peter.p.waskiewicz.jr, john.ronciak,
donald.c.skidmore, netdev, e1000-devel
Le mercredi 15 septembre 2010 à 09:37 -0700, Brandon Philips a écrit :
> Currently vlan devices don't have GRO by default as none of the Ethernet
> drivers add NETIF_F_GRO to their vlan_features.
>
> As GRO is a software feature just propogate GRO from the real_dev in the
> vlan core. There is no need to have the drivers each add NETIF_F_GRO to
> their vlan_features.
>
> Signed-off-by: Brandon Philips <bphilips@suse.de>
> ---
> net/8021q/vlan_dev.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
> index 3bccdd1..6fbc445 100644
> --- a/net/8021q/vlan_dev.c
> +++ b/net/8021q/vlan_dev.c
> @@ -735,6 +735,7 @@ static int vlan_dev_init(struct net_device *dev)
> (1<<__LINK_STATE_DORMANT))) |
> (1<<__LINK_STATE_PRESENT);
>
> + dev->features |= real_dev->features & NETIF_F_GRO;
> dev->features |= real_dev->features & real_dev->vlan_features;
> dev->gso_max_size = real_dev->gso_max_size;
>
Hmm, this is only part of a generic solution.
If I enable gro with "ethtool -K eth0 gro on", should we propagate GRO
on vlan eth0.555 ?
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] vlan: enable GRO if real_dev supports it
2010-09-15 17:08 ` Eric Dumazet
@ 2010-09-15 17:14 ` David Miller
2010-09-15 17:24 ` Ben Hutchings
2010-09-15 18:59 ` [PATCH] vlan: enable GRO if real_dev supports it Brandon Philips
1 sibling, 1 reply; 22+ messages in thread
From: David Miller @ 2010-09-15 17:14 UTC (permalink / raw)
To: eric.dumazet
Cc: bphilips, jeffrey.t.kirsher, jesse.brandeburg, bruce.w.allan,
alexander.h.duyck, peter.p.waskiewicz.jr, john.ronciak,
donald.c.skidmore, netdev, e1000-devel
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 15 Sep 2010 19:08:40 +0200
> Le mercredi 15 septembre 2010 à 09:37 -0700, Brandon Philips a écrit :
>> Currently vlan devices don't have GRO by default as none of the Ethernet
>> drivers add NETIF_F_GRO to their vlan_features.
>>
>> As GRO is a software feature just propogate GRO from the real_dev in the
>> vlan core. There is no need to have the drivers each add NETIF_F_GRO to
>> their vlan_features.
>>
>> Signed-off-by: Brandon Philips <bphilips@suse.de>
>> ---
>> net/8021q/vlan_dev.c | 1 +
>> 1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
>> index 3bccdd1..6fbc445 100644
>> --- a/net/8021q/vlan_dev.c
>> +++ b/net/8021q/vlan_dev.c
>> @@ -735,6 +735,7 @@ static int vlan_dev_init(struct net_device *dev)
>> (1<<__LINK_STATE_DORMANT))) |
>> (1<<__LINK_STATE_PRESENT);
>>
>> + dev->features |= real_dev->features & NETIF_F_GRO;
>> dev->features |= real_dev->features & real_dev->vlan_features;
>> dev->gso_max_size = real_dev->gso_max_size;
>>
>
> Hmm, this is only part of a generic solution.
>
> If I enable gro with "ethtool -K eth0 gro on", should we propagate GRO
> on vlan eth0.555 ?
I think a better way to implement this is to set NETIF_F_GRO in
netdev->vlan_features at register_netdev() time if it is set in
netdev->features
That is the implementation I was implying earlier in this thread.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] vlan: enable GRO if real_dev supports it
2010-09-15 17:14 ` David Miller
@ 2010-09-15 17:24 ` Ben Hutchings
2010-09-15 17:29 ` David Miller
2010-09-15 19:24 ` [PATCH] net: enable GRO by default for vlan devices Brandon Philips
0 siblings, 2 replies; 22+ messages in thread
From: Ben Hutchings @ 2010-09-15 17:24 UTC (permalink / raw)
To: David Miller
Cc: eric.dumazet, bphilips, jeffrey.t.kirsher, jesse.brandeburg,
bruce.w.allan, alexander.h.duyck, peter.p.waskiewicz.jr,
john.ronciak, donald.c.skidmore, netdev, e1000-devel
On Wed, 2010-09-15 at 10:14 -0700, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Wed, 15 Sep 2010 19:08:40 +0200
>
> > Le mercredi 15 septembre 2010 à 09:37 -0700, Brandon Philips a écrit :
> >> Currently vlan devices don't have GRO by default as none of the Ethernet
> >> drivers add NETIF_F_GRO to their vlan_features.
> >>
> >> As GRO is a software feature just propogate GRO from the real_dev in the
> >> vlan core. There is no need to have the drivers each add NETIF_F_GRO to
> >> their vlan_features.
> >>
> >> Signed-off-by: Brandon Philips <bphilips@suse.de>
> >> ---
> >> net/8021q/vlan_dev.c | 1 +
> >> 1 files changed, 1 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
> >> index 3bccdd1..6fbc445 100644
> >> --- a/net/8021q/vlan_dev.c
> >> +++ b/net/8021q/vlan_dev.c
> >> @@ -735,6 +735,7 @@ static int vlan_dev_init(struct net_device *dev)
> >> (1<<__LINK_STATE_DORMANT))) |
> >> (1<<__LINK_STATE_PRESENT);
> >>
> >> + dev->features |= real_dev->features & NETIF_F_GRO;
> >> dev->features |= real_dev->features & real_dev->vlan_features;
> >> dev->gso_max_size = real_dev->gso_max_size;
> >>
> >
> > Hmm, this is only part of a generic solution.
> >
> > If I enable gro with "ethtool -K eth0 gro on", should we propagate GRO
> > on vlan eth0.555 ?
>
> I think a better way to implement this is to set NETIF_F_GRO in
> netdev->vlan_features at register_netdev() time if it is set in
> netdev->features
>
> That is the implementation I was implying earlier in this thread.
We can even add it unconditionally there, since we only ever use
dev->features & dev->vlan_features.
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
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] 22+ messages in thread
* Re: [PATCH] vlan: enable GRO if real_dev supports it
2010-09-15 17:24 ` Ben Hutchings
@ 2010-09-15 17:29 ` David Miller
2010-09-15 19:24 ` [PATCH] net: enable GRO by default for vlan devices Brandon Philips
1 sibling, 0 replies; 22+ messages in thread
From: David Miller @ 2010-09-15 17:29 UTC (permalink / raw)
To: bhutchings
Cc: eric.dumazet, bphilips, jeffrey.t.kirsher, jesse.brandeburg,
bruce.w.allan, alexander.h.duyck, peter.p.waskiewicz.jr,
john.ronciak, donald.c.skidmore, netdev, e1000-devel
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Wed, 15 Sep 2010 18:24:36 +0100
> On Wed, 2010-09-15 at 10:14 -0700, David Miller wrote:
>> I think a better way to implement this is to set NETIF_F_GRO in
>> netdev->vlan_features at register_netdev() time if it is set in
>> netdev->features
>>
>> That is the implementation I was implying earlier in this thread.
>
> We can even add it unconditionally there, since we only ever use
> dev->features & dev->vlan_features.
Good point!
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] net: enable GRO by default for vlan devices
2010-09-15 17:24 ` Ben Hutchings
2010-09-15 17:29 ` David Miller
@ 2010-09-15 19:24 ` Brandon Philips
2010-09-15 22:03 ` Eric Dumazet
1 sibling, 1 reply; 22+ messages in thread
From: Brandon Philips @ 2010-09-15 19:24 UTC (permalink / raw)
To: Ben Hutchings
Cc: eric.dumazet, e1000-devel, bruce.w.allan, jesse.brandeburg,
john.ronciak, jeffrey.t.kirsher, netdev, David Miller
Currently vlan devices don't have GRO by default as none of the Ethernet
drivers add NETIF_F_GRO to their vlan_features.
As GRO is a software feature add GRO to dev->vlan_features in
register_netdevice() and let vlan_dev_init() take care that it gets
enabled only when dev->features has NETIF_F_GRO too.
Signed-off-by: Brandon Philips <bphilips@suse.de>
---
net/core/dev.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index fc2dc93..e7a6bff 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5057,6 +5057,11 @@ int register_netdevice(struct net_device *dev)
if (dev->features & NETIF_F_SG)
dev->features |= NETIF_F_GSO;
+ /* Enable GRO for vlans by default if dev->features has GRO also.
+ * vlan_dev_init() will do the dev->features check.
+ */
+ dev->vlan_features |= NETIF_F_GRO;
+
ret = call_netdevice_notifiers(NETDEV_POST_INIT, dev);
ret = notifier_to_errno(ret);
if (ret)
--
1.7.1
------------------------------------------------------------------------------
Start uncovering the many advantages of virtual appliances
and start using them to simplify application deployment and
accelerate your shift to cloud computing.
http://p.sf.net/sfu/novell-sfdev2dev
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH] net: enable GRO by default for vlan devices
2010-09-15 19:24 ` [PATCH] net: enable GRO by default for vlan devices Brandon Philips
@ 2010-09-15 22:03 ` Eric Dumazet
2010-09-15 22:28 ` Eric Dumazet
2010-09-16 5:32 ` [PATCH] net: enable GRO by default for vlan devices David Miller
0 siblings, 2 replies; 22+ messages in thread
From: Eric Dumazet @ 2010-09-15 22:03 UTC (permalink / raw)
To: Brandon Philips
Cc: Ben Hutchings, David Miller, jeffrey.t.kirsher, jesse.brandeburg,
bruce.w.allan, alexander.h.duyck, peter.p.waskiewicz.jr,
john.ronciak, donald.c.skidmore, netdev, e1000-devel
Le mercredi 15 septembre 2010 à 12:24 -0700, Brandon Philips a écrit :
> Currently vlan devices don't have GRO by default as none of the Ethernet
> drivers add NETIF_F_GRO to their vlan_features.
>
> As GRO is a software feature add GRO to dev->vlan_features in
> register_netdevice() and let vlan_dev_init() take care that it gets
> enabled only when dev->features has NETIF_F_GRO too.
>
> Signed-off-by: Brandon Philips <bphilips@suse.de>
>
Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] net: enable GRO by default for vlan devices
2010-09-15 22:03 ` Eric Dumazet
@ 2010-09-15 22:28 ` Eric Dumazet
2010-09-16 1:16 ` Stephen Hemminger
2010-09-16 5:32 ` [PATCH] net: enable GRO by default for vlan devices David Miller
1 sibling, 1 reply; 22+ messages in thread
From: Eric Dumazet @ 2010-09-15 22:28 UTC (permalink / raw)
To: Brandon Philips
Cc: e1000-devel, bruce.w.allan, jesse.brandeburg, john.ronciak,
jeffrey.t.kirsher, netdev, Ben Hutchings, David Miller
Le jeudi 16 septembre 2010 à 00:03 +0200, Eric Dumazet a écrit :
> Le mercredi 15 septembre 2010 à 12:24 -0700, Brandon Philips a écrit :
> > Currently vlan devices don't have GRO by default as none of the Ethernet
> > drivers add NETIF_F_GRO to their vlan_features.
> >
> > As GRO is a software feature add GRO to dev->vlan_features in
> > register_netdevice() and let vlan_dev_init() take care that it gets
> > enabled only when dev->features has NETIF_F_GRO too.
> >
> > Signed-off-by: Brandon Philips <bphilips@suse.de>
> >
>
> Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
>
BTW, we have a similar problem for bonding ( GRO is disabled )
# ethtool -K bond0 gro off
# ethtool -K bond0 gro on
Cannot set device GRO settings: Invalid argument
Same for vlans on top of bond0
------------------------------------------------------------------------------
Start uncovering the many advantages of virtual appliances
and start using them to simplify application deployment and
accelerate your shift to cloud computing.
http://p.sf.net/sfu/novell-sfdev2dev
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] net: enable GRO by default for vlan devices
2010-09-15 22:28 ` Eric Dumazet
@ 2010-09-16 1:16 ` Stephen Hemminger
2010-09-16 5:51 ` [E1000-devel] " David Miller
0 siblings, 1 reply; 22+ messages in thread
From: Stephen Hemminger @ 2010-09-16 1:16 UTC (permalink / raw)
To: Eric Dumazet
Cc: Brandon Philips, e1000-devel, netdev, bruce.w.allan,
jesse.brandeburg, David, john.ronciak, jeffrey.t.kirsher,
Ben Hutchings, Miller
On Thu, 16 Sep 2010 00:28:11 +0200
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le jeudi 16 septembre 2010 à 00:03 +0200, Eric Dumazet a écrit :
> > Le mercredi 15 septembre 2010 à 12:24 -0700, Brandon Philips a écrit :
> > > Currently vlan devices don't have GRO by default as none of the Ethernet
> > > drivers add NETIF_F_GRO to their vlan_features.
> > >
> > > As GRO is a software feature add GRO to dev->vlan_features in
> > > register_netdevice() and let vlan_dev_init() take care that it gets
> > > enabled only when dev->features has NETIF_F_GRO too.
> > >
> > > Signed-off-by: Brandon Philips <bphilips@suse.de>
> > >
> >
> > Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
> >
>
> BTW, we have a similar problem for bonding ( GRO is disabled )
>
> # ethtool -K bond0 gro off
> # ethtool -K bond0 gro on
> Cannot set device GRO settings: Invalid argument
>
> Same for vlans on top of bond0
Bridge has same problem as well.
--
------------------------------------------------------------------------------
Start uncovering the many advantages of virtual appliances
and start using them to simplify application deployment and
accelerate your shift to cloud computing.
http://p.sf.net/sfu/novell-sfdev2dev
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [E1000-devel] [PATCH] net: enable GRO by default for vlan devices
2010-09-16 1:16 ` Stephen Hemminger
@ 2010-09-16 5:51 ` David Miller
2010-09-16 6:21 ` Stephen Hemminger
0 siblings, 1 reply; 22+ messages in thread
From: David Miller @ 2010-09-16 5:51 UTC (permalink / raw)
To: shemminger
Cc: eric.dumazet, bphilips, e1000-devel, bruce.w.allan,
jesse.brandeburg, john.ronciak, jeffrey.t.kirsher, netdev,
bhutchings
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Wed, 15 Sep 2010 18:16:16 -0700
> On Thu, 16 Sep 2010 00:28:11 +0200
> Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>> BTW, we have a similar problem for bonding ( GRO is disabled )
>>
>> # ethtool -K bond0 gro off
>> # ethtool -K bond0 gro on
>> Cannot set device GRO settings: Invalid argument
>>
>> Same for vlans on top of bond0
>
> Bridge has same problem as well.
Looking this over it should be a simple matter of:
1) Adding the generic get/set gro ethtool ops to bonding
and bridge device ops.
2) Set NETIF_F_GRO in bond/bridge device flags
3) Maybe some minor fiddling with netdev_increment_features
and the flag macros it uses?
Patches welcome :-)
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [E1000-devel] [PATCH] net: enable GRO by default for vlan devices
2010-09-16 5:51 ` [E1000-devel] " David Miller
@ 2010-09-16 6:21 ` Stephen Hemminger
2010-09-16 6:38 ` David Miller
0 siblings, 1 reply; 22+ messages in thread
From: Stephen Hemminger @ 2010-09-16 6:21 UTC (permalink / raw)
To: David Miller
Cc: eric.dumazet, bphilips, e1000-devel, bruce.w.allan,
jesse.brandeburg, john.ronciak, jeffrey.t.kirsher, netdev,
bhutchings
On Wed, 15 Sep 2010 22:51:46 -0700 (PDT)
David Miller <davem@davemloft.net> wrote:
> From: Stephen Hemminger <shemminger@vyatta.com>
> Date: Wed, 15 Sep 2010 18:16:16 -0700
>
> > On Thu, 16 Sep 2010 00:28:11 +0200
> > Eric Dumazet <eric.dumazet@gmail.com> wrote:
> >
> >> BTW, we have a similar problem for bonding ( GRO is disabled )
> >>
> >> # ethtool -K bond0 gro off
> >> # ethtool -K bond0 gro on
> >> Cannot set device GRO settings: Invalid argument
> >>
> >> Same for vlans on top of bond0
> >
> > Bridge has same problem as well.
>
> Looking this over it should be a simple matter of:
>
> 1) Adding the generic get/set gro ethtool ops to bonding
> and bridge device ops.
>
> 2) Set NETIF_F_GRO in bond/bridge device flags
>
> 3) Maybe some minor fiddling with netdev_increment_features
> and the flag macros it uses?
>
> Patches welcome :-)
I think it is more complex than that. GRO is tied to NAPI,
and bridge/bond don't use NAPI directly. They use netif_rx() for receiving
because layered drivers can't directly up call because of possible
issues with stack depth.
To get GRO working for netif_rx case,
the logic in process_backlog would have to change.
But this queue is processing packets from multiple devices so
it is not clear if GRO could be used.
--
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [E1000-devel] [PATCH] net: enable GRO by default for vlan devices
2010-09-16 6:21 ` Stephen Hemminger
@ 2010-09-16 6:38 ` David Miller
2010-09-16 6:42 ` Eric Dumazet
0 siblings, 1 reply; 22+ messages in thread
From: David Miller @ 2010-09-16 6:38 UTC (permalink / raw)
To: shemminger
Cc: eric.dumazet, bphilips, e1000-devel, bruce.w.allan,
jesse.brandeburg, john.ronciak, jeffrey.t.kirsher, netdev,
bhutchings
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Wed, 15 Sep 2010 23:21:25 -0700
> I think it is more complex than that. GRO is tied to NAPI,
> and bridge/bond don't use NAPI directly. They use netif_rx() for receiving
> because layered drivers can't directly up call because of possible
> issues with stack depth.
>
> To get GRO working for netif_rx case,
> the logic in process_backlog would have to change.
> But this queue is processing packets from multiple devices so
> it is not clear if GRO could be used.
Bonding's un-layering on RX is done in the normal netif_receive_skb()
control flow.
And bridging only uses netif_rx for multicast replication.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [E1000-devel] [PATCH] net: enable GRO by default for vlan devices
2010-09-16 6:38 ` David Miller
@ 2010-09-16 6:42 ` Eric Dumazet
2010-09-16 8:11 ` [PATCH] ethtool: change ethtool_set_gro() to use ethtool_op_get_rx_csum Eric Dumazet
0 siblings, 1 reply; 22+ messages in thread
From: Eric Dumazet @ 2010-09-16 6:42 UTC (permalink / raw)
To: David Miller
Cc: shemminger, bphilips, e1000-devel, bruce.w.allan,
jesse.brandeburg, john.ronciak, jeffrey.t.kirsher, netdev,
bhutchings
Le mercredi 15 septembre 2010 à 23:38 -0700, David Miller a écrit :
> From: Stephen Hemminger <shemminger@vyatta.com>
> Date: Wed, 15 Sep 2010 23:21:25 -0700
>
> > I think it is more complex than that. GRO is tied to NAPI,
> > and bridge/bond don't use NAPI directly. They use netif_rx() for receiving
> > because layered drivers can't directly up call because of possible
> > issues with stack depth.
> >
> > To get GRO working for netif_rx case,
> > the logic in process_backlog would have to change.
> > But this queue is processing packets from multiple devices so
> > it is not clear if GRO could be used.
>
> Bonding's un-layering on RX is done in the normal netif_receive_skb()
> control flow.
>
> And bridging only uses netif_rx for multicast replication.
Yes, bonding case should be easy, I'll take a look today.
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] ethtool: change ethtool_set_gro() to use ethtool_op_get_rx_csum
2010-09-16 6:42 ` Eric Dumazet
@ 2010-09-16 8:11 ` Eric Dumazet
2010-09-17 18:57 ` David Miller
0 siblings, 1 reply; 22+ messages in thread
From: Eric Dumazet @ 2010-09-16 8:11 UTC (permalink / raw)
To: David Miller
Cc: shemminger, bphilips, e1000-devel, bruce.w.allan,
jesse.brandeburg, john.ronciak, jeffrey.t.kirsher, netdev,
bhutchings
Le jeudi 16 septembre 2010 à 08:43 +0200, Eric Dumazet a écrit :
> Le mercredi 15 septembre 2010 à 23:38 -0700, David Miller a écrit :
> > From: Stephen Hemminger <shemminger@vyatta.com>
> > Date: Wed, 15 Sep 2010 23:21:25 -0700
> >
> > > I think it is more complex than that. GRO is tied to NAPI,
> > > and bridge/bond don't use NAPI directly. They use netif_rx() for receiving
> > > because layered drivers can't directly up call because of possible
> > > issues with stack depth.
> > >
> > > To get GRO working for netif_rx case,
> > > the logic in process_backlog would have to change.
> > > But this queue is processing packets from multiple devices so
> > > it is not clear if GRO could be used.
> >
> > Bonding's un-layering on RX is done in the normal netif_receive_skb()
> > control flow.
> >
> > And bridging only uses netif_rx for multicast replication.
>
> Yes, bonding case should be easy, I'll take a look today.
>
[PATCH] ethtool: change ethtool_set_gro() to use ethtool_op_get_rx_csum
To be able to switch on GRO on a device, ethtool_set_gro() checks this
device provides a get_rx_csum() method.
Some devices dont provide this method, while they do support RX
checksumming.
This patch allows bonding to support GRO :
ethtool -K bond0 gro on
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
net/core/ethtool.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 7a85367..8c8c6d4 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -1175,8 +1175,11 @@ static int ethtool_set_gro(struct net_device *dev, char __user *useraddr)
return -EFAULT;
if (edata.data) {
- if (!dev->ethtool_ops->get_rx_csum ||
- !dev->ethtool_ops->get_rx_csum(dev))
+ u32 rxcsum = dev->ethtool_ops->get_rx_csum ?
+ dev->ethtool_ops->get_rx_csum(dev) :
+ ethtool_op_get_rx_csum(dev);
+
+ if (!rxcsum)
return -EINVAL;
dev->features |= NETIF_F_GRO;
} else
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH] ethtool: change ethtool_set_gro() to use ethtool_op_get_rx_csum
2010-09-16 8:11 ` [PATCH] ethtool: change ethtool_set_gro() to use ethtool_op_get_rx_csum Eric Dumazet
@ 2010-09-17 18:57 ` David Miller
2010-09-17 19:25 ` [PATCH] bonding: enable gro by default Eric Dumazet
0 siblings, 1 reply; 22+ messages in thread
From: David Miller @ 2010-09-17 18:57 UTC (permalink / raw)
To: eric.dumazet
Cc: shemminger, bphilips, e1000-devel, bruce.w.allan,
jesse.brandeburg, john.ronciak, jeffrey.t.kirsher, netdev,
bhutchings
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 16 Sep 2010 10:11:03 +0200
> [PATCH] ethtool: change ethtool_set_gro() to use ethtool_op_get_rx_csum
>
> To be able to switch on GRO on a device, ethtool_set_gro() checks this
> device provides a get_rx_csum() method.
>
> Some devices dont provide this method, while they do support RX
> checksumming.
>
> This patch allows bonding to support GRO :
>
> ethtool -K bond0 gro on
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied, thanks Eric.
Can you do whatever change is needed to get it enabled by default?
Thanks again.
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] bonding: enable gro by default
2010-09-17 18:57 ` David Miller
@ 2010-09-17 19:25 ` Eric Dumazet
2010-09-17 23:54 ` David Miller
0 siblings, 1 reply; 22+ messages in thread
From: Eric Dumazet @ 2010-09-17 19:25 UTC (permalink / raw)
To: David Miller, Jay Vosburgh
Cc: shemminger, bphilips, e1000-devel, bruce.w.allan,
jesse.brandeburg, john.ronciak, jeffrey.t.kirsher, netdev,
bhutchings
Le vendredi 17 septembre 2010 à 11:57 -0700, David Miller a écrit :
> Applied, thanks Eric.
>
> Can you do whatever change is needed to get it enabled by default?
>
Sure ;)
[PATCH] bonding: enable gro by default
gro can be enabled by default on bonding devices.
Actual support depends on the lower devices.
One can still use ethtool to switch off GRO if needed.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
drivers/net/bonding/bond_main.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 3b16f62..80610a6 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -4678,6 +4678,10 @@ static void bond_setup(struct net_device *bond_dev)
NETIF_F_HW_VLAN_RX |
NETIF_F_HW_VLAN_FILTER);
+ /* By default, we enable GRO on bonding devices.
+ * Actual support requires lowlevel drivers are GRO ready.
+ */
+ bond_dev->features |= NETIF_F_GRO;
}
static void bond_work_cancel_all(struct bonding *bond)
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH] bonding: enable gro by default
2010-09-17 19:25 ` [PATCH] bonding: enable gro by default Eric Dumazet
@ 2010-09-17 23:54 ` David Miller
0 siblings, 0 replies; 22+ messages in thread
From: David Miller @ 2010-09-17 23:54 UTC (permalink / raw)
To: eric.dumazet
Cc: bphilips, e1000-devel, netdev, fubar, bruce.w.allan,
jesse.brandeburg, john.ronciak, jeffrey.t.kirsher, bhutchings
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 17 Sep 2010 21:25:07 +0200
> gro can be enabled by default on bonding devices.
>
> Actual support depends on the lower devices.
>
> One can still use ethtool to switch off GRO if needed.
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied, thanks Eric.
------------------------------------------------------------------------------
Start uncovering the many advantages of virtual appliances
and start using them to simplify application deployment and
accelerate your shift to cloud computing.
http://p.sf.net/sfu/novell-sfdev2dev
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] net: enable GRO by default for vlan devices
2010-09-15 22:03 ` Eric Dumazet
2010-09-15 22:28 ` Eric Dumazet
@ 2010-09-16 5:32 ` David Miller
1 sibling, 0 replies; 22+ messages in thread
From: David Miller @ 2010-09-16 5:32 UTC (permalink / raw)
To: eric.dumazet
Cc: bphilips, bhutchings, jeffrey.t.kirsher, jesse.brandeburg,
bruce.w.allan, alexander.h.duyck, peter.p.waskiewicz.jr,
john.ronciak, donald.c.skidmore, netdev, e1000-devel
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 16 Sep 2010 00:03:23 +0200
> Le mercredi 15 septembre 2010 à 12:24 -0700, Brandon Philips a écrit :
>> Currently vlan devices don't have GRO by default as none of the Ethernet
>> drivers add NETIF_F_GRO to their vlan_features.
>>
>> As GRO is a software feature add GRO to dev->vlan_features in
>> register_netdevice() and let vlan_dev_init() take care that it gets
>> enabled only when dev->features has NETIF_F_GRO too.
>>
>> Signed-off-by: Brandon Philips <bphilips@suse.de>
>>
>
> Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] vlan: enable GRO if real_dev supports it
2010-09-15 17:08 ` Eric Dumazet
2010-09-15 17:14 ` David Miller
@ 2010-09-15 18:59 ` Brandon Philips
1 sibling, 0 replies; 22+ messages in thread
From: Brandon Philips @ 2010-09-15 18:59 UTC (permalink / raw)
To: Eric Dumazet
Cc: David Miller, jeffrey.t.kirsher, jesse.brandeburg, bruce.w.allan,
alexander.h.duyck, peter.p.waskiewicz.jr, john.ronciak,
donald.c.skidmore, netdev, e1000-devel
On 19:08 Wed 15 Sep 2010, Eric Dumazet wrote:
> Le mercredi 15 septembre 2010 à 09:37 -0700, Brandon Philips a écrit :
> > Currently vlan devices don't have GRO by default as none of the Ethernet
> > drivers add NETIF_F_GRO to their vlan_features.
> >
> > As GRO is a software feature just propogate GRO from the real_dev in the
> > vlan core. There is no need to have the drivers each add NETIF_F_GRO to
> > their vlan_features.
> >
> > Signed-off-by: Brandon Philips <bphilips@suse.de>
> > ---
> > net/8021q/vlan_dev.c | 1 +
> > 1 files changed, 1 insertions(+), 0 deletions(-)
> >
> > diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
> > index 3bccdd1..6fbc445 100644
> > --- a/net/8021q/vlan_dev.c
> > +++ b/net/8021q/vlan_dev.c
> > @@ -735,6 +735,7 @@ static int vlan_dev_init(struct net_device *dev)
> > (1<<__LINK_STATE_DORMANT))) |
> > (1<<__LINK_STATE_PRESENT);
> >
> > + dev->features |= real_dev->features & NETIF_F_GRO;
> > dev->features |= real_dev->features & real_dev->vlan_features;
> > dev->gso_max_size = real_dev->gso_max_size;
> >
>
> Hmm, this is only part of a generic solution.
>
> If I enable gro with "ethtool -K eth0 gro on", should we propagate GRO
> on vlan eth0.555 ?
That works fine as far as I can see. I don't think it should go the
other way though.
Cheers, Brandon
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] ixgbe: add GRO to vlan_features
2010-09-15 4:35 ` David Miller
2010-09-15 16:37 ` [PATCH] vlan: enable GRO if real_dev supports it Brandon Philips
@ 2010-09-15 16:38 ` Brandon Philips
1 sibling, 0 replies; 22+ messages in thread
From: Brandon Philips @ 2010-09-15 16:38 UTC (permalink / raw)
To: David Miller
Cc: jeffrey.t.kirsher, jesse.brandeburg, bruce.w.allan,
alexander.h.duyck, peter.p.waskiewicz.jr, john.ronciak,
donald.c.skidmore, netdev, e1000-devel
On 21:35 Tue 14 Sep 2010, David Miller wrote:
> From: Brandon Philips <bphilips@suse.de>
> Date: Tue, 14 Sep 2010 18:50:53 -0700
>
> > When running a vlan -> bridge -> domU setup I was noticing poor RX
> > performance. It seems ixgbe and other drivers don't have NETIF_F_GRO set
> > in vlan_features and thus GRO is off by default for vlans.
> >
> > I see no reason why the other drivers shouldn't be fixed. If this is OK
> > I will go off and fix the rest.
> >
> > Signed-off-by: Brandon Philips <bphilips@suse.de>
>
> I think it should be set, but why don't we do this another way?
>
> This feature is a software feature, essentially, so if it is published
> in netdev->features we can simply propagate it automatically to
> netdev->vlan_features at device registry time in the core networking.
Agreed. I will just send a patch to vlan_dev.c then.
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2010-09-17 23:54 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-09-15 1:50 [PATCH] ixgbe: add GRO to vlan_features Brandon Philips
2010-09-15 4:35 ` David Miller
2010-09-15 16:37 ` [PATCH] vlan: enable GRO if real_dev supports it Brandon Philips
2010-09-15 17:08 ` Eric Dumazet
2010-09-15 17:14 ` David Miller
2010-09-15 17:24 ` Ben Hutchings
2010-09-15 17:29 ` David Miller
2010-09-15 19:24 ` [PATCH] net: enable GRO by default for vlan devices Brandon Philips
2010-09-15 22:03 ` Eric Dumazet
2010-09-15 22:28 ` Eric Dumazet
2010-09-16 1:16 ` Stephen Hemminger
2010-09-16 5:51 ` [E1000-devel] " David Miller
2010-09-16 6:21 ` Stephen Hemminger
2010-09-16 6:38 ` David Miller
2010-09-16 6:42 ` Eric Dumazet
2010-09-16 8:11 ` [PATCH] ethtool: change ethtool_set_gro() to use ethtool_op_get_rx_csum Eric Dumazet
2010-09-17 18:57 ` David Miller
2010-09-17 19:25 ` [PATCH] bonding: enable gro by default Eric Dumazet
2010-09-17 23:54 ` David Miller
2010-09-16 5:32 ` [PATCH] net: enable GRO by default for vlan devices David Miller
2010-09-15 18:59 ` [PATCH] vlan: enable GRO if real_dev supports it Brandon Philips
2010-09-15 16:38 ` [PATCH] ixgbe: add GRO to vlan_features Brandon Philips
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.