[dpdk-dev] [PATCH] net/bonding: fix link properties with autoneg
Matan Azrad
matan at mellanox.com
Mon Apr 16 21:09:02 CEST 2018
Hi Chas
From: Chas Williams, Monday, April 16, 2018 7:44 PM
> On Mon, Apr 16, 2018 at 4:06 AM, Matan Azrad <matan at mellanox.com>
> 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 <chas3 at 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
> >
More information about the dev
mailing list