Fri, Sep 18, 2015 at 12:00:51AM CEST, sridhar.samudrala(a)intel.com wrote:
On 9/17/2015 2:10 PM, Flavio Leitner wrote:
>On Thu, Sep 17, 2015 at 11:45:20AM -0700, Samudrala, Sridhar wrote:
>>On 9/17/2015 12:42 AM, Jiri Pirko wrote:
>>>Can you please provide some more info about this change, like when in
>>>the spec this is decribed? Thanks!
>>Section 5.3.15 in 802.1ax spec defines these flags
>>http://varlyakov.com/diplomna/diplomna/802.1ax/802.1ax.pdf
>>Here is another link that describes the LACP flags.
>>http://networkingbodges.blogspot.com/2012/12/all-sorts-of-things-about-lacp-and-lags.html
>Are you actually seeing a bug without this patch?
>
>COLLECTING and DISTRIBUTING are informative bits about the port's
>state, but they are secondary to other states like SYNCHRONIZATION,
>so I don't see how that can cause an issue.
IIRC, I originally decided to set those 2 bits always, because there is
not an actual state machine inside teamd_lacp as it is described in the
standard, so the internal status does not make a lot of sense.
>
>With the proposed patch applied, there could be an outage if
>the state moves to EXPIRED or DEFAULTED.
With the current code, these bits are always set even if the partner is not
present. Is this OK?
When the state moves to EXPIRED or DEFAULTED, isn't it correct to mark these
bits as FALSE?
It looks like teamd implementation is not really looking at these bits in the
received LACPDUs.
This is more of an issue detected by our validation team looking at the LACP
frames generated by
teamd rather than an actual interoperability issue with other
implementations.
If it does not break interoperability, I would leave this as it is.
Unless there is an argument againt it. Thoughts?
>
>So, without more details for this change, NACK.
>
>fbl
>
>>Thanks
>>Sridhar
>>
>>>
>>>Thu, Sep 17, 2015 at 12:47:36AM CEST, sridhar.samudrala(a)intel.com wrote:
>>>>Signed-off-by: Sridhar Samudrala <sridhar.samudrala(a)intel.com>
>>>>---
>>>>teamd/teamd_runner_lacp.c | 3 ++-
>>>>1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>>diff --git a/teamd/teamd_runner_lacp.c b/teamd/teamd_runner_lacp.c
>>>>index 09899ce..f663016 100644
>>>>--- a/teamd/teamd_runner_lacp.c
>>>>+++ b/teamd/teamd_runner_lacp.c
>>>>@@ -900,7 +900,8 @@ static int lacp_port_actor_update(struct lacp_port
*lacp_port)
>>>> 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->state == PORT_STATE_CURRENT)
>>>>+ state |= INFO_STATE_COLLECTING | INFO_STATE_DISTRIBUTING;
>>>> if (lacp_port->state == PORT_STATE_EXPIRED)
>>>> state |= INFO_STATE_EXPIRED;
>>>> if (lacp_port->state == PORT_STATE_DEFAULTED)
>>>>--
>>>>2.1.0
>>>>
>>_______________________________________________
>>libteam mailing list
>>libteam(a)lists.fedorahosted.org
>>https://lists.fedorahosted.org/mailman/listinfo/libteam
_______________________________________________
libteam mailing list
libteam(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/libteam