DPDK patches and discussions
 help / color / mirror / Atom feed
From: Rasesh Mody <rmody@marvell.com>
To: Jerin Jacob <jerinjacobk@gmail.com>, Gavin Hu <gavin.hu@arm.com>
Cc: dpdk-dev <dev@dpdk.org>,
	Jerin Jacob Kollanukkaran <jerinj@marvell.com>,
	Ferruh Yigit <ferruh.yigit@intel.com>,
	GR-Everest-DPDK-Dev <GR-Everest-DPDK-Dev@marvell.com>,
	dpdk stable <stable@dpdk.org>
Subject: Re: [dpdk-dev] [EXT] Re: [PATCH 1/3] net/bnx2x: fix to use required mem barriers in Rx path
Date: Wed, 15 Jan 2020 01:57:11 +0000	[thread overview]
Message-ID: <BYAPR18MB2838B4BA898EA20A89054FA9B5340@BYAPR18MB2838.namprd18.prod.outlook.com> (raw)
In-Reply-To: <CALBAE1OhLgx_4iO0g8v1bVimRpRzw9AWButE0iuoV3rL=SR56Q@mail.gmail.com>

Hi Jerin,

>From: Jerin Jacob <jerinjacobk@gmail.com>
>Sent: Tuesday, January 14, 2020 5:49 AM
>To: Rasesh Mody <rmody@marvell.com>; Gavin Hu <gavin.hu@arm.com>
>Cc: dpdk-dev <dev@dpdk.org>; Jerin Jacob Kollanukkaran
><jerinj@marvell.com>; Ferruh Yigit <ferruh.yigit@intel.com>; GR-Everest-
>DPDK-Dev <GR-Everest-DPDK-Dev@marvell.com>; dpdk stable
><stable@dpdk.org>
>Subject: [EXT] Re: [dpdk-dev] [PATCH 1/3] net/bnx2x: fix to use required
>mem barriers in Rx path
>
>External Email
>
>----------------------------------------------------------------------
>+ Gavin
>
>On Tue, Jan 14, 2020 at 6:09 AM Rasesh Mody <rmody@marvell.com> wrote:
>>
>> When handling RX completion queue PMD is not using required read/write
>> barriers before reading completion queue element (CQE) indices,
>> updating/writing hardware consumer and producer.
>> This patch adds appropriate read/write memory barriers in places which
>> are required by driver and adapter to read or update indices.
>>
>> Fixes: 540a211084a7 ("bnx2x: driver core")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Rasesh Mody <rmody@marvell.com>
>> ---
>>  drivers/net/bnx2x/bnx2x.c      |  5 +++++
>>  drivers/net/bnx2x/bnx2x_rxtx.c | 22 ++++++++++++++++++++++
>>  2 files changed, 27 insertions(+)
>>
>> diff --git a/drivers/net/bnx2x/bnx2x.c b/drivers/net/bnx2x/bnx2x.c
>> index ed31335ac..9c5e7995d 100644
>> --- a/drivers/net/bnx2x/bnx2x.c
>> +++ b/drivers/net/bnx2x/bnx2x.c
>> @@ -1255,6 +1255,11 @@ static uint8_t bnx2x_rxeof(struct bnx2x_softc
>*sc, struct bnx2x_fastpath *fp)
>>                 return 0;
>>         }
>>
>> +       /* Add memory barrier as status block fields can change. This memory
>> +        * barrier will flush out all the read/write operations to status block
>> +        * generated before the barrier. It will ensure stale data is not read
>> +        */
>> +       mb();
>
># Do you need full barriers here?

Yes

># Which architecture did you saw this issue?
># rte_cio_* barriers are performance Friday, Have you checked
>rte_cio_* would suffice the requirements.
>See the discussion in  https://urldefense.proofpoint.com/v2/url?u=http-
>3A__patches.dpdk.org_patch_64038_&d=DwIBaQ&c=nKjWec2b6R0mOyPaz7
>xtfQ&r=9aB46H7c7TYTnBun6ODgtnNLQdw3jNiVKHbs9eOyBSU&m=lmdOnuAp
>3MUDqX100Z4E2BDgaSSAy9oBgksySHVfEBI&s=oNq78smfHMB5fUZW_ew0_p
>e6gp_5C0MTw0TSPPWR8qQ&e=

This patch is to prevent potential issue which can happen in absence of required barriers.
The above barrier is in slow path. However, there is another full barrier in fast path as part of this patch which can use rte_cio_* .
I'll revise the patch and resend.

>
>I assume 2/3 and 3/3 patches are for the slow path. if so, it is fine to use full
>barriers on those patches.

The 2/3 is slow path fix and 3/3 is fixing a race condition that can occur between slow path and fast path.

Thanks!
-Rasesh



  reply	other threads:[~2020-01-15  1:57 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-14  0:39 [dpdk-dev] " Rasesh Mody
2020-01-14  0:39 ` [dpdk-dev] [PATCH 2/3] net/bnx2x: fix reset of scan FP flag Rasesh Mody
2020-01-14  0:39 ` [dpdk-dev] [PATCH 3/3] net/bnx2x: fix to sync fastpath Rx queue access Rasesh Mody
2020-01-14 13:49 ` [dpdk-dev] [PATCH 1/3] net/bnx2x: fix to use required mem barriers in Rx path Jerin Jacob
2020-01-15  1:57   ` Rasesh Mody [this message]
2020-01-26 22:55     ` [dpdk-dev] [EXT] " Rasesh Mody
2020-01-26 22:54 ` [dpdk-dev] [PATCH v2 1/2] net/bnx2x: fix reset of scan FP flag Rasesh Mody
2020-01-28  9:59   ` Jerin Jacob
2020-01-26 22:54 ` [dpdk-dev] [PATCH v2 2/2] net/bnx2x: fix to sync fastpath Rx queue access Rasesh Mody

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=BYAPR18MB2838B4BA898EA20A89054FA9B5340@BYAPR18MB2838.namprd18.prod.outlook.com \
    --to=rmody@marvell.com \
    --cc=GR-Everest-DPDK-Dev@marvell.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=gavin.hu@arm.com \
    --cc=jerinj@marvell.com \
    --cc=jerinjacobk@gmail.com \
    --cc=stable@dpdk.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).