-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
[Ufispace][PDDF] Add PDDF support on S9180-32X #14909
Conversation
Signed-off-by: cytsai0409 <cytsai0409@gmail.com>
Signed-off-by: cytsai0409 <cytsai0409@gmail.com>
@FuzailBrcm pls help review |
#self._sfp_list[int(index)].check_sfp_optoe_type() | ||
return ret_dict | ||
|
||
def get_sfp(self, index): |
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 might not be required as he parent class chassi_base.py provide the same definition of the func.
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.
Ok. I will remove this function.
|
||
return rpm_speed | ||
|
||
def get_direction(self): |
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.
Can you check this again?
I think same implementation is present in the pddf_fan.py class.
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 device is old design and we can't get fan direction information from CPU or BMC. I tried to take implementation from pddf_fan.py class, the direction will be "N/a". So I still need to overwrite the get_direction() function and set the direction to FAN_DIRECTION_EXHAUST to fit our device. I will update the code later.
if self.is_psu_fan: | ||
return 75 | ||
else: | ||
return 65 |
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.
Is your system fan doesnt support speed variations etc?
How is the target speed has a fixed value? This is supposed to be the speed intended by the user. Its calculation is based on the duty-cycle/pwm calculations.
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.
Yes, The fan speed is fully controlled by BMC and not open for the users. Still I will update the code to get current speed to avoid fixed value which is quite misleading. Thanks.
1. remove duplicate get_sfp() in chassis.py 2. update get_direction() and get_target_speed() in fan.py Signed-off-by: cytsai0409 <cytsai0409@gmail.com>
@FuzailBrcm update committed and pass the check, pls help review, thanks |
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.
Changes look okay to me.
@lguohan Please help review and merge, thanks. |
@lguohan please help merge, thanks. |
1 similar comment
@lguohan please help merge, thanks. |
@yxieca please help merge, thanks. |
Please resolve the merge conflict. |
@yxieca Merge conflict is resolved. Thanks. |
@cytsai0409 , bfnplatform-ufispace_1.0.0_amd64.deb conflicts with bfnsdk_20221130_sai_1.11.0_deb11.deb. |
It breaks barefoot SONiC image's build. |
…)" This reverts commit d2b5d77.
@liushilongbuaa We checked the build logs and found sometimes bfnplatform-ufispace_1.0.0_amd64.deb is installed before bfnsdk_20221130_sai_1.11.0_deb11.deb. In this case the build process failed. We will add dependency rule in the makefile to avoid such installation conflict soon. Is this ok to you? Tks. |
@cytsai0409 , if install ufispace after bfnsdk, /opt/intall/libsai.so will be override. |
@liushilongbuaa Could you provide your build commands? We would like to reproduce the issue in our local environment. By the way the master branch has another build issue #16087 that we need time to walkaround it. Tks. |
@cytsai0409 , you can check https://dev.azure.com/mssonic/build/_build?definitionId=146&_a=summary&branchFilter=11237%2C11237%2C11237%2C11237 |
And you can login to sonic-slave-bullseye-$USER container to reproduce.
|
@liushilongbuaa We plan to create a new bfnplatform-ufispace_1.0.0_amd64.deb that create new folder under /opt/bfn/install_x1_tofino and /opt/bfn/install_x2_tofino and put our files there. This way the files in bfnsdk_20221130_sai_1.11.0_deb11.deb are not affected and there will be no race issue. We will test it and let you know the result later. Tks. |
@liushilongbuaa We created a test deb package and verified the file structures are the same under /opt/bfn regardless the package installation ordering. The logs are in the attachment files. Also the build process is successful in my environment with this test deb. I will update the bfnplatform-ufispace_1.0.0_amd64.deb on github and submit a new PR later. Tks. |
I'm not sure how to check if there are other conflicts. |
ok. I will stress the build process for a while locally and check if there are other conflicts not found so far. tks. |
* Add s9180-32x pddf support Signed-off-by: cytsai0409 <cytsai0409@gmail.com> * Fix memset_s parameter Signed-off-by: cytsai0409 <cytsai0409@gmail.com> * Update chassis.py and fan.py 1. remove duplicate get_sfp() in chassis.py 2. update get_direction() and get_target_speed() in fan.py Signed-off-by: cytsai0409 <cytsai0409@gmail.com> --------- Signed-off-by: cytsai0409 <cytsai0409@gmail.com>
…)" (sonic-net#16092) This reverts commit d2b5d77.
Signed-off-by: Jason Tsai jason.cy.tsai@ufispace.com
Why I did it
Add PDDF support on Ufispace S9180-32X with Barefoot ASIC
How I did it
Add PDDF configuration files, scripts and python files
How to verify it
Run pddf commands and show commands.
Which release branch to backport (provide reason below if selected)
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)