All of lore.kernel.org
 help / color / mirror / Atom feed
* zero features for a vlan over bond
@ 2009-04-06 13:00 Or Gerlitz
  2009-04-07 14:16 ` Jay Vosburgh
  0 siblings, 1 reply; 19+ messages in thread
From: Or Gerlitz @ 2009-04-06 13:00 UTC (permalink / raw)
  To: Jay Vosburgh; +Cc: netdev

Hi,

Using vconfig and the 8021q modules I have created a vlan device over a
bond (e.g bond0.4001 over bond0 which is over eth0,eth1) - but for some
reason the vlan device has no features - any idea what may be the reason
for that? below is the flags/features matrix (eth0/1 are igb, 2.6.29)

device		flags	features
================================
bond0.4001	0x1403	0x0
bond0		0x1403	0x1113a9
eth0/1		0x1803	0x111ba9

Or.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: zero features for a vlan over bond
  2009-04-06 13:00 zero features for a vlan over bond Or Gerlitz
@ 2009-04-07 14:16 ` Jay Vosburgh
  2009-04-07 22:16   ` Or Gerlitz
  2009-04-16 14:11   ` zero features for a vlan over bond / vlan features Or Gerlitz
  0 siblings, 2 replies; 19+ messages in thread
From: Jay Vosburgh @ 2009-04-07 14:16 UTC (permalink / raw)
  To: Or Gerlitz; +Cc: netdev

Or Gerlitz <ogerlitz@voltaire.com> wrote:

>Using vconfig and the 8021q modules I have created a vlan device over a
>bond (e.g bond0.4001 over bond0 which is over eth0,eth1) - but for some
>reason the vlan device has no features - any idea what may be the reason
>for that? below is the flags/features matrix (eth0/1 are igb, 2.6.29)
>
>device		flags	features
>================================
>bond0.4001	0x1403	0x0
>bond0		0x1403	0x1113a9
>eth0/1		0x1803	0x111ba9

	The features system now has a dev->vlan_features that lists the
features that will work through a vlan; bond_compute_features isn't
using netdev_increment/fix_features to additionally compute the
vlan_features, so that's ending up always empty even if the underlying
device supports vlan passthrough.

	I suspect a bridge will display the same symptoms.

	I'm working on a patch; I'll see what I can come up with.  Note,
however, that at this writing, the set of drivers that explicitly
supports VLAN passthrough (a non-zero vlan_features) is rather limited.

	-J

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: zero features for a vlan over bond
  2009-04-07 14:16 ` Jay Vosburgh
@ 2009-04-07 22:16   ` Or Gerlitz
  2009-04-16 14:11   ` zero features for a vlan over bond / vlan features Or Gerlitz
  1 sibling, 0 replies; 19+ messages in thread
From: Or Gerlitz @ 2009-04-07 22:16 UTC (permalink / raw)
  To: Jay Vosburgh; +Cc: Or Gerlitz, netdev

On Tue, Apr 7, 2009 at 5:16 PM, Jay Vosburgh <fubar@us.ibm.com> wrote:
> I suspect a bridge will display the same symptoms. I'm working on a patch; I'll see
> what I can come up with.  Note, however, that at this writing, the set of drivers that
> explicitly supports VLAN passthrough (a non-zero vlan_features) is rather limited.

Hi Jay,

Thanks for your quick notes. I'm looking forward to see the patch and
then better understand the impact on bridges and how VLANs are going
along with bonds and bridges, but the way are later two coexist
peacefully? I remember discussions in the past whether one can bridge
bonds and/or bond bridges, however, I wasn't sure what was the
conclusion and what is possible, I would be more interested to see
that the specific case of a bond  connected to a bridge is supported.
We are going into the Passover vacation and as such I would be somehow
slow in responding over the next week or so.

Or.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: zero features for a vlan over bond / vlan features
  2009-04-07 14:16 ` Jay Vosburgh
  2009-04-07 22:16   ` Or Gerlitz
@ 2009-04-16 14:11   ` Or Gerlitz
  2009-04-23 23:47     ` Jay Vosburgh
  1 sibling, 1 reply; 19+ messages in thread
From: Or Gerlitz @ 2009-04-16 14:11 UTC (permalink / raw)
  To: Jay Vosburgh, David Miller; +Cc: netdev

Jay Vosburgh wrote:
> The features system now has a dev->vlan_features that lists the
> features that will work through a vlan; bond_compute_features isn't
> using netdev_increment/fix_features to additionally compute the vlan_features, 
> so that's ending up always empty even if the underlying device supports vlan passthrough.
> I'm working on a patch; I'll see what I can come up with.  

Great, I will be happy to test the patch once you have it...

> Note, however, that at this writing, the set of drivers that explicitly
> supports VLAN passthrough (a non-zero vlan_features) is rather limited.

Please correct me if I'm wrong - would be it correct to say that there's a concept 
of "VLAN acceleration" which comes into play when the NIC device advertises  
NETIF_F_HW_VLAN_xxx features and a concept of "VLAN passthrough" which means the
NIC device have a non empty vlan_features bit mask? 

Looking on Linus tree I noted that indeed only few drivers have non-empty vlan_features
and I wonder if the reason for that is indeed lack of HW support or just the driver
maintainers being somehow slow to catch up with the vlan_features concept, Dave - 
any insight here? 

Or.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: zero features for a vlan over bond / vlan features
  2009-04-16 14:11   ` zero features for a vlan over bond / vlan features Or Gerlitz
