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

[device]: Add SAI checksum verify to TD3 config #8857

Merged
merged 5 commits into from
Jul 8, 2022

Conversation

theasianpianist
Copy link
Contributor

Signed-off-by: Lawrence Lee [email protected]

Why I did it

A new config option was added to control the value of IPV4_INCR_CHECKSUM_ORIGINAL_VALUE_VERIFY in the EGR_FLEX_CONFIG control register (this prevents checksums of 0xffff from being propagated to other devices)

How I did it

How to verify it

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106

Description for the changelog

A picture of a cute animal (not mandatory but encouraged)

gechiang
gechiang previously approved these changes Sep 30, 2021
Copy link
Collaborator

@gechiang gechiang left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@theasianpianist
Copy link
Contributor Author

/Azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@gechiang
Copy link
Collaborator

@theasianpianist Please note that the necessary changes in BRCM SAI is only available in 202012 BRCM SAI 4.3.5.1-3. This means that this PR ideally should wait until the same fix is applied to the master branch BRCM SAI 5.0 before you merge it...
I would suggest that you raise a separate PR for the 202012 branch with this same change so that the 202012 branch which is ready with the BRCM SAI support can start getting this complete fix.

@gechiang
Copy link
Collaborator

gechiang commented Sep 30, 2021

@theasianpianist you should also press BRCM to make sure they push this same BRCM SAI change into their REL_5.0 SUG repo and any future release that they are also developing such as 6.0. They miss this type of things quite often and needs to be reminded unfortunately or else there will be a surprised gap...

@theasianpianist theasianpianist changed the base branch from master to 202012 September 30, 2021 23:58
@theasianpianist theasianpianist changed the base branch from 202012 to master September 30, 2021 23:59
@theasianpianist theasianpianist dismissed gechiang’s stale review September 30, 2021 23:59

The base branch was changed.

@theasianpianist theasianpianist requested a review from a team as a code owner June 7, 2022 17:17
@liushilongbuaa
Copy link
Contributor

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@gechiang
Copy link
Collaborator

@theasianpianist , Should this SOC property be applied to all TD3 based HW SKUs or just the two you updated?

@theasianpianist
Copy link
Contributor Author

@theasianpianist , Should this SOC property be applied to all TD3 based HW SKUs or just the two you updated?

Checked offline with @lguohan, we only need to update the 7050CX3 SKUs

@prsunny prsunny merged commit 1b7fcb4 into sonic-net:master Jul 8, 2022
yxieca pushed a commit that referenced this pull request Jul 17, 2022
* [device]: Add SAI checksum verify to TD3 config
* A new config option was added to control the value of IPV4_INCR_CHECKSUM_ORIGINAL_VALUE_VERIFY in the EGR_FLEX_CONFIG control register (this prevents checksums of 0xffff from being propagated to other devices)
skbarista pushed a commit to skbarista/sonic-buildimage that referenced this pull request Aug 17, 2022
* [device]: Add SAI checksum verify to TD3 config
* A new config option was added to control the value of IPV4_INCR_CHECKSUM_ORIGINAL_VALUE_VERIFY in the EGR_FLEX_CONFIG control register (this prevents checksums of 0xffff from being propagated to other devices)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants