On Fri, Jan 29, 2021 at 7:09 PM Jacob Keller <jacob.e.kel...@intel.com> wrote:
>
>
>
> On 1/29/2021 12:23 PM, Willem de Bruijn wrote:
> > On Thu, Jan 28, 2021 at 4:45 PM Tony Nguyen <anthony.l.ngu...@intel.com>
> > wrote:
> >>
> >> From: Aleksandr Loktionov <aleksandr.loktio...@intel.com>
> >>
> >> This reverts commit 2ad1274fa35ace5c6360762ba48d33b63da2396c
> >>
> >> VF queues were not brought up when PF was brought up after being
> >> downed if the VF driver disabled VFs queues during PF down.
> >> This could happen in some older or external VF driver implementations.
> >> The problem was that PF driver used vf->queues_enabled as a condition
> >> to decide what link-state it would send out which caused the issue.
> >>
> >> Remove the check for vf->queues_enabled in the VF link notify.
> >> Now VF will always be notified of the current link status.
> >> Also remove the queues_enabled member from i40e_vf structure as it is
> >> not used anymore. Otherwise VNF implementation was broken and caused
> >> a link flap.
> >>
> >> Fixes: 2ad1274fa35a ("i40e: don't report link up for a VF who hasn't
> >> enabled")
> >> Signed-off-by: Aleksandr Loktionov <aleksandr.loktio...@intel.com>
> >> Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalew...@intel.com>
> >> Tested-by: Konrad Jankowski <konrad0.jankow...@intel.com>
> >> Signed-off-by: Tony Nguyen <anthony.l.ngu...@intel.com>
> >
> > Doesn't this reintroduce the bug that the original patch aimed to solve?
> >
> > Commit 2ad1274fa35a itself was also a fix.
> >
>
> Yea this might re-introduce the issue described in that commit. However
> I believe the bug in question was due to very old versions of VF
> drivers, (including an ancient version of FreeBSD if I recall).
>
> Perhaps there is some better mechanism for handling this, but I think
> reverting this is ok given that it causes problems in certain situations
> where the link status wasn't reported properly.
>
> Maybe there is a solution for both cases? but I would worry less about
> an issue with the incredibly old VFs because we know that the issue is
> fixed in newer VF code and the real problem is that the VF driver is
> incorrectly assuming link up means it is ready to send.
>
> Thus, I am comfortable with this revert: It simplifies the state for
> both the PF and VF.
>
> I would be open to alternatives as long as the issue described here is
> also fixed.
>
> Caveat: I was not involved in the decision to revert this and wasn't
> aware of it until now, so I almost certainly have out of date information.
That's reasonable. The original patch is over three years old.
If it is considered safe to revert now, I would just articulate that
point in the commit.