@ 2009-04-23 23:47     ` Jay Vosburgh
  2009-06-08  9:34       ` Or Gerlitz
  0 siblings, 1 reply; 19+ messages in thread
From: Jay Vosburgh @ 2009-04-23 23:47 UTC (permalink / raw)
  To: Or Gerlitz; +Cc: David Miller, netdev

Or Gerlitz <ogerlitz@voltaire.com> wrote:

>Jay Vosburgh wrote:
>> The features system now has a dev->vlan_features that lists the
>> features that will work through a vlan; bond_compute_features isn't
>> using netdev_increment/fix_features to additionally compute the vlan_features, 
>> so that's ending up always empty even if the underlying device supports vlan passthrough.
>> I'm working on a patch; I'll see what I can come up with.  
>
>Great, I will be happy to test the patch once you have it...

	Ok, here's a patch, but I'm not sure it's the right patch.

	I'm not entirely sure if the vlan_features should be amassed as
the regular features are, or if it should be a strict subset
(slave0->vlan_features & slave1->vlan_features, etc).  This patch does
the former, collecting the vlan_features in a manner analagous to the
regular features.

	This is strictly a test patch, it's still got printks and such
in it.

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 99610f3..967c5b6 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1341,6 +1341,7 @@ static int bond_compute_features(struct bonding *bond)
 	struct slave *slave;
 	struct net_device *bond_dev = bond->dev;
 	unsigned long features = bond_dev->features;
+	unsigned long vlan_features = 0;
 	unsigned short max_hard_header_len = max((u16)ETH_HLEN,
 						bond_dev->hard_header_len);
 	int i;
@@ -1352,11 +1353,18 @@ static int bond_compute_features(struct bonding *bond)
 		goto done;
 
 	features &= ~NETIF_F_ONE_FOR_ALL;
-
+	vlan_features = bond->first_slave->dev->vlan_features;
+	printk("vf %lx\n", vlan_features);
 	bond_for_each_slave(bond, slave, i) {
 		features = netdev_increment_features(features,
 						     slave->dev->features,
 						     NETIF_F_ONE_FOR_ALL);
+		vlan_features = netdev_increment_features(vlan_features,
+						  slave->dev->vlan_features,
+							  NETIF_F_ONE_FOR_ALL);
+		printk("slave %s (f %lx vf %lx) new f %lx vf %lx\n",
+		       slave->dev->name, slave->dev->features,
+		       slave->dev->vlan_features, features, vlan_features);
 		if (slave->dev->hard_header_len > max_hard_header_len)
 			max_hard_header_len = slave->dev->hard_header_len;
 	}
@@ -1364,6 +1372,8 @@ static int bond_compute_features(struct bonding *bond)
 done:
 	features |= (bond_dev->features & BOND_VLAN_FEATURES);
 	bond_dev->features = netdev_fix_features(features, NULL);
+	bond_dev->vlan_features = netdev_fix_features(vlan_features, NULL);
+	printk("bd->vf %lx\n", bond_dev->vlan_features);
 	bond_dev->hard_header_len = max_hard_header_len;
 
 	return 0;


	-J

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: zero features for a vlan over bond / vlan features
  2009-04-23 23:47     ` Jay Vosburgh
@ 2009-06-08  9:34       ` Or Gerlitz
  2009-07-20  8:08         ` Or Gerlitz
  0 siblings, 1 reply; 19+ messages in thread
From: Or Gerlitz @ 2009-06-08  9:34 UTC (permalink / raw)
  To: Jay Vosburgh; +Cc: David Miller, netdev

Jay Vosburgh wrote:
>>> The features system now has a dev->vlan_features that lists the
>>> features that will work through a vlan; bond_compute_features isn't
>>> using netdev_increment/fix_features to additionally compute the vlan_features, so
>>> that's ending up always empty even if the underlying device supports vlan passthrough

> Ok, here's a patch, but I'm not sure it's the right patch.
> I'm not entirely sure if the vlan_features should be amassed as
> the regular features are, or if it should be a strict subset
> (slave0->vlan_features & slave1->vlan_features, etc).  This patch does the
> former, collecting the vlan_features in a manner analogous to the regular features.

Jay, 

I have tested with your patch and indeed now, vlan-over-bond (e.g bond0.4001) advertises features to the stack wheres before it didn't. Below is the patch you sent with the debugging prints removed and my signature added, I would be happy if you set the change-log && your signature and push it further to Dave. I'm also find if you prefer that I'll do that.

Signed-off-by: Or Gerlitz <ogerlitz@voltaire.com>

Index: net-next-2.6/drivers/net/bonding/bond_main.c
===================================================================
--- net-next-2.6.orig/drivers/net/bonding/bond_main.c
+++ net-next-2.6/drivers/net/bonding/bond_main.c
@@ -1339,6 +1339,7 @@ static int bond_compute_features(struct 
 	struct slave *slave;
 	struct net_device *bond_dev = bond->dev;
 	unsigned long features = bond_dev->features;
+	unsigned long vlan_features = 0;
 	unsigned short max_hard_header_len = max((u16)ETH_HLEN,
 						bond_dev->hard_header_len);
 	int i;
@@ -1351,10 +1352,14 @@ static int bond_compute_features(struct 
 
 	features &= ~NETIF_F_ONE_FOR_ALL;
 
+	vlan_features = bond->first_slave->dev->vlan_features;
 	bond_for_each_slave(bond, slave, i) {
 		features = netdev_increment_features(features,
 						     slave->dev->features,
 						     NETIF_F_ONE_FOR_ALL);
+		vlan_features = netdev_increment_features(vlan_features,
+							slave->dev->vlan_features,
+							NETIF_F_ONE_FOR_ALL);
 		if (slave->dev->hard_header_len > max_hard_header_len)
 			max_hard_header_len = slave->dev->hard_header_len;
 	}
@@ -1362,6 +1367,7 @@ static int bond_compute_features(struct 
 done:
 	features |= (bond_dev->features & BOND_VLAN_FEATURES);
 	bond_dev->features = netdev_fix_features(features, NULL);
+	bond_dev->vlan_features = netdev_fix_features(vlan_features, NULL);
 	bond_dev->hard_header_len = max_hard_header_len;
 
 	return 0;

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: zero features for a vlan over bond / vlan features
  2009-06-08  9:34       ` Or Gerlitz
