Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[SAI]Fix build in v1.7 and remove unnecessary change #1468

Merged
merged 10 commits into from
Apr 26, 2022
Merged

[SAI]Fix build in v1.7 and remove unnecessary change #1468

merged 10 commits into from
Apr 26, 2022

Conversation

richardyu-ms
Copy link
Collaborator

@richardyu-ms richardyu-ms commented Apr 8, 2022 •
edited
Loading

In order to enable the function for Tunnel Scoped TC MAP in #1452, we
need to enable the tunnel related interfaces, attributes and method
tables.
But other changes like changes related to ECN, IPSec, and VxLan UDP source port in PR #1452, should not be needed.

Besides, some other interfaces are needed for compiling the meta in SAI.

Cause a lack of the reference to the switch tunnel object and some unnecessary interface added, there are some errors as below

ERROR: invalid enum specified 'sai_tunnel_vxlan_udp_sport_mode_t' on SAI_SWITCH_TUNNEL_ATTR_TUNNEL_VXLAN_UDP_SPORT_MODE
ERROR: invalid enum specified 'sai_tunnel_vxlan_udp_sport_mode_t' on SAI_TUNNEL_ATTR_VXLAN_UDP_SPORT_MODE
ERROR: condition value 'SAI_TUNNEL_TYPE_IPINIP_ESP' in 'SAI_TUNNEL_ATTR_TYPE == SAI_TUNNEL_TYPE_IPINIP_ESP' on SAI_TUNNEL_ATTR_SA_INDEX is not present in sai_tunnel_type_t
ERROR: condition value 'SAI_TUNNEL_TYPE_IPINIP_UDP_ESP' in 'SAI_TUNNEL_ATTR_TYPE == SAI_TUNNEL_TYPE_IPINIP_UDP_ESP' on SAI_TUNNEL_ATTR_SA_INDEX is not present in sai_tunnel_type_t
ERROR: condition value 'SAI_TUNNEL_TYPE_VXLAN_UDP_ESP' in 'SAI_TUNNEL_ATTR_TYPE == SAI_TUNNEL_TYPE_VXLAN_UDP_ESP' on SAI_TUNNEL_ATTR_SA_INDEX is not present in sai_tunnel_type_t

Some of the objects need other related objects and some of them need to be removed.

what I have done:

  • Add objects, methods, and attributes in sai headers files to satisfy build dependencies.
  • Remove unnecessary changes related to ECN, IPSec, and VxLan UDP source port

Test done:
make -C ./meta/

TODO: Add the enum check when compare with master meta

Signed-off-by: richardyu-ms richard.yu@microsoft.com

@richardyu-ms richardyu-ms force-pushed the fix-meta-build-error branch from 8dabf8d to 74ae97b Compare April 8, 2022 16:48
kcudnik
kcudnik reviewed Apr 8, 2022
View reviewed changes
inc/saitunnel.h Outdated Show resolved Hide resolved
@richardyu-ms richardyu-ms changed the title [SAI]Add tunnel related interfaces in sai [SAI]Fix build in v1.7, Add tunnel related interfaces in sai and add IPSec keywords Apr 10, 2022
@richardyu-ms richardyu-ms mentioned this pull request Apr 10, 2022
Merged
@richardyu-ms richardyu-ms requested a review from kcudnik April 10, 2022 02:00
@richardyu-ms
Copy link
Collaborator Author

richardyu-ms commented Apr 10, 2022 •
edited
Loading

@bingwang-ms @JaiOCP please help confirm the interfaces in the modification are necessary and sufficient.

itaibaz
itaibaz reviewed Apr 10, 2022
View reviewed changes
inc/saitunnel.h Outdated Show resolved Hide resolved
itaibaz
itaibaz reviewed Apr 10, 2022
View reviewed changes
meta/aspell.en.pws Outdated Show resolved Hide resolved
@richardyu-ms richardyu-ms requested a review from itaibaz April 10, 2022 07:51
itaibaz
itaibaz reviewed Apr 10, 2022
View reviewed changes
inc/saitunnel.h Outdated Show resolved Hide resolved
itaibaz
itaibaz reviewed Apr 10, 2022
View reviewed changes
inc/saitunnel.h Outdated Show resolved Hide resolved
@richardyu-ms richardyu-ms changed the title [SAI]Fix build in v1.7, Add tunnel related interfaces in sai and add IPSec keywords [SAI]Fix build in v1.7 and remove unnecessary change Apr 10, 2022
@richardyu-ms richardyu-ms requested a review from itaibaz April 10, 2022 15:31
@bingwang-ms
Copy link
Contributor

