Hi Chas From: Chas Williams, Monday, April 16, 2018 7:44 PM > On Mon, Apr 16, 2018 at 4:06 AM, Matan Azrad <[email protected]> > wrote: > > 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? > > Yes, I am pretty sure bonding doesn't get this right when the interfaces > aren't link up. While what bonding is doing is likely wrong, it doesn't mean > that the behavior of the PMDs are correct in leaving the link_status unset > until the first LSC interrupt. > > I plan to get around to looking at this bonding problem in a little bit. > Luckily it > seems that we always tend to get matched links and even if bonding is > advertising the wrong aggregate speed and duplex we are find for now. It > wouldn't pass close inspection by a protocol analyzer though. >
So, Are you going to fix it, If no, I think you can open a bug in Bugzilla. > >> > >> Signed-off-by: Chas Williams <[email protected]> > >> --- > >> 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 > >
