* [Intel-wired-lan] [PATCH net] ice: Clear default forwarding VSI during VSI release
@ 2022-03-22 14:25 ` Ivan Vecera
0 siblings, 0 replies; 11+ messages in thread
From: Ivan Vecera @ 2022-03-22 14:25 UTC (permalink / raw)
To: intel-wired-lan
VSI is set as default forwarding one when promisc mode is set for
PF interface, when PF is switched to switchdev mode or when VF
driver asks to enable allmulticast or promisc mode for the VF
interface (when vf-true-promisc-support priv flag is off).
The third case is buggy because in that case VSI associated with
VF remains as default one after VF removal.
Reproducer:
1. Create VF
echo 1 > sys/class/net/ens7f0/device/sriov_numvfs
2. Enable allmulticast or promisc mode on VF
ip link set ens7f0v0 allmulticast on
ip link set ens7f0v0 promisc on
3. Delete VF
echo 0 > sys/class/net/ens7f0/device/sriov_numvfs
4. Try to enable promisc mode on PF
ip link set ens7f0 promisc on
Although it looks that promisc mode on PF is enabled the opposite
is true because ice_vsi_sync_fltr() responsible for IFF_PROMISC
handling first checks if any other VSI is set as default forwarding
one and if so the function does not do anything. At this point
it is not possible to enable promisc mode on PF without re-probe
device.
To resolve the issue this patch clear default forwarding VSI
during ice_vsi_release() when the VSI to be released is the default
one.
Fixes: 01b5e89aab49 ("ice: Add VF promiscuous support")
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
drivers/net/ethernet/intel/ice/ice_lib.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
index 53256aca27c7..20d755822d43 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_lib.c
@@ -3147,6 +3147,8 @@ int ice_vsi_release(struct ice_vsi *vsi)
}
}
+ if (ice_is_vsi_dflt_vsi(pf->first_sw, vsi))
+ ice_clear_dflt_vsi(pf->first_sw);
ice_fltr_remove_all(vsi);
ice_rm_vsi_lan_cfg(vsi->port_info, vsi->idx);
err = ice_rm_vsi_rdma_cfg(vsi->port_info, vsi->idx);
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH net] ice: Clear default forwarding VSI during VSI release @ 2022-03-22 14:25 ` Ivan Vecera 0 siblings, 0 replies; 11+ messages in thread From: Ivan Vecera @ 2022-03-22 14:25 UTC (permalink / raw) To: netdev Cc: poros, mschmidt, Jesse Brandeburg, Tony Nguyen, David S. Miller, Jakub Kicinski, Paolo Abeni, Brett Creeley, Jeff Kirsher, moderated list:INTEL ETHERNET DRIVERS, open list VSI is set as default forwarding one when promisc mode is set for PF interface, when PF is switched to switchdev mode or when VF driver asks to enable allmulticast or promisc mode for the VF interface (when vf-true-promisc-support priv flag is off). The third case is buggy because in that case VSI associated with VF remains as default one after VF removal. Reproducer: 1. Create VF echo 1 > sys/class/net/ens7f0/device/sriov_numvfs 2. Enable allmulticast or promisc mode on VF ip link set ens7f0v0 allmulticast on ip link set ens7f0v0 promisc on 3. Delete VF echo 0 > sys/class/net/ens7f0/device/sriov_numvfs 4. Try to enable promisc mode on PF ip link set ens7f0 promisc on Although it looks that promisc mode on PF is enabled the opposite is true because ice_vsi_sync_fltr() responsible for IFF_PROMISC handling first checks if any other VSI is set as default forwarding one and if so the function does not do anything. At this point it is not possible to enable promisc mode on PF without re-probe device. To resolve the issue this patch clear default forwarding VSI during ice_vsi_release() when the VSI to be released is the default one. Fixes: 01b5e89aab49 ("ice: Add VF promiscuous support") Signed-off-by: Ivan Vecera <ivecera@redhat.com> --- drivers/net/ethernet/intel/ice/ice_lib.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c index 53256aca27c7..20d755822d43 100644 --- a/drivers/net/ethernet/intel/ice/ice_lib.c +++ b/drivers/net/ethernet/intel/ice/ice_lib.c @@ -3147,6 +3147,8 @@ int ice_vsi_release(struct ice_vsi *vsi) } } + if (ice_is_vsi_dflt_vsi(pf->first_sw, vsi)) + ice_clear_dflt_vsi(pf->first_sw); ice_fltr_remove_all(vsi); ice_rm_vsi_lan_cfg(vsi->port_info, vsi->idx); err = ice_rm_vsi_rdma_cfg(vsi->port_info, vsi->idx); -- 2.34.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Intel-wired-lan] [PATCH net] ice: Clear default forwarding VSI during VSI release 2022-03-22 14:25 ` Ivan Vecera @ 2022-03-23 17:39 ` Marcin Szycik -1 siblings, 0 replies; 11+ messages in thread From: Marcin Szycik @ 2022-03-23 17:39 UTC (permalink / raw) To: intel-wired-lan On 22-Mar-22 15:25, Ivan Vecera wrote: > VSI is set as default forwarding one when promisc mode is set for > PF interface, when PF is switched to switchdev mode or when VF > driver asks to enable allmulticast or promisc mode for the VF > interface (when vf-true-promisc-support priv flag is off). > The third case is buggy because in that case VSI associated with > VF remains as default one after VF removal. > > Reproducer: > 1. Create VF > echo 1 > sys/class/net/ens7f0/device/sriov_numvfs > 2. Enable allmulticast or promisc mode on VF > ip link set ens7f0v0 allmulticast on > ip link set ens7f0v0 promisc on > 3. Delete VF > echo 0 > sys/class/net/ens7f0/device/sriov_numvfs > 4. Try to enable promisc mode on PF > ip link set ens7f0 promisc on > > Although it looks that promisc mode on PF is enabled the opposite > is true because ice_vsi_sync_fltr() responsible for IFF_PROMISC > handling first checks if any other VSI is set as default forwarding > one and if so the function does not do anything. At this point > it is not possible to enable promisc mode on PF without re-probe > device. > > To resolve the issue this patch clear default forwarding VSI > during ice_vsi_release() when the VSI to be released is the default > one. > > Fixes: 01b5e89aab49 ("ice: Add VF promiscuous support") > Signed-off-by: Ivan Vecera <ivecera@redhat.com> > --- > drivers/net/ethernet/intel/ice/ice_lib.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c > index 53256aca27c7..20d755822d43 100644 > --- a/drivers/net/ethernet/intel/ice/ice_lib.c > +++ b/drivers/net/ethernet/intel/ice/ice_lib.c > @@ -3147,6 +3147,8 @@ int ice_vsi_release(struct ice_vsi *vsi) > } > } > > + if (ice_is_vsi_dflt_vsi(pf->first_sw, vsi)) > + ice_clear_dflt_vsi(pf->first_sw); It would probably be good to check `ice_clear_dflt_vsi` return code. > ice_fltr_remove_all(vsi); > ice_rm_vsi_lan_cfg(vsi->port_info, vsi->idx); > err = ice_rm_vsi_rdma_cfg(vsi->port_info, vsi->idx); ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net] ice: Clear default forwarding VSI during VSI release @ 2022-03-23 17:39 ` Marcin Szycik 0 siblings, 0 replies; 11+ messages in thread From: Marcin Szycik @ 2022-03-23 17:39 UTC (permalink / raw) To: Ivan Vecera, netdev Cc: poros, mschmidt, Jesse Brandeburg, Tony Nguyen, David S. Miller, Jakub Kicinski, Paolo Abeni, Brett Creeley, Jeff Kirsher, moderated list:INTEL ETHERNET DRIVERS, open list On 22-Mar-22 15:25, Ivan Vecera wrote: > VSI is set as default forwarding one when promisc mode is set for > PF interface, when PF is switched to switchdev mode or when VF > driver asks to enable allmulticast or promisc mode for the VF > interface (when vf-true-promisc-support priv flag is off). > The third case is buggy because in that case VSI associated with > VF remains as default one after VF removal. > > Reproducer: > 1. Create VF > echo 1 > sys/class/net/ens7f0/device/sriov_numvfs > 2. Enable allmulticast or promisc mode on VF > ip link set ens7f0v0 allmulticast on > ip link set ens7f0v0 promisc on > 3. Delete VF > echo 0 > sys/class/net/ens7f0/device/sriov_numvfs > 4. Try to enable promisc mode on PF > ip link set ens7f0 promisc on > > Although it looks that promisc mode on PF is enabled the opposite > is true because ice_vsi_sync_fltr() responsible for IFF_PROMISC > handling first checks if any other VSI is set as default forwarding > one and if so the function does not do anything. At this point > it is not possible to enable promisc mode on PF without re-probe > device. > > To resolve the issue this patch clear default forwarding VSI > during ice_vsi_release() when the VSI to be released is the default > one. > > Fixes: 01b5e89aab49 ("ice: Add VF promiscuous support") > Signed-off-by: Ivan Vecera <ivecera@redhat.com> > --- > drivers/net/ethernet/intel/ice/ice_lib.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c > index 53256aca27c7..20d755822d43 100644 > --- a/drivers/net/ethernet/intel/ice/ice_lib.c > +++ b/drivers/net/ethernet/intel/ice/ice_lib.c > @@ -3147,6 +3147,8 @@ int ice_vsi_release(struct ice_vsi *vsi) > } > } > > + if (ice_is_vsi_dflt_vsi(pf->first_sw, vsi)) > + ice_clear_dflt_vsi(pf->first_sw); It would probably be good to check `ice_clear_dflt_vsi` return code. > ice_fltr_remove_all(vsi); > ice_rm_vsi_lan_cfg(vsi->port_info, vsi->idx); > err = ice_rm_vsi_rdma_cfg(vsi->port_info, vsi->idx); ^ permalink raw reply [flat|nested] 11+ messages in thread
* [Intel-wired-lan] [PATCH net] ice: Clear default forwarding VSI during VSI release 2022-03-23 17:39 ` Marcin Szycik (?) @ 2022-03-23 17:54 ` Ivan Vecera 2022-03-23 18:19 ` Marcin Szycik -1 siblings, 1 reply; 11+ messages in thread From: Ivan Vecera @ 2022-03-23 17:54 UTC (permalink / raw) To: intel-wired-lan On Wed, 23 Mar 2022 18:39:11 +0100 Marcin Szycik <marcin.szycik@linux.intel.com> wrote: > On 22-Mar-22 15:25, Ivan Vecera wrote: > > VSI is set as default forwarding one when promisc mode is set for > > PF interface, when PF is switched to switchdev mode or when VF > > driver asks to enable allmulticast or promisc mode for the VF > > interface (when vf-true-promisc-support priv flag is off). > > The third case is buggy because in that case VSI associated with > > VF remains as default one after VF removal. > > > > Reproducer: > > 1. Create VF > > echo 1 > sys/class/net/ens7f0/device/sriov_numvfs > > 2. Enable allmulticast or promisc mode on VF > > ip link set ens7f0v0 allmulticast on > > ip link set ens7f0v0 promisc on > > 3. Delete VF > > echo 0 > sys/class/net/ens7f0/device/sriov_numvfs > > 4. Try to enable promisc mode on PF > > ip link set ens7f0 promisc on > > > > Although it looks that promisc mode on PF is enabled the opposite > > is true because ice_vsi_sync_fltr() responsible for IFF_PROMISC > > handling first checks if any other VSI is set as default forwarding > > one and if so the function does not do anything. At this point > > it is not possible to enable promisc mode on PF without re-probe > > device. > > > > To resolve the issue this patch clear default forwarding VSI > > during ice_vsi_release() when the VSI to be released is the default > > one. > > > > Fixes: 01b5e89aab49 ("ice: Add VF promiscuous support") > > Signed-off-by: Ivan Vecera <ivecera@redhat.com> > > --- > > drivers/net/ethernet/intel/ice/ice_lib.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c > > index 53256aca27c7..20d755822d43 100644 > > --- a/drivers/net/ethernet/intel/ice/ice_lib.c > > +++ b/drivers/net/ethernet/intel/ice/ice_lib.c > > @@ -3147,6 +3147,8 @@ int ice_vsi_release(struct ice_vsi *vsi) > > } > > } > > > > + if (ice_is_vsi_dflt_vsi(pf->first_sw, vsi)) > > + ice_clear_dflt_vsi(pf->first_sw); > > It would probably be good to check `ice_clear_dflt_vsi` return code. Check and report potential warning when error occurs? because we are in ice_vsi_release() so any rollback does not make sense. Ivan ^ permalink raw reply [flat|nested] 11+ messages in thread
* [Intel-wired-lan] [PATCH net] ice: Clear default forwarding VSI during VSI release 2022-03-23 17:54 ` [Intel-wired-lan] " Ivan Vecera @ 2022-03-23 18:19 ` Marcin Szycik 0 siblings, 0 replies; 11+ messages in thread From: Marcin Szycik @ 2022-03-23 18:19 UTC (permalink / raw) To: intel-wired-lan On 23-Mar-22 18:54, Ivan Vecera wrote: > On Wed, 23 Mar 2022 18:39:11 +0100 > Marcin Szycik <marcin.szycik@linux.intel.com> wrote: > >> On 22-Mar-22 15:25, Ivan Vecera wrote: >>> VSI is set as default forwarding one when promisc mode is set for >>> PF interface, when PF is switched to switchdev mode or when VF >>> driver asks to enable allmulticast or promisc mode for the VF >>> interface (when vf-true-promisc-support priv flag is off). >>> The third case is buggy because in that case VSI associated with >>> VF remains as default one after VF removal. >>> >>> Reproducer: >>> 1. Create VF >>> echo 1 > sys/class/net/ens7f0/device/sriov_numvfs >>> 2. Enable allmulticast or promisc mode on VF >>> ip link set ens7f0v0 allmulticast on >>> ip link set ens7f0v0 promisc on >>> 3. Delete VF >>> echo 0 > sys/class/net/ens7f0/device/sriov_numvfs >>> 4. Try to enable promisc mode on PF >>> ip link set ens7f0 promisc on >>> >>> Although it looks that promisc mode on PF is enabled the opposite >>> is true because ice_vsi_sync_fltr() responsible for IFF_PROMISC >>> handling first checks if any other VSI is set as default forwarding >>> one and if so the function does not do anything. At this point >>> it is not possible to enable promisc mode on PF without re-probe >>> device. >>> >>> To resolve the issue this patch clear default forwarding VSI >>> during ice_vsi_release() when the VSI to be released is the default >>> one. >>> >>> Fixes: 01b5e89aab49 ("ice: Add VF promiscuous support") >>> Signed-off-by: Ivan Vecera <ivecera@redhat.com> >>> --- >>> drivers/net/ethernet/intel/ice/ice_lib.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c >>> index 53256aca27c7..20d755822d43 100644 >>> --- a/drivers/net/ethernet/intel/ice/ice_lib.c >>> +++ b/drivers/net/ethernet/intel/ice/ice_lib.c >>> @@ -3147,6 +3147,8 @@ int ice_vsi_release(struct ice_vsi *vsi) >>> } >>> } >>> >>> + if (ice_is_vsi_dflt_vsi(pf->first_sw, vsi)) >>> + ice_clear_dflt_vsi(pf->first_sw); >> >> It would probably be good to check `ice_clear_dflt_vsi` return code. > > Check and report potential warning when error occurs? because we are in ice_vsi_release() so > any rollback does not make sense. Right. ice_clear_dflt_vsi already reports errors so it should be good as is. LGTM, thanks! > > Ivan > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net] ice: Clear default forwarding VSI during VSI release @ 2022-03-23 18:19 ` Marcin Szycik 0 siblings, 0 replies; 11+ messages in thread From: Marcin Szycik @ 2022-03-23 18:19 UTC (permalink / raw) To: Ivan Vecera Cc: netdev, poros, mschmidt, Jesse Brandeburg, Tony Nguyen, David S. Miller, Jakub Kicinski, Paolo Abeni, Brett Creeley, Jeff Kirsher, moderated list:INTEL ETHERNET DRIVERS", open list On 23-Mar-22 18:54, Ivan Vecera wrote: > On Wed, 23 Mar 2022 18:39:11 +0100 > Marcin Szycik <marcin.szycik@linux.intel.com> wrote: > >> On 22-Mar-22 15:25, Ivan Vecera wrote: >>> VSI is set as default forwarding one when promisc mode is set for >>> PF interface, when PF is switched to switchdev mode or when VF >>> driver asks to enable allmulticast or promisc mode for the VF >>> interface (when vf-true-promisc-support priv flag is off). >>> The third case is buggy because in that case VSI associated with >>> VF remains as default one after VF removal. >>> >>> Reproducer: >>> 1. Create VF >>> echo 1 > sys/class/net/ens7f0/device/sriov_numvfs >>> 2. Enable allmulticast or promisc mode on VF >>> ip link set ens7f0v0 allmulticast on >>> ip link set ens7f0v0 promisc on >>> 3. Delete VF >>> echo 0 > sys/class/net/ens7f0/device/sriov_numvfs >>> 4. Try to enable promisc mode on PF >>> ip link set ens7f0 promisc on >>> >>> Although it looks that promisc mode on PF is enabled the opposite >>> is true because ice_vsi_sync_fltr() responsible for IFF_PROMISC >>> handling first checks if any other VSI is set as default forwarding >>> one and if so the function does not do anything. At this point >>> it is not possible to enable promisc mode on PF without re-probe >>> device. >>> >>> To resolve the issue this patch clear default forwarding VSI >>> during ice_vsi_release() when the VSI to be released is the default >>> one. >>> >>> Fixes: 01b5e89aab49 ("ice: Add VF promiscuous support") >>> Signed-off-by: Ivan Vecera <ivecera@redhat.com> >>> --- >>> drivers/net/ethernet/intel/ice/ice_lib.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c >>> index 53256aca27c7..20d755822d43 100644 >>> --- a/drivers/net/ethernet/intel/ice/ice_lib.c >>> +++ b/drivers/net/ethernet/intel/ice/ice_lib.c >>> @@ -3147,6 +3147,8 @@ int ice_vsi_release(struct ice_vsi *vsi) >>> } >>> } >>> >>> + if (ice_is_vsi_dflt_vsi(pf->first_sw, vsi)) >>> + ice_clear_dflt_vsi(pf->first_sw); >> >> It would probably be good to check `ice_clear_dflt_vsi` return code. > > Check and report potential warning when error occurs? because we are in ice_vsi_release() so > any rollback does not make sense. Right. ice_clear_dflt_vsi already reports errors so it should be good as is. LGTM, thanks! > > Ivan > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [Intel-wired-lan] [PATCH net] ice: Clear default forwarding VSI during VSI release 2022-03-23 18:19 ` Marcin Szycik @ 2022-03-24 11:10 ` Maciej Fijalkowski -1 siblings, 0 replies; 11+ messages in thread From: Maciej Fijalkowski @ 2022-03-24 11:10 UTC (permalink / raw) To: intel-wired-lan On Wed, Mar 23, 2022 at 07:19:55PM +0100, Marcin Szycik wrote: > > > On 23-Mar-22 18:54, Ivan Vecera wrote: > > On Wed, 23 Mar 2022 18:39:11 +0100 > > Marcin Szycik <marcin.szycik@linux.intel.com> wrote: > > > >> On 22-Mar-22 15:25, Ivan Vecera wrote: > >>> VSI is set as default forwarding one when promisc mode is set for > >>> PF interface, when PF is switched to switchdev mode or when VF > >>> driver asks to enable allmulticast or promisc mode for the VF > >>> interface (when vf-true-promisc-support priv flag is off). > >>> The third case is buggy because in that case VSI associated with > >>> VF remains as default one after VF removal. > >>> > >>> Reproducer: > >>> 1. Create VF > >>> echo 1 > sys/class/net/ens7f0/device/sriov_numvfs > >>> 2. Enable allmulticast or promisc mode on VF > >>> ip link set ens7f0v0 allmulticast on > >>> ip link set ens7f0v0 promisc on > >>> 3. Delete VF > >>> echo 0 > sys/class/net/ens7f0/device/sriov_numvfs > >>> 4. Try to enable promisc mode on PF > >>> ip link set ens7f0 promisc on > >>> > >>> Although it looks that promisc mode on PF is enabled the opposite > >>> is true because ice_vsi_sync_fltr() responsible for IFF_PROMISC > >>> handling first checks if any other VSI is set as default forwarding > >>> one and if so the function does not do anything. At this point > >>> it is not possible to enable promisc mode on PF without re-probe > >>> device. > >>> > >>> To resolve the issue this patch clear default forwarding VSI tiny nit: s/clear/clears Also it's more welcome to use imperative mood. > >>> during ice_vsi_release() when the VSI to be released is the default > >>> one. > >>> > >>> Fixes: 01b5e89aab49 ("ice: Add VF promiscuous support") > >>> Signed-off-by: Ivan Vecera <ivecera@redhat.com> > >>> --- > >>> drivers/net/ethernet/intel/ice/ice_lib.c | 2 ++ > >>> 1 file changed, 2 insertions(+) > >>> > >>> diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c > >>> index 53256aca27c7..20d755822d43 100644 > >>> --- a/drivers/net/ethernet/intel/ice/ice_lib.c > >>> +++ b/drivers/net/ethernet/intel/ice/ice_lib.c > >>> @@ -3147,6 +3147,8 @@ int ice_vsi_release(struct ice_vsi *vsi) > >>> } > >>> } > >>> > >>> + if (ice_is_vsi_dflt_vsi(pf->first_sw, vsi)) > >>> + ice_clear_dflt_vsi(pf->first_sw); > >> > >> It would probably be good to check `ice_clear_dflt_vsi` return code. > > > > Check and report potential warning when error occurs? because we are in ice_vsi_release() so > > any rollback does not make sense. I believe that comment wouldn't hurt that it's ok to ignore the retval, but then again i'm fine with what it is currently :) Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> > > Right. ice_clear_dflt_vsi already reports errors so it should be good as is. > LGTM, thanks! > > > > > Ivan > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net] ice: Clear default forwarding VSI during VSI release @ 2022-03-24 11:10 ` Maciej Fijalkowski 0 siblings, 0 replies; 11+ messages in thread From: Maciej Fijalkowski @ 2022-03-24 11:10 UTC (permalink / raw) To: Marcin Szycik Cc: Ivan Vecera, netdev, poros, mschmidt, Jesse Brandeburg, Tony Nguyen, David S. Miller, Jakub Kicinski, Paolo Abeni, Brett Creeley, Jeff Kirsher, moderated list:INTEL ETHERNET DRIVERS", open list On Wed, Mar 23, 2022 at 07:19:55PM +0100, Marcin Szycik wrote: > > > On 23-Mar-22 18:54, Ivan Vecera wrote: > > On Wed, 23 Mar 2022 18:39:11 +0100 > > Marcin Szycik <marcin.szycik@linux.intel.com> wrote: > > > >> On 22-Mar-22 15:25, Ivan Vecera wrote: > >>> VSI is set as default forwarding one when promisc mode is set for > >>> PF interface, when PF is switched to switchdev mode or when VF > >>> driver asks to enable allmulticast or promisc mode for the VF > >>> interface (when vf-true-promisc-support priv flag is off). > >>> The third case is buggy because in that case VSI associated with > >>> VF remains as default one after VF removal. > >>> > >>> Reproducer: > >>> 1. Create VF > >>> echo 1 > sys/class/net/ens7f0/device/sriov_numvfs > >>> 2. Enable allmulticast or promisc mode on VF > >>> ip link set ens7f0v0 allmulticast on > >>> ip link set ens7f0v0 promisc on > >>> 3. Delete VF > >>> echo 0 > sys/class/net/ens7f0/device/sriov_numvfs > >>> 4. Try to enable promisc mode on PF > >>> ip link set ens7f0 promisc on > >>> > >>> Although it looks that promisc mode on PF is enabled the opposite > >>> is true because ice_vsi_sync_fltr() responsible for IFF_PROMISC > >>> handling first checks if any other VSI is set as default forwarding > >>> one and if so the function does not do anything. At this point > >>> it is not possible to enable promisc mode on PF without re-probe > >>> device. > >>> > >>> To resolve the issue this patch clear default forwarding VSI tiny nit: s/clear/clears Also it's more welcome to use imperative mood. > >>> during ice_vsi_release() when the VSI to be released is the default > >>> one. > >>> > >>> Fixes: 01b5e89aab49 ("ice: Add VF promiscuous support") > >>> Signed-off-by: Ivan Vecera <ivecera@redhat.com> > >>> --- > >>> drivers/net/ethernet/intel/ice/ice_lib.c | 2 ++ > >>> 1 file changed, 2 insertions(+) > >>> > >>> diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c > >>> index 53256aca27c7..20d755822d43 100644 > >>> --- a/drivers/net/ethernet/intel/ice/ice_lib.c > >>> +++ b/drivers/net/ethernet/intel/ice/ice_lib.c > >>> @@ -3147,6 +3147,8 @@ int ice_vsi_release(struct ice_vsi *vsi) > >>> } > >>> } > >>> > >>> + if (ice_is_vsi_dflt_vsi(pf->first_sw, vsi)) > >>> + ice_clear_dflt_vsi(pf->first_sw); > >> > >> It would probably be good to check `ice_clear_dflt_vsi` return code. > > > > Check and report potential warning when error occurs? because we are in ice_vsi_release() so > > any rollback does not make sense. I believe that comment wouldn't hurt that it's ok to ignore the retval, but then again i'm fine with what it is currently :) Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> > > Right. ice_clear_dflt_vsi already reports errors so it should be good as is. > LGTM, thanks! > > > > > Ivan > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [Intel-wired-lan] [PATCH net] ice: Clear default forwarding VSI during VSI release 2022-03-22 14:25 ` Ivan Vecera @ 2022-03-24 7:09 ` Michal Swiatkowski -1 siblings, 0 replies; 11+ messages in thread From: Michal Swiatkowski @ 2022-03-24 7:09 UTC (permalink / raw) To: intel-wired-lan On Tue, Mar 22, 2022 at 03:25:54PM +0100, Ivan Vecera wrote: > VSI is set as default forwarding one when promisc mode is set for > PF interface, when PF is switched to switchdev mode or when VF > driver asks to enable allmulticast or promisc mode for the VF > interface (when vf-true-promisc-support priv flag is off). > The third case is buggy because in that case VSI associated with > VF remains as default one after VF removal. > > Reproducer: > 1. Create VF > echo 1 > sys/class/net/ens7f0/device/sriov_numvfs > 2. Enable allmulticast or promisc mode on VF > ip link set ens7f0v0 allmulticast on > ip link set ens7f0v0 promisc on > 3. Delete VF > echo 0 > sys/class/net/ens7f0/device/sriov_numvfs > 4. Try to enable promisc mode on PF > ip link set ens7f0 promisc on > > Although it looks that promisc mode on PF is enabled the opposite > is true because ice_vsi_sync_fltr() responsible for IFF_PROMISC > handling first checks if any other VSI is set as default forwarding > one and if so the function does not do anything. At this point > it is not possible to enable promisc mode on PF without re-probe > device. > > To resolve the issue this patch clear default forwarding VSI > during ice_vsi_release() when the VSI to be released is the default > one. > > Fixes: 01b5e89aab49 ("ice: Add VF promiscuous support") > Signed-off-by: Ivan Vecera <ivecera@redhat.com> > --- > drivers/net/ethernet/intel/ice/ice_lib.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c > index 53256aca27c7..20d755822d43 100644 > --- a/drivers/net/ethernet/intel/ice/ice_lib.c > +++ b/drivers/net/ethernet/intel/ice/ice_lib.c > @@ -3147,6 +3147,8 @@ int ice_vsi_release(struct ice_vsi *vsi) > } > } > > + if (ice_is_vsi_dflt_vsi(pf->first_sw, vsi)) > + ice_clear_dflt_vsi(pf->first_sw); > ice_fltr_remove_all(vsi); > ice_rm_vsi_lan_cfg(vsi->port_info, vsi->idx); > err = ice_rm_vsi_rdma_cfg(vsi->port_info, vsi->idx); Thanks for fixing it. Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com> > -- > 2.34.1 > > _______________________________________________ > Intel-wired-lan mailing list > Intel-wired-lan at osuosl.org > https://lists.osuosl.org/mailman/listinfo/intel-wired-lan ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Intel-wired-lan] [PATCH net] ice: Clear default forwarding VSI during VSI release @ 2022-03-24 7:09 ` Michal Swiatkowski 0 siblings, 0 replies; 11+ messages in thread From: Michal Swiatkowski @ 2022-03-24 7:09 UTC (permalink / raw) To: Ivan Vecera Cc: netdev, moderated list:INTEL ETHERNET DRIVERS, mschmidt, Brett Creeley, open list, poros, Jeff Kirsher, Jakub Kicinski, Paolo Abeni, David S. Miller On Tue, Mar 22, 2022 at 03:25:54PM +0100, Ivan Vecera wrote: > VSI is set as default forwarding one when promisc mode is set for > PF interface, when PF is switched to switchdev mode or when VF > driver asks to enable allmulticast or promisc mode for the VF > interface (when vf-true-promisc-support priv flag is off). > The third case is buggy because in that case VSI associated with > VF remains as default one after VF removal. > > Reproducer: > 1. Create VF > echo 1 > sys/class/net/ens7f0/device/sriov_numvfs > 2. Enable allmulticast or promisc mode on VF > ip link set ens7f0v0 allmulticast on > ip link set ens7f0v0 promisc on > 3. Delete VF > echo 0 > sys/class/net/ens7f0/device/sriov_numvfs > 4. Try to enable promisc mode on PF > ip link set ens7f0 promisc on > > Although it looks that promisc mode on PF is enabled the opposite > is true because ice_vsi_sync_fltr() responsible for IFF_PROMISC > handling first checks if any other VSI is set as default forwarding > one and if so the function does not do anything. At this point > it is not possible to enable promisc mode on PF without re-probe > device. > > To resolve the issue this patch clear default forwarding VSI > during ice_vsi_release() when the VSI to be released is the default > one. > > Fixes: 01b5e89aab49 ("ice: Add VF promiscuous support") > Signed-off-by: Ivan Vecera <ivecera@redhat.com> > --- > drivers/net/ethernet/intel/ice/ice_lib.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c > index 53256aca27c7..20d755822d43 100644 > --- a/drivers/net/ethernet/intel/ice/ice_lib.c > +++ b/drivers/net/ethernet/intel/ice/ice_lib.c > @@ -3147,6 +3147,8 @@ int ice_vsi_release(struct ice_vsi *vsi) > } > } > > + if (ice_is_vsi_dflt_vsi(pf->first_sw, vsi)) > + ice_clear_dflt_vsi(pf->first_sw); > ice_fltr_remove_all(vsi); > ice_rm_vsi_lan_cfg(vsi->port_info, vsi->idx); > err = ice_rm_vsi_rdma_cfg(vsi->port_info, vsi->idx); Thanks for fixing it. Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com> > -- > 2.34.1 > > _______________________________________________ > Intel-wired-lan mailing list > Intel-wired-lan@osuosl.org > https://lists.osuosl.org/mailman/listinfo/intel-wired-lan ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2022-03-24 11:10 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-03-22 14:25 [Intel-wired-lan] [PATCH net] ice: Clear default forwarding VSI during VSI release Ivan Vecera 2022-03-22 14:25 ` Ivan Vecera 2022-03-23 17:39 ` [Intel-wired-lan] " Marcin Szycik 2022-03-23 17:39 ` Marcin Szycik 2022-03-23 17:54 ` [Intel-wired-lan] " Ivan Vecera 2022-03-23 18:19 ` Marcin Szycik 2022-03-23 18:19 ` Marcin Szycik 2022-03-24 11:10 ` [Intel-wired-lan] " Maciej Fijalkowski 2022-03-24 11:10 ` Maciej Fijalkowski 2022-03-24 7:09 ` [Intel-wired-lan] " Michal Swiatkowski 2022-03-24 7:09 ` Michal Swiatkowski
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.