[net PATCH V3 0/2] octeontx2-af: Fix issues with NPC field hash

[net PATCH V3 0/2] octeontx2-af: Fix issues with NPC field hash

[Suman Ghosh]

This patchset fixes the issues with NPC field hash extract. This feature is
supported only for CN10KB variant of CN10K series of silicons. This features
helps to hash reduce 128 bit IPv6 source/destination address to 32 bit(also)
configurable) which can save 96 bit and can be used to store other key
information during packet filtering.

Patch #1 Fix hash extraction mbox message

Patch #2 Fix hash extraction enable configuration

Suman Ghosh (2):
  octeontx2-af: Fix hash extraction mbox message
  octeontx2-af: Fix hash extraction enable configuration

---
v3 changes:
	1. Rebased the patchset on top of 'main' branch
	2. Removed patch #3 as it is already merged in the 'main' branch

v2 changes:
	Fixed review comment from Jakub Kicinski
	1. Updated detailed commit messages
	2. Updated cover letter.

 .../net/ethernet/marvell/octeontx2/af/mbox.h  |  6 +-
 .../marvell/octeontx2/af/rvu_npc_hash.c       | 70 +++++++++++++++----
 .../marvell/octeontx2/af/rvu_npc_hash.h       | 17 +++--
 3 files changed, 69 insertions(+), 24 deletions(-)

-- 
2.25.1

[net PATCH V3 1/2] octeontx2-af: Fix hash extraction mbox message

[Suman Ghosh]

As of today, hash extraction mbox message response supports only the
secret key which is not a complete solution. This patch fixes that and
adds support to extract both hash mask and hash control along with the
secret key. These are needed to use hash reduction of 128 bit IPv6
address to 32 bit. Hash mask decides on the bits from the 128 bit IPv6
address which should be used for 32 bit hash calculation. After
generating the 32 bit hash, hash control decides how many bits from the
32 bit hash can be taken into consideration.

Fixes: a95ab93550d3 ("octeontx2-af: Use hashed field in MCAM key")
Signed-off-by: Suman Ghosh <sumang@marvell.com>
---
 .../net/ethernet/marvell/octeontx2/af/mbox.h  |  6 ++---
 .../marvell/octeontx2/af/rvu_npc_hash.c       | 27 ++++++++++---------
 .../marvell/octeontx2/af/rvu_npc_hash.h       |  9 ++++---
 3 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
