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

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

Jiri Bohac <jbohac@suse.cz> wrote:

>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?

	Your understanding is correct.

>> 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? ;)

	Personally, for ease of reading debug messages, I would prefer
the globally unique ID (or a patch to update the pr_debugs to add the
bond name).  From a technical point of view either way will function
correctly.  I'm not too worried about the overflow of the ID.

	-J

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