patches for DPDK stable branches
 help / color / mirror / Atom feed
From: Ruifeng Wang <Ruifeng.Wang@arm.com>
To: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>,
	"dev@dpdk.org" <dev@dpdk.org>
Cc: "beilei.xing@intel.com" <beilei.xing@intel.com>,
	"qi.z.zhang@intel.com" <qi.z.zhang@intel.com>,
	"bruce.richardson@intel.com" <bruce.richardson@intel.com>,
	"jerinj@marvell.com" <jerinj@marvell.com>,
	"hemant.agrawal@nxp.com" <hemant.agrawal@nxp.com>,
	"drc@linux.vnet.ibm.com" <drc@linux.vnet.ibm.com>,
	"stable@dpdk.org" <stable@dpdk.org>, nd <nd@arm.com>,
	nd <nd@arm.com>, nd <nd@arm.com>
Subject: Re: [dpdk-stable] [PATCH 1/2] net/i40e: fix risk in Rx descriptor read in NEON vector path
Date: Wed, 15 Sep 2021 08:42:53 +0000
Message-ID: <AM5PR0802MB246547ACB89D159841F9C7CD9EDB9@AM5PR0802MB2465.eurprd08.prod.outlook.com> (raw)
In-Reply-To: <DBAPR08MB581411CA53E62C81A83B419D98DA9@DBAPR08MB5814.eurprd08.prod.outlook.com>

> -----Original Message-----
> From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> Sent: Wednesday, September 15, 2021 2:33 AM
> To: Ruifeng Wang <Ruifeng.Wang@arm.com>; dev@dpdk.org
> Cc: beilei.xing@intel.com; qi.z.zhang@intel.com;
> bruce.richardson@intel.com; jerinj@marvell.com;
> hemant.agrawal@nxp.com; drc@linux.vnet.ibm.com; stable@dpdk.org; nd
> <nd@arm.com>; Ruifeng Wang <Ruifeng.Wang@arm.com>; Honnappa
> Nagarahalli <Honnappa.Nagarahalli@arm.com>; nd <nd@arm.com>
> Subject: RE: [PATCH 1/2] net/i40e: fix risk in Rx descriptor read in NEON
> vector path
> 
> <snip>
> Similar comments that I have to patch 2/2
> 
> >
> > Rx descriptor is 16B/32B in size and consists of multiple words.
> > The word that includes DD field should be read first. Read result with
> > DD bit set indicates the rest part in a descriptor is valid.
> Suggest rewording as follows:
> Rx descriptor is 16B/32B in size. If the DD bit is set, it indicates that the rest of
> the descriptor words have valid values. Hence, the word containing DD bit
> must be read first before reading the rest of the descriptor words.
> 
> >
> > In NEON vector PMD, vector load loads two contiguous 8B of descriptor
> > data into vector register. Given vector load ensures no 16B atomicity,
> > read of the word that includes DD field could be reordered after read
> > of other words. In this case, some words could be invalid data.
> "some words could contain invalid data"
> 
> >
> > Read barrier is added after read of qword1 that includes DD field.
> > And qword0 is reloaded to update vector register. This ensures what
> > fetched is correct descriptor data.
> "This ensures that the fetched data is correct".
> 
> Suggest capturing the performance impact, so it is clearly documented.

Added performance impact to commit message in v2.
> >
> > Fixes: ae0eb310f253 ("net/i40e: implement vector PMD for ARM")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Ruifeng Wang <ruifeng.wang@arm.com>
> With the above comments,
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> 

