netdev - Re: PATCH: fix bridged 802.3ad bonding

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 3 Jun 2008 13:13:26 -0700
From:	Stephen Hemminger <shemminger@...ux-foundation.org>
To:	Jiri Bohac <jbohac@...e.cz>
Cc:	Jiri Bohac <jbohac@...e.cz>, netdev@...r.kernel.org,
	David Miller <davem@...emloft.net>,
	Jay Vosburgh <fubar@...ibm.com>
Subject: Re: PATCH: fix bridged 802.3ad bonding

On Tue, 3 Jun 2008 21:32:27 +0200
Jiri Bohac <jbohac@...e.cz> wrote:

> Hi,
> 
> On Tue, Jun 03, 2008 at 09:46:04AM -0700, Stephen Hemminger wrote:
> > On Tue, 3 Jun 2008 15:21:06 +0200 Jiri Bohac <jbohac@...e.cz> wrote:
> > 
> > > Bonding in 802.3ad mode breaks when the bond interface is added
> > > to a bridge (which makes 802.3ad unusable in XEN, for example).
> > > 
> > > The problem is that br_pass_frame_up() will change the skb's dev
> > > pointer to point to the bridge interface. As a result, the LACP
> > > packets will not reach the bond_3ad_lacpdu_recv() protocol
> > > handler registered on the bonding device. Even if they did, the
> > > handler won't work with the changed skb->dev.
> 
> ... actually, now I realized the above statement is wrong. The
> bridging code does not change the LACP frames, it just drops
> them, because the LACP frames always have the link-local
> destination MAC address.
>  
> > The packets do arrive on the bridge
> > interface which is an aggregation of all the interfaces in the bridge.
> > 
> > The LACPDU's are received via now on the bond device. If you moved
> > the packet type handler down to the physical interface, this problem
> > would go away because the packets would be handled to bond_3ad_lacpdu_recv
> > prior to being handled by the bridge.
> 
> This wouldn't really work, because with bonding the packet only
> passes through netif_receive_skb() once, it just has the skb->dev
> modified by skb_bond() at the very beginning.
> 
> 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)?
2) Do you want to process after netfilter processing to allow
   some firewalling possiblity?

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html