@ 2009-07-20  8:08         ` Or Gerlitz
  2009-07-22 21:34           ` [PATCH net-next-2.6] bonding: propogate vlan_features to bonding master Jay Vosburgh
  0 siblings, 1 reply; 19+ messages in thread
From: Or Gerlitz @ 2009-07-20  8:08 UTC (permalink / raw)
  To: Jay Vosburgh; +Cc: David Miller, netdev

Or Gerlitz wrote:
> Jay, I have tested with your patch and indeed now, vlan-over-bond (e.g bond0.4001) advertises features to the stack wheres before it didn't. Below is the patch you sent with the debugging prints removed and my signature added, I would be happy if you set the change-log && your signature and push it further to Dave. I'm also find if you prefer that I'll do that.
>
> Signed-off-by: Or Gerlitz <ogerlitz@voltaire.com>
>   
Hi Jay,

This (http://marc.info/?l=linux-netdev&m=124445367820790) is pending 
quite a lot of time and already missed 2.6.31, please let me know if you 
prefer that I will push it directly to Dave, or you want to do so, thanks

Or.



^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH net-next-2.6] bonding: propogate vlan_features to bonding master
  2009-07-20  8:08         ` Or Gerlitz
@ 2009-07-22 21:34           ` Jay Vosburgh
  2009-07-23  6:03             ` Eric Dumazet
  2009-07-23 11:03             ` [PATCH net-next-2.6] bonding: propogate vlan_features to bonding master Patrick McHardy
  0 siblings, 2 replies; 19+ messages in thread
From: Jay Vosburgh @ 2009-07-22 21:34 UTC (permalink / raw)
  To: Or Gerlitz; +Cc: David Miller, netdev


	Propogate the vlan_features of the slave devices to the bonding
master device, using the same logic as for regular features.

	Tested by Or Gerlitz <ogerlitz@voltaire.com>, who also removed
the debug logic from the original test patch.

Signed-off-by: Or Gerlitz <ogerlitz@voltaire.com>
Signed-off-by: Jay Vosburgh <fubar@us.ibm.com>

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 3bf0cc6..5a8b882 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1331,6 +1331,7 @@ static int bond_compute_features(struct bonding *bond)
 	struct slave *slave;
 	struct net_device *bond_dev = bond->dev;
 	unsigned long features = bond_dev->features;
+	unsigned long vlan_features;
 	unsigned short max_hard_header_len = max((u16)ETH_HLEN,
 						bond_dev->hard_header_len);
 	int i;
@@ -1343,10 +1344,14 @@ static int bond_compute_features(struct bonding *bond)
 
 	features &= ~NETIF_F_ONE_FOR_ALL;
 
+	vlan_features = bond->first_slave->dev->vlan_features;
 	bond_for_each_slave(bond, slave, i) {
 		features = netdev_increment_features(features,
 						     slave->dev->features,
 						     NETIF_F_ONE_FOR_ALL);
+		vlan_features = netdev_increment_features(vlan_features,
+							slave->dev->vlan_features,
+							NETIF_F_ONE_FOR_ALL);
 		if (slave->dev->hard_header_len > max_hard_header_len)
 			max_hard_header_len = slave->dev->hard_header_len;
 	}
@@ -1354,6 +1359,7 @@ static int bond_compute_features(struct bonding *bond)
 done:
 	features |= (bond_dev->features & BOND_VLAN_FEATURES);
 	bond_dev->features = netdev_fix_features(features, NULL);
+	bond_dev->vlan_features = netdev_fix_features(vlan_features, NULL);
 	bond_dev->hard_header_len = max_hard_header_len;
 
 	return 0;

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH net-next-2.6] bonding: propogate vlan_features to bonding master
  2009-07-22 21:34           ` [PATCH net-next-2.6] bonding: propogate vlan_features to bonding master Jay Vosburgh
@ 2009-07-23  6:03             ` Eric Dumazet
  2009-07-23  7:24               ` Or Gerlitz
  2009-07-23 11:03             ` [PATCH net-next-2.6] bonding: propogate vlan_features to bonding master Patrick McHardy
  1 sibling, 1 reply; 19+ messages in thread
