DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] net/bnxt: mark handling needs to look at meta_fmt instead of meta data
@ 2020-04-30 20:39 Venkat Duvvuru
  2020-05-01 19:16 ` [dpdk-dev] [PATCH v2] net/bnxt: fix mark handling to check metafmt not metadata Ajit Khaparde
  0 siblings, 1 reply; 3+ messages in thread
From: Venkat Duvvuru @ 2020-04-30 20:39 UTC (permalink / raw)
  To: dev; +Cc: Mike Baucom

From: Mike Baucom <michael.baucom@broadcom.com>

The current mark handling uses the meta data field of the rxcmp as the
first level check for determining gfid vs lfid.  When the meta data is
zero due to only the lowest 16bits of the gfid being set, the cfa code
is incorrectly interpreted as being an lfid.  Changing code to look at
meta fmt instead of the meta data directly for the determination.

Signed-off-by: Mike Baucom <michael.baucom@broadcom.com>
Reviewed-by: Ajit Kumar Khaparde <ajit.khaparde@broadcom.com>
Reviewed-by: Lance Richardson <lance.richardson@broadcom.com>
---
 drivers/net/bnxt/bnxt_rxr.c | 85 +++++++++++++++++++++++++++------------------
 drivers/net/bnxt/bnxt_rxr.h |  2 ++
 2 files changed, 53 insertions(+), 34 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_rxr.c b/drivers/net/bnxt/bnxt_rxr.c
index a657150..9ecdf5e 100644
--- a/drivers/net/bnxt/bnxt_rxr.c
+++ b/drivers/net/bnxt/bnxt_rxr.c
@@ -409,7 +409,7 @@ bnxt_ulp_set_mark_in_mbuf(struct bnxt *bp, struct rx_pkt_cmpl_hi *rxcmp1,
 	uint32_t cfa_code;
 	uint32_t meta_fmt;
 	uint32_t meta;
-	uint32_t eem = 0;
+	bool gfid = false;
 	uint32_t mark_id;
 	uint32_t flags2;
 	int rc;
@@ -417,53 +417,70 @@ bnxt_ulp_set_mark_in_mbuf(struct bnxt *bp, struct rx_pkt_cmpl_hi *rxcmp1,
 	cfa_code = rte_le_to_cpu_16(rxcmp1->cfa_code);
 	flags2 = rte_le_to_cpu_32(rxcmp1->flags2);
 	meta = rte_le_to_cpu_32(rxcmp1->metadata);
-	if (meta) {
-		meta >>= BNXT_RX_META_CFA_CODE_SHIFT;
 
-		/* The flags field holds extra bits of info from [6:4]
-		 * which indicate if the flow is in TCAM or EM or EEM
+	/*
+	 * The flags field holds extra bits of info from [6:4]
+	 * which indicate if the flow is in TCAM or EM or EEM
+	 */
+	meta_fmt = (flags2 & BNXT_CFA_META_FMT_MASK) >>
+		BNXT_CFA_META_FMT_SHFT;
+
+	switch (meta_fmt) {
+	case 0:
+		/* Not an LFID or GFID, a flush cmd. */
+		goto skip_mark;
+	case 4:
+	case 5:
+		/*
+		 * EM/TCAM case
+		 * Assume that EM doesn't support Mark due to GFID
+		 * collisions with EEM.  Simply return without setting the mark
+		 * in the mbuf.
 		 */
-		meta_fmt = (flags2 & BNXT_CFA_META_FMT_MASK) >>
-			    BNXT_CFA_META_FMT_SHFT;
-		/* meta_fmt == 4 => 'b100 => 'b10x => EM.
-		 * meta_fmt == 5 => 'b101 => 'b10x => EM + VLAN
-		 * meta_fmt == 6 => 'b110 => 'b11x => EEM
-		 * meta_fmt == 7 => 'b111 => 'b11x => EEM + VLAN.
+		if (BNXT_CFA_META_EM_TEST(meta))
+			goto skip_mark;
+		/*
+		 * It is a TCAM entry, so it is an LFID. The TCAM IDX and Mode
+		 * can also be determined by decoding the meta_data.  We are not
+		 * using these for now.
 		 */
-		meta_fmt >>= BNXT_CFA_META_FMT_EM_EEM_SHFT;
-
-		eem = meta_fmt == BNXT_CFA_META_FMT_EEM;
+		break;
+	case 6:
+	case 7:
+		/* EEM Case, only using gfid in EEM for now. */
+		gfid = true;
 
-		/* For EEM flows, The first part of cfa_code is 16 bits.
+		/*
+		 * For EEM flows, The first part of cfa_code is 16 bits.
 		 * The second part is embedded in the
 		 * metadata field from bit 19 onwards. The driver needs to
 		 * ignore the first 19 bits of metadata and use the next 12
 		 * bits as higher 12 bits of cfa_code.
 		 */
-		if (eem)
-			cfa_code |= meta << BNXT_CFA_CODE_META_SHIFT;
+		meta >>= BNXT_RX_META_CFA_CODE_SHIFT;
+		cfa_code |= meta << BNXT_CFA_CODE_META_SHIFT;
+		break;
+	default:
+		/* For other values, the cfa_code is assumed to be an LFID. */
+		break;
 	}
 
 	if (cfa_code) {
-		mbuf->hash.fdir.hi = 0;
-		mbuf->hash.fdir.id = 0;
-		if (eem)
-			rc = ulp_mark_db_mark_get(&bp->ulp_ctx, true,
-						  cfa_code, &mark_id);
-		else
-			rc = ulp_mark_db_mark_get(&bp->ulp_ctx, false,
-						  cfa_code, &mark_id);
-		/* If the above fails, simply return and don't add the mark to
-		 * mbuf
-		 */
-		if (rc)
+		rc = ulp_mark_db_mark_get(&bp->ulp_ctx, gfid,
+					  cfa_code, &mark_id);
+		if (!rc) {
+			/* Got the mark, write it to the mbuf and return */
+			mbuf->hash.fdir.hi = mark_id;
+			mbuf->udata64 = (cfa_code & 0xffffffffull) << 32;
+			mbuf->hash.fdir.id = rxcmp1->cfa_code;
+			mbuf->ol_flags |= PKT_RX_FDIR | PKT_RX_FDIR_ID;
 			return;
-
-		mbuf->hash.fdir.hi	= mark_id;
-		mbuf->udata64		= (cfa_code & 0xffffffffull) << 32;
-		mbuf->hash.fdir.id	= rxcmp1->cfa_code;
-		mbuf->ol_flags		|= PKT_RX_FDIR | PKT_RX_FDIR_ID;
+		}
 	}
+
+skip_mark:
+	mbuf->hash.fdir.hi = 0;
+	mbuf->hash.fdir.id = 0;
 }
 
 void bnxt_set_mark_in_mbuf(struct bnxt *bp,
diff --git a/drivers/net/bnxt/bnxt_rxr.h b/drivers/net/bnxt/bnxt_rxr.h
index d10dae2..811dcd8 100644
--- a/drivers/net/bnxt/bnxt_rxr.h
+++ b/drivers/net/bnxt/bnxt_rxr.h
@@ -238,5 +238,7 @@ void bnxt_set_mark_in_mbuf(struct bnxt *bp,
 #define BNXT_CFA_META_FMT_SHFT			4
 #define BNXT_CFA_META_FMT_EM_EEM_SHFT		1
 #define BNXT_CFA_META_FMT_EEM			3
+#define BNXT_CFA_META_EEM_TCAM_SHIFT		31
+#define BNXT_CFA_META_EM_TEST(x) ((x) >> BNXT_CFA_META_EEM_TCAM_SHIFT)
 
 #endif
-- 
2.7.4


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [dpdk-dev] [PATCH v2] net/bnxt: fix mark handling to check metafmt not metadata
  2020-04-30 20:39 [dpdk-dev] [PATCH] net/bnxt: mark handling needs to look at meta_fmt instead of meta data Venkat Duvvuru
@ 2020-05-01 19:16 ` Ajit Khaparde
  2020-05-01 22:02   ` Ajit Khaparde
  0 siblings, 1 reply; 3+ messages in thread
From: Ajit Khaparde @ 2020-05-01 19:16 UTC (permalink / raw)
  To: dev; +Cc: Mike Baucom, Lance Richardson

From: Mike Baucom <michael.baucom@broadcom.com>

The current mark handling uses the meta data field of the rxcmp as the
first level check for determining gfid vs lfid.  When the meta data is
zero due to only the lowest 16bits of the gfid being set, the cfa code
is incorrectly interpreted as being an lfid.  Changing code to look at
meta fmt instead of the meta data directly for the determination.

Fixes: b87abb2e55cb ("net/bnxt: support marking packet")

Signed-off-by: Mike Baucom <michael.baucom@broadcom.com>
Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
Reviewed-by: Lance Richardson <lance.richardson@broadcom.com>
--
v1->v2: fixed the commit log.
---
 drivers/net/bnxt/bnxt_rxr.c | 85 ++++++++++++++++++++++---------------
 drivers/net/bnxt/bnxt_rxr.h |  2 +
 2 files changed, 53 insertions(+), 34 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_rxr.c b/drivers/net/bnxt/bnxt_rxr.c
index a657150d1..9ecdf5e65 100644
--- a/drivers/net/bnxt/bnxt_rxr.c
+++ b/drivers/net/bnxt/bnxt_rxr.c
@@ -409,7 +409,7 @@ bnxt_ulp_set_mark_in_mbuf(struct bnxt *bp, struct rx_pkt_cmpl_hi *rxcmp1,
 	uint32_t cfa_code;
 	uint32_t meta_fmt;
 	uint32_t meta;
-	uint32_t eem = 0;
+	bool gfid = false;
 	uint32_t mark_id;
 	uint32_t flags2;
 	int rc;
@@ -417,53 +417,70 @@ bnxt_ulp_set_mark_in_mbuf(struct bnxt *bp, struct rx_pkt_cmpl_hi *rxcmp1,
 	cfa_code = rte_le_to_cpu_16(rxcmp1->cfa_code);
 	flags2 = rte_le_to_cpu_32(rxcmp1->flags2);
 	meta = rte_le_to_cpu_32(rxcmp1->metadata);
-	if (meta) {
-		meta >>= BNXT_RX_META_CFA_CODE_SHIFT;
 
-		/* The flags field holds extra bits of info from [6:4]
-		 * which indicate if the flow is in TCAM or EM or EEM
+	/*
+	 * The flags field holds extra bits of info from [6:4]
+	 * which indicate if the flow is in TCAM or EM or EEM
+	 */
+	meta_fmt = (flags2 & BNXT_CFA_META_FMT_MASK) >>
+		BNXT_CFA_META_FMT_SHFT;
+
+	switch (meta_fmt) {
+	case 0:
+		/* Not an LFID or GFID, a flush cmd. */
+		goto skip_mark;
+	case 4:
+	case 5:
+		/*
+		 * EM/TCAM case
+		 * Assume that EM doesn't support Mark due to GFID
+		 * collisions with EEM.  Simply return without setting the mark
+		 * in the mbuf.
 		 */
-		meta_fmt = (flags2 & BNXT_CFA_META_FMT_MASK) >>
-			    BNXT_CFA_META_FMT_SHFT;
-		/* meta_fmt == 4 => 'b100 => 'b10x => EM.
-		 * meta_fmt == 5 => 'b101 => 'b10x => EM + VLAN
-		 * meta_fmt == 6 => 'b110 => 'b11x => EEM
-		 * meta_fmt == 7 => 'b111 => 'b11x => EEM + VLAN.
+		if (BNXT_CFA_META_EM_TEST(meta))
+			goto skip_mark;
+		/*
+		 * It is a TCAM entry, so it is an LFID. The TCAM IDX and Mode
+		 * can also be determined by decoding the meta_data.  We are not
+		 * using these for now.
 		 */
-		meta_fmt >>= BNXT_CFA_META_FMT_EM_EEM_SHFT;
-
-		eem = meta_fmt == BNXT_CFA_META_FMT_EEM;
+		break;
+	case 6:
+	case 7:
+		/* EEM Case, only using gfid in EEM for now. */
+		gfid = true;
 
-		/* For EEM flows, The first part of cfa_code is 16 bits.
+		/*
+		 * For EEM flows, The first part of cfa_code is 16 bits.
 		 * The second part is embedded in the
 		 * metadata field from bit 19 onwards. The driver needs to
 		 * ignore the first 19 bits of metadata and use the next 12
 		 * bits as higher 12 bits of cfa_code.
 		 */
-		if (eem)
-			cfa_code |= meta << BNXT_CFA_CODE_META_SHIFT;
+		meta >>= BNXT_RX_META_CFA_CODE_SHIFT;
+		cfa_code |= meta << BNXT_CFA_CODE_META_SHIFT;
+		break;
+	default:
+		/* For other values, the cfa_code is assumed to be an LFID. */
+		break;
 	}
 
 	if (cfa_code) {
-		mbuf->hash.fdir.hi = 0;
-		mbuf->hash.fdir.id = 0;
-		if (eem)
-			rc = ulp_mark_db_mark_get(&bp->ulp_ctx, true,
-						  cfa_code, &mark_id);
-		else
-			rc = ulp_mark_db_mark_get(&bp->ulp_ctx, false,
-						  cfa_code, &mark_id);
-		/* If the above fails, simply return and don't add the mark to
-		 * mbuf
-		 */
-		if (rc)
+		rc = ulp_mark_db_mark_get(&bp->ulp_ctx, gfid,
+					  cfa_code, &mark_id);
+		if (!rc) {
+			/* Got the mark, write it to the mbuf and return */
+			mbuf->hash.fdir.hi = mark_id;
+			mbuf->udata64 = (cfa_code & 0xffffffffull) << 32;
+			mbuf->hash.fdir.id = rxcmp1->cfa_code;
+			mbuf->ol_flags |= PKT_RX_FDIR | PKT_RX_FDIR_ID;
 			return;
-
-		mbuf->hash.fdir.hi	= mark_id;
-		mbuf->udata64		= (cfa_code & 0xffffffffull) << 32;
-		mbuf->hash.fdir.id	= rxcmp1->cfa_code;
-		mbuf->ol_flags		|= PKT_RX_FDIR | PKT_RX_FDIR_ID;
+		}
 	}
+
+skip_mark:
+	mbuf->hash.fdir.hi = 0;
+	mbuf->hash.fdir.id = 0;
 }
 
 void bnxt_set_mark_in_mbuf(struct bnxt *bp,
diff --git a/drivers/net/bnxt/bnxt_rxr.h b/drivers/net/bnxt/bnxt_rxr.h
index d10dae25a..811dcd86b 100644
--- a/drivers/net/bnxt/bnxt_rxr.h
+++ b/drivers/net/bnxt/bnxt_rxr.h
@@ -238,5 +238,7 @@ void bnxt_set_mark_in_mbuf(struct bnxt *bp,
 #define BNXT_CFA_META_FMT_SHFT			4
 #define BNXT_CFA_META_FMT_EM_EEM_SHFT		1
 #define BNXT_CFA_META_FMT_EEM			3
+#define BNXT_CFA_META_EEM_TCAM_SHIFT		31
+#define BNXT_CFA_META_EM_TEST(x) ((x) >> BNXT_CFA_META_EEM_TCAM_SHIFT)
 
 #endif
-- 
2.21.1 (Apple Git-122.3)


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [dpdk-dev] [PATCH v2] net/bnxt: fix mark handling to check metafmt not metadata
  2020-05-01 19:16 ` [dpdk-dev] [PATCH v2] net/bnxt: fix mark handling to check metafmt not metadata Ajit Khaparde
