[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <SJ0PR18MB52168D1146609692E01FC2F9DB3FA@SJ0PR18MB5216.namprd18.prod.outlook.com>
Date: Fri, 21 Jul 2023 05:43:29 +0000
From: Suman Ghosh <sumang@...vell.com>
To: Simon Horman <simon.horman@...igine.com>
CC: Sunil Kovvuri Goutham <sgoutham@...vell.com>,
Geethasowjanya Akula <gakula@...vell.com>,
Subbaraya Sundeep Bhatta <sbhatta@...vell.com>,
Hariprasad Kelam <hkelam@...vell.com>,
"davem@...emloft.net" <davem@...emloft.net>,
"edumazet@...gle.com" <edumazet@...gle.com>,
"kuba@...nel.org" <kuba@...nel.org>,
"pabeni@...hat.com" <pabeni@...hat.com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [EXT] Re: [net PATCH V3 1/2] octeontx2-af: Fix hash extraction
mbox message
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@...vell.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
>>
>>
Powered by blists - more mailing lists