From: Eric Dumazet @ 2009-07-23  6:03 UTC (permalink / raw)
  To: Jay Vosburgh; +Cc: Or Gerlitz, David Miller, netdev

Jay Vosburgh a écrit :
> 	Propogate the vlan_features of the slave devices to the bonding
> master device, using the same logic as for regular features.
> 
> 	Tested by Or Gerlitz <ogerlitz@voltaire.com>, who also removed
> the debug logic from the original test patch.
> 
> Signed-off-by: Or Gerlitz <ogerlitz@voltaire.com>
> Signed-off-by: Jay Vosburgh <fubar@us.ibm.com>

Seems pretty cool, but I could not test it on my dev machine,
since tg3 and bnx2 drivers dont advertize yet vlan_features

> 
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 3bf0cc6..5a8b882 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -1331,6 +1331,7 @@ static int bond_compute_features(struct bonding *bond)
>  	struct slave *slave;
>  	struct net_device *bond_dev = bond->dev;
>  	unsigned long features = bond_dev->features;
> +	unsigned long vlan_features;
>  	unsigned short max_hard_header_len = max((u16)ETH_HLEN,
>  						bond_dev->hard_header_len);
>  	int i;
> @@ -1343,10 +1344,14 @@ static int bond_compute_features(struct bonding *bond)
>  
>  	features &= ~NETIF_F_ONE_FOR_ALL;
>  
> +	vlan_features = bond->first_slave->dev->vlan_features;
>  	bond_for_each_slave(bond, slave, i) {
>  		features = netdev_increment_features(features,
>  						     slave->dev->features,
>  						     NETIF_F_ONE_FOR_ALL);
> +		vlan_features = netdev_increment_features(vlan_features,
> +							slave->dev->vlan_features,
> +							NETIF_F_ONE_FOR_ALL);
>  		if (slave->dev->hard_header_len > max_hard_header_len)
>  			max_hard_header_len = slave->dev->hard_header_len;
>  	}
> @@ -1354,6 +1359,7 @@ static int bond_compute_features(struct bonding *bond)
>  done:
>  	features |= (bond_dev->features & BOND_VLAN_FEATURES);
>  	bond_dev->features = netdev_fix_features(features, NULL);
> +	bond_dev->vlan_features = netdev_fix_features(vlan_features, NULL);
>  	bond_dev->hard_header_len = max_hard_header_len;
>  
>  	return 0;


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH net-next-2.6] bonding: propogate vlan_features to bonding master
  2009-07-23  6:03             ` Eric Dumazet
@ 2009-07-23  7:24               ` Or Gerlitz
  2009-07-23  8:29                 ` [PATCH net-next-2.6] bnx2: Update vlan_features Eric Dumazet
  0 siblings, 1 reply; 19+ messages in thread
From: Or Gerlitz @ 2009-07-23  7:24 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Jay Vosburgh, David Miller, netdev, Eilon Greenstein, Michael Chan

Eric Dumazet wrote:
> Jay Vosburgh a écrit :
>> Propogate the vlan_features of the slave devices to the bonding master device, using the same logic as for regular features.
> Seems pretty cool, but I could not test it on my dev machine, since tg3 and bnx2 drivers dont advertize yet vlan_features
The bnx2x maintainer posted yesterday a patch that does so, I assume a 
similar patch could work for at least one of tg3 or bnx2. I copied both 
maintainers on this email, in  the hope they can come up with a patch.

Or.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH net-next-2.6] bnx2: Update vlan_features
  2009-07-23  7:24               ` Or Gerlitz
@ 2009-07-23  8:29                 ` Eric Dumazet
  2009-07-23 12:01                   ` Eric Dumazet
  0 siblings, 1 reply; 19+ messages in thread
From: Eric Dumazet @ 2009-07-23  8:29 UTC (permalink / raw)
  To: Or Gerlitz
  Cc: Jay Vosburgh, David Miller, netdev, Eilon Greenstein, Michael Chan

Or Gerlitz a écrit :
> Eric Dumazet wrote:
>> Jay Vosburgh a écrit :
>>> Propogate the vlan_features of the slave devices to the bonding
>>> master device, using the same logic as for regular features.
>> Seems pretty cool, but I could not test it on my dev machine, since
>> tg3 and bnx2 drivers dont advertize yet vlan_features
> The bnx2x maintainer posted yesterday a patch that does so, I assume a
> similar patch could work for at least one of tg3 or bnx2. I copied both
> maintainers on this email, in  the hope they can come up with a patch.
> 

Here is the bnx2 patch I cooked to test this vlan_features propagation on bonding.

Everything fine so far !

# cat /proc/net/bonding/bond0
Ethernet Channel Bonding Driver: v3.5.0 (November 4, 2008)

Bonding Mode: fault-tolerance (active-backup)
Primary Slave: None
Currently Active Slave: eth1
MII Status: up
MII Polling Interval (ms): 100
Up Delay (ms): 0
Down Delay (ms): 0

Slave Interface: eth1    (bnx2 driver)
MII Status: up
Link Failure Count: 2
Permanent HW addr: 00:1e:0b:ec:d3:d2

Slave Interface: eth2    (tg3 driver)
MII Status: up
Link Failure Count: 0
Permanent HW addr: 00:1e:0b:92:78:50


