Hashing Enhancements for Efficient RoCE Traffic Distribution by sathk-marvell · Pull Request #2144 · opencomputeproject/SAI

sathk-marvell · 2025-02-27T11:20:13Z

This Feature adds New Hash fields to support hashing based on RDMA fields

kcudnik · 2025-02-27T16:13:19Z

/azp run

azure-pipelines · 2025-02-27T16:13:28Z

Azure Pipelines successfully started running 1 pipeline(s).

kcudnik · 2025-02-27T16:13:53Z

+
    /** No field - for compatibility, must be last */
-    SAI_NATIVE_HASH_FIELD_NONE = 0x00000021,
+    SAI_NATIVE_HASH_FIELD_NONE = 0x00000023,


this is not backward compatible change and you are adding exception for that, please remove

Modified this based on comment on Line No:178. This might change in future even with new hash-field additions.

Can we deprecate this enum definition and start adding new fields after this.? will add a comment saying not a valid field used only for backward compatibility

bring it back to expected value and remove exception in ancestry, NONE is none, no mather what value it has binary

@kcudnik, @sathk-marvell - this field is still causing meta checker to fail. Please check

kcudnik · 2025-02-27T16:15:12Z

            next if $enumName eq "SAI_OBJECT_TYPE_MAX";
            next if $enumName eq "SAI_PORT_INTERFACE_TYPE_MAX";
            next if $enumName eq "SAI_PORT_BREAKOUT_MODE_TYPE_MAX";
+            next if $enumName eq "SAI_NATIVE_HASH_FIELD_NONE";


adding exception is not backward compatible, please revert

For modules with last attribute != XXX_END, we need this. tried all combinations, meta always failing

This only applies to attributes enum, previously last enum was flow lubelskie and it was passing, please remote those exceptions as semestr like the error maybe in diferent place

tjchadaga · 2025-03-06T22:23:03Z

/azp run

azure-pipelines · 2025-03-06T22:23:11Z

Azure Pipelines successfully started running 1 pipeline(s).

tjchadaga · 2025-03-06T22:23:28Z

@vmittal-msft - FYI

tjchadaga · 2025-03-06T22:24:47Z

@JaiOCP - could you please help review?

tjchadaga · 2025-03-12T04:32:11Z

/azp run

azure-pipelines · 2025-03-12T04:32:19Z

Azure Pipelines successfully started running 1 pipeline(s).

tjchadaga · 2025-03-12T20:22:24Z

/azp run

azure-pipelines · 2025-03-12T20:22:32Z

Azure Pipelines successfully started running 1 pipeline(s).

tjchadaga · 2025-03-13T05:36:03Z

@kcudnik - could you please help check why the meta checker is failing for this?

kcudnik · 2025-03-13T12:00:55Z

processing commit 9f5fc22
ERROR: check ! SAI_NATIVE_HASH_FIELD_NONE value is 0x00000022, but on was 0x00000021 on commit 3132018
WARNING: Both enums have the same value SAI_NATIVE_HASH_FIELD_RDMA_BTH_DEST_QP and SAI_NATIVE_HASH_FIELD_NONE = 0x00000021
processing commit 7df1c94
ERROR: check ! SAI_NATIVE_HASH_FIELD_NONE value is 0x00000021, but on was 0x00000022 on commit 9f5fc22
processing commit 00ecb4d
ERROR: check ! SAI_NATIVE_HASH_FIELD_NONE value is 0x00000022, but on was 0x00000021 on commit 7df1c94
ERROR: please correct all 3 error(s) and all 1 warnings before continue
make: *** [Makefile:89: all] Error 1

you need to squash your commits, and force push again, since you changed NONE field and shifted enum values

sathk-marvell · 2025-03-13T12:39:59Z

@kcudnik still seeing the same issue. Can i add below diff..

builder@sonic-skarra:~/SAI$ git diff
diff --git a/meta/ancestry.pl b/meta/ancestry.pl
index 29f6115..1e002f7 100755
--- a/meta/ancestry.pl
+++ b/meta/ancestry.pl
@@ -178,6 +178,7 @@ sub BuildCommitHistory
next if $enumName eq "SAI_OBJECT_TYPE_MAX";
next if $enumName eq "SAI_PORT_INTERFACE_TYPE_MAX";
next if $enumName eq "SAI_PORT_BREAKOUT_MODE_TYPE_MAX";

       next if $enumName eq "SAI_NATIVE_HASH_FIELD_NONE";

       LogError "wrong initializer on $enumName $enumValue" if not $enumValue =~ /^0x[0-9a-f]{8}$/;

