[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Zshw1yW2mjEKk3aO@boxer>
Date: Fri, 23 Aug 2024 13:21:59 +0200
From: Maciej Fijalkowski <maciej.fijalkowski@...el.com>
To: Larysa Zaremba <larysa.zaremba@...el.com>
CC: <intel-wired-lan@...ts.osuosl.org>, Tony Nguyen
<anthony.l.nguyen@...el.com>, "David S. Miller" <davem@...emloft.net>, "Jacob
Keller" <jacob.e.keller@...el.com>, Eric Dumazet <edumazet@...gle.com>,
"Jakub Kicinski" <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, "Alexei
Starovoitov" <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>,
"Jesper Dangaard Brouer" <hawk@...nel.org>, John Fastabend
<john.fastabend@...il.com>, <netdev@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <bpf@...r.kernel.org>,
<magnus.karlsson@...el.com>, Michal Kubiak <michal.kubiak@...el.com>,
Wojciech Drewek <wojciech.drewek@...el.com>, Amritha Nambiar
<amritha.nambiar@...el.com>, <przemyslaw.kitszel@...el.com>,
<anirudh.venkataramanan@...el.com>, <sridhar.samudrala@...el.com>, "Chandan
Kumar Rout" <chandanx.rout@...el.com>
Subject: Re: [PATCH iwl-net v4 4/6] ice: check ICE_VSI_DOWN under rtnl_lock
when preparing for reset
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@...el.com>
> Reviewed-by: Jacob Keller <jacob.e.keller@...el.com>
> Tested-by: Chandan Kumar Rout <chandanx.rout@...el.com>
> Signed-off-by: Larysa Zaremba <larysa.zaremba@...el.com>
Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@...el.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
>
Powered by blists - more mailing lists