# ethtool -k bond0
Offload parameters for bond0:
Cannot get device rx csum settings: Operation not supported
rx-checksumming: off
tx-checksumming: on
scatter-gather: on
tcp-segmentation-offload: on
udp-fragmentation-offload: off
generic-segmentation-offload: off
generic-receive-offload: off
large-receive-offload: off

# ip link add link bond0 vlan.103 type vlan id 103
# ethtool -k vlan.103
Offload parameters for vlan.103:
rx-checksumming: off
tx-checksumming: on
scatter-gather: on
tcp-segmentation-offload: on
udp-fragmentation-offload: off
generic-segmentation-offload: on
generic-receive-offload: off
large-receive-offload: off


Thanks Or & Jay

[PATCH net-next-2.6] bnx2: Update vlan_features

In order to get full use of some advanced features of BNX2, we now need to
fill dev->vlan_features.

Patch successfully tested with vlan devices built on top of bonding.
(bond0 : one bnx2 slave, one tg3 slave (not yet vlan_features enabled)

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 drivers/net/bnx2.c |   36 ++++++++++++++++++++++++++++++------
 1 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
index b70cc99..0868673 100644
--- a/drivers/net/bnx2.c
+++ b/drivers/net/bnx2.c
@@ -3485,6 +3485,20 @@ static int bnx2_poll(struct napi_struct *napi, int budget)
 	return work_done;
 }
 
+static void inline vlan_features_add(struct net_device *dev, unsigned long flags)
+{
+#ifdef BCM_VLAN
+	dev->vlan_features |= flags;
+#endif
+}
+
+static void inline vlan_features_del(struct net_device *dev, unsigned long flags)
+{
+#ifdef BCM_VLAN
+	dev->vlan_features &= ~flags;
+#endif
+}
+
 /* Called with rtnl_lock from vlan functions and also netif_tx_lock
  * from set_multicast.
  */
@@ -7068,11 +7082,17 @@ bnx2_set_tso(struct net_device *dev, u32 data)
 
 	if (data) {
 		dev->features |= NETIF_F_TSO | NETIF_F_TSO_ECN;
-		if (CHIP_NUM(bp) == CHIP_NUM_5709)
+		vlan_features_add(dev, NETIF_F_TSO | NETIF_F_TSO_ECN);
+		if (CHIP_NUM(bp) == CHIP_NUM_5709) {
 			dev->features |= NETIF_F_TSO6;
-	} else
+			vlan_features_add(dev, NETIF_F_TSO6);
+		}
+	} else {
 		dev->features &= ~(NETIF_F_TSO | NETIF_F_TSO6 |
 				   NETIF_F_TSO_ECN);
+		vlan_features_del(dev, NETIF_F_TSO | NETIF_F_TSO6 |
+				  NETIF_F_TSO_ECN);
+	}
 	return 0;
 }
 
@@ -8064,16 +8084,20 @@ bnx2_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	memcpy(dev->perm_addr, bp->mac_addr, 6);
 
 	dev->features |= NETIF_F_IP_CSUM | NETIF_F_SG;
-	if (CHIP_NUM(bp) == CHIP_NUM_5709)
+	vlan_features_add(dev, NETIF_F_IP_CSUM | NETIF_F_SG);
+	if (CHIP_NUM(bp) == CHIP_NUM_5709) {
 		dev->features |= NETIF_F_IPV6_CSUM;
-
+		vlan_features_add(dev, NETIF_F_IPV6_CSUM);
+	}
 #ifdef BCM_VLAN
 	dev->features |= NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX;
 #endif
 	dev->features |= NETIF_F_TSO | NETIF_F_TSO_ECN;
-	if (CHIP_NUM(bp) == CHIP_NUM_5709)
+	vlan_features_add(dev, NETIF_F_TSO | NETIF_F_TSO_ECN);
+	if (CHIP_NUM(bp) == CHIP_NUM_5709) {
 		dev->features |= NETIF_F_TSO6;
-
+		vlan_features_add(dev, NETIF_F_TSO6);
+	}
 	if ((rc = register_netdev(dev))) {
 		dev_err(&pdev->dev, "Cannot register net device\n");
 		goto error;

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH net-next-2.6] bonding: propogate vlan_features to bonding master
  2009-07-22 21:34           ` [PATCH net-next-2.6] bonding: propogate vlan_features to bonding master Jay Vosburgh
  2009-07-23  6:03             ` Eric Dumazet
@ 2009-07-23 11:03             ` Patrick McHardy
  1 sibling, 0 replies; 19+ messages in thread
From: Patrick McHardy @ 2009-07-23 11:03 UTC (permalink / raw)
  To: Jay Vosburgh; +Cc: Or Gerlitz, David Miller, netdev

Jay Vosburgh wrote:
> 	Propogate the vlan_features of the slave devices to the bonding
> master device, using the same logic as for regular features.
> 
> 	Tested by Or Gerlitz <ogerlitz@voltaire.com>, who also removed
> the debug logic from the original test patch.
> 
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 3bf0cc6..5a8b882 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -1354,6 +1359,7 @@ static int bond_compute_features(struct bonding *bond)
>  done:
>  	features |= (bond_dev->features & BOND_VLAN_FEATURES);
>  	bond_dev->features = netdev_fix_features(features, NULL);
> +	bond_dev->vlan_features = netdev_fix_features(vlan_features, NULL);


You need to call netdev_features_change() to make sure the VLAN
code notices the change.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH net-next-2.6] bnx2: Update vlan_features
  2009-07-23  8:29                 ` [PATCH net-next-2.6] bnx2: Update vlan_features Eric Dumazet