@@ -209,6 +210,7 @@ sub BuildCommitHistory
#print "elsif (defined $enumName $IGNORED{$enumName} and $IGNORED{$enumName} eq $HISTORY{$enumTypeName}{$enumName}{name})";

                 next if $HISTORY{$enumTypeName}{$enumValue} eq "SAI_PORT_BREAKOUT_MODE_TYPE_MAX";

               next if $HISTORY{$enumTypeName}{$enumValue} eq "SAI_NATIVE_HASH_FIELD_NONE";
               LogWarning "Both enums have the same value $enumName and $HISTORY{$enumTypeName}{$enumValue} = $enumValue";
           }
       }

kcudnik · 2025-03-13T18:55:18Z

+    /** Native hash field RDMA packet BTH(Base Transport Header) opcode */
+    SAI_NATIVE_HASH_FIELD_RDMA_BTH_OPCODE = 0x00000022,
+
+    /** Native hash field RDMA packet BTH destination queue pair */
+    SAI_NATIVE_HASH_FIELD_RDMA_BTH_DEST_QP = 0x00000023,
+


place those after NONE

why is this set to 0x22 & 0x23? are enum values 0x19, 0x1a, 0x1b .. 0x20 not usable?

placed after NONE only. these values are already in use, but not in order

/**
* @brief Native hash field destination IPv6
*
* Also, refers to the outer source IPv6
* in case for encapsulated packets
*/
SAI_NATIVE_HASH_FIELD_DST_IPV6 = 0x0000001c,

/** Native hash field inner source IPv4 */ SAI_NATIVE_HASH_FIELD_INNER_SRC_IPV4 = 0x0000001d, /** Native hash field inner destination IPv4 */ SAI_NATIVE_HASH_FIELD_INNER_DST_IPV4 = 0x0000001e, /** Native hash field inner source IPv6 */ SAI_NATIVE_HASH_FIELD_INNER_SRC_IPV6 = 0x0000001f, /** Native hash field inner destination IPv6 */ SAI_NATIVE_HASH_FIELD_INNER_DST_IPV6 = 0x00000020,

kcudnik · 2025-03-13T18:56:51Z

is your change rebased to top of masteR ?

sathk-marvell · 2025-03-14T01:25:44Z

is your change rebased to top of masteR ?

yes

kcudnik · 2025-03-14T07:35:29Z

/azp run

azure-pipelines · 2025-03-14T07:35:37Z

Azure Pipelines successfully started running 1 pipeline(s).

kcudnik · 2025-03-14T07:54:27Z

all passed now :)

eddyk-nvidia · 2025-03-19T20:11:20Z

+
+- Queue Pair (QP) Number
+- RDMA opcode(Operation type)
+


It is in addition to the option to use it via UDF ?

Yes, this will allow the users to use native H/w support

kcudnik · 2025-03-20T06:20:08Z

            next if $enumName eq "SAI_OBJECT_TYPE_MAX";
            next if $enumName eq "SAI_PORT_INTERFACE_TYPE_MAX";
            next if $enumName eq "SAI_PORT_BREAKOUT_MODE_TYPE_MAX";
+            next if $enumName eq "SAI_NATIVE_HASH_FIELD_NONE";


This only applies to attributes enum, previously last enum was flow lubelskie and it was passing, please remote those exceptions as semestr like the error maybe in diferent place

sathk-marvell · 2025-03-20T07:19:30Z

seeing below errors when i remove that line:
ERROR: check ! SAI_NATIVE_HASH_FIELD_NONE value is 0x00000022, but on was 0x00000021 on commit 3132018
ERROR: please correct all 1 error(s) and all 0 warnings before continue
Makefile:85: recipe for target 'all' failed
make: *** [all] Error 1
make: Leaving directory '/home/builder/SAI/meta'

kcudnik · 2025-03-20T17:19:17Z

did you rebased your PR to SAI master ?

kcudnik · 2025-03-20T17:20:46Z

you should squash your changes and rebase to SAI/master, then this error will go away

tjchadaga · 2025-03-21T06:04:57Z

@sathk-marvell - could you please resolve conflicts?

sathk-marvell · 2025-03-21T15:09:21Z

@sathk-marvell - could you please resolve conflicts?

Done

sathk-marvell · 2025-03-21T15:14:30Z

you should squash your changes and rebase to SAI/master, then this error will go away

tried that.

Reset to Head to before my commit
do the pull to latest master and clean build: Build is successful
applied my patches again w/o above line
Tried building, seeing same failure again
added that Line:build went through
Pull to latest master and tried rebuild: Build is working w/o any issues

tjchadaga · 2025-03-21T16:31:22Z

/azp run

azure-pipelines · 2025-03-21T16:31:31Z

Azure Pipelines successfully started running 1 pipeline(s).

kcudnik · 2025-03-21T18:45:02Z

you should squash your changes and rebase to SAI/master, then this error will go away

