On Sat, Apr 15, 2017 at 8:03 AM, Jamie Bainbridge jamie.bainbridge@gmail.com wrote:
On 14 April 2017 at 17:24, Xin Long lucien.xin@gmail.com wrote:
Patrick reported an issue about COLLECTING, DISTRIBUTING and SYNCHRONIZATION flags when the ports with standby agg. :
The ports associated with the non-selected (standby) aggregation should not sent the Synchronising, Collecting, and Distributing flags in their LACPDUs. So a port state of 7 for fast rate or 5 for slow rate.
It also causes team work different from bonding.
I've noticed this thread: https://lists.fedorahosted.org/archives/list/libteam@lists.fedorahosted.org/...
But here we will talk more about 'the ports with standby agg'.
He thinks:
- Collecting & Distributing bits indicate to the partner that the
port can actively be used to both receive and transmit normal network traffic. If a port is part of a *standby* aggregator, it just seems wrong to set these bits. With the bits set, the switch is being told that this port can be used to receive regular network traffic which is not true.
From The IEEE 802.1AX-2008 standard section 5.4.2.2 description of the Collecting & Distributing bits:
Collecting is encoded in bit 4. TRUE (encoded as a 1) means collection of incoming frames on this link is definitely enabled; i.e., collection is currently enabled and is not expected to be disabled in the absence of administrative changes or changes in received protocol information. Its value is otherwise FALSE (encoded as a 0).
Distributing is encoded in bit 5. FALSE (encoded as a 0) means distribution of outgoing frames on this link is definitely disabled; i.e., distribution is currently disabled and is not expected to be enabled in the absence of administrative changes or changes in received protocol information. Its value is otherwise TRUE (encoded as a 1).
- The bonding driver clearly agrees with me :p as it does not set
these bits on ports of an standby aggregator. It only sets bits 1 & 3 (& 2 depending on lacp rate) for ports in a standby aggregator.
- In 5.4.12.2 Selection Logic Requirements:
If there are further constraints on the attachment of ports that have selected an Aggregator, those ports may be selected as standby in accordance with the rules specified in 5.6.1. Selection or deselection of that Aggregator can cause the Selection Logic to re-evaluate the ports to be selected as standby.
.... and in 5.6.1 Use of system and port priorities:
For each link that a given System cannot include in the aggregation, the Selection Logic identifies the Selection state of the corresponding port as STANDBY, preventing the link from becoming active. The synchronization state signaled in transmitted LACPDUs for such links will be OUT_OF_SYNC.
(out of sync means the Synchornization bit should not be set).
Basically, there should be some logic where each port should check whether it is a member of a 'selected' aggregator and if not, then it should set its Synchronization, Collecting, and Distributing bits to 0.
From 5.6.1 the correct behaviour seems to be:
- Standby ports MAY set Collecting and Distributing bits
- Standby ports MUST NOT set Synchronization bit
The lack of Synchronization bit should prevent the standby port being included in the selection logic.
Thanks Jamie.
I think we are on same page about that standby port shouldn't set Synchronization bit.
As for Collecting and Distributing, if no other special reasons, to keep consistent with bonding, I agree with Patrick to clear them as well.
--- a/teamd/teamd_runner_lacp.c +++ b/teamd/teamd_runner_lacp.c @@ -913,9 +913,11 @@ static void lacp_port_actor_update(struct lacp_port *lacp_port) state |= INFO_STATE_LACP_ACTIVITY; if (lacp_port->lacp->cfg.fast_rate) state |= INFO_STATE_LACP_TIMEOUT; - if (lacp_port_selected(lacp_port)) - state |= INFO_STATE_SYNCHRONIZATION; - state |= INFO_STATE_COLLECTING | INFO_STATE_DISTRIBUTING; + if (lacp_port_selected(lacp_port) && + lacp_port_agg_selected(lacp_port)) { + state |= INFO_STATE_SYNCHRONIZATION | + INFO_STATE_COLLECTING | INFO_STATE_DISTRIBUTING; + }
Jiri, what do you think ?