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

Port configuration incremental update support #985

Merged
merged 2 commits into from
Jul 6, 2022

Conversation

Junchao-Mellanox
Copy link
Contributor

@Junchao-Mellanox Junchao-Mellanox commented Apr 19, 2022

Due to historical reason, portsyncd and portmgrd both handle PORT table changes in CONFIG_DB and write APPL_DB according to configuration change. portmgrd handles fields including "mtu", "admin_status" and "learn_mode"; portsyncd handles all fields. There are a few issues here:

  1. portsyncd is designed to listen to kernel port status change and fire the change event to high level, it should not handle CONFIG_DB change. portmgrd is the right place to handle port configuration change according to SONiC architecture.
  2. Configuration change for "mtu", "admin_status" and "learn_mode" will be handled twice which is not necessary
  3. portsyncd put all configuration to APPL_DB even if user only changes part of them. E.g. user changes "speed" of the port, portsyncd will put configuration like "mtu", "admin_status", "autoneg" to APPL_DB. This is not necessary too.

This HLD describes how to fix them.

PR:

PR title state context
[sonic-swss] Port configuration incremental update support GitHub issue/pull request detail GitHub pull request check contexts

keboliu
keboliu previously approved these changes Apr 22, 2022
@liat-grozovik
Copy link
Collaborator

@prgeor , @qiluo-msft could you please review and signoff? coding is ready and feature is required for 202205.

prsunny
prsunny previously approved these changes Jul 6, 2022
@Junchao-Mellanox Junchao-Mellanox dismissed stale reviews from prsunny and keboliu via 7892315 July 6, 2022 05:33
@liat-grozovik liat-grozovik merged commit fd1e310 into sonic-net:master Jul 6, 2022
itamar-talmon pushed a commit to itamar-talmon/SONiC that referenced this pull request Oct 2, 2022
Due to historical reason, portsyncd and portmgrd both handle PORT table changes in CONFIG_DB and write APPL_DB according to configuration change. portmgrd handles fields including "mtu", "admin_status" and "learn_mode"; portsyncd handles all fields. There are a few issues here:

portsyncd is designed to listen to kernel port status change and fire the change event to high level, it should not handle CONFIG_DB change. portmgrd is the right place to handle port configuration change according to SONiC architecture.
Configuration change for "mtu", "admin_status" and "learn_mode" will be handled twice which is not necessary
portsyncd put all configuration to APPL_DB even if user only changes part of them. E.g. user changes "speed" of the port, portsyncd will put configuration like "mtu", "admin_status", "autoneg" to APPL_DB. This is not necessary too.
This HLD describes how to fix them.
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.

4 participants