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

[buffers] Add handler for the 'create_only_config_db_buffers' configuration knob #2883

Merged

Conversation

vadymhlushko-mlnx
Copy link
Contributor

@vadymhlushko-mlnx vadymhlushko-mlnx commented Aug 22, 2023

What I did
Add support in the orchagent for the configuration knob:

{
    "DEVICE_METADATA": {
        "localhost": {
            "create_only_config_db_buffers": "true"
            ...
        }
    }
}

Why I did it
If the "create_only_config_db_buffers" is equal to "true" - buffers will be created according to the config_db configuration (for example BUFFER_QUEUE|* table).
If the "create_only_config_db_buffers" is equal to "false" or does not exist - the maximum available buffers (which are read from SAI) will be created, regardless of the config_db buffer config.

How I verified it
Add UT.

Manual verification:

  1. Install the image with this PR included on the not MSFT SKU switch
  2. Check the show queue counters output and verify that only configured in CONFIG_DB buffers are created
root@sonic:/home/admin# show queue counters
     Port    TxQ    Counter/pkts    Counter/bytes    Drop/pkts    Drop/bytes
---------  -----  --------------  ---------------  -----------  ------------
Ethernet0    UC0               0                0            0           N/A
Ethernet0    UC1               0                0            0           N/A
Ethernet0    UC2               0                0            0           N/A
Ethernet0    UC3               0                0            0           N/A
Ethernet0    UC4               0                0            0           N/A
Ethernet0    UC5               0                0            0           N/A
Ethernet0    UC6               0                0            0           N/A
  1. Open the /usr/share/sonic/device/$DEVICE/$SKU/create_only_config_db_buffers.json and change it to:
"create_only_config_db_buffers": "false"
  1. Do config reload
  2. Check the show queue counters output and verify that all available buffers are created
root@sonic:/home/admin# show queue counters
     Port    TxQ    Counter/pkts    Counter/bytes    Drop/pkts    Drop/bytes
---------  -----  --------------  ---------------  -----------  ------------
Ethernet0    UC0               0                0            0           N/A
Ethernet0    UC1               0                0            0           N/A
Ethernet0    UC2               0                0            0           N/A
Ethernet0    UC3               0                0            0           N/A
Ethernet0    UC4               0                0            0           N/A
Ethernet0    UC5               0                0            0           N/A
Ethernet0    UC6               0                0            0           N/A
Ethernet0    UC7              60            15346            0           N/A
Ethernet0    MC8             N/A              N/A          N/A           N/A
Ethernet0    MC9             N/A              N/A          N/A           N/A
Ethernet0   MC10             N/A              N/A          N/A           N/A
Ethernet0   MC11             N/A              N/A          N/A           N/A
Ethernet0   MC12             N/A              N/A          N/A           N/A
Ethernet0   MC13             N/A              N/A          N/A           N/A
Ethernet0   MC14             N/A              N/A          N/A           N/A
Ethernet0   MC15             N/A              N/A          N/A           N/A

Details if related

@vadymhlushko-mlnx vadymhlushko-mlnx changed the title [buffers] Add handler for the 'create_all_available_buffers' configuration knob [buffers] Add handler for the 'create_only_config_db_buffers' configuration knob Aug 23, 2023
@vadymhlushko-mlnx vadymhlushko-mlnx force-pushed the master-create-all-buffers branch 3 times, most recently from b0f6e07 to 34ca6e4 Compare August 29, 2023 13:04
@vadymhlushko-mlnx vadymhlushko-mlnx marked this pull request as ready for review August 30, 2023 18:04
@vadymhlushko-mlnx
Copy link
Contributor Author

/azpw run Azure.sonic-swss

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-swss

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vadymhlushko-mlnx
Copy link
Contributor Author

/azpw run Azure.sonic-swss

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-swss

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@prsunny
Copy link
Collaborator

prsunny commented Sep 6, 2023

@vadymhlushko-mlnx , can you check why the test is failing?

@vadymhlushko-mlnx vadymhlushko-mlnx force-pushed the master-create-all-buffers branch 4 times, most recently from e98dcbd to aa23409 Compare September 12, 2023 08:40
…on knob

Signed-off-by: vadymhlushko-mlnx <vadymh@nvidia.com>
Signed-off-by: vadymhlushko-mlnx <vadymh@nvidia.com>
Signed-off-by: vadymhlushko-mlnx <vadymh@nvidia.com>
@vadymhlushko-mlnx
Copy link
Contributor Author

@abdosi @prsunny could you please approve and merge?

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.

lgtm, minor comment. @abdosi , @prabhataravind , please review/sign-off.

@neethajohn for viz

}
catch(const std::system_error& e)
{
SWSS_LOG_ERROR("System error: %s", e.what());
Copy link
Collaborator

Choose a reason for hiding this comment

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

can you change this to WARNING? for non existent config_db, we don't want an ERR log printed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The error statement in the catch block will be printed only if the deviceMetadataConfigTable.hget() throws an exception.
If the create_only_config_db_buffers do not exist in the config_db the deviceMetadataConfigTable.hget() returns false.

Copy link
Contributor

@prabhataravind prabhataravind left a comment

Choose a reason for hiding this comment

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

lgtm

@prsunny prsunny merged commit 91e7a27 into sonic-net:master Oct 4, 2023
16 checks passed
@vadymhlushko-mlnx
Copy link
Contributor Author

@prsunny could we include this PR in the 202305 branch?
I've already checked - it is a clean cherry-pick

@StormLiangMS
Copy link
Contributor

@vadymhlushko-mlnx have you tested 202305 image with this PR?

@prsunny could we have ADO to track if support cherry pick to 202305?

@Blueve
Copy link

Blueve commented Oct 18, 2023

@StormLiangMS Please use this ADO: 25518393

StormLiangMS pushed a commit that referenced this pull request Nov 2, 2023
…ration knob (#2883)

* [buffers] Add handler for "create_only_config_db_buffers" configuration knob

If the "create_only_config_db_buffers" is equal to "true" - buffers will be created according to the config_db configuration (for example BUFFER_QUEUE|* table).
If the "create_only_config_db_buffers" is equal to "false" or does not exist - the maximum available buffers (which are read from SAI) will be created, regardless of the config_db buffer config.
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