Re: [patch libteam] teamd: lacp: Do not unselect port if it changes state to "expired"

[Jiri Pirko]

Thu, Aug 25, 2016 at 05:51:44PM CEST, jiri(a)resnulli.us wrote:
> From: Jiri Pirko <jiri(a)mellanox.com&gt;
>
> This fixes a long standing issue in select/unselect logic. We need to
> leave port selected in case it changes state to "expired".
>
> Reported-by: Ido Schimmel <idosch(a)mellanox.com&gt;
> Fixes: 68861ff780df ("teamd: lacp: rewrite lacp port selecting/unselecting
> logic")
> Signed-off-by: Jiri Pirko <jiri(a)mellanox.com&gt;
> ---
>  teamd/teamd_runner_lacp.c | 21 ++++++++++++++++++---
>  1 file changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/teamd/teamd_runner_lacp.c b/teamd/teamd_runner_lacp.c
> index e959cb3..75576f4 100644
> --- a/teamd/teamd_runner_lacp.c
> +++ b/teamd/teamd_runner_lacp.c
> @@ -297,7 +297,7 @@ static int lacp_load_config(struct teamd_context *ctx, struct lacp
> *lacp)
>  	return 0;
>  }
>  
> -static bool lacp_port_selectable(struct lacp_port *lacp_port)
> +static bool lacp_port_loopback_free(struct lacp_port *lacp_port)
>  {
>  	if (!memcmp(lacp_port->actor.system,
>  		    lacp_port->partner.system, ETH_ALEN)) {
> @@ -305,11 +305,24 @@ static bool lacp_port_selectable(struct lacp_port *lacp_port)
>  			       "team device.", lacp_port->tdport->ifname);
>  		return false;
>  	}
> +	return true;
> +}
> +
> +static bool lacp_port_selectable_state(struct lacp_port *lacp_port)
> +{
>  	if (lacp_port->state == PORT_STATE_CURRENT)
>  		return true;
>  	return false;
>  }
>  
> +static bool lacp_port_unselectable_state(struct lacp_port *lacp_port)
> +{
> +	if (lacp_port->state == PORT_STATE_CURRENT ||
> +	    lacp_port->state == PORT_STATE_EXPIRED)
> +		return false;
> +	return true;
> +}
> +
>  static bool lacp_port_selected(struct lacp_port *lacp_port)
>  {
>  	return lacp_port->agg_lead;
> @@ -726,13 +739,15 @@ static bool lacp_port_mergeable(struct lacp_port *lacp_port)
>  static int lacp_port_agg_update(struct lacp_port *lacp_port)
>  {
>  	if (lacp_port_selected(lacp_port) &&
> -	    (!lacp_port_selectable(lacp_port) ||
> +	    (lacp_port_unselectable_state(lacp_port) ||
> +	     !lacp_port_loopback_free(lacp_port) ||
>  	     !lacp_port_correct_aggregation(lacp_port) ||
>  	     lacp_port_mergeable(lacp_port)))
>  		lacp_port_agg_unselect(lacp_port);
>  
>  	if (!lacp_port_selected(lacp_port) &&
> -	    lacp_port_selectable(lacp_port))
> +	    (lacp_port_selectable_state(lacp_port) || 
Actually, this is bug :) Will send v2

>+	     lacp_port_loopback_free(lacp_port)))
> 		lacp_port_agg_select(lacp_port);
> 
> 	return lacp_selected_agg_update(lacp_port->lacp, NULL);
>-- 
>2.5.5
>