[SAI submodule] Update SAI to 1.7.1 by vmittal-msft · Pull Request #748 · sonic-net/sonic-sairedis

vmittal-msft · 2020-12-15T19:23:37Z

No description provided.

kcudnik · 2020-12-15T19:30:58Z

what about this one #733?

lib/src/sai_redis_hash.cpp

vslib/src/sai_vs_hash.cpp

vmittal-msft · 2020-12-15T20:59:42Z

retest vs please

lib/src/sai_redis_hash.cpp

vmittal-msft · 2020-12-16T18:50:24Z

retest vs please

kcudnik · 2020-12-16T19:53:17Z

please fix compilation issues:
../../lib/inc/sai_redis.h:73:32: error: 'SAI_OBJECT_TYPE_FINE_GRAINED_HASH' was not declared in this scope
compile locally before submitting
object type is called "FINE_GRAINED_HASH_FIELD"

…sonic-sairedis into sairedis_sai_1.7.1

vmittal-msft · 2020-12-17T05:40:59Z

retest vs please

kcudnik · 2020-12-17T12:12:31Z

vslib/src/SwitchStateBaseMACsec.cpp

    macsecAttr.m_an = attr->value.u8;

-    SAI_METADATA_GET_ATTR_BY_ID(attr, SAI_MACSEC_SA_ATTR_ENCRYPTION_ENABLE, attrCount, attrList);
+    SAI_METADATA_GET_ATTR_BY_ID(attr, SAI_MACSEC_SC_ATTR_ENCRYPTION_ENABLE, attrCount, attrList);


why this is changed? even if wrong, this code is not used, and did you tested this whether this will work ?

@Pterosaur, can you look at those changes ?

There was compilation error due to this macro hence i corrected it.

this function is called loadMACsecAttrFromMACsecSA, so it's getting "SA" attributes, but you changed this to "SC" attribute, how come this was compiling before ?

those are 2 different object types, and this is SA object, and now you query SC attribute

Commented the code and added SWSS_LOG_THROW to generate exception for author to fix it in future as per discussion.

sure, just add comments for other 2 attributes you changed

kcudnik · 2020-12-17T12:14:01Z

retest vs please

kcudnik · 2020-12-17T17:23:57Z

vslib/src/SwitchStateBaseMACsec.cpp

    macsecAttr.m_an = attr->value.u8;

-    SAI_METADATA_GET_ATTR_BY_ID(attr, SAI_MACSEC_SA_ATTR_ENCRYPTION_ENABLE, attrCount, attrList);
+    SAI_METADATA_GET_ATTR_BY_ID(attr, SAI_MACSEC_SC_ATTR_ENCRYPTION_ENABLE, attrCount, attrList);


this function is called loadMACsecAttrFromMACsecSA, so it's getting "SA" attributes, but you changed this to "SC" attribute, how come this was compiling before ?

…sonic-sairedis into sairedis_sai_1.7.1

vslib/src/SwitchStateBaseMACsec.cpp

Fix for vslib

vslib/src/SwitchStateBaseMACsec.cpp

…sonic-sairedis into sairedis_sai_1.7.1

daall · 2020-12-18T00:12:28Z

It looks like the same set of tests are failing consistently and all of them seem to be related to next hop groups.

