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.

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

[saisubmodule] Update SAI submodule #1409

Merged
merged 4 commits into from
Sep 6, 2024

Conversation

jimmyzhai
Copy link
Contributor

@jimmyzhai jimmyzhai commented Aug 7, 2024

Update SAI submodule to include opencomputeproject/SAI#2026, which updates outbound_routing_entry. It updates serialization/deserialization, test, etc for outbound_routing_entry.

Includes the following SAI commits:

2024-07-23 684a8ee Mukesh Moopath Velayudhan: Add DPU acronym (#2053)
2024-07-18 7455ddf mikeberesford: Add port attribute to get the max debug data size. (#2033)
2024-07-15 dff0e34 Riff: Update DASH SAI API comments (#2050)
2024-07-15 8e74c55 Riff: Update DASH pipeline and HA related counters. (#2051)
2024-07-14 d15eca7 Kamil Cudnik: [meta][sai] Move extensions SAI and API to 0x20000000 range (#2028)
2024-07-10 19b30c1 chikkaiah-work: SAI proposal for icmp echo offload (#2020)
2024-06-27 18ba20f Marian Pritsak: [DASH] Add Routing Group API (#2026)
2024-06-25 abc8f02 Marian Pritsak: [DASH] Add missing prefixes to PL mapping (#2035)

@kperumalbfn
Copy link

@jimmyzhai Please update the PR description about adding serialize/deserialize change.

@@ -118,6 +118,7 @@ namespace syncd
.on_switch_asic_sdk_health_event = &Slot<context>::onSwitchAsicSdkHealthEvent,
.on_port_host_tx_ready = &Slot<context>::onPortHostTxReady,
.on_twamp_session_event = &Slot<context>::onTwampSessionEvent,
.on_icmp_echo_session_state_change = nullptr,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any other changes required due to SAI refpoint update?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any other changes required due to SAI refpoint update?

No. Later let icmp echo feature owner implement its detail notification callback.

@theasianpianist
Copy link
Contributor

Just FYI I have a PR that updates the submodule + also updates sairedis code for the route group API changes #1410

@jimmyzhai
Copy link
Contributor Author

@jimmyzhai Please update the PR description about adding serialize/deserialize change.

done

@prabhataravind
Copy link
Contributor

@jimmyzhai, there are some more changes in public sairedis repo that includes metering related changes. Could you please help include those as well in this PR?

@kcudnik
Copy link
Collaborator

kcudnik commented Aug 16, 2024

Please fix build errors

@jimmyzhai
Copy link
Contributor Author

@jimmyzhai, there are some more changes in public sairedis repo that includes metering related changes. Could you please help include those as well in this PR?

Had tried that, but PR opencomputeproject/SAI#2056 for metering changes caused other sairedis build failure, which is not easy to handle in this PR.

@jimmyzhai
Copy link
Contributor Author

Please fix build errors

It depends on sonic-net/sonic-swss#3048, which has fix for outbound_routing_entry changes in swss. The sairedis PR won't pass without the swss change, but the swss PR also won't pass without the sairedis PR. It's a sort of circular dependency.

After a discussion with Lawrence(theasianpianist), he had proposed the below solution:

  1. I will open my swss PR when it is ready, and also change the swss PR tests to use the artifacts from Junhua's submodule update PR
  2. Assuming that the PR tests pass with these changes, force merge Junhua's sairedis PR, then my swss PR.
  3. Finally submit another swss PR to change the PR test artifacts back to the latest build

@saiarcot895 saiarcot895 merged commit dbcba69 into sonic-net:master Sep 6, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants