-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
[bfn][saiserver]support bfn saiserverv1 and saiserverv2 #9684
[bfn][saiserver]support bfn saiserverv1 and saiserverv2 #9684
Conversation
a6eab9c
to
e2c2230
Compare
86fbcb0
to
e631fdf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
9fc1297
to
9508855
Compare
@msosyak could you help make a review about this update? BTW, I found you removed two dependencies After add those library, |
Also I found few one more issue while trying to build saiserver.
|
Signed-off-by: Myron Sosyak <[email protected]>
@richardyu-ms Here are the changes needed to fix saiserverv2 startup richardyu-ms#5. Please review and try it. |
9508855
to
3af6eb8
Compare
Fix saisrver startup process and dependencies
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is dependent on opencomputeproject/SAI#1433 which is not yet included in sonic-sairedis and sonic-buildimage. Without this, the build will fail.
I left a few minor comments, but I still, have some concerns regarding work on different devices.
Will check and come back.
@@ -13,18 +13,19 @@ $(LIBSAITHRIFT_DEV)_DEPENDS += $(LIBTHRIFT_0_14_1) $(LIBTHRIFT_0_14_1_DEV) \ | |||
#$(LIBSAIVS) $(LIBSAIVS_DEV) $(LIBSAIMETADATA) $(LIBSAIMETADATA_DEV) | |||
|
|||
# $(LIBSAITHRIFT_DEV)_BUILD_ENV = platform=v | |||
$(LIBSAITHRIFT_DEV)_BUILD_ENV = SAITHRIFTV2=true SAITHRIFT_VER=v2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't it be under
ifeq ($(SAITHRIFT_V2),y)
@@ -295,6 +295,7 @@ endif | |||
$(info "ENABLE_SYNCD_RPC" : "$(ENABLE_SYNCD_RPC)") | |||
$(info "SAITHRIFT_V2" : "$(SAITHRIFT_V2)") | |||
$(info "ENABLE_ORGANIZATION_EXTENSIONS" : "$(ENABLE_ORGANIZATION_EXTENSIONS)") | |||
$(info "SAITHRIFT_V2" : "$(SAITHRIFT_V2)") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will be great to add this variable to rules/config file
export ONIE_PLATFORM=`grep onie_platform /etc/machine.conf | awk 'BEGIN { FS = "=" } ; { print $2 }'` | ||
export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:/opt/bfn/install/lib:/opt/bfn/install/lib/platform/$ONIE_PLATFORM:/opt/bfn/install/lib:/opt/bfn/install/lib/tofinopd/switch | ||
./opt/bfn/install/bin/dma_setup.sh | ||
/usr/sbin/saiserver -p /etc/sai/profile.ini -f /etc/sai/portmap.ini |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where /etc/sai/portmap.ini is taken?
@richardyu-ms shouldn't this PR be closed in favor of #13166? |
close this one as all the change finished in #13166 |
Why I did it
support bfn saiserverv1 and saiserverv2
How I did it
Support saiserver v2 with python3 and thrift 0.13.0
add variables to support the saiserverv2
build different thrift in saithrift depends on saiserver version
build differernt versions of saiserver
make the saiserver and saiserver docker with version number
How to verify it
build two different versions of sasiserver in local build environment
todo: test on devices
Which release branch to backport (provide reason below if selected)
Description for the changelog
A picture of a cute animal (not mandatory but encouraged)