Re: [dpdk-dev] [PATCH] net/bonding: fix link properties with autoneg

[Matan Azrad <matan@mellanox.com>]

Hi Chas

From: Chas Williams, Wednesday, February 14, 2018 12:55 AM
> If a link is carrier down and using autonegotiation, then the PMD may not
> have detected a speed yet.  In this case the best we can do is ignore the link
> speed and duplex since they aren't valid.

Ok for this.

>  To be completely correct, there
> should be additional checks to prevent a slave that negotiates a different
> speed from being activated.

Looks like every changing in the link properties should cause LSC interrupt.
In the bonding LCS interrupt you could handle and to deactivate the device.
Also you should deal with the case of the first slave, what is happen if the first slave has invalid link properties?
How can you know that the speed\duplex_mode is invalid?
Are we sure LACP mode can run with auto negotiation?
  

> 
> Signed-off-by: Chas Williams <chas3@att.com>
> ---
>  drivers/net/bonding/rte_eth_bond_pmd.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c
> b/drivers/net/bonding/rte_eth_bond_pmd.c
> index 92ad688..5559879 100644
> --- a/drivers/net/bonding/rte_eth_bond_pmd.c
> +++ b/drivers/net/bonding/rte_eth_bond_pmd.c
> @@ -1545,9 +1545,10 @@ link_properties_valid(struct rte_eth_dev
> *ethdev,
>  	if (bond_ctx->mode == BONDING_MODE_8023AD) {
>  		struct rte_eth_link *bond_link = &bond_ctx-
> >mode4.slave_link;
> 
> -		if (bond_link->link_duplex != slave_link->link_duplex ||
> -			bond_link->link_autoneg != slave_link->link_autoneg
> ||
> -			bond_link->link_speed != slave_link->link_speed)
> +		if (bond_link->link_autoneg != slave_link->link_autoneg ||
> +		    (bond_link->link_autoneg != ETH_LINK_AUTONEG &&
> +		     (bond_link->link_duplex != slave_link->link_duplex ||
> +		      bond_link->link_speed != slave_link->link_speed)))
>  			return -1;
>  	}
> 
> --
> 2.9.5