Thanks for the fix. @richardyu-ms

@richardyu-ms richardyu-ms requested a review from kcudnik April 11, 2022 06:33
@richardyu-ms richardyu-ms force-pushed the fix-meta-build-error branch 7 times, most recently from 7070203 to 9520b1f Compare April 11, 2022 06:50
@richardyu-ms richardyu-ms force-pushed the fix-meta-build-error branch from 5df7d3b to 83fb5a2 Compare April 11, 2022 08:38
@JaiOCP
Copy link
Contributor

JaiOCP commented Apr 11, 2022

Looks good to me

@JaiOCP
Copy link
Contributor

JaiOCP commented Apr 11, 2022

@bingwang-ms @JaiOCP please help confirm the interfaces in the modification are necessary and sufficient.

I do not see the review button on this PR. Don't know why.
But changes looks good to me.

lguohan
lguohan reviewed Apr 11, 2022
View reviewed changes
inc/saitunnel.h Outdated Show resolved Hide resolved
@richardyu-ms richardyu-ms requested a review from lguohan April 12, 2022 04:40
@richardyu-ms
In order to keep the enum value compatiable with latter version, add …
2e3af6f 
…vxlan source port and ECN

Signed-off-by: richardyu-ms <richard.yu@microsoft.com>
@richardyu-ms richardyu-ms force-pushed the fix-meta-build-error branch from 9422fec to 2e3af6f Compare April 12, 2022 04:45
@richardyu-ms
Set enum values
003c080 
Signed-off-by: richardyu-ms <richard.yu@microsoft.com>
richardyu-ms
richardyu-ms commented Apr 13, 2022
View reviewed changes
inc/saiswitch.h Outdated Show resolved Hide resolved
richardyu-ms
richardyu-ms commented Apr 13, 2022
View reviewed changes
inc/saitunnel.h Outdated Show resolved Hide resolved
richardyu-ms
richardyu-ms commented Apr 13, 2022
View reviewed changes
inc/saiswitch.h Outdated Show resolved Hide resolved
richardyu-ms
richardyu-ms commented Apr 13, 2022
View reviewed changes
inc/saiswitch.h Outdated Show resolved Hide resolved
@richardyu-ms
add test to make sure the value of enum is right
d0f1723 
Signed-off-by: richardyu-ms <richard.yu@microsoft.com>
@richardyu-ms richardyu-ms force-pushed the fix-meta-build-error branch 2 times, most recently from 55ed0dd to eecedb7 Compare April 15, 2022 02:48
@richardyu-ms
refactor the testing code
42d766b 
Signed-off-by: richardyu-ms <richard.yu@microsoft.com>
@richardyu-ms richardyu-ms force-pushed the fix-meta-build-error branch from eecedb7 to 42d766b Compare April 15, 2022 02:59
kcudnik
kcudnik requested changes Apr 15, 2022
View reviewed changes
inc/saitunnel.h Outdated Show resolved Hide resolved
meta/test.pm Outdated Show resolved Hide resolved
@richardyu-ms
remove a test method and remove a unused enum which might cause enum …
131280c 
…inconsistence between versions

Signed-off-by: richardyu-ms <richard.yu@microsoft.com>
@richardyu-ms richardyu-ms force-pushed the fix-meta-build-error branch from 0d8da2d to 131280c Compare April 22, 2022 06:36
@richardyu-ms richardyu-ms requested a review from kcudnik April 23, 2022 02:35
@richardyu-ms
remove the enum values which might cause value holes
9066cdf 
Signed-off-by: richardyu-ms <richard.yu@microsoft.com>
@richardyu-ms richardyu-ms force-pushed the fix-meta-build-error branch from b2ef4de to 9066cdf Compare April 23, 2022 02:43
@richardyu-ms richardyu-ms merged commit c460675 into opencomputeproject:v1.7 Apr 26, 2022
@richardyu-ms richardyu-ms deleted the fix-meta-build-error branch July 3, 2022 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lguohan lguohan lguohan approved these changes

@itaibaz itaibaz itaibaz approved these changes

@kcudnik kcudnik kcudnik approved these changes

@bingwang-ms bingwang-ms bingwang-ms approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@richardyu-ms @bingwang-ms @JaiOCP @lguohan @itaibaz @kcudnik