All of lore.kernel.org
 help / color / mirror / Atom feed
From: Przemek Kitszel <przemyslaw.kitszel@intel.com>
To: Manoj Vishwanathan <manojvishy@google.com>
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Eric Dumazet <edumazet@google.com>,
	Tony Nguyen <anthony.l.nguyen@intel.com>,
	intel-wired-lan@lists.osuosl.org,
	David Decotigny <decot@google.com>,
	"David S. Miller" <davem@davemloft.net>
Subject: Re: [Intel-wired-lan] [PATCH] idpf: Acquire the lock before accessing the xn->salt
Date: Mon, 5 Aug 2024 15:37:03 +0200	[thread overview]
Message-ID: <6830e66d-e019-4197-ba40-0d1662360fdb@intel.com> (raw)
In-Reply-To: <20240803182548.2932270-1-manojvishy@google.com>

On 8/3/24 20:25, Manoj Vishwanathan wrote:
> The transaction salt was being accessed
> before acquiring the idpf_vc_xn_lock when idpf has to forward the
> virtchnl reply

nits: This text would fit into two lines (the column limit is 75).
Missing dot at the very end.

> 
> Signed-off-by: Manoj Vishwanathan <manojvishy@google.com>

thank you!, code looks fine

it would be great if you could add a Fixes: tag, and add iwl-net as
the target tree

> ---
>   drivers/net/ethernet/intel/idpf/idpf_virtchnl.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
> index 70986e12da28..30eec674d594 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
> @@ -612,14 +612,15 @@ idpf_vc_xn_forward_reply(struct idpf_adapter *adapter,
>   		return -EINVAL;
>   	}
>   	xn = &adapter->vcxn_mngr->ring[xn_idx];
> +	idpf_vc_xn_lock(xn);
>   	salt = FIELD_GET(IDPF_VC_XN_SALT_M, msg_info);
>   	if (xn->salt != salt) {
>   		dev_err_ratelimited(&adapter->pdev->dev, "Transaction salt does not match (%02x != %02x)\n",
>   				    xn->salt, salt);
> +		idpf_vc_xn_unlock(xn);
>   		return -EINVAL;
>   	}
>   
> -	idpf_vc_xn_lock(xn);
>   	switch (xn->state) {
>   	case IDPF_VC_XN_WAITING:
>   		/* success */


WARNING: multiple messages have this Message-ID (diff)
From: Przemek Kitszel <przemyslaw.kitszel@intel.com>
To: Manoj Vishwanathan <manojvishy@google.com>
Cc: <netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	David Decotigny <decot@google.com>,
	Tony Nguyen <anthony.l.nguyen@intel.com>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	<intel-wired-lan@lists.osuosl.org>
Subject: Re: [PATCH] idpf: Acquire the lock before accessing the xn->salt
Date: Mon, 5 Aug 2024 15:37:03 +0200	[thread overview]
Message-ID: <6830e66d-e019-4197-ba40-0d1662360fdb@intel.com> (raw)
In-Reply-To: <20240803182548.2932270-1-manojvishy@google.com>

On 8/3/24 20:25, Manoj Vishwanathan wrote:
> The transaction salt was being accessed
> before acquiring the idpf_vc_xn_lock when idpf has to forward the
> virtchnl reply

nits: This text would fit into two lines (the column limit is 75).
Missing dot at the very end.

> 
> Signed-off-by: Manoj Vishwanathan <manojvishy@google.com>

thank you!, code looks fine

it would be great if you could add a Fixes: tag, and add iwl-net as
the target tree

> ---
>   drivers/net/ethernet/intel/idpf/idpf_virtchnl.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
> index 70986e12da28..30eec674d594 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
> @@ -612,14 +612,15 @@ idpf_vc_xn_forward_reply(struct idpf_adapter *adapter,
>   		return -EINVAL;
>   	}
>   	xn = &adapter->vcxn_mngr->ring[xn_idx];
> +	idpf_vc_xn_lock(xn);
>   	salt = FIELD_GET(IDPF_VC_XN_SALT_M, msg_info);
>   	if (xn->salt != salt) {
>   		dev_err_ratelimited(&adapter->pdev->dev, "Transaction salt does not match (%02x != %02x)\n",
>   				    xn->salt, salt);
> +		idpf_vc_xn_unlock(xn);
>   		return -EINVAL;
>   	}
>   
> -	idpf_vc_xn_lock(xn);
>   	switch (xn->state) {
>   	case IDPF_VC_XN_WAITING:
>   		/* success */


  reply	other threads:[~2024-08-05 13:37 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-03 18:25 [Intel-wired-lan] [PATCH] idpf: Acquire the lock before accessing the xn->salt Manoj Vishwanathan
2024-08-03 18:25 ` Manoj Vishwanathan
2024-08-05 13:37 ` Przemek Kitszel [this message]
2024-08-05 13:37   ` Przemek Kitszel
2024-08-05 18:21 ` [Intel-wired-lan] [PATCH] [PATCH iwl-net] " Manoj Vishwanathan
2024-08-05 18:21   ` Manoj Vishwanathan
2024-08-06 10:23   ` [Intel-wired-lan] " Przemek Kitszel
2024-08-06 10:23     ` Przemek Kitszel
2024-08-07 11:04   ` [Intel-wired-lan] " Alexander Lobakin
2024-08-07 11:04     ` Alexander Lobakin
2024-08-07 13:58     ` Manoj Vishwanathan
2024-08-07 13:58       ` Manoj Vishwanathan
2024-08-07 14:04       ` Alexander Lobakin
2024-08-07 14:04         ` Alexander Lobakin
2024-08-07 17:05       ` Simon Horman
2024-08-07 17:05         ` Simon Horman

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=6830e66d-e019-4197-ba40-0d1662360fdb@intel.com \
    --to=przemyslaw.kitszel@intel.com \
    --cc=anthony.l.nguyen@intel.com \
    --cc=davem@davemloft.net \
    --cc=decot@google.com \
    --cc=edumazet@google.com \
    --cc=intel-wired-lan@lists.osuosl.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=manojvishy@google.com \
    --cc=netdev@vger.kernel.org \
    /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.