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

[portsyncd] Remove the port_config.ini dependency from portsyncd #1107

Merged
merged 2 commits into from
Oct 27, 2019

Conversation

zhenggen-xu
Copy link
Collaborator

[portsyncd] Remove the port_config.ini dependency from portsyncd

portsyncd will rely on configDB port table, not from port_config.ini

port_config.ini is used when we convert minigraph to configDB, then
everything should come from configDB.

Also for the DPB feature, we are going to deprecate port_config.ini

Signed-off-by: Zhenggen Xu zxu@linkedin.com

What I did
Remove the port_config.ini dependency from portsyncd

Why I did it
port_config.ini is used when we convert minigraph to configDB, then
everything should come from configDB.

Also for the DPB feature, we are going to deprecate port_config.ini

How I verified it
vs-test

Details if related

@lguohan lguohan requested review from qiluo-msft and daall October 25, 2019 19:52
@lguohan
Copy link
Contributor

lguohan commented Oct 25, 2019

@qiluo-msft , do we have port_config.ini dependency with mellanox script?

Copy link
Contributor

@daall daall left a comment

Choose a reason for hiding this comment

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

In the same vein as Guohan I'd like to understand what is dependent on this -p option before we remove it. I agree that it does seem redundant now that the data should be available in config DB, but there could be scripts that currently depend on this option. We should at least give those folks a heads up first.

@qiluo-msft qiluo-msft requested a review from stcheng October 25, 2019 21:14
Copy link
Collaborator

@prsunny prsunny left a comment

Choose a reason for hiding this comment

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

Minor comment - Please check comments from others on if this has any other dependencies.

portsyncd/portsyncd.cpp Outdated Show resolved Hide resolved
@zhenggen-xu
Copy link
Collaborator Author

zhenggen-xu commented Oct 25, 2019

@lguohan @daall @qiluo-msft @prsunny This PR is not intended to remove the port_config.ini file itself. It just to break the dependencies of portsyncd with port_config.ini. port_config.ini would still exist for other purposes like transceiver show etc. I have some pending changes for the vs test cases and PR with sonic-buildimage along with this PR. Will send out soon.

To remove port_config.ini completely, we are taking graceful steps and keep backwards compatible. We will introduce platform.json which was described in DPB design (sonic-net/SONiC#450). In case configDB does not have the information available, we will get from platform.json, but in the platforms/HWSKUs that still uses port_config.ini, the logic will keep the same. It will up to the platform vendors to add the platform.json for each platform/HWSKU and completely remove the port_config.ini. The code to use the new platform.json etc is coming out soon.

In short, this PR should not break any functions unless you miss the PORT TABLE in configDB.

@daall
Copy link
Contributor

daall commented Oct 25, 2019

Thanks for the explanation @zhenggen-xu! 🚀

portsyncd will rely on configDB port table, not from port_config.ini

port_config.ini is used when we convert minigraph to configDB, then
everything should come from configDB.

Also for the DPB feature, we are going to deprecate port_config.ini

Signed-off-by: Zhenggen Xu <zxu@linkedin.com>
Signed-off-by: Zhenggen Xu <zxu@linkedin.com>
@qiluo-msft qiluo-msft merged commit 2a1ffe8 into sonic-net:master Oct 27, 2019
EdenGri pushed a commit to EdenGri/sonic-swss that referenced this pull request Feb 28, 2022
)

This is not deemed as error, hence don't report as error.
oleksandrivantsiv pushed a commit to oleksandrivantsiv/sonic-swss that referenced this pull request Mar 1, 2023
- Update SAI VoQ support (sonic-net#1107) …
- Voq system (sonic-net#1081) …
- [meta] Add support for ignored enum values (sonic-net#1099)
- TPID SAI proposal (sonic-net#1089) …
- ACL GRE key match (sonic-net#1076) …
- Add IPv6 NS and NA Traps (sonic-net#1092) …
- MACsec flow list attribute added in MACsec object (sonic-net#1095) …
- Add Enterprise Number for IPFIX Report Type (sonic-net#1072) …
- Provide TTL and QoS treatment during MPLS encap and decap (sonic-net#1079)
- Create and Set for Tunnel Attributes (sonic-net#1086) …
oleksandrivantsiv pushed a commit to oleksandrivantsiv/sonic-swss that referenced this pull request Mar 1, 2023
Add SAI_PORT_ATTR_OPER_SPEED get in vslib.
If the SAI_PORT_ATTR_OPER_STATUS is DOWN, SAI_PORT_ATTR_OPER_SPEED should be 0, otherwise its value comes from /sys/class/net/eth{X}/speed
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.

6 participants