@ 2009-07-23 12:01                   ` Eric Dumazet
  2009-07-23 17:59                     ` David Miller
  0 siblings, 1 reply; 19+ messages in thread
From: Eric Dumazet @ 2009-07-23 12:01 UTC (permalink / raw)
  To: Or Gerlitz
  Cc: Patrick McHardy, Jay Vosburgh, David Miller, netdev,
	Eilon Greenstein, Michael Chan

Eric Dumazet a écrit :
> Or Gerlitz a écrit :
>> Eric Dumazet wrote:
>>> Jay Vosburgh a écrit :
>>>> Propogate the vlan_features of the slave devices to the bonding
>>>> master device, using the same logic as for regular features.
>>> Seems pretty cool, but I could not test it on my dev machine, since
>>> tg3 and bnx2 drivers dont advertize yet vlan_features
>> The bnx2x maintainer posted yesterday a patch that does so, I assume a
>> similar patch could work for at least one of tg3 or bnx2. I copied both
>> maintainers on this email, in  the hope they can come up with a patch.
>>
> 
> Here is the bnx2 patch I cooked to test this vlan_features propagation on bonding.
> 
> Everything fine so far !
> 
> # cat /proc/net/bonding/bond0
> Ethernet Channel Bonding Driver: v3.5.0 (November 4, 2008)
> 
> Bonding Mode: fault-tolerance (active-backup)
> Primary Slave: None
> Currently Active Slave: eth1
> MII Status: up
> MII Polling Interval (ms): 100
> Up Delay (ms): 0
> Down Delay (ms): 0
> 
> Slave Interface: eth1    (bnx2 driver)
> MII Status: up
> Link Failure Count: 2
> Permanent HW addr: 00:1e:0b:ec:d3:d2
> 
> Slave Interface: eth2    (tg3 driver)
> MII Status: up
> Link Failure Count: 0
> Permanent HW addr: 00:1e:0b:92:78:50
> 
> 
> # ethtool -k bond0
> Offload parameters for bond0:
> Cannot get device rx csum settings: Operation not supported
> rx-checksumming: off
> tx-checksumming: on
> scatter-gather: on
> tcp-segmentation-offload: on
> udp-fragmentation-offload: off
> generic-segmentation-offload: off
> generic-receive-offload: off
> large-receive-offload: off
> 
> # ip link add link bond0 vlan.103 type vlan id 103
> # ethtool -k vlan.103
> Offload parameters for vlan.103:
> rx-checksumming: off
> tx-checksumming: on
> scatter-gather: on
> tcp-segmentation-offload: on
> udp-fragmentation-offload: off
> generic-segmentation-offload: on
> generic-receive-offload: off
> large-receive-offload: off
> 
> 
> Thanks Or & Jay
> 

Updated to take into account Patrick feedback : We dont need to
change vlan_features in bnx2_set_tso()

Quoting Patrick from another thread :
  "vlan_features doesn't need to be updated, the resulting dev->features
  of the VLAN device is computed as the intersection of dev->features
  and dev->vlan_features."

[PATCH net-next-2.6] bnx2: Update vlan_features

In order to get full use of some advanced features of BNX2, we now need to
fill dev->vlan_features.