Thanks for your review.
Comments are addressed in v2.
> > ---
> >  drivers/net/i40e/i40e_rxtx_vec_neon.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/net/i40e/i40e_rxtx_vec_neon.c
> > b/drivers/net/i40e/i40e_rxtx_vec_neon.c
> > index b2683fda60..71191c7cc8 100644
> > --- a/drivers/net/i40e/i40e_rxtx_vec_neon.c
> > +++ b/drivers/net/i40e/i40e_rxtx_vec_neon.c
> > @@ -286,6 +286,14 @@ _recv_raw_pkts_vec(struct i40e_rx_queue
> > *__rte_restrict rxq,
> >  		descs[1] =  vld1q_u64((uint64_t *)(rxdp + 1));
> >  		descs[0] =  vld1q_u64((uint64_t *)(rxdp));
> >
> > +		/* Use acquire fence to order loads of descriptor qwords */
> > +		rte_atomic_thread_fence(__ATOMIC_ACQUIRE);
> > +		/* A.2 reload qword0 to make it ordered after qword1 load
> */
> > +		descs[3] = vld1q_lane_u64((uint64_t *)(rxdp + 3), descs[3],
> > 0);
> > +		descs[2] = vld1q_lane_u64((uint64_t *)(rxdp + 2), descs[2],
> > 0);
> > +		descs[1] = vld1q_lane_u64((uint64_t *)(rxdp + 1), descs[1],
> > 0);
> > +		descs[0] = vld1q_lane_u64((uint64_t *)(rxdp), descs[0], 0);
> > +
> >  		/* B.1 load 4 mbuf point */
> >  		mbp1 = vld1q_u64((uint64_t *)&sw_ring[pos]);
> >  		mbp2 = vld1q_u64((uint64_t *)&sw_ring[pos + 2]);
> > --
> > 2.25.1


  reply	other threads:[~2021-09-15  8:43 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-06  3:31 [dpdk-stable] [PATCH 0/2] i40e Rx descriptor loads ordering Ruifeng Wang
2021-09-06  3:32 ` [dpdk-stable] [PATCH 1/2] net/i40e: fix risk in Rx descriptor read in NEON vector path Ruifeng Wang
2021-09-14 18:33   ` Honnappa Nagarahalli
2021-09-15  8:42     ` Ruifeng Wang [this message]
2021-09-06  3:32 ` [dpdk-stable] [PATCH 2/2] net/i40e: fix risk in Rx descriptor read in scalar path Ruifeng Wang
2021-09-14 18:06   ` Honnappa Nagarahalli
2021-09-15  8:33 ` [dpdk-stable] [PATCH v2 0/2] i40e Rx descriptor loads ordering Ruifeng Wang
2021-09-15  8:33   ` [dpdk-stable] [PATCH v2 1/2] net/i40e: fix risk in Rx descriptor read in NEON vector path Ruifeng Wang
2021-09-15  8:33   ` [dpdk-stable] [PATCH v2 2/2] net/i40e: fix risk in Rx descriptor read in scalar path Ruifeng Wang
2021-09-29 15:05     ` Ferruh Yigit
2021-09-29 15:29       ` Honnappa Nagarahalli
2021-10-11 16:26         ` Ferruh Yigit
2021-10-19 11:14           ` Zhang, Qi Z
2021-11-05  6:57             ` Ruifeng Wang
2021-11-11 10:27               ` Ruifeng Wang
2021-11-11 12:27     ` Zhang, Qi Z
2021-09-24 11:08   ` [dpdk-stable] [PATCH v2 0/2] i40e Rx descriptor loads ordering Zhang, Qi Z

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=AM5PR0802MB246547ACB89D159841F9C7CD9EDB9@AM5PR0802MB2465.eurprd08.prod.outlook.com \
    --to=ruifeng.wang@arm.com \
    --cc=Honnappa.Nagarahalli@arm.com \
    --cc=beilei.xing@intel.com \
    --cc=bruce.richardson@intel.com \
    --cc=dev@dpdk.org \
    --cc=drc@linux.vnet.ibm.com \
    --cc=hemant.agrawal@nxp.com \
    --cc=jerinj@marvell.com \
    --cc=nd@arm.com \
    --cc=qi.z.zhang@intel.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

patches for DPDK stable branches

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://inbox.dpdk.org/stable/0 stable/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 stable stable/ http://inbox.dpdk.org/stable \
		stable@dpdk.org
	public-inbox-index stable

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.stable


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git