index eba307eee2b2..5a5c23a02261 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
+++ b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
@@ -246,9 +246,9 @@ M(NPC_MCAM_READ_BASE_RULE, 0x6011, npc_read_base_steer_rule,            \
 M(NPC_MCAM_GET_STATS, 0x6012, npc_mcam_entry_stats,                     \
 				   npc_mcam_get_stats_req,              \
 				   npc_mcam_get_stats_rsp)              \
-M(NPC_GET_FIELD_HASH_INFO, 0x6013, npc_get_field_hash_info,                     \
-				   npc_get_field_hash_info_req,              \
-				   npc_get_field_hash_info_rsp)              \
+M(NPC_GET_FIELD_HASH_INFO, 0x6013, npc_get_field_hash_info,             \
+				   npc_get_field_hash_info_req,         \
+				   npc_get_field_hash_info_rsp)         \
 M(NPC_GET_FIELD_STATUS, 0x6014, npc_get_field_status,                     \
 				   npc_get_field_status_req,              \
 				   npc_get_field_status_rsp)              \
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c
index 6fe67f3a7f6f..a3bc53d22dc0 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c
@@ -96,7 +96,7 @@ u32 npc_field_hash_calc(u64 *ldata, struct npc_get_field_hash_info_rsp rsp,
 	field_hash = rvu_npc_toeplitz_hash(data_padded, hash_key, 128, 159);
 
 	field_hash &= FIELD_GET(GENMASK(63, 32), rsp.hash_ctrl[intf][hash_idx]);
-	field_hash += FIELD_GET(GENMASK(31, 0), rsp.hash_ctrl[intf][hash_idx]);
+	field_hash |= FIELD_GET(GENMASK(31, 0), rsp.hash_ctrl[intf][hash_idx]);
 	return field_hash;
 }
 
@@ -253,7 +253,8 @@ void npc_update_field_hash(struct rvu *rvu, u8 intf,
 	}
 
 	req.intf = intf;
-	rvu_mbox_handler_npc_get_field_hash_info(rvu, &req, &rsp);
+	if (rvu_mbox_handler_npc_get_field_hash_info(rvu, &req, &rsp))
+		return;
 
 	for (hash_idx = 0; hash_idx < NPC_MAX_HASH; hash_idx++) {
 		cfg = rvu_read64(rvu, blkaddr, NPC_AF_INTFX_HASHX_CFG(intf, hash_idx));
@@ -319,9 +320,9 @@ int rvu_mbox_handler_npc_get_field_hash_info(struct rvu *rvu,
 					     struct npc_get_field_hash_info_req *req,
 					     struct npc_get_field_hash_info_rsp *rsp)
 {
+	int hash_idx, hash_mask_idx, blkaddr;
 	u64 *secret_key = rsp->secret_key;
 	u8 intf = req->intf;
-	int i, j, blkaddr;
 
 	blkaddr = rvu_get_blkaddr(rvu, BLKTYPE_NPC, 0);
 	if (blkaddr < 0) {
@@ -333,18 +334,18 @@ int rvu_mbox_handler_npc_get_field_hash_info(struct rvu *rvu,
 	secret_key[1] = rvu_read64(rvu, blkaddr, NPC_AF_INTFX_SECRET_KEY1(intf));
 	secret_key[2] = rvu_read64(rvu, blkaddr, NPC_AF_INTFX_SECRET_KEY2(intf));
 
-	for (i = 0; i < NPC_MAX_HASH; i++) {
-		for (j = 0; j < NPC_MAX_HASH_MASK; j++) {
-			rsp->hash_mask[NIX_INTF_RX][i][j] =
-				GET_KEX_LD_HASH_MASK(NIX_INTF_RX, i, j);
-			rsp->hash_mask[NIX_INTF_TX][i][j] =
-				GET_KEX_LD_HASH_MASK(NIX_INTF_TX, i, j);
+	for (hash_idx = 0; hash_idx < NPC_MAX_HASH; hash_idx++)
+		for (hash_mask_idx = 0; hash_mask_idx < NPC_MAX_HASH_MASK; hash_mask_idx++) {
+			rsp->hash_mask[NIX_INTF_RX][hash_idx][hash_mask_idx] =
+				GET_KEX_LD_HASH_MASK(NIX_INTF_RX, hash_idx, hash_mask_idx);
+			rsp->hash_mask[NIX_INTF_TX][hash_idx][hash_mask_idx] =
+				GET_KEX_LD_HASH_MASK(NIX_INTF_TX, hash_idx, hash_mask_idx);
 		}
-	}
 
-	for (i = 0; i < NPC_MAX_INTF; i++)
-		for (j = 0; j < NPC_MAX_HASH; j++)
-			rsp->hash_ctrl[i][j] = GET_KEX_LD_HASH_CTRL(i, j);
+	for (hash_idx = 0; hash_idx < NPC_MAX_INTF; hash_idx++)
+		for (hash_mask_idx = 0; hash_mask_idx < NPC_MAX_HASH; hash_mask_idx++)
+			rsp->hash_ctrl[hash_idx][hash_mask_idx] =
+				GET_KEX_LD_HASH_CTRL(hash_idx, hash_mask_idx);
 
 	return 0;
 }
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.h b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.h
index a1c3d987b804..eb9cb311b934 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.h
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.h
@@ -12,9 +12,6 @@
 #define RVU_NPC_HASH_SECRET_KEY1 0xa9d5af4c9fbc87b4
 #define RVU_NPC_HASH_SECRET_KEY2 0x5954c9e7
 
-#define NPC_MAX_HASH 2
-#define NPC_MAX_HASH_MASK 2
-
 #define KEX_LD_CFG_USE_HASH(use_hash, bytesm1, hdr_ofs, ena, flags_ena, key_ofs) \
 			    ((use_hash) << 20 | ((bytesm1) << 16) | ((hdr_ofs) << 8) | \
 			     ((ena) << 7) | ((flags_ena) << 6) | ((key_ofs) & 0x3F))
@@ -41,6 +38,12 @@
 	rvu_write64(rvu, blkaddr,	\
 		    NPC_AF_INTFX_HASHX_RESULT_CTRL(intf, ld), cfg)
 
+#define GET_KEX_LD_HASH_CTRL(intf, ld)  \
+	rvu_read64(rvu, blkaddr, NPC_AF_INTFX_HASHX_RESULT_CTRL(intf, ld))
+
+#define GET_KEX_LD_HASH_MASK(intf, ld, mask_idx)	\
+	rvu_read64(rvu, blkaddr, NPC_AF_INTFX_HASHX_MASKX(intf, ld, mask_idx))
+
 struct npc_mcam_kex_hash {
 	/* NPC_AF_INTF(0..1)_LID(0..7)_LT(0..15)_LD(0..1)_CFG */
 	bool lid_lt_ld_hash_en[NPC_MAX_INTF][NPC_MAX_LID][NPC_MAX_LT][NPC_MAX_LD];
-- 
2.25.1

[net PATCH V3 2/2] octeontx2-af: Fix hash extraction enable configuration

[Suman Ghosh]

As of today, hash extraction support is enabled for all the silicons.
Because of which we are facing initialization issues when the silicon
does not support hash extraction. During creation of the hardware
parsing table for IPv6 address, we need to consider if hash extraction
is enabled then extract only 32 bit, otherwise 128 bit needs to be
extracted. This patch fixes the issue and configures the hardware parser
based on the availability of the feature.

Fixes: a95ab93550d3 ("octeontx2-af: Use hashed field in MCAM key")
Signed-off-by: Suman Ghosh <sumang@marvell.com>
---
 .../marvell/octeontx2/af/rvu_npc_hash.c       | 43 ++++++++++++++++++-
 .../marvell/octeontx2/af/rvu_npc_hash.h       |  8 ++--
 2 files changed, 46 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c
index a3bc53d22dc0..c4c45dea0f39 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c
@@ -218,13 +218,54 @@ void npc_config_secret_key(struct rvu *rvu, int blkaddr)
 
 void npc_program_mkex_hash(struct rvu *rvu, int blkaddr)
 {
+	struct npc_mcam_kex_hash *mh = rvu->kpu.mkex_hash;
 	struct hw_cap *hwcap = &rvu->hw->cap;
+	u8 intf, ld, hdr_offset, byte_len;
 	struct rvu_hwinfo *hw = rvu->hw;
-	u8 intf;
+	u64 cfg;
 
+	/* Check if hardware supports hash extraction */
 	if (!hwcap->npc_hash_extract)
 		return;
 
+	/* Check if IPv6 source/destination address
+	 * should be hash enabled.
+	 * Hashing reduces 128bit SIP/DIP fields to 32bit
+	 * so that 224 bit X2 key can be used for IPv6 based filters as well,
+	 * which in turn results in more number of MCAM entries available for
+	 * use.
+	 *
+	 * Hashing of IPV6 SIP/DIP is enabled in below scenarios
+	 * 1. If the silicon variant supports hashing feature
+	 * 2. If the number of bytes of IP addr being extracted is 4 bytes ie
+	 *    32bit. The assumption here is that if user wants 8bytes of LSB of
+	 *    IP addr or full 16 bytes then his intention is not to use 32bit
+	 *    hash.
+	 */
+	for (intf = 0; intf < hw->npc_intfs; intf++) {
+		for (ld = 0; ld < NPC_MAX_LD; ld++) {
+			cfg = rvu_read64(rvu, blkaddr,
+					 NPC_AF_INTFX_LIDX_LTX_LDX_CFG(intf,
+								       NPC_LID_LC,
+								       NPC_LT_LC_IP6,
+								       ld));
+			hdr_offset = FIELD_GET(NPC_HDR_OFFSET, cfg);
+			byte_len = FIELD_GET(NPC_BYTESM, cfg);
+			/* Hashing of IPv6 source/destination address should be
+			 * enabled if,
+			 * hdr_offset == 8 (offset of source IPv6 address) or
+			 * hdr_offset == 24 (offset of destination IPv6)
+			 * address) and the number of byte to be
+			 * extracted is 4. As per hardware configuration
+			 * byte_len should be == actual byte_len - 1.
+			 * Hence byte_len is checked against 3 but nor 4.
+			 */
+			if ((hdr_offset == 8 || hdr_offset == 24) && byte_len == 3)
+				mh->lid_lt_ld_hash_en[intf][NPC_LID_LC][NPC_LT_LC_IP6][ld] = true;
+		}
+	}
+
+	/* Update hash configuration if the field is hash enabled */
 	for (intf = 0; intf < hw->npc_intfs; intf++) {
 		npc_program_mkex_hash_rx(rvu, blkaddr, intf);
 		npc_program_mkex_hash_tx(rvu, blkaddr, intf);
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.h b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.h
index eb9cb311b934..df6c6016827d 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.h
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.h
@@ -73,8 +73,8 @@ static struct npc_mcam_kex_hash npc_mkex_hash_default __maybe_unused = {
 	[NIX_INTF_RX] = {
 		[NPC_LID_LC] = {
 			[NPC_LT_LC_IP6] = {
-				true,
-				true,
+				false,
+				false,
 			},
 		},
 	},
@@ -82,8 +82,8 @@ static struct npc_mcam_kex_hash npc_mkex_hash_default __maybe_unused = {
 	[NIX_INTF_TX] = {
 		[NPC_LID_LC] = {
 			[NPC_LT_LC_IP6] = {
-				true,
-				true,
+				false,
+				false,
 			},
 		},
 	},
-- 
2.25.1

Re: [net PATCH V3 1/2] octeontx2-af: Fix hash extraction mbox message

[Simon Horman]

On Mon, Jul 17, 2023 at 01:00:48AM +0530, Suman Ghosh wrote:
> As of today, hash extraction mbox message response supports only the
> secret key which is not a complete solution. This patch fixes that and
> adds support to extract both hash mask and hash control along with the
> secret key. These are needed to use hash reduction of 128 bit IPv6
> address to 32 bit. Hash mask decides on the bits from the 128 bit IPv6
> address which should be used for 32 bit hash calculation. After
> generating the 32 bit hash, hash control decides how many bits from the
> 32 bit hash can be taken into consideration.
> 
> Fixes: a95ab93550d3 ("octeontx2-af: Use hashed field in MCAM key")
> Signed-off-by: Suman Ghosh <sumang@marvell.com>
> ---
>  .../net/ethernet/marvell/octeontx2/af/mbox.h  |  6 ++---
>  .../marvell/octeontx2/af/rvu_npc_hash.c       | 27 ++++++++++---------
>  .../marvell/octeontx2/af/rvu_npc_hash.h       |  9 ++++---
>  3 files changed, 23 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> index eba307eee2b2..5a5c23a02261 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> @@ -246,9 +246,9 @@ M(NPC_MCAM_READ_BASE_RULE, 0x6011, npc_read_base_steer_rule,            \
>  M(NPC_MCAM_GET_STATS, 0x6012, npc_mcam_entry_stats,                     \
>  				   npc_mcam_get_stats_req,              \
>  				   npc_mcam_get_stats_rsp)              \
> -M(NPC_GET_FIELD_HASH_INFO, 0x6013, npc_get_field_hash_info,                     \
> -				   npc_get_field_hash_info_req,              \
> -				   npc_get_field_hash_info_rsp)              \
> +M(NPC_GET_FIELD_HASH_INFO, 0x6013, npc_get_field_hash_info,             \
> +				   npc_get_field_hash_info_req,         \
> +				   npc_get_field_hash_info_rsp)         \
>  M(NPC_GET_FIELD_STATUS, 0x6014, npc_get_field_status,                     \
>  				   npc_get_field_status_req,              \
>  				   npc_get_field_status_rsp)              \

This hunk is a white-space only change that doesn't seem
related to the patch description.

> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c
> index 6fe67f3a7f6f..a3bc53d22dc0 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c
> @@ -96,7 +96,7 @@ u32 npc_field_hash_calc(u64 *ldata, struct npc_get_field_hash_info_rsp rsp,
>  	field_hash = rvu_npc_toeplitz_hash(data_padded, hash_key, 128, 159);
>  
>  	field_hash &= FIELD_GET(GENMASK(63, 32), rsp.hash_ctrl[intf][hash_idx]);
> -	field_hash += FIELD_GET(GENMASK(31, 0), rsp.hash_ctrl[intf][hash_idx]);
> +	field_hash |= FIELD_GET(GENMASK(31, 0), rsp.hash_ctrl[intf][hash_idx]);
>  	return field_hash;
>  }
>  
> @@ -253,7 +253,8 @@ void npc_update_field_hash(struct rvu *rvu, u8 intf,
>  	}
>  
>  	req.intf = intf;
> -	rvu_mbox_handler_npc_get_field_hash_info(rvu, &req, &rsp);
> +	if (rvu_mbox_handler_npc_get_field_hash_info(rvu, &req, &rsp))
> +		return;
>  
>  	for (hash_idx = 0; hash_idx < NPC_MAX_HASH; hash_idx++) {
>  		cfg = rvu_read64(rvu, blkaddr, NPC_AF_INTFX_HASHX_CFG(intf, hash_idx));
> @@ -319,9 +320,9 @@ int rvu_mbox_handler_npc_get_field_hash_info(struct rvu *rvu,
>  					     struct npc_get_field_hash_info_req *req,
>  					     struct npc_get_field_hash_info_rsp *rsp)
>  {
> +	int hash_idx, hash_mask_idx, blkaddr;
>  	u64 *secret_key = rsp->secret_key;
>  	u8 intf = req->intf;
> -	int i, j, blkaddr;
>  
>  	blkaddr = rvu_get_blkaddr(rvu, BLKTYPE_NPC, 0);
>  	if (blkaddr < 0) {
> @@ -333,18 +334,18 @@ int rvu_mbox_handler_npc_get_field_hash_info(struct rvu *rvu,
>  	secret_key[1] = rvu_read64(rvu, blkaddr, NPC_AF_INTFX_SECRET_KEY1(intf));
>  	secret_key[2] = rvu_read64(rvu, blkaddr, NPC_AF_INTFX_SECRET_KEY2(intf));
>  
> -	for (i = 0; i < NPC_MAX_HASH; i++) {
> -		for (j = 0; j < NPC_MAX_HASH_MASK; j++) {
> -			rsp->hash_mask[NIX_INTF_RX][i][j] =
> -				GET_KEX_LD_HASH_MASK(NIX_INTF_RX, i, j);
> -			rsp->hash_mask[NIX_INTF_TX][i][j] =
> -				GET_KEX_LD_HASH_MASK(NIX_INTF_TX, i, j);
> +	for (hash_idx = 0; hash_idx < NPC_MAX_HASH; hash_idx++)
> +		for (hash_mask_idx = 0; hash_mask_idx < NPC_MAX_HASH_MASK; hash_mask_idx++) {
> +			rsp->hash_mask[NIX_INTF_RX][hash_idx][hash_mask_idx] =
> +				GET_KEX_LD_HASH_MASK(NIX_INTF_RX, hash_idx, hash_mask_idx);
> +			rsp->hash_mask[NIX_INTF_TX][hash_idx][hash_mask_idx] =
> +				GET_KEX_LD_HASH_MASK(NIX_INTF_TX, hash_idx, hash_mask_idx);
>  		}
> -	}
>  
> -	for (i = 0; i < NPC_MAX_INTF; i++)
> -		for (j = 0; j < NPC_MAX_HASH; j++)
> -			rsp->hash_ctrl[i][j] = GET_KEX_LD_HASH_CTRL(i, j);
> +	for (hash_idx = 0; hash_idx < NPC_MAX_INTF; hash_idx++)
> +		for (hash_mask_idx = 0; hash_mask_idx < NPC_MAX_HASH; hash_mask_idx++)
> +			rsp->hash_ctrl[hash_idx][hash_mask_idx] =
> +				GET_KEX_LD_HASH_CTRL(hash_idx, hash_mask_idx);
>  
>  	return 0;
>  }

The three hunks above appear to change the iterator variables for the loops
without changing functionality. This doesn't seem to match the patch
description.

> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.h b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.h
> index a1c3d987b804..eb9cb311b934 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.h
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.h
> @@ -12,9 +12,6 @@
>  #define RVU_NPC_HASH_SECRET_KEY1 0xa9d5af4c9fbc87b4
>  #define RVU_NPC_HASH_SECRET_KEY2 0x5954c9e7
>  
> -#define NPC_MAX_HASH 2
> -#define NPC_MAX_HASH_MASK 2
> -

This seems to remove duplicated #defines.
This doesn't seem to match the patch description.

>  #define KEX_LD_CFG_USE_HASH(use_hash, bytesm1, hdr_ofs, ena, flags_ena, key_ofs) \
>  			    ((use_hash) << 20 | ((bytesm1) << 16) | ((hdr_ofs) << 8) | \
>  			     ((ena) << 7) | ((flags_ena) << 6) | ((key_ofs) & 0x3F))
> @@ -41,6 +38,12 @@
>  	rvu_write64(rvu, blkaddr,	\
>  		    NPC_AF_INTFX_HASHX_RESULT_CTRL(intf, ld), cfg)
>  
> +#define GET_KEX_LD_HASH_CTRL(intf, ld)  \
> +	rvu_read64(rvu, blkaddr, NPC_AF_INTFX_HASHX_RESULT_CTRL(intf, ld))
> +
> +#define GET_KEX_LD_HASH_MASK(intf, ld, mask_idx)	\
> +	rvu_read64(rvu, blkaddr, NPC_AF_INTFX_HASHX_MASKX(intf, ld, mask_idx))
> +

This seems to duplicate existing MACROS,
which appear a few lines further above in this file.

>  struct npc_mcam_kex_hash {
>  	/* NPC_AF_INTF(0..1)_LID(0..7)_LT(0..15)_LD(0..1)_CFG */
>  	bool lid_lt_ld_hash_en[NPC_MAX_INTF][NPC_MAX_LID][NPC_MAX_LT][NPC_MAX_LD];
> -- 
> 2.25.1
> 
> 

RE: [EXT] Re: [net PATCH V3 1/2] octeontx2-af: Fix hash extraction mbox message

[Suman Ghosh]

My bad, I will discard the patch since it is already merged. 

>On Mon, Jul 17, 2023 at 01:00:48AM +0530, Suman Ghosh wrote:
>> As of today, hash extraction mbox message response supports only the
>> secret key which is not a complete solution. This patch fixes that and
>> adds support to extract both hash mask and hash control along with the
>> secret key. These are needed to use hash reduction of 128 bit IPv6
>> address to 32 bit. Hash mask decides on the bits from the 128 bit IPv6
>> address which should be used for 32 bit hash calculation. After
>> generating the 32 bit hash, hash control decides how many bits from
>> the
>> 32 bit hash can be taken into consideration.
>>
>> Fixes: a95ab93550d3 ("octeontx2-af: Use hashed field in MCAM key")
>> Signed-off-by: Suman Ghosh <sumang@marvell.com>
>> ---
>>  .../net/ethernet/marvell/octeontx2/af/mbox.h  |  6 ++---
>>  .../marvell/octeontx2/af/rvu_npc_hash.c       | 27 ++++++++++--------
>-
>>  .../marvell/octeontx2/af/rvu_npc_hash.h       |  9 ++++---
>>  3 files changed, 23 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
>> b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
>> index eba307eee2b2..5a5c23a02261 100644
>> --- a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
>> +++ b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
>> @@ -246,9 +246,9 @@ M(NPC_MCAM_READ_BASE_RULE, 0x6011,
>npc_read_base_steer_rule,            \
>>  M(NPC_MCAM_GET_STATS, 0x6012, npc_mcam_entry_stats,
>\
>>  				   npc_mcam_get_stats_req,              \
>>  				   npc_mcam_get_stats_rsp)              \
>> -M(NPC_GET_FIELD_HASH_INFO, 0x6013, npc_get_field_hash_info,
>\
>> -				   npc_get_field_hash_info_req,              \
>> -				   npc_get_field_hash_info_rsp)              \
>> +M(NPC_GET_FIELD_HASH_INFO, 0x6013, npc_get_field_hash_info,
>\
>> +				   npc_get_field_hash_info_req,         \
>> +				   npc_get_field_hash_info_rsp)         \
>>  M(NPC_GET_FIELD_STATUS, 0x6014, npc_get_field_status,
>\
>>  				   npc_get_field_status_req,              \
>>  				   npc_get_field_status_rsp)              \
>
>This hunk is a white-space only change that doesn't seem related to the
>patch description.
>
>> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c
>> b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c
>> index 6fe67f3a7f6f..a3bc53d22dc0 100644
>> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c
>> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c
>> @@ -96,7 +96,7 @@ u32 npc_field_hash_calc(u64 *ldata, struct
>npc_get_field_hash_info_rsp rsp,
>>  	field_hash = rvu_npc_toeplitz_hash(data_padded, hash_key, 128,
>159);
>>
>>  	field_hash &= FIELD_GET(GENMASK(63, 32),
>rsp.hash_ctrl[intf][hash_idx]);
>> -	field_hash += FIELD_GET(GENMASK(31, 0),
>rsp.hash_ctrl[intf][hash_idx]);
>> +	field_hash |= FIELD_GET(GENMASK(31, 0),
>> +rsp.hash_ctrl[intf][hash_idx]);
>>  	return field_hash;
>>  }
>>
>> @@ -253,7 +253,8 @@ void npc_update_field_hash(struct rvu *rvu, u8
>intf,
>>  	}
>>
>>  	req.intf = intf;
>> -	rvu_mbox_handler_npc_get_field_hash_info(rvu, &req, &rsp);
>> +	if (rvu_mbox_handler_npc_get_field_hash_info(rvu, &req, &rsp))
>> +		return;
>>
>>  	for (hash_idx = 0; hash_idx < NPC_MAX_HASH; hash_idx++) {
>>  		cfg = rvu_read64(rvu, blkaddr, NPC_AF_INTFX_HASHX_CFG(intf,
>> hash_idx)); @@ -319,9 +320,9 @@ int
>rvu_mbox_handler_npc_get_field_hash_info(struct rvu *rvu,
>>  					     struct npc_get_field_hash_info_req *req,
>>  					     struct npc_get_field_hash_info_rsp *rsp)  {
>> +	int hash_idx, hash_mask_idx, blkaddr;
>>  	u64 *secret_key = rsp->secret_key;
>>  	u8 intf = req->intf;
>> -	int i, j, blkaddr;
>>
>>  	blkaddr = rvu_get_blkaddr(rvu, BLKTYPE_NPC, 0);
>>  	if (blkaddr < 0) {
>> @@ -333,18 +334,18 @@ int
>rvu_mbox_handler_npc_get_field_hash_info(struct rvu *rvu,
>>  	secret_key[1] = rvu_read64(rvu, blkaddr,
>NPC_AF_INTFX_SECRET_KEY1(intf));
>>  	secret_key[2] = rvu_read64(rvu, blkaddr,
>> NPC_AF_INTFX_SECRET_KEY2(intf));
>>
>> -	for (i = 0; i < NPC_MAX_HASH; i++) {
>> -		for (j = 0; j < NPC_MAX_HASH_MASK; j++) {
>> -			rsp->hash_mask[NIX_INTF_RX][i][j] =
>> -				GET_KEX_LD_HASH_MASK(NIX_INTF_RX, i, j);
>> -			rsp->hash_mask[NIX_INTF_TX][i][j] =
>> -				GET_KEX_LD_HASH_MASK(NIX_INTF_TX, i, j);
>> +	for (hash_idx = 0; hash_idx < NPC_MAX_HASH; hash_idx++)
>> +		for (hash_mask_idx = 0; hash_mask_idx < NPC_MAX_HASH_MASK;
>hash_mask_idx++) {
>> +			rsp->hash_mask[NIX_INTF_RX][hash_idx][hash_mask_idx] =
>> +				GET_KEX_LD_HASH_MASK(NIX_INTF_RX, hash_idx,
>hash_mask_idx);
>> +			rsp->hash_mask[NIX_INTF_TX][hash_idx][hash_mask_idx] =
>> +				GET_KEX_LD_HASH_MASK(NIX_INTF_TX, hash_idx,
>hash_mask_idx);
>>  		}
>> -	}
>>
>> -	for (i = 0; i < NPC_MAX_INTF; i++)
>> -		for (j = 0; j < NPC_MAX_HASH; j++)
>> -			rsp->hash_ctrl[i][j] = GET_KEX_LD_HASH_CTRL(i, j);
>> +	for (hash_idx = 0; hash_idx < NPC_MAX_INTF; hash_idx++)
>> +		for (hash_mask_idx = 0; hash_mask_idx < NPC_MAX_HASH;
>hash_mask_idx++)
>> +			rsp->hash_ctrl[hash_idx][hash_mask_idx] =
>> +				GET_KEX_LD_HASH_CTRL(hash_idx, hash_mask_idx);
>>
>>  	return 0;
>>  }
>
>The three hunks above appear to change the iterator variables for the
>loops without changing functionality. This doesn't seem to match the
>patch description.
>
>> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.h
>> b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.h
>> index a1c3d987b804..eb9cb311b934 100644
>> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.h
>> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.h
>> @@ -12,9 +12,6 @@
>>  #define RVU_NPC_HASH_SECRET_KEY1 0xa9d5af4c9fbc87b4  #define
>> RVU_NPC_HASH_SECRET_KEY2 0x5954c9e7
>>
>> -#define NPC_MAX_HASH 2
>> -#define NPC_MAX_HASH_MASK 2
>> -
>
>This seems to remove duplicated #defines.
>This doesn't seem to match the patch description.
>
>>  #define KEX_LD_CFG_USE_HASH(use_hash, bytesm1, hdr_ofs, ena,
>flags_ena, key_ofs) \
>>  			    ((use_hash) << 20 | ((bytesm1) << 16) | ((hdr_ofs) <<
>8) | \
>>  			     ((ena) << 7) | ((flags_ena) << 6) | ((key_ofs) &
>0x3F)) @@
>> -41,6 +38,12 @@
>>  	rvu_write64(rvu, blkaddr,	\
>>  		    NPC_AF_INTFX_HASHX_RESULT_CTRL(intf, ld), cfg)
>>
>> +#define GET_KEX_LD_HASH_CTRL(intf, ld)  \
>> +	rvu_read64(rvu, blkaddr, NPC_AF_INTFX_HASHX_RESULT_CTRL(intf, ld))
>> +
>> +#define GET_KEX_LD_HASH_MASK(intf, ld, mask_idx)	\
>> +	rvu_read64(rvu, blkaddr, NPC_AF_INTFX_HASHX_MASKX(intf, ld,
>> +mask_idx))
>> +
>
>This seems to duplicate existing MACROS, which appear a few lines
>further above in this file.
>
>>  struct npc_mcam_kex_hash {
>>  	/* NPC_AF_INTF(0..1)_LID(0..7)_LT(0..15)_LD(0..1)_CFG */
>>  	bool
>> lid_lt_ld_hash_en[NPC_MAX_INTF][NPC_MAX_LID][NPC_MAX_LT][NPC_MAX_LD];
>> --
>> 2.25.1
>>
>>