I took a look at the logs from one of the failures (https://sonic-jenkins.westus2.cloudapp.azure.com/job/vs/job/sonic-sairedis-build-pr/845/artifact/swss/tests/log/test_acl_portchannel/log/) and found this:

Dec 17 13:55:20.029414 3ea5d8b82340 NOTICE #orchagent: :- addNextHopGroup: Create next hop group 40.0.0.1@PortChannel001,40.0.0.3@PortChannel002,40.0.0.5@PortChannel003,40.0.0.7@PortChannel004
Dec 17 13:55:20.029444 3ea5d8b82340 ERR #orchagent: :- bulkCreate: mode vlaue 0 is not in range on sai_stats_mode_t
Dec 17 13:55:20.029451 3ea5d8b82340 ERR #orchagent: :- addNextHopGroup: Failed to create next hop group 50000000005e8 member 0: 0

I see similar messages for the other tests that are failing as well.

kcudnik · 2020-12-18T00:39:56Z

there is a bug in metadata where sai_metadata_enum_sai_stats_mode_t is used instead of sai_metadata_enum_sai_bulk_op_error_mode_t, and i will fix that, but coincidently, value 0 is in both of those, that is interesting fail :D this is interesting, since this was working with bulk operations from 201811 branch to master which i recently checked with @shi-su and no issues on that mode
this bulkCreate: mode vlaue 0 is not in range on sai_stats_mode_t it's like medatada was corrupted really strange, if you have a consistent repro on that it would be good to put some more logs in case where this happens

fixing here: #753, but it can produce same error as above, but i dont have idea why

Signed-off-by: kcudnik <[email protected]>

lguohan · 2020-12-18T06:15:44Z

retest this please

…sonic-sairedis into sairedis_sai_1.7.1-tmp

smaheshm · 2020-12-18T17:39:21Z

vslib/src/SwitchStateBaseMACsec.cpp

+    //SAI_METADATA_GET_ATTR_BY_ID(attr, SAI_MACSEC_SA_ATTR_ENCRYPTION_ENABLE, attrCount, attrList);
+    //macsecAttr.m_encryptionEnable = attr->value.booldata;


Prefer not to have commented code.

you are welcome to jump in and make a proper fix for this issue if you are knowledgeable of macsec

kcudnik requested changes Dec 15, 2020

View reviewed changes

lib/src/sai_redis_hash.cpp Outdated Show resolved Hide resolved

vslib/src/sai_vs_hash.cpp Outdated Show resolved Hide resolved

vmittal-msft mentioned this pull request Dec 15, 2020

[sonic-sairedis submodule] Updated SAI header 1.7.1 for BRCM and Mellanox SDK/SAI sonic-net/sonic-buildimage#6218

Merged

3 tasks

vmittal-msft force-pushed the sairedis_sai_1.7.1 branch from 48efe7e to 6ae8503 Compare December 15, 2020 23:32

kcudnik reviewed Dec 16, 2020

View reviewed changes

lib/src/sai_redis_hash.cpp Outdated Show resolved Hide resolved

kcudnik requested changes Dec 16, 2020

View reviewed changes

lib/src/sai_redis_hash.cpp Outdated Show resolved Hide resolved

vmittal-msft force-pushed the sairedis_sai_1.7.1 branch 2 times, most recently from 0111156 to e687dfd Compare December 16, 2020 17:48

Update SAI to 1.7.1

f4040f4

vmittal-msft force-pushed the sairedis_sai_1.7.1 branch from e687dfd to f4040f4 Compare December 16, 2020 18:14

kcudnik previously approved these changes Dec 16, 2020

View reviewed changes

vmittal-msft added 2 commits December 16, 2020 20:57

Update SAI to 1.7.1

45f9b40

Merge branch 'sairedis_sai_1.7.1' of https://github.com/vmittal-msft/…

4258790

…sonic-sairedis into sairedis_sai_1.7.1

vmittal-msft dismissed kcudnik’s stale review via 4258790 December 16, 2020 21:42

Fix for vslib

f3ac59b

kcudnik reviewed Dec 17, 2020

View reviewed changes

kcudnik mentioned this pull request Dec 17, 2020

[SAI] Update to v1.6.6 #733

Closed

kcudnik requested changes Dec 17, 2020

View reviewed changes

vmittal-msft added 2 commits December 17, 2020 18:50

Fix for vslib

0a49316

Merge branch 'sairedis_sai_1.7.1' of https://github.com/vmittal-msft/…

64cf87a

…sonic-sairedis into sairedis_sai_1.7.1

kcudnik reviewed Dec 17, 2020

View reviewed changes

vslib/src/SwitchStateBaseMACsec.cpp Outdated Show resolved Hide resolved

kcudnik reviewed Dec 17, 2020

View reviewed changes

vslib/src/SwitchStateBaseMACsec.cpp Outdated Show resolved Hide resolved

Update SAI to 1.7.1

ef6684f

Fix for vslib

vmittal-msft force-pushed the sairedis_sai_1.7.1 branch from 64cf87a to ef6684f Compare December 17, 2020 20:04

kcudnik reviewed Dec 17, 2020

View reviewed changes

vslib/src/SwitchStateBaseMACsec.cpp Outdated Show resolved Hide resolved

vmittal-msft added 2 commits December 17, 2020 20:58

Merge branch 'sairedis_sai_1.7.1' of https://github.com/vmittal-msft/…

25f7aac

…sonic-sairedis into sairedis_sai_1.7.1

Fixed comment.

9a19098

kcudnik previously approved these changes Dec 17, 2020

View reviewed changes

[meta] Fix stat_mode enums to sai_bulk_op_error_mode_t (sonic-net#753)

ac27701

Signed-off-by: kcudnik <[email protected]>

vmittal-msft added 2 commits December 18, 2020 07:59

Update SAI to 1.7.1

4fbb3ab

Merge branch 'sairedis_sai_1.7.1' of https://github.com/vmittal-msft/…

7f494a1

…sonic-sairedis into sairedis_sai_1.7.1-tmp

vmittal-msft dismissed kcudnik’s stale review via 7f494a1 December 18, 2020 08:04

kcudnik approved these changes Dec 18, 2020

View reviewed changes

kcudnik merged commit 7096989 into sonic-net:master Dec 18, 2020

vmittal-msft deleted the sairedis_sai_1.7.1 branch December 18, 2020 16:05

smaheshm reviewed Dec 18, 2020

View reviewed changes

lguohan pushed a commit that referenced this pull request Dec 19, 2020

[SAI submodule] Update SAI to 1.7.1 (#748)

36e912b

pettershao-ragilenetworks pushed a commit to pettershao-ragilenetworks/sonic-sairedis that referenced this pull request Nov 18, 2022

[SAI submodule] Update SAI to 1.7.1 (sonic-net#748)

2537bca

		//SAI_METADATA_GET_ATTR_BY_ID(attr, SAI_MACSEC_SA_ATTR_ENCRYPTION_ENABLE, attrCount, attrList);
		//macsecAttr.m_encryptionEnable = attr->value.booldata;

Conversation

vmittal-msft commented Dec 15, 2020

Uh oh!

kcudnik commented Dec 15, 2020

Uh oh!

Uh oh!

Uh oh!

vmittal-msft commented Dec 15, 2020

Uh oh!

Uh oh!

Uh oh!

vmittal-msft commented Dec 16, 2020

Uh oh!

kcudnik commented Dec 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vmittal-msft commented Dec 17, 2020

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!

kcudnik commented Dec 17, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

daall commented Dec 18, 2020

Uh oh!

kcudnik commented Dec 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lguohan commented Dec 18, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

kcudnik commented Dec 16, 2020 •

edited

Loading

kcudnik commented Dec 18, 2020 •

edited

Loading