From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5AE8DC7618E for ; Wed, 26 Apr 2023 06:49:53 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S239397AbjDZGtv (ORCPT ); Wed, 26 Apr 2023 02:49:51 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59254 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S239542AbjDZGts (ORCPT ); Wed, 26 Apr 2023 02:49:48 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 526C910EB for ; Tue, 25 Apr 2023 23:49:47 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id E1C6062321 for ; Wed, 26 Apr 2023 06:49:46 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 7987FC433D2; Wed, 26 Apr 2023 06:49:45 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1682491786; bh=05Qxei+c9rVh3k+ab+XsF3LuJB+Pw5m0VNFNSgtpjuM=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=YTXggkBffKLk/o5HKBt5R5zL4oxPHY8ljiGv1dlXRJNNVN53fmhzcqBshukeqAtfG 9wEGfgsY3aPoXyeIr7TwO3emxUR0tOQqI53oFRQQc0HGoD8xMU/XEU3ElSSxXBvOjQ lkM10JDW/0A8cLFeJUvWMlU8WSi5RiF5YT1212BBBWLfVjEX7nS/yDV1JXPnzC+Ylr gF2boOK3wdDJ8UJbNkxauwAp4G0mdCSvhFWuMiHAs7NkpFtJPO9clLrMJ1ziVe3aPG 8MOy32e5tZcA7mwwNwX/fG7buQsPqoAjWzooQloehl7xh4PEtODMx+gMBZtZwByRhI 3s8IhYrUc8zOg== Date: Wed, 26 Apr 2023 09:49:41 +0300 From: Leon Romanovsky To: Tony Nguyen Cc: davem@davemloft.net, kuba@kernel.org, pabeni@redhat.com, edumazet@google.com, netdev@vger.kernel.org, Dawid Wesierski , Kamil Maziarz , Jacob Keller , Rafal Romanowski Subject: Re: [PATCH net 2/3] ice: Fix ice VF reset during iavf initialization Message-ID: <20230426064941.GF27649@unreal> References: <20230425170127.2522312-1-anthony.l.nguyen@intel.com> <20230425170127.2522312-3-anthony.l.nguyen@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230425170127.2522312-3-anthony.l.nguyen@intel.com> Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Tue, Apr 25, 2023 at 10:01:26AM -0700, Tony Nguyen wrote: > From: Dawid Wesierski > > Fix the current implementation that causes ice_trigger_vf_reset() > to start resetting the VF even when the VF is still resetting itself > and initializing adminq. This leads to a series of -53 errors > (failed to init adminq) from the IAVF. > > Change the state of the vf_state field to be not active when the IAVF > asks for a reset. To avoid issues caused by the VF being reset too > early, make sure to wait until receiving the message on the message > box to know the exact state of the IAVF driver. > > Fixes: c54d209c78b8 ("ice: Wait for VF to be reset/ready before configuration") > Signed-off-by: Dawid Wesierski > Signed-off-by: Kamil Maziarz > Acked-by: Jacob Keller > Tested-by: Rafal Romanowski > Signed-off-by: Tony Nguyen > --- > drivers/net/ethernet/intel/ice/ice_sriov.c | 8 ++++---- > drivers/net/ethernet/intel/ice/ice_vf_lib.c | 19 +++++++++++++++++++ > drivers/net/ethernet/intel/ice/ice_vf_lib.h | 1 + > drivers/net/ethernet/intel/ice/ice_virtchnl.c | 1 + > 4 files changed, 25 insertions(+), 4 deletions(-) <...> > - ret = ice_check_vf_ready_for_cfg(vf); > + ret = ice_check_vf_ready_for_reset(vf); > if (ret) > goto out_put_vf; <...> > +/** > + * ice_check_vf_ready_for_reset - check if VF is ready to be reset > + * @vf: VF to check if it's ready to be reset > + * > + * The purpose of this function is to ensure that the VF is not in reset, > + * disabled, and is both initialized and active, thus enabling us to safely > + * initialize another reset. > + */ > +int ice_check_vf_ready_for_reset(struct ice_vf *vf) > +{ > + int ret; > + > + ret = ice_check_vf_ready_for_cfg(vf); > + if (!ret && !test_bit(ICE_VF_STATE_ACTIVE, vf->vf_states)) > + ret = -EAGAIN; I don't know your driver enough to say how it is it possible to find VF "resetting itself" and PF trying to reset VF at the same time. But what I see is that ICE_VF_STATE_ACTIVE bit check is racy and you don't really fix the root cause of calling to reset without proper locking. Thanks > + > + return ret; > +} <...> > case VIRTCHNL_OP_RESET_VF: > + clear_bit(ICE_VF_STATE_ACTIVE, vf->vf_states); > ops->reset_vf(vf); > break; > case VIRTCHNL_OP_ADD_ETH_ADDR: > -- > 2.38.1 >