Patch successfully tested with vlan devices built on top of bonding.
(bond0 : one bnx2 slave, one tg3 slave (not yet vlan_features enabled)

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 drivers/net/bnx2.c |   19 +++++++++++++++----
 1 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
index b70cc99..cec1b17 100644
--- a/drivers/net/bnx2.c
+++ b/drivers/net/bnx2.c
@@ -8023,6 +8023,13 @@ static const struct net_device_ops bnx2_netdev_ops = {
 #endif
 };
 
+static void inline vlan_features_add(struct net_device *dev, unsigned long flags)
+{
+#ifdef BCM_VLAN
+	dev->vlan_features |= flags;
+#endif
+}
+
 static int __devinit
 bnx2_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 {
@@ -8064,16 +8071,20 @@ bnx2_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	memcpy(dev->perm_addr, bp->mac_addr, 6);
 
 	dev->features |= NETIF_F_IP_CSUM | NETIF_F_SG;
-	if (CHIP_NUM(bp) == CHIP_NUM_5709)
+	vlan_features_add(dev, NETIF_F_IP_CSUM | NETIF_F_SG);
+	if (CHIP_NUM(bp) == CHIP_NUM_5709) {
 		dev->features |= NETIF_F_IPV6_CSUM;
-
+		vlan_features_add(dev, NETIF_F_IPV6_CSUM);
+	}
 #ifdef BCM_VLAN
 	dev->features |= NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX;
 #endif
 	dev->features |= NETIF_F_TSO | NETIF_F_TSO_ECN;
-	if (CHIP_NUM(bp) == CHIP_NUM_5709)
+	vlan_features_add(dev, NETIF_F_TSO | NETIF_F_TSO_ECN);
+	if (CHIP_NUM(bp) == CHIP_NUM_5709) {
 		dev->features |= NETIF_F_TSO6;
-
+		vlan_features_add(dev, NETIF_F_TSO6);
+	}
 	if ((rc = register_netdev(dev))) {
 		dev_err(&pdev->dev, "Cannot register net device\n");
 		goto error;

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH net-next-2.6] bnx2: Update vlan_features
  2009-07-23 12:01                   ` Eric Dumazet
@ 2009-07-23 17:59                     ` David Miller
  2009-07-24  0:12                       ` Michael Chan
  0 siblings, 1 reply; 19+ messages in thread
From: David Miller @ 2009-07-23 17:59 UTC (permalink / raw)
  To: eric.dumazet; +Cc: ogerlitz, kaber, fubar, netdev, eilong, mchan

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 23 Jul 2009 14:01:38 +0200

> [PATCH net-next-2.6] bnx2: Update vlan_features
> 
> In order to get full use of some advanced features of BNX2, we now need to
> fill dev->vlan_features.
> 
> Patch successfully tested with vlan devices built on top of bonding.
> (bond0 : one bnx2 slave, one tg3 slave (not yet vlan_features enabled)
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Can I get some Broadcom ACKs for this?

Thanks.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH net-next-2.6] bnx2: Update vlan_features
  2009-07-23 17:59                     ` David Miller
@ 2009-07-24  0:12                       ` Michael Chan
  2009-07-24  7:21                         ` [PATCH net-next-2.6] bnx2x: Dont update vlan_features in bnx2x_set_tso() Eric Dumazet
  2009-07-27  2:50                         ` [PATCH net-next-2.6] bnx2: Update vlan_features David Miller
  0 siblings, 2 replies; 19+ messages in thread
From: Michael Chan @ 2009-07-24  0:12 UTC (permalink / raw)
  To: David Miller
  Cc: eric.dumazet, ogerlitz, kaber, fubar, netdev, Eilon Greenstein


On Thu, 2009-07-23 at 10:59 -0700, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Thu, 23 Jul 2009 14:01:38 +0200
> 
> > [PATCH net-next-2.6] bnx2: Update vlan_features
> > 
> > In order to get full use of some advanced features of BNX2, we now need to
> > fill dev->vlan_features.
> > 
> > Patch successfully tested with vlan devices built on top of bonding.
> > (bond0 : one bnx2 slave, one tg3 slave (not yet vlan_features enabled)
> > 
> > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> 
> Can I get some Broadcom ACKs for this?
> 
I've reviewed and tested Eric's 2nd patch.  Thanks.

Acked-by: Michael Chan <mchan@broadcom.com>



^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH net-next-2.6] bnx2x: Dont update vlan_features in bnx2x_set_tso()
  2009-07-24  0:12                       ` Michael Chan
@ 2009-07-24  7:21                         ` Eric Dumazet
  2009-07-26 10:53                           ` Eilon Greenstein
  2009-07-27  2:50                         ` [PATCH net-next-2.6] bnx2: Update vlan_features David Miller
  1 sibling, 1 reply; 19+ messages in thread
From: Eric Dumazet @ 2009-07-24  7:21 UTC (permalink / raw)
  To: Michael Chan
  Cc: David Miller, ogerlitz, kaber, fubar, netdev, Eilon Greenstein

Michael Chan a écrit :
> On Thu, 2009-07-23 at 10:59 -0700, David Miller wrote:
>> From: Eric Dumazet <eric.dumazet@gmail.com>
>> Date: Thu, 23 Jul 2009 14:01:38 +0200
>>
>>> [PATCH net-next-2.6] bnx2: Update vlan_features
>>>
>>> In order to get full use of some advanced features of BNX2, we now need to
>>> fill dev->vlan_features.
>>>
>>> Patch successfully tested with vlan devices built on top of bonding.
>>> (bond0 : one bnx2 slave, one tg3 slave (not yet vlan_features enabled)
>>>
>>> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
>> Can I get some Broadcom ACKs for this?
>>
> I've reviewed and tested Eric's 2nd patch.  Thanks.
> 
> Acked-by: Michael Chan <mchan@broadcom.com>

Thanks

Based on Patrick feedback, I believe we can add this followup too

[PATCH net-next-2.6] bnx2x: Dont update vlan_features in bnx2x_set_tso()

Patrick said : "vlan_features doesn't need to be updated, the resulting
dev->features of the VLAN device is computed as the intersection of
dev->features and dev->vlan_features."

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---

diff --git a/drivers/net/bnx2x_main.c b/drivers/net/bnx2x_main.c
index c4c42b3..a2de0cd 100644
--- a/drivers/net/bnx2x_main.c
+++ b/drivers/net/bnx2x_main.c
@@ -9349,17 +9349,9 @@ static int bnx2x_set_tso(struct net_device *dev, u32 data)
 	if (data) {
 		dev->features |= (NETIF_F_TSO | NETIF_F_TSO_ECN);
 		dev->features |= NETIF_F_TSO6;
-#ifdef BCM_VLAN
-		dev->vlan_features |= (NETIF_F_TSO | NETIF_F_TSO_ECN);
-		dev->vlan_features |= NETIF_F_TSO6;
-#endif
 	} else {
 		dev->features &= ~(NETIF_F_TSO | NETIF_F_TSO_ECN);
 		dev->features &= ~NETIF_F_TSO6;
-#ifdef BCM_VLAN
-		dev->vlan_features &= ~(NETIF_F_TSO | NETIF_F_TSO_ECN);
-		dev->vlan_features &= ~NETIF_F_TSO6;
-#endif
 	}
 
 	return 0;

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH net-next-2.6] bnx2x: Dont update vlan_features in bnx2x_set_tso()
  2009-07-24  7:21                         ` [PATCH net-next-2.6] bnx2x: Dont update vlan_features in bnx2x_set_tso() Eric Dumazet
@ 2009-07-26 10:53                           ` Eilon Greenstein
  2009-07-27  2:49                             ` David Miller
  0 siblings, 1 reply; 19+ messages in thread
From: Eilon Greenstein @ 2009-07-26 10:53 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Michael Chan, David Miller, ogerlitz, kaber, fubar, netdev

On Fri, 2009-07-24 at 00:21 -0700, Eric Dumazet wrote:
> Based on Patrick feedback, I believe we can add this followup too
> 
> [PATCH net-next-2.6] bnx2x: Dont update vlan_features in bnx2x_set_tso()
> 
> Patrick said : "vlan_features doesn't need to be updated, the resulting
> dev->features of the VLAN device is computed as the intersection of
> dev->features and dev->vlan_features."
> 
Thanks Eric

> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Acked-by: Eilon Greenstein <eilong@broadcom.com>




^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH net-next-2.6] bnx2x: Dont update vlan_features in bnx2x_set_tso()
  2009-07-26 10:53                           ` Eilon Greenstein
@ 2009-07-27  2:49                             ` David Miller
  0 siblings, 0 replies; 19+ messages in thread
From: David Miller @ 2009-07-27  2:49 UTC (permalink / raw)
  To: eilong; +Cc: eric.dumazet, mchan, ogerlitz, kaber, fubar, netdev

From: "Eilon Greenstein" <eilong@broadcom.com>
Date: Sun, 26 Jul 2009 13:53:43 +0300

> On Fri, 2009-07-24 at 00:21 -0700, Eric Dumazet wrote:
>> Based on Patrick feedback, I believe we can add this followup too
>> 
>> [PATCH net-next-2.6] bnx2x: Dont update vlan_features in bnx2x_set_tso()
>> 
>> Patrick said : "vlan_features doesn't need to be updated, the resulting
>> dev->features of the VLAN device is computed as the intersection of
>> dev->features and dev->vlan_features."
>> 
> Thanks Eric
> 
>> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> Acked-by: Eilon Greenstein <eilong@broadcom.com>

Applied, thanks.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH net-next-2.6] bnx2: Update vlan_features
  2009-07-24  0:12                       ` Michael Chan
  2009-07-24  7:21                         ` [PATCH net-next-2.6] bnx2x: Dont update vlan_features in bnx2x_set_tso() Eric Dumazet
