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

Fix ecmp hash polarization by enable hash seed/offset config on Broadcom T1 #19454

Closed
wants to merge 1 commit into from

Conversation

Gfrom2016
Copy link
Contributor

@Gfrom2016 Gfrom2016 commented Jul 2, 2024

Why I did it

To fix ecmp hash polarization issue on 202311 image.

Work item tracking
  • Microsoft ADO (number only): 28601762

How I did it

Braodcom's fix for ecmp hash polarization has been included in 10.1.4.0 in #18044
Cherry-pick image fix from #17505
Add sai_hash_seed_config_hash_offset_enable=1 in all config.bcm that Broadcom T1 uses.

HardwareSku
Force10-S6100-T1
Force10-S6100-ITPAC-T1
Force10-S6100
Celestica-DX010-C32
Arista-7260CX3-C64
Arista-7060CX-32S-Q32
Arista-7060CX-32S-C32-T1
Arista-7060CX-32S-C32
Arista-7050QX32S-Q32
Arista-7050QX-32S-S4Q31
Arista-7050-QX32
Arista-7050-QX-32S

How to verify it

Run ecmp/test_ecmp_sai_values.py on 7260 T1 DUT and passed.
https://dev.azure.com/mssonic/internal/_build/results?buildId=586225&view=results

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

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205
  • 202211
  • 202305

Tested branch (Please provide the tested image version)

Description for the changelog

Link to config_db schema for YANG module changes

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

Why I did it
To fix ecmp hash polarization issue.

How I did it
Add sai_hash_seed_config_hash_offset_enable=1 in all config.bcm that Broadcom T1 uses.

HardwareSku
Force10-S6100-T1
Force10-S6100-ITPAC-T1
Force10-S6100
Celestica-DX010-C32
Arista-7260CX3-C64
Arista-7060CX-32S-Q32
Arista-7060CX-32S-C32-T1
Arista-7060CX-32S-C32
Arista-7050QX32S-Q32
Arista-7050QX-32S-S4Q31
Arista-7050-QX32
Arista-7050-QX-32S

Signed-off-by: zitingguo-ms <zitingguo@microsoft.com>
@Gfrom2016 Gfrom2016 marked this pull request as ready for review July 3, 2024 05:58
@Gfrom2016 Gfrom2016 requested a review from lguohan as a code owner July 3, 2024 05:58
@Gfrom2016 Gfrom2016 changed the title [Build only]Fix ecmp hash polarization by enable hash seed/offset config on T1 Fix ecmp hash polarization by enable hash seed/offset config on Broadcom T1 Jul 3, 2024
Copy link
Contributor

@ZhaohuiS ZhaohuiS left a comment

Choose a reason for hiding this comment

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

LGTM

@StormLiangMS
Copy link
Contributor

hi @yxieca @kperumalbfn could you help to review?

@StormLiangMS
Copy link
Contributor

hi @Gfrom2016 do we have ADO to track this one?

@Gfrom2016
Copy link
Contributor Author

@yxieca
Copy link
Contributor

yxieca commented Jul 3, 2024

@Gfrom2016 why are we trying to cherry-pick a change from 202311 to 202405? If the change is needed in both branches but not the master, can you oepn a separate PR for 202405 branch? Thanks!

@kperumalbfn
Copy link
Contributor

@Gfrom2016 Why do we need this change in addition to

#18912

Thanks,

@Gfrom2016
Copy link
Contributor Author

@Gfrom2016 why are we trying to cherry-pick a change from 202311 to 202405? If the change is needed in both branches but not the master, can you oepn a separate PR for 202405 branch? Thanks!

Sure, will do.

@yxieca
Copy link
Contributor

yxieca commented Jul 18, 2024

@Gfrom2016 is this change still needed?

@Gfrom2016
Copy link
Contributor Author

Gfrom2016 commented Jul 19, 2024

@Gfrom2016 is this change still needed?

We don't need it anymore, I'll close it.

@Gfrom2016 Gfrom2016 closed this Jul 19, 2024
@Gfrom2016 Gfrom2016 deleted the fix_ecmp_hash branch September 2, 2024 02:24
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.

5 participants