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

[Chas Williams <3chas3@gmail.com>]

On Thu, Jun 14, 2018 at 1:04 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:

> On 4/16/2018 8:09 PM, Matan Azrad wrote:
> > Hi Chas
> >
> > From: Chas Williams, Monday, April 16, 2018 7:44 PM
> >> On Mon, Apr 16, 2018 at 4:06 AM, Matan Azrad <matan@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.
>
> Hi Matan, Chas,
>
> What is the latest status of the patch?
> And I guess there is another issue as well discussed here, is it still
> valid?
>
> Thanks,
> ferruh
>


I think this issue is better addressed by
http://dpdk.org/dev/patchwork/patch/40572/

There's just a little more cleanup that needs to be done in that patch.