@ 2020-05-01 22:02   ` Ajit Khaparde
  0 siblings, 0 replies; 3+ messages in thread
From: Ajit Khaparde @ 2020-05-01 22:02 UTC (permalink / raw)
  To: dpdk-dev; +Cc: Mike Baucom, Lance Richardson, Venkat Duvvuru

On Fri, May 1, 2020 at 12:16 PM Ajit Khaparde <ajit.khaparde@broadcom.com>
wrote:

> From: Mike Baucom <michael.baucom@broadcom.com>
>
> The current mark handling uses the meta data field of the rxcmp as the
> first level check for determining gfid vs lfid.  When the meta data is
> zero due to only the lowest 16bits of the gfid being set, the cfa code
> is incorrectly interpreted as being an lfid.  Changing code to look at
> meta fmt instead of the meta data directly for the determination.
>
> Fixes: b87abb2e55cb ("net/bnxt: support marking packet")
>
> Signed-off-by: Mike Baucom <michael.baucom@broadcom.com>
> Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> Reviewed-by: Lance Richardson <lance.richardson@broadcom.com>
> --
> v1->v2: fixed the commit log.
> ---
>  drivers/net/bnxt/bnxt_rxr.c | 85 ++++++++++++++++++++++---------------
>  drivers/net/bnxt/bnxt_rxr.h |  2 +
>  2 files changed, 53 insertions(+), 34 deletions(-)
>
> Patch applied to dpdk-next-net-brcm.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-05-01 22:03 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-30 20:39 [dpdk-dev] [PATCH] net/bnxt: mark handling needs to look at meta_fmt instead of meta data Venkat Duvvuru
2020-05-01 19:16 ` [dpdk-dev] [PATCH v2] net/bnxt: fix mark handling to check metafmt not metadata Ajit Khaparde
2020-05-01 22:02   ` Ajit Khaparde

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://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/ http://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