Laurent Chavey <[EMAIL PROTECTED]> wrote: >On 6/1/07, Jay Vosburgh <[EMAIL PROTECTED]> 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 configured 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, [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html