Re: PATCH: fix bridged 802.3ad bonding

[Jay Vosburgh <fubar@us.ibm.com>]

Stephen Hemminger <shemminger@linux-foundation.org> wrote:

>On Tue, 3 Jun 2008 21:32:27 +0200
>Jiri Bohac <jbohac@suse.cz> wrote:
[...]
>> But I think I found a much nicer fix for the problem:
>> 
>> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
>> --- a/net/bridge/br_input.c
>> +++ b/net/bridge/br_input.c
>> @@ -136,6 +136,10 @@ struct sk_buff *br_handle_frame(struct net_bridge_port *p, struct sk_buff *skb)
>>  		if (skb->protocol == htons(ETH_P_PAUSE))
>>  			goto drop;
>>  
>> +		/* Don't touch SLOW frames (LACP, etc.) */
>> +		if (skb->protocol == htons(ETH_P_SLOW))
>> +			return skb;
>> +
>>  		/* Process STP BPDU's through normal netif_receive_skb() path */
>>  		if (p->br->stp_enabled != BR_NO_STP) {
>>  			if (NF_HOOK(PF_BRIDGE, NF_BR_LOCAL_IN, skb, skb->dev,
>> 
>> The LACP frames always have the link-local destination MAC
>> address and so they are not handled by the bridge anyway. They
>> are only dropped, unless STP is turned on. So let's just not drop
>> the SLOW packets. Does this look better?
>> 
>
>Better, but still have a couple of questions:
>1) Do you want to processing frames when bridge port is in blocking
>   state (because STP detected a loop)?

	I believe so.  If I'm reading correctly, the layout is something
like:

bridge -> bond0 -> [ eth0, eth1, etc ]

	so bonding needs to see the LACPDUs in order to decide which
subset of the slaves (eth0, eth1, etc) should be active and which should
not.  That, in turn, may affect the topology of the network.  In other
words, the presence or absence of a loop is determined by the set of
interfaces (or, really, the location of the peer of that set) made
active by link aggregation.  For 802.3ad, the set of active slaves
(active aggregator) will always connect to the same peer, but link
failures could move the active aggregator from one peer to a different
peer.

	This seems to agree with my (brief) examination of standards and
documentation: 802.3ad doesn't really say much about STP, 802.1d 6.5.1
discusses link aggregation a bit, in particular:

a) For a MAC entity that contains a Link Aggregation sublayer, the value
of MAC_Enabled is directly determined by the value of the aAggAdminState
attribute (30.7.1.13 in IEEE Std 802.3-2002), and the value of
MAC_Operational is directly determined by the value of the aAggOperState
attribute (30.7.1.13 in IEEE Std 802.3).

	suggests that the aggregation is treated as a unit (I'm not that
familiar with 802.1d, so I could be misreading it here).

	Lastly, Cisco's Etherchannel implementation treats a LACP
aggregation as a single bridge port.

	Thoughts?

	-J

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