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

Moving BRCM SAI from 3.7.5.1-1 to 3.7.5.1-2 to pick up the stubbed SA… #5101

Merged
merged 2 commits into from
Aug 5, 2020
Merged

Conversation

gechiang
Copy link
Collaborator

@gechiang gechiang commented Aug 5, 2020

…I API that were present in the header but missing in SAI implementation that cause link error during build

- Why I did it
In SAI header 1.5.2 there are defined SAI APIs such as "sai_query_attribute_capability() and 3 other APIs that were missing the stub code in BRCM 3.7.5.1 SAI code delivery. This caused a link issue during build when we implemented the common handler to support sai_query_attribute_capability() that other platform do have the support. The expectation is that all SAI vendors must implement all the SAI header defined SAI APIs by stubbing those that they don't have the actual code support to return "SAI_STATUS_NOT_IMPLEMENTED" so that all SAI APIs will be linkable during build. BRCM has agreed on the fix and provided the patch code that this PR is raised to pick up those missing stubbed SAI APIs.
See BRCM case CS00010790550 for more details

- How I did it
Added the patch and built the new debians packages for brcm SAI.

- How to verify it
Moved the sai.mk to pick up the new BRCM SAI debians and temporarily re-enabled the "sai_query_attribute_capability()" calling support to check if the build now can pass during link time. the build was successful and I loaded it to str-s6000-acs-8 DUT to ensure that the DUT boots fine, all docker services were up and running, all interface were able to be be brought up to Operational state.

- Which release branch to backport (provide reason below if seleted)

  • 201811
  • 201911
  • 202006

It is very likely that those missing BRCM SAI APIs may be needed as part of some other future fixes that may be needed in 202006 branch

- Description for the changelog
Move from BRCM SAI 3.7.5.1-1 to BRCM SAI 3.7.5.1-2 to pick up missing stubbed SAI APIs defined in SAI header 1.5.2

- A picture of a cute animal (not mandatory but encouraged)
ValidationOfNewSaiOnDUT.txt

…I API that were present in the header but missing in SAI implementation that cause link error during build
@abdosi
Copy link
Contributor

abdosi commented Aug 5, 2020

why not moving to 3.7.5.1-2 ?

@gechiang
Copy link
Collaborator Author

gechiang commented Aug 5, 2020

why not moving to 3.7.5.1-2 ?

There was already such named package in the 3.7 folder which I do not want to name this new one with same name to avoid confusion.

@abdosi
Copy link
Contributor

abdosi commented Aug 5, 2020

why not moving to 3.7.5.1-2 ?

There was already such named package in the 3.7 folder which I do not want to name this new one with same name to avoid confusion.

@gechiang let discuss offline

…I API that were present in the header but missing in SAI implementation that cause link error during build
@gechiang gechiang changed the title Moving BRCM SAI from 3.7.5.1-1 to 3.7.5.1-3 to pick up the stubbed SA… Moving BRCM SAI from 3.7.5.1-1 to 3.7.5.1-2 to pick up the stubbed SA… Aug 5, 2020
@gechiang
Copy link
Collaborator Author

gechiang commented Aug 5, 2020

Broadcom build failed due to in the middle of it I renamed the BRCM SAI debian package and it was not retriggered to rebuild.
I manually restarted it and it passed:
https://sonic-jenkins.westus2.cloudapp.azure.com/job/broadcom/job/buildimage-brcm-all-pr/4323/
Here is the portion that previously failed the build but in this run passed:
...
19:04:21 [ building ] [ target/debs/stretch/libsaibcm_3.7.5.1-2_amd64.deb ]
19:04:24 [ finished ] [ target/debs/stretch/libsaibcm_3.7.5.1-2_amd64.deb ]
19:04:24 [ building ] [ target/debs/stretch/libsairedis_1.0.0_amd64.deb-install ]
19:04:25 [ finished ] [ target/debs/stretch/libsairedis_1.0.0_amd64.deb-install ]
19:04:26 [ building ] [ target/debs/stretch/libsaibcm_3.7.5.1-2_amd64.deb-install ]
19:04:31 [ finished ] [ target/debs/stretch/libsaibcm_3.7.5.1-2_amd64.deb-install ]
19:04:31 [ building ] [ target/debs/stretch/libsaibcm-dev_3.7.5.1-2_amd64.deb-install ]
19:04:31 [ finished ] [ target/debs/stretch/libsaibcm-dev_3.7.5.1-2_amd64.deb-install ]
19:04:31 libsaibcm-dev_3.7.5.1-2_amd64.deb
...

@gechiang
Copy link
Collaborator Author

gechiang commented Aug 5, 2020

VS image build/test case failure had nothing to do with my changes. The BRCM SAI change had no functional changes so it should not have any impact on VS.

@gechiang
Copy link
Collaborator Author

gechiang commented Aug 5, 2020

retest broadcom please

@gechiang
Copy link
Collaborator Author

gechiang commented Aug 5, 2020

retest vsimage please

@gechiang gechiang merged commit 569289a into sonic-net:master Aug 5, 2020
gechiang added a commit to sonic-net/sonic-sairedis that referenced this pull request Aug 6, 2020
)

Previously the code to support query SAI attribute capabilities were commented out due to discovered missing BRCM Vendor SAI API missing in their 3.7.5.1 debian package that caused linking error during image build.
See #645 for details on the SAI attribute capability query support.

Since BRCM has provided the patch for the missing API stub code and included them in BRCM SAI 3.7.5.1-2 (see sonic-net/sonic-buildimage#5101), I am raising this PR to re-enable the calling to Vendor SAI API sai_query_attribute_capability().

I have tested this with the new BRCM SAI 3.7.5.1-2 debians and ensured that the linking issue was fixed by that patch.
santhosh-kt pushed a commit to santhosh-kt/sonic-buildimage that referenced this pull request Feb 25, 2021
sonic-net#5101)

* Moving BRCM SAI from 3.7.5.1-1 to 3.7.5.1-2 to pick up the stubbed SAI API that were present in the header but missing in SAI implementation that cause link error during build.
In SAI header 1.5.2 there are defined SAI APIs such as "sai_query_attribute_capability()" and few other APIs that were missing the stub code in BRCM 3.7.5.1 SAI code delivery. This causes linking issue during build when we implemented the common handler to support "sai_query_attribute_capability()" that other platform supports. The expectation is that all SAI vendors must implement all the SAI header defined SAI APIs by stubbing those that they don't have the actual code support to return "SAI_STATUS_NOT_IMPLEMENTED".  This allows common code for all platforms to be developed without having linking issue during build for all platforms. BRCM has provided the patch code and this PR moves the BRCM SAI from 3.7.5.1-1 to 3.7.5.1-2 to pick up those missing stubbed SAI APIs.
See BRCM case CS00010790550 for more details
pettershao-ragilenetworks pushed a commit to pettershao-ragilenetworks/sonic-sairedis that referenced this pull request Nov 18, 2022
…onic-net#651)

Previously the code to support query SAI attribute capabilities were commented out due to discovered missing BRCM Vendor SAI API missing in their 3.7.5.1 debian package that caused linking error during image build.
See sonic-net#645 for details on the SAI attribute capability query support.

Since BRCM has provided the patch for the missing API stub code and included them in BRCM SAI 3.7.5.1-2 (see sonic-net/sonic-buildimage#5101), I am raising this PR to re-enable the calling to Vendor SAI API sai_query_attribute_capability().

I have tested this with the new BRCM SAI 3.7.5.1-2 debians and ensured that the linking issue was fixed by that patch.
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.

3 participants