netdev - Re: [PATCH]: bonding: Fix 802.3ad no carrier on "no partner found" instance

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] [day] [month] [year] [list]

Message-ID: <22892.1180725300@death>
Date:	Fri, 01 Jun 2007 12:15:00 -0700
From:	Jay Vosburgh <fubar@...ibm.com>
To:	"Laurent Chavey" <chavey@...gle.com>
cc:	netdev@...r.kernel.org
Subject: Re: [PATCH]: bonding: Fix 802.3ad no carrier on "no partner found" instance 

Laurent Chavey <chavey@...gle.com> wrote:

>On 6/1/07, Jay Vosburgh <fubar@...ibm.com> wrote:
[...]
>>         Prior to the change in question, the carrier state of the master
>> device was always on, regardless of the state of the slaves (so even if
>> things didn't work, bonding would claim to be up).
>
>looking at the code, this will only work if there is an actual active
>aggregator. If that is not the case, and as you mention there could
>be an active aggregator before completing all LACP pahses (on all the slaves)
>then we would need to add a check (not for the mac) but for the churned state.
>(I added a churned state machine that could be use there, I did not submit
>the patch yet)

	I'm not exactly sure what you mean, but I'm guessing that you're
saying that the current code won't work properly when there isn't an
active aggregator.  That's true, but the only times there isn't an
active aggregator is when there are no slaves, and temporarily during
802.3ad negoitation (if there are no 802.3ad responses, an aggregator is
chosen from the set after a timeout).  During those times, I believe it
to be correct for the bonding master to be down, as no transmission or
reception is possible.

	I'm not at all sure what you mean by a churned state, but I
don't think any additional state checks are needed here, for the
following reason.

	I checked the 802.3ad standard, and I believe that the switches
I'm testing against are not in compliance with the standard,
particularly the following:

43.3.9 Attaching a link to an Aggregator

Links that are not successful candidates for aggregation (e.g., links
that are attached to other devices that cannot perform aggregation or
links that have been manually conﬁgured to be non-aggregatable) are
enabled to operate as individual IEEE 802.3 links. For consistency of
modeling, such a link is regarded as being attached to a compatible
Aggregator that can only be associated with a single link. That is, from
the perspective of Link Aggregation, non-aggregated links are not a
special case; they compose an aggregation with a maximum membership of
one link.

	The switches I have (Cisco 2960, 2970) do not appear to enable
links that are not successful candidates for aggregation as individual
802.3 links, rather, those links are disabled entirely.  

	Anyway, in light of this, I don't see a problem with your patch,
I believe it would produce results in compliance with the standard.  It
may result in false "carrier up" for cases of, e.g., negotiation failure
with 802.3ad switches such as I test with, but the cause there appears
to be due to noncompliance on the switch.

	Can you update your patch to change the comments above
bond_3ad_set_carrier() to remove the word "partnered" and note that the
code is implementing compliance with section 43.3.9 of IEEE 802.3, and
then resubmit your patch with the proper "Signed-off-by" attribution?

	-J

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@...ibm.com
-
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