DPDK patches and discussions
 help / color / mirror / Atom feed
From: Harman Kalra <hkalra@marvell.com>
To: Jerin Jacob <jerinj@marvell.com>,
	Nithin Dabilpuram <ndabilpuram@marvell.com>,
	Pavan Nikhilesh <pbhagavatula@marvell.com>,
	"Kiran Kumar K" <kirankumark@marvell.com>
Cc: <dev@dpdk.org>, Harman Kalra <hkalra@marvell.com>, <stable@dpdk.org>
Subject: [dpdk-dev] [PATCH v2 3/4] net/octeontx2: fix jumbo frame crash
Date: Fri,  9 Oct 2020 00:18:45 +0530
Message-ID: <1602182927-18254-3-git-send-email-hkalra@marvell.com> (raw)
In-Reply-To: <1602182927-18254-1-git-send-email-hkalra@marvell.com>

Issue has been observed in case of multi segments where mbuf
data gets corrupted due to missing barriers. Changes made to
mbuf just before LMTST by one core gets updatded when the
same mbuf is in use by another core, leading to corruption.
It should be ensured that all changes made to mbuf should be
written before LMTST.

Fixes: cbd5710db48d ("net/octeontx2: add Tx multi segment version")
Cc: stable@dpdk.org

Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
Signed-off-by: Harman Kalra <hkalra@marvell.com>
---
V2:
* replace rte_cio_wmb with rte_io_wmb

 drivers/common/octeontx2/otx2_io_arm64.h     | 12 ++++++++++
 drivers/common/octeontx2/otx2_io_generic.h   | 16 +++++++++++---
 drivers/event/octeontx2/otx2_worker.h        | 20 +++++++++++++----
 drivers/mempool/octeontx2/otx2_mempool_ops.c |  4 ++++
 drivers/net/octeontx2/otx2_tx.c              | 23 ++++++++++++++------
 drivers/net/octeontx2/otx2_tx.h              | 23 ++++++++++++++++++++
 6 files changed, 84 insertions(+), 14 deletions(-)

diff --git a/drivers/common/octeontx2/otx2_io_arm64.h b/drivers/common/octeontx2/otx2_io_arm64.h
index 7e45329b3..b5c85d9a6 100644
--- a/drivers/common/octeontx2/otx2_io_arm64.h
+++ b/drivers/common/octeontx2/otx2_io_arm64.h
@@ -63,6 +63,18 @@ otx2_lmt_submit(rte_iova_t io_address)
 	return result;
 }
 
+static __rte_always_inline uint64_t
+otx2_lmt_submit_release(rte_iova_t io_address)
+{
+	uint64_t result;
+
+	asm volatile (
+		".cpu  generic+lse\n"
+		"ldeorl xzr,%x[rf],[%[rs]]" :
+		 [rf] "=r"(result) : [rs] "r"(io_address));
+	return result;
+}
+
 static __rte_always_inline void
 otx2_lmt_mov(void *out, const void *in, const uint32_t lmtext)
 {
diff --git a/drivers/common/octeontx2/otx2_io_generic.h b/drivers/common/octeontx2/otx2_io_generic.h
index b1d754008..da64c9b31 100644
--- a/drivers/common/octeontx2/otx2_io_generic.h
+++ b/drivers/common/octeontx2/otx2_io_generic.h
@@ -45,12 +45,22 @@ otx2_lmt_submit(uint64_t io_address)
 	return 0;
 }
 
+static inline int64_t
+otx2_lmt_submit_release(uint64_t io_address)
+{
+	RTE_SET_USED(io_address);
+
+	return 0;
+}
+
 static __rte_always_inline void
 otx2_lmt_mov(void *out, const void *in, const uint32_t lmtext)
 {
-	RTE_SET_USED(out);
-	RTE_SET_USED(in);
-	RTE_SET_USED(lmtext);
+	/* Copy four words if lmtext = 0
+	 *      six words if lmtext = 1
+	 *      eight words if lmtext =2
+	 */
+	memcpy(out, in, (4 + (2 * lmtext)) * sizeof(uint64_t));
 }
 
 static __rte_always_inline void
