All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Tony Nguyen <anthony.l.nguyen@intel.com>
Cc: davem@davemloft.net, pabeni@redhat.com,
	Michal Maloszewski <michal.maloszewski@intel.com>,
	netdev@vger.kernel.org, sassmann@redhat.com,
	Sylwester Dziedziuch <sylwesterx.dziedziuch@intel.com>,
	Konrad Jankowski <konrad0.jankowski@intel.com>
Subject: Re: [PATCH net 1/2] iavf: Fix error when changing ring parameters on ice PF
Date: Fri, 22 Apr 2022 15:47:52 -0700	[thread overview]
Message-ID: <20220422154752.1fab6496@kernel.org> (raw)
In-Reply-To: <20220420172624.931237-2-anthony.l.nguyen@intel.com>

On Wed, 20 Apr 2022 10:26:23 -0700 Tony Nguyen wrote:
> From: Michal Maloszewski <michal.maloszewski@intel.com>
> 
> Reset is triggered when ring parameters are being changed through
> ethtool and queues are reconfigured for VF's VSI. If ring is changed
> again immediately, then the next reset could be executed before
> queues could be properly reinitialized on VF's VSI. It caused ice PF
> to mess up the VSI resource tree.
> 
> Add a check in iavf_set_ringparam for adapter and VF's queue
> state. If VF is currently resetting or queues are disabled for the VF
> return with EAGAIN error.

Can't we wait for the device to get into the right state?
Throwing EAGAIN back to user space is not very friendly.

> diff --git a/drivers/net/ethernet/intel/iavf/iavf_ethtool.c b/drivers/net/ethernet/intel/iavf/iavf_ethtool.c
> index 3bb56714beb0..08efbc50fbe9 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf_ethtool.c
> +++ b/drivers/net/ethernet/intel/iavf/iavf_ethtool.c
> @@ -631,6 +631,11 @@ static int iavf_set_ringparam(struct net_device *netdev,
>  	if ((ring->rx_mini_pending) || (ring->rx_jumbo_pending))
>  		return -EINVAL;
>  
> +	if (adapter->state == __IAVF_RESETTING ||
> +	    (adapter->state == __IAVF_RUNNING &&
> +	     (adapter->flags & IAVF_FLAG_QUEUES_DISABLED)))
> +		return -EAGAIN;

nit: why add this check in the middle of input validation 
(i.e. checking the ring params are supported)?

>  	if (ring->tx_pending > IAVF_MAX_TXD ||
>  	    ring->tx_pending < IAVF_MIN_TXD ||
>  	    ring->rx_pending > IAVF_MAX_RXD ||

  reply	other threads:[~2022-04-22 23:12 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-20 17:26 [PATCH net 0/2][pull request] Intel Wired LAN Driver Updates 2022-04-20 Tony Nguyen
2022-04-20 17:26 ` [PATCH net 1/2] iavf: Fix error when changing ring parameters on ice PF Tony Nguyen
2022-04-22 22:47   ` Jakub Kicinski [this message]
2022-04-28 18:10     ` Maloszewski, Michal
2022-04-28 18:28       ` Jakub Kicinski
2022-04-29 13:14         ` Maloszewski, Michal
2022-04-29 17:57           ` Jakub Kicinski
2022-05-09 15:25             ` Maloszewski, Michal
2022-04-20 17:26 ` [PATCH net 2/2] i40e: i40e_main: fix a missing check on list iterator Tony Nguyen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220422154752.1fab6496@kernel.org \
    --to=kuba@kernel.org \
    --cc=anthony.l.nguyen@intel.com \
    --cc=davem@davemloft.net \
    --cc=konrad0.jankowski@intel.com \
    --cc=michal.maloszewski@intel.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=sassmann@redhat.com \
    --cc=sylwesterx.dziedziuch@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.