netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net] bonding: Don't assume 802.1Q when sending alb learning packets.
@ 2014-05-21 15:24 Vlad Yasevich
  2014-05-21 15:48 ` Veaceslav Falico
  2014-05-22 19:45 ` David Miller
  0 siblings, 2 replies; 4+ messages in thread
From: Vlad Yasevich @ 2014-05-21 15:24 UTC (permalink / raw)
  To: netdev; +Cc: Vlad Yasevich, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek

TLB/ALB learning packets always assume 802.1Q vlan protocol, but
that is no longer the case since we now have support for Q-in-Q
on top of bonding.  Pass the vlan protocol to alb_send_lp_vid()
so that the packets are properly tagged.

CC: Jay Vosburgh <j.vosburgh@gmail.com>
CC: Veaceslav Falico <vfalico@gmail.com>
CC: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
---
 drivers/net/bonding/bond_alb.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index e538478..2ec945c 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -995,7 +995,7 @@ static void rlb_clear_vlan(struct bonding *bond, unsigned short vlan_id)
 /*********************** tlb/rlb shared functions *********************/
 
 static void alb_send_lp_vid(struct slave *slave, u8 mac_addr[],
-			    u16 vid)
+			    __be16 vlan_proto, u16 vid)
 {
 	struct learning_pkt pkt;
 	struct sk_buff *skb;
@@ -1021,7 +1021,7 @@ static void alb_send_lp_vid(struct slave *slave, u8 mac_addr[],
 	skb->dev = slave->dev;
 
 	if (vid) {
-		skb = vlan_put_tag(skb, htons(ETH_P_8021Q), vid);
+		skb = vlan_put_tag(skb, vlan_proto, vid);
 		if (!skb) {
 			pr_err("%s: Error: failed to insert VLAN tag\n",
 			       slave->bond->dev->name);
@@ -1040,13 +1040,14 @@ static void alb_send_learning_packets(struct slave *slave, u8 mac_addr[])
 	struct list_head *iter;
 
 	/* send untagged */
-	alb_send_lp_vid(slave, mac_addr, 0);
+	alb_send_lp_vid(slave, mac_addr, 0, 0);
 
 	/* loop through vlans and send one packet for each */
 	rcu_read_lock();
 	netdev_for_each_all_upper_dev_rcu(bond->dev, upper, iter) {
 		if (is_vlan_dev(upper) && vlan_get_encap_level(upper) == 0)
 			alb_send_lp_vid(slave, mac_addr,
+					vlan_dev_vlan_proto(upper),
 					vlan_dev_vlan_id(upper));
 	}
 	rcu_read_unlock();
-- 
1.9.0

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

* Re: [PATCH net] bonding: Don't assume 802.1Q when sending alb learning packets.
  2014-05-21 15:24 [PATCH net] bonding: Don't assume 802.1Q when sending alb learning packets Vlad Yasevich
@ 2014-05-21 15:48 ` Veaceslav Falico
  2014-05-21 20:10   ` Vlad Yasevich
  2014-05-22 19:45 ` David Miller
  1 sibling, 1 reply; 4+ messages in thread
From: Veaceslav Falico @ 2014-05-21 15:48 UTC (permalink / raw)
  To: Vlad Yasevich; +Cc: netdev, Jay Vosburgh, Andy Gospodarek

On Wed, May 21, 2014 at 11:24:39AM -0400, Vlad Yasevich wrote:
>TLB/ALB learning packets always assume 802.1Q vlan protocol, but
>that is no longer the case since we now have support for Q-in-Q
>on top of bonding.  Pass the vlan protocol to alb_send_lp_vid()
>so that the packets are properly tagged.
>
>CC: Jay Vosburgh <j.vosburgh@gmail.com>
>CC: Veaceslav Falico <vfalico@gmail.com>

There one more left - the rlb_update_client(), but it'll require a bit more
hacking with rlb_client_info struct to work. Maybe some day in the future
:).

Acked-by: Veaceslav Falico <vfalico@gmail.com>

>CC: Andy Gospodarek <andy@greyhouse.net>
>Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
>---
> drivers/net/bonding/bond_alb.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
>index e538478..2ec945c 100644
>--- a/drivers/net/bonding/bond_alb.c
>+++ b/drivers/net/bonding/bond_alb.c
>@@ -995,7 +995,7 @@ static void rlb_clear_vlan(struct bonding *bond, unsigned short vlan_id)
> /*********************** tlb/rlb shared functions *********************/
>
> static void alb_send_lp_vid(struct slave *slave, u8 mac_addr[],
>-			    u16 vid)
>+			    __be16 vlan_proto, u16 vid)
> {
> 	struct learning_pkt pkt;
> 	struct sk_buff *skb;
>@@ -1021,7 +1021,7 @@ static void alb_send_lp_vid(struct slave *slave, u8 mac_addr[],
> 	skb->dev = slave->dev;
>
> 	if (vid) {
>-		skb = vlan_put_tag(skb, htons(ETH_P_8021Q), vid);
>+		skb = vlan_put_tag(skb, vlan_proto, vid);
> 		if (!skb) {
> 			pr_err("%s: Error: failed to insert VLAN tag\n",
> 			       slave->bond->dev->name);
>@@ -1040,13 +1040,14 @@ static void alb_send_learning_packets(struct slave *slave, u8 mac_addr[])
> 	struct list_head *iter;
>
> 	/* send untagged */
>-	alb_send_lp_vid(slave, mac_addr, 0);
>+	alb_send_lp_vid(slave, mac_addr, 0, 0);
>
> 	/* loop through vlans and send one packet for each */
> 	rcu_read_lock();
> 	netdev_for_each_all_upper_dev_rcu(bond->dev, upper, iter) {
> 		if (is_vlan_dev(upper) && vlan_get_encap_level(upper) == 0)
> 			alb_send_lp_vid(slave, mac_addr,
>+					vlan_dev_vlan_proto(upper),
> 					vlan_dev_vlan_id(upper));
> 	}
> 	rcu_read_unlock();
>-- 
>1.9.0
>

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

* Re: [PATCH net] bonding: Don't assume 802.1Q when sending alb learning packets.
  2014-05-21 15:48 ` Veaceslav Falico
@ 2014-05-21 20:10   ` Vlad Yasevich
  0 siblings, 0 replies; 4+ messages in thread
From: Vlad Yasevich @ 2014-05-21 20:10 UTC (permalink / raw)
  To: Veaceslav Falico; +Cc: netdev, Jay Vosburgh, Andy Gospodarek

On 05/21/2014 11:48 AM, Veaceslav Falico wrote:
> On Wed, May 21, 2014 at 11:24:39AM -0400, Vlad Yasevich wrote:
>> TLB/ALB learning packets always assume 802.1Q vlan protocol, but
>> that is no longer the case since we now have support for Q-in-Q
>> on top of bonding.  Pass the vlan protocol to alb_send_lp_vid()
>> so that the packets are properly tagged.
>>
>> CC: Jay Vosburgh <j.vosburgh@gmail.com>
>> CC: Veaceslav Falico <vfalico@gmail.com>
> 
> There one more left - the rlb_update_client(), but it'll require a bit more
> hacking with rlb_client_info struct to work. Maybe some day in the future
> :).

Just looked at it and it looks like q-in-q support is very incompletely
in for alb mode bonding.

Some things I noticed:
  1) bond_start_xmit() looks at skb->protocol only which could be 8021Q
     or 8021ad.  Need to look for the protocol harder.
  2) all the arp recv_probe handlers need to know how skip past vlan
     headers to see if this is an ARP or not.
  3) rlb_client_info needs to know about all vlan headers, not just the
     outer one so that rlb_update_client() can function correctly.

-vlad

> 
> Acked-by: Veaceslav Falico <vfalico@gmail.com>
> 
>> CC: Andy Gospodarek <andy@greyhouse.net>
>> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
>> ---
>> drivers/net/bonding/bond_alb.c | 7 ++++---
>> 1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/bonding/bond_alb.c
>> b/drivers/net/bonding/bond_alb.c
>> index e538478..2ec945c 100644
>> --- a/drivers/net/bonding/bond_alb.c
>> +++ b/drivers/net/bonding/bond_alb.c
>> @@ -995,7 +995,7 @@ static void rlb_clear_vlan(struct bonding *bond,
>> unsigned short vlan_id)
>> /*********************** tlb/rlb shared functions *********************/
>>
>> static void alb_send_lp_vid(struct slave *slave, u8 mac_addr[],
>> -                u16 vid)
>> +                __be16 vlan_proto, u16 vid)
>> {
>>     struct learning_pkt pkt;
>>     struct sk_buff *skb;
>> @@ -1021,7 +1021,7 @@ static void alb_send_lp_vid(struct slave *slave,
>> u8 mac_addr[],
>>     skb->dev = slave->dev;
>>
>>     if (vid) {
>> -        skb = vlan_put_tag(skb, htons(ETH_P_8021Q), vid);
>> +        skb = vlan_put_tag(skb, vlan_proto, vid);
>>         if (!skb) {
>>             pr_err("%s: Error: failed to insert VLAN tag\n",
>>                    slave->bond->dev->name);
>> @@ -1040,13 +1040,14 @@ static void alb_send_learning_packets(struct
>> slave *slave, u8 mac_addr[])
>>     struct list_head *iter;
>>
>>     /* send untagged */
>> -    alb_send_lp_vid(slave, mac_addr, 0);
>> +    alb_send_lp_vid(slave, mac_addr, 0, 0);
>>
>>     /* loop through vlans and send one packet for each */
>>     rcu_read_lock();
>>     netdev_for_each_all_upper_dev_rcu(bond->dev, upper, iter) {
>>         if (is_vlan_dev(upper) && vlan_get_encap_level(upper) == 0)
>>             alb_send_lp_vid(slave, mac_addr,
>> +                    vlan_dev_vlan_proto(upper),
>>                     vlan_dev_vlan_id(upper));
>>     }
>>     rcu_read_unlock();
>> -- 
>> 1.9.0
>>

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

* Re: [PATCH net] bonding: Don't assume 802.1Q when sending alb learning packets.
  2014-05-21 15:24 [PATCH net] bonding: Don't assume 802.1Q when sending alb learning packets Vlad Yasevich
  2014-05-21 15:48 ` Veaceslav Falico
@ 2014-05-22 19:45 ` David Miller
  1 sibling, 0 replies; 4+ messages in thread
From: David Miller @ 2014-05-22 19:45 UTC (permalink / raw)
  To: vyasevic; +Cc: netdev, j.vosburgh, vfalico, andy

From: Vlad Yasevich <vyasevic@redhat.com>
Date: Wed, 21 May 2014 11:24:39 -0400

> TLB/ALB learning packets always assume 802.1Q vlan protocol, but
> that is no longer the case since we now have support for Q-in-Q
> on top of bonding.  Pass the vlan protocol to alb_send_lp_vid()
> so that the packets are properly tagged.
> 
> CC: Jay Vosburgh <j.vosburgh@gmail.com>
> CC: Veaceslav Falico <vfalico@gmail.com>
> CC: Andy Gospodarek <andy@greyhouse.net>
> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>

Applied, thanks Vlad.

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

end of thread, other threads:[~2014-05-22 19:45 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-05-21 15:24 [PATCH net] bonding: Don't assume 802.1Q when sending alb learning packets Vlad Yasevich
2014-05-21 15:48 ` Veaceslav Falico
2014-05-21 20:10   ` Vlad Yasevich
2014-05-22 19:45 ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).