tried that.

Reset to Head to before my commit

do the pull to latest master and clean build: Build is successful

applied my patches again w/o above line

Tried building, seeing same failure again

added that Line:build went through

Pull to latest master and tried rebuild: Build is working w/o any issues

there is a limitation in validator, which is not able to handle enums added in the middle of the enum, please put your enums after NONE, and it will PASS, and remove those exceptions

kcudnik · 2025-03-21T19:40:04Z

i will try to fix this issue

kcudnik · 2025-03-21T20:09:05Z

@sathk-marvell i fixed it here: #2157, please remove exceptions from ancestry, and you can now put your flags before NONE field, enjoy

Signed-off-by: Satheesh Kumar Karra <skarra@marvell.com>

sathk-marvell · 2025-03-22T03:16:42Z

@sathk-marvell i fixed it here: #2157, please remove exceptions from ancestry, and you can now put your flags before NONE field, enjoy

Update the review now, Thanks a lot @kcudnik for all your timely help

kcudnik · 2025-03-22T11:17:23Z

/azp run

azure-pipelines · 2025-03-22T11:17:32Z

Azure Pipelines successfully started running 1 pipeline(s).

kcudnik requested changes Feb 27, 2025

View reviewed changes

sathk-marvell force-pushed the sathk_hash_rdma branch from 8f628fb to 25122d8 Compare March 6, 2025 07:32

kcudnik approved these changes Mar 6, 2025

View reviewed changes

tjchadaga requested a review from abdosi March 6, 2025 22:22

tjchadaga added the reviewed PR is discussed in SAI Meeting label Mar 6, 2025

sathk-marvell force-pushed the sathk_hash_rdma branch 3 times, most recently from e739d0d to 9f5fc22 Compare March 12, 2025 09:02

kcudnik reviewed Mar 13, 2025

View reviewed changes

sathk-marvell force-pushed the sathk_hash_rdma branch from 9f5fc22 to 8a99a47 Compare March 14, 2025 07:21

eddyk-nvidia reviewed Mar 19, 2025

View reviewed changes

kcudnik requested changes Mar 20, 2025

View reviewed changes

eddyk-nvidia approved these changes Mar 20, 2025

View reviewed changes

tjchadaga requested a review from kcudnik March 21, 2025 05:43

sathk-marvell force-pushed the sathk_hash_rdma branch 2 times, most recently from e6d7876 to 035a3a1 Compare March 21, 2025 15:08

Adding RDMA fields to Hash Object

bcb98fa

Signed-off-by: Satheesh Kumar Karra <skarra@marvell.com>

sathk-marvell force-pushed the sathk_hash_rdma branch from 035a3a1 to bcb98fa Compare March 22, 2025 03:15

tjchadaga approved these changes Mar 22, 2025

View reviewed changes

kcudnik approved these changes Mar 24, 2025

View reviewed changes

tjchadaga merged commit 28751b9 into opencomputeproject:master Mar 24, 2025
3 checks passed

Conversation

sathk-marvell commented Feb 27, 2025

Uh oh!

kcudnik commented Feb 27, 2025

Uh oh!

azure-pipelines Bot commented Feb 27, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tjchadaga commented Mar 6, 2025

Uh oh!

azure-pipelines Bot commented Mar 6, 2025

Uh oh!

tjchadaga commented Mar 6, 2025

Uh oh!

tjchadaga commented Mar 6, 2025

Uh oh!

tjchadaga commented Mar 12, 2025

Uh oh!

azure-pipelines Bot commented Mar 12, 2025

Uh oh!

tjchadaga commented Mar 12, 2025

Uh oh!

azure-pipelines Bot commented Mar 12, 2025

Uh oh!

tjchadaga commented Mar 13, 2025

Uh oh!

kcudnik commented Mar 13, 2025

Uh oh!

sathk-marvell commented Mar 13, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kcudnik commented Mar 13, 2025

Uh oh!

sathk-marvell commented Mar 14, 2025

Uh oh!

kcudnik commented Mar 14, 2025

Uh oh!

azure-pipelines Bot commented Mar 14, 2025

Uh oh!

kcudnik commented Mar 14, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sathk-marvell commented Mar 20, 2025

Uh oh!

kcudnik commented Mar 20, 2025

Uh oh!

kcudnik commented Mar 20, 2025

Uh oh!

tjchadaga commented Mar 21, 2025

Uh oh!

sathk-marvell commented Mar 21, 2025

Uh oh!

sathk-marvell commented Mar 21, 2025

Uh oh!

tjchadaga commented Mar 21, 2025

Uh oh!

azure-pipelines Bot commented Mar 21, 2025

kcudnik commented Mar 21, 2025 •

edited

Loading