@ 2009-07-27  2:50                         ` David Miller
  1 sibling, 0 replies; 19+ messages in thread
From: David Miller @ 2009-07-27  2:50 UTC (permalink / raw)
  To: mchan; +Cc: eric.dumazet, ogerlitz, kaber, fubar, netdev, eilong

From: "Michael Chan" <mchan@broadcom.com>
Date: Thu, 23 Jul 2009 17:12:59 -0700

> 
> On Thu, 2009-07-23 at 10:59 -0700, David Miller wrote:
>> From: Eric Dumazet <eric.dumazet@gmail.com>
>> Date: Thu, 23 Jul 2009 14:01:38 +0200
>> 
>> > [PATCH net-next-2.6] bnx2: Update vlan_features
>> > 
>> > In order to get full use of some advanced features of BNX2, we now need to
>> > fill dev->vlan_features.
>> > 
>> > Patch successfully tested with vlan devices built on top of bonding.
>> > (bond0 : one bnx2 slave, one tg3 slave (not yet vlan_features enabled)
>> > 
>> > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
>> 
>> Can I get some Broadcom ACKs for this?
>> 
> I've reviewed and tested Eric's 2nd patch.  Thanks.
> 
> Acked-by: Michael Chan <mchan@broadcom.com>

Applied.

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2009-07-27  2:50 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-04-06 13:00 zero features for a vlan over bond Or Gerlitz
2009-04-07 14:16 ` Jay Vosburgh
2009-04-07 22:16   ` Or Gerlitz
2009-04-16 14:11   ` zero features for a vlan over bond / vlan features Or Gerlitz
2009-04-23 23:47     ` Jay Vosburgh
2009-06-08  9:34       ` Or Gerlitz
2009-07-20  8:08         ` Or Gerlitz
2009-07-22 21:34           ` [PATCH net-next-2.6] bonding: propogate vlan_features to bonding master Jay Vosburgh
2009-07-23  6:03             ` Eric Dumazet
2009-07-23  7:24               ` Or Gerlitz
2009-07-23  8:29                 ` [PATCH net-next-2.6] bnx2: Update vlan_features Eric Dumazet
2009-07-23 12:01                   ` Eric Dumazet
2009-07-23 17:59                     ` David Miller
2009-07-24  0:12                       ` Michael Chan
2009-07-24  7:21                         ` [PATCH net-next-2.6] bnx2x: Dont update vlan_features in bnx2x_set_tso() Eric Dumazet
2009-07-26 10:53                           ` Eilon Greenstein
2009-07-27  2:49                             ` David Miller
2009-07-27  2:50                         ` [PATCH net-next-2.6] bnx2: Update vlan_features David Miller
2009-07-23 11:03             ` [PATCH net-next-2.6] bonding: propogate vlan_features to bonding master Patrick McHardy

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.