diff --git a/drivers/event/octeontx2/otx2_worker.h b/drivers/event/octeontx2/otx2_worker.h
index 757fa6fe5..5eb83435e 100644
--- a/drivers/event/octeontx2/otx2_worker.h
+++ b/drivers/event/octeontx2/otx2_worker.h
@@ -280,7 +280,19 @@ otx2_ssogws_event_tx(struct otx2_ssogws *ws, struct rte_event ev[],
 
 	/* Perform header writes before barrier for TSO */
 	otx2_nix_xmit_prepare_tso(m, flags);
-	rte_io_wmb();
+	/* Lets commit any changes in the packet here in case of single seg as
+	 * no further changes to mbuf will be done.
+	 * While for multi seg all mbufs used are set to NULL in
+	 * otx2_nix_prepare_mseg() after preparing the sg list and these changes
+	 * should be committed before LMTST.
+	 * Also in no fast free case some mbuf fields are updated in
+	 * otx2_nix_prefree_seg
+	 * Hence otx2_nix_xmit_submit_lmt_release/otx2_nix_xmit_mseg_one_release
+	 * has store barrier for multiseg.
+	 */
+	if (!(flags & NIX_TX_MULTI_SEG_F) &&
+	    !(flags & NIX_TX_OFFLOAD_MBUF_NOFF_F))
+		rte_io_wmb();
 	txq = otx2_ssogws_xtract_meta(m, txq_data);
 	otx2_ssogws_prepare_pkt(txq, m, cmd, flags);
 
@@ -291,12 +303,12 @@ otx2_ssogws_event_tx(struct otx2_ssogws *ws, struct rte_event ev[],
 		if (!ev->sched_type) {
 			otx2_nix_xmit_mseg_prep_lmt(cmd, txq->lmt_addr, segdw);
 			otx2_ssogws_head_wait(ws);
-			if (otx2_nix_xmit_submit_lmt(txq->io_addr) == 0)
+			if (otx2_nix_xmit_submit_lmt_release(txq->io_addr) == 0)
 				otx2_nix_xmit_mseg_one(cmd, txq->lmt_addr,
 						       txq->io_addr, segdw);
 		} else {
-			otx2_nix_xmit_mseg_one(cmd, txq->lmt_addr, txq->io_addr,
-					       segdw);
+			otx2_nix_xmit_mseg_one_release(cmd, txq->lmt_addr,
+						       txq->io_addr, segdw);
 		}
 	} else {
 		/* Passing no of segdw as 4: HDR + EXT + SG + SMEM */
diff --git a/drivers/mempool/octeontx2/otx2_mempool_ops.c b/drivers/mempool/octeontx2/otx2_mempool_ops.c
index 5229a7cfb..9ff71bcf6 100644
--- a/drivers/mempool/octeontx2/otx2_mempool_ops.c
+++ b/drivers/mempool/octeontx2/otx2_mempool_ops.c
@@ -15,6 +15,10 @@ otx2_npa_enq(struct rte_mempool *mp, void * const *obj_table, unsigned int n)
 	const uint64_t addr = npa_lf_aura_handle_to_base(aura_handle) +
 				 NPA_LF_AURA_OP_FREE0;
 
+	/* Ensure mbuf init changes are written before the free pointers
+	 * are enqueued to the stack.
+	 */
+	rte_io_wmb();
 	for (index = 0; index < n; index++)
 		otx2_store_pair((uint64_t)obj_table[index], reg, addr);
 
diff --git a/drivers/net/octeontx2/otx2_tx.c b/drivers/net/octeontx2/otx2_tx.c
index 1b75cd559..4458d8bca 100644
--- a/drivers/net/octeontx2/otx2_tx.c
+++ b/drivers/net/octeontx2/otx2_tx.c
@@ -38,8 +38,11 @@ nix_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 			otx2_nix_xmit_prepare_tso(tx_pkts[i], flags);
 	}
 
-	/* Lets commit any changes in the packet */
-	rte_io_wmb();
+	/* Lets commit any changes in the packet here as no further changes
+	 * to the packet will be done unless no fast free is enabled.
+	 */
+	if (!(flags & NIX_TX_OFFLOAD_MBUF_NOFF_F))
+		rte_io_wmb();
 
 	for (i = 0; i < pkts; i++) {
 		otx2_nix_xmit_prepare(tx_pkts[i], cmd, flags);
@@ -74,12 +77,11 @@ nix_xmit_pkts_mseg(void *tx_queue, struct rte_mbuf **tx_pkts,
 			otx2_nix_xmit_prepare_tso(tx_pkts[i], flags);
 	}
 
-	/* Lets commit any changes in the packet */
-	rte_io_wmb();
-
 	for (i = 0; i < pkts; i++) {
 		otx2_nix_xmit_prepare(tx_pkts[i], cmd, flags);
 		segdw = otx2_nix_prepare_mseg(tx_pkts[i], cmd, flags);
+		/* Lets commit any changes in the packet */
+		rte_io_wmb();
 		otx2_nix_xmit_prepare_tstamp(cmd, &txq->cmd[0],
 					     tx_pkts[i]->ol_flags, segdw,
 					     flags);
@@ -127,8 +129,11 @@ nix_xmit_pkts_vector(void *tx_queue, struct rte_mbuf **tx_pkts,
 	/* Reduce the cached count */
 	txq->fc_cache_pkts -= pkts;
 
-	/* Lets commit any changes in the packet */
-	rte_io_wmb();
+	/* Lets commit any changes in the packet here as no further changes
+	 * to the packet will be done unless no fast free is enabled.
+	 */
+	if (!(flags & NIX_TX_OFFLOAD_MBUF_NOFF_F))
+		rte_io_wmb();
 
 	senddesc01_w0 = vld1q_dup_u64(&txq->cmd[0]);
 	senddesc23_w0 = senddesc01_w0;
@@ -221,6 +226,10 @@ nix_xmit_pkts_vector(void *tx_queue, struct rte_mbuf **tx_pkts,
 							1, 0);
 			senddesc01_w0 = vorrq_u64(senddesc01_w0, xmask01);
 			senddesc23_w0 = vorrq_u64(senddesc23_w0, xmask23);
+			/* Ensuring mbuf fields which got updated in
+			 * otx2_nix_prefree_seg are written before LMTST.
+			 */
+			rte_io_wmb();
 		} else {
 			struct rte_mbuf *mbuf;
 			/* Mark mempool object as "put" since
diff --git a/drivers/net/octeontx2/otx2_tx.h b/drivers/net/octeontx2/otx2_tx.h
index caf170fd1..d6ea3b487 100644
--- a/drivers/net/octeontx2/otx2_tx.h
+++ b/drivers/net/octeontx2/otx2_tx.h
@@ -363,6 +363,10 @@ otx2_nix_xmit_prepare(struct rte_mbuf *m, uint64_t *cmd, const uint16_t flags)
 			 * DF bit = 0 otherwise
 			 */
 			send_hdr->w0.df = otx2_nix_prefree_seg(m);
+			/* Ensuring mbuf fields which got updated in
+			 * otx2_nix_prefree_seg are written before LMTST.
+			 */
+			rte_io_wmb();
 		}
 		/* Mark mempool object as "put" since it is freed by NIX */
 		if (!send_hdr->w0.df)
@@ -395,6 +399,12 @@ otx2_nix_xmit_submit_lmt(const rte_iova_t io_addr)
 	return otx2_lmt_submit(io_addr);
 }
 
+static __rte_always_inline uint64_t
+otx2_nix_xmit_submit_lmt_release(const rte_iova_t io_addr)
+{
+	return otx2_lmt_submit_release(io_addr);
+}
+
 static __rte_always_inline uint16_t
 otx2_nix_prepare_mseg(struct rte_mbuf *m, uint64_t *cmd, const uint16_t flags)
 {
@@ -483,6 +493,19 @@ otx2_nix_xmit_mseg_one(uint64_t *cmd, void *lmt_addr,
 	} while (lmt_status == 0);
 }
 
+static __rte_always_inline void
+otx2_nix_xmit_mseg_one_release(uint64_t *cmd, void *lmt_addr,
+		       rte_iova_t io_addr, uint16_t segdw)
+{
+	uint64_t lmt_status;
+
+	rte_io_wmb();
+	do {
+		otx2_lmt_mov_seg(lmt_addr, (const void *)cmd, segdw);
+		lmt_status = otx2_lmt_submit(io_addr);
+	} while (lmt_status == 0);
+}
+
 #define L3L4CSUM_F   NIX_TX_OFFLOAD_L3_L4_CSUM_F
 #define OL3OL4CSUM_F NIX_TX_OFFLOAD_OL3_OL4_CSUM_F
 #define VLAN_F       NIX_TX_OFFLOAD_VLAN_QINQ_F
-- 
2.18.0


  parent reply	other threads:[~2020-10-08 18:50 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-15 18:56 [dpdk-dev] [PATCH 1/4] event/octeontx2: add switch tag flush op Harman Kalra
2020-09-15 18:56 ` [dpdk-dev] [PATCH 2/4] event/octeontx2: improve single flow performance Harman Kalra
2020-10-05  9:29   ` Jerin Jacob
2020-10-08 18:48     ` [dpdk-dev] [PATCH v2 1/4] event/octeontx2: add switch tag flush op Harman Kalra
2020-10-08 18:48       ` [dpdk-dev] [PATCH v2 2/4] event/octeontx2: improve single flow performance Harman Kalra
2020-10-08 18:48       ` Harman Kalra [this message]
2020-10-16  4:04         ` [dpdk-dev] [dpdk-stable] [PATCH v2 3/4] net/octeontx2: fix jumbo frame crash Thomas Monjalon
2020-10-08 18:48       ` [dpdk-dev] [PATCH v2 4/4] app/eventdev: enable fast free offload Harman Kalra
2020-10-11 10:33         ` Jerin Jacob
2020-10-13 19:06           ` Jerin Jacob
2020-10-11 10:40       ` [dpdk-dev] [PATCH v2 1/4] event/octeontx2: add switch tag flush op Jerin Jacob
2020-09-15 18:56 ` [dpdk-dev] [PATCH 3/4] net/octeontx2: fix jumbo frame crash Harman Kalra
2020-09-15 18:56 ` [dpdk-dev] [PATCH 4/4] app/eventdev: enable fast free offload Harman Kalra
2020-10-05  9:26   ` Jerin Jacob

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=1602182927-18254-3-git-send-email-hkalra@marvell.com \
    --to=hkalra@marvell.com \
    --cc=dev@dpdk.org \
    --cc=jerinj@marvell.com \
    --cc=kirankumark@marvell.com \
    --cc=ndabilpuram@marvell.com \
    --cc=pbhagavatula@marvell.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

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/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 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

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


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