Subject: Re: [PATCH] bonding: 802.3ad: make aggregator_identifier bond-private

From: Jiri Bohac <[email-masked]>
Date: Fri, 14 Feb 2014 21:51:47 +0100

On Fri, Feb 14, 2014 at 05:12:43PM -0200, Flavio Leitner wrote:
> On Fri, Feb 14, 2014 at 06:13:50PM +0100, Jiri Bohac wrote:
> > Fix this by making aggregator_identifier private to the bond.
> 
> I don't see how you fix the duplicate agg id with this patch because
> you initialize for each bond to 0, then use the same algo further on.
> So, what is changing?

My understanding is that the aggregator identifier is used
internally by the bond and never appears anywhere in the LACP
traffic.

So having duplicate aggregator ids between two bonds on the same
machine does not matter. But it is a problem if two aggregators
in the same bond share the same id.

Is my understanding wrong?
 
> Actually, aggregator_identifier is a global variable to make sure the
> counter is always increasing for new bonds.  So, the fix would be to
> not reset it to zero, isn't it?

I was considering this fix, but my concern was that the variable
(u16) would overflow sooner than it does now. It would take 2^16
enslavings on the machine, while with my patch you need 2^16
enslavings on a single bond.

Hypothetically, a rogue NET_ADMIN in one net namespace may cause
this overflow to break a bond in another nemespace.

Maybe I'm being paranoid? ;)

-- 
Jiri Bohac <[email-masked]>
SUSE Labs, SUSE CZ

-- 
Jiri Bohac <[email-masked]>
SUSE Labs, SUSE CZ