2023-08-07 12:00:14

by Petr Oros

[permalink] [raw]
Subject: [PATCH net 0/2] Fix VF to VM attach detach

Petr Oros (2):
Revert "ice: Fix ice VF reset during iavf initialization"
ice: Fix NULL pointer deref during VF reset

drivers/net/ethernet/intel/ice/ice_sriov.c | 8 ++---
drivers/net/ethernet/intel/ice/ice_vf_lib.c | 34 +++++--------------
drivers/net/ethernet/intel/ice/ice_vf_lib.h | 1 -
drivers/net/ethernet/intel/ice/ice_virtchnl.c | 1 -
4 files changed, 12 insertions(+), 32 deletions(-)

--
2.41.0



2023-08-08 22:11:17

by Przemek Kitszel

[permalink] [raw]
Subject: Re: [Intel-wired-lan] [PATCH net 0/2] Fix VF to VM attach detach

On 8/7/23 11:48, Petr Oros wrote:
> Petr Oros (2):
> Revert "ice: Fix ice VF reset during iavf initialization"
> ice: Fix NULL pointer deref during VF reset
>
> drivers/net/ethernet/intel/ice/ice_sriov.c | 8 ++---
> drivers/net/ethernet/intel/ice/ice_vf_lib.c | 34 +++++--------------
> drivers/net/ethernet/intel/ice/ice_vf_lib.h | 1 -
> drivers/net/ethernet/intel/ice/ice_virtchnl.c | 1 -
> 4 files changed, 12 insertions(+), 32 deletions(-)
>

There were 3 typos reported, but this series is good anyway

Reviewed-by: Przemek Kitszel <[email protected]>

2023-08-09 01:30:21

by Jacob Keller

[permalink] [raw]
Subject: Re: [PATCH net 0/2] Fix VF to VM attach detach



On 8/7/2023 2:48 AM, Petr Oros wrote:
> Petr Oros (2):
> Revert "ice: Fix ice VF reset during iavf initialization"
> ice: Fix NULL pointer deref during VF reset
>
> drivers/net/ethernet/intel/ice/ice_sriov.c | 8 ++---
> drivers/net/ethernet/intel/ice/ice_vf_lib.c | 34 +++++--------------
> drivers/net/ethernet/intel/ice/ice_vf_lib.h | 1 -
> drivers/net/ethernet/intel/ice/ice_virtchnl.c | 1 -
> 4 files changed, 12 insertions(+), 32 deletions(-)
>


I reviewed the commit message for the reverted commit and I concur that
any sort of issue that it fixed with respect to concurrent resets is
better fixed by the 2nd commit of this series.

The motivation for the original commit appears to be to prevent a reset
from happening while the VF is still resetting. I think this motivation
is incorrect for a few reasons:

1) as described by the revert above, if the VF has not had iAVF load on
it yet (such as when its been assigned to the pass through PCI driver),
it will never initialize, and thus we can't ever reset the device.

2) It does not make sense for the PF to rely on or assume behavior of
the VF (i.e. that it will properly initialize) and I believe it should
be allowed to reset the device without regard to the state of the VF
driver. It is a bug in the VF driver if this causes problems for it. I
think we recently fixed several issues here in iAVF.

With that in mind, I think this series of fixes is good.

Reviewed-by: Jacob Keller <[email protected]>