| Message ID | 20240823095933.17922-5-larysa.zaremba@intel.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | ice: fix synchronization between .ndo_bpf() and reset | expand |
On Fri, Aug 23, 2024 at 11:59:29AM +0200, Larysa Zaremba wrote: > Consider the following scenario: > > .ndo_bpf() | ice_prepare_for_reset() | > ________________________|_______________________________________| > rtnl_lock() | | > ice_down() | | > | test_bit(ICE_VSI_DOWN) - true | > | ice_dis_vsi() returns | > ice_up() | | > | proceeds to rebuild a running VSI | > > .ndo_bpf() is not the only rtnl-locked callback that toggles the interface > to apply new configuration. Another example is .set_channels(). > > To avoid the race condition above, act only after reading ICE_VSI_DOWN > under rtnl_lock. > > Fixes: 0f9d5027a749 ("ice: Refactor VSI allocation, deletion and rebuild flow") > Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com> > Reviewed-by: Jacob Keller <jacob.e.keller@intel.com> > Tested-by: Chandan Kumar Rout <chandanx.rout@intel.com> > Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com> Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> > --- > drivers/net/ethernet/intel/ice/ice_lib.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c > index 14257e036d4b..2405e5ed9128 100644 > --- a/drivers/net/ethernet/intel/ice/ice_lib.c > +++ b/drivers/net/ethernet/intel/ice/ice_lib.c > @@ -2665,8 +2665,7 @@ int ice_ena_vsi(struct ice_vsi *vsi, bool locked) > */ > void ice_dis_vsi(struct ice_vsi *vsi, bool locked) > { > - if (test_bit(ICE_VSI_DOWN, vsi->state)) > - return; > + bool already_down = test_bit(ICE_VSI_DOWN, vsi->state); > > set_bit(ICE_VSI_NEEDS_RESTART, vsi->state); > > @@ -2674,15 +2673,16 @@ void ice_dis_vsi(struct ice_vsi *vsi, bool locked) > if (netif_running(vsi->netdev)) { > if (!locked) > rtnl_lock(); > - > - ice_vsi_close(vsi); > + already_down = test_bit(ICE_VSI_DOWN, vsi->state); > + if (!already_down) > + ice_vsi_close(vsi); > > if (!locked) > rtnl_unlock(); > - } else { > + } else if (!already_down) { > ice_vsi_close(vsi); > } > - } else if (vsi->type == ICE_VSI_CTRL) { > + } else if (vsi->type == ICE_VSI_CTRL && !already_down) { > ice_vsi_close(vsi); > } > } > -- > 2.43.0 >
diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c index 14257e036d4b..2405e5ed9128 100644 --- a/drivers/net/ethernet/intel/ice/ice_lib.c +++ b/drivers/net/ethernet/intel/ice/ice_lib.c @@ -2665,8 +2665,7 @@ int ice_ena_vsi(struct ice_vsi *vsi, bool locked) */ void ice_dis_vsi(struct ice_vsi *vsi, bool locked) { - if (test_bit(ICE_VSI_DOWN, vsi->state)) - return; + bool already_down = test_bit(ICE_VSI_DOWN, vsi->state); set_bit(ICE_VSI_NEEDS_RESTART, vsi->state); @@ -2674,15 +2673,16 @@ void ice_dis_vsi(struct ice_vsi *vsi, bool locked) if (netif_running(vsi->netdev)) { if (!locked) rtnl_lock(); - - ice_vsi_close(vsi); + already_down = test_bit(ICE_VSI_DOWN, vsi->state); + if (!already_down) + ice_vsi_close(vsi); if (!locked) rtnl_unlock(); - } else { + } else if (!already_down) { ice_vsi_close(vsi); } - } else if (vsi->type == ICE_VSI_CTRL) { + } else if (vsi->type == ICE_VSI_CTRL && !already_down) { ice_vsi_close(vsi); } }
Consider the following scenario: .ndo_bpf() | ice_prepare_for_reset() | ________________________|_______________________________________| rtnl_lock() | | ice_down() | | | test_bit(ICE_VSI_DOWN) - true | | ice_dis_vsi() returns | ice_up() | | | proceeds to rebuild a running VSI | .ndo_bpf() is not the only rtnl-locked callback that toggles the interface to apply new configuration. Another example is .set_channels(). To avoid the race condition above, act only after reading ICE_VSI_DOWN under rtnl_lock. Fixes: 0f9d5027a749 ("ice: Refactor VSI allocation, deletion and rebuild flow") Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com> Reviewed-by: Jacob Keller <jacob.e.keller@intel.com> Tested-by: Chandan Kumar Rout <chandanx.rout@intel.com> Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com> --- drivers/net/ethernet/intel/ice/ice_lib.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)