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

Support multiple database for DPU #188

Merged
merged 13 commits into from
Mar 18, 2024
Merged

Conversation

ganglyu
Copy link
Contributor

@ganglyu ganglyu commented Feb 7, 2024

Why I did it

We have updated database configuration to support multiple databases for smartswitch.
And then we need to update sonic-gnmi to support new database configuration and new GNMI path for smartswitch.
HLD PR: sonic-net/SONiC#1625

How I did it

  • Support new database configuration
{
    "INCLUDES" : [
        {
            "include" : "../../redis/sonic-db/database_config.json"
        },
        {
            "container_name" : "dpu0",
            "include" : "../../redisdpu0/sonic-db/database_config.json"
        }
    ],
    "VERSION" : "1.0"
}

Use a single layer instance for each device to support multiple asic and multiple dpu.
For example, for multiple asic, the instances can be named localhost, asic0, asic1, and so on. For multiple dpu, the instances can be named localhost, dpu0, dpu1, and so on. For multiple asic npu and multiple dpu, the instances can be named localhost, asic0, asic1, ..., dpu0, dpu1, and so on.

        "DPU_APPL_DB" : {
            "id" : 15,
            "separator": ":",
            "instance" : "redis",
            "format": "proto"
        },
  • Update GNMI path to support multiple database

I add a new element in GNMI path to support database instance.

/APPL_DB/dpu0/DASH_VNET_TABLE/vnet1/

The second element is used for target database instance, and it can be localhost, asic0, asic1, ..., dpu0, dpu1, and so on.

How to verify it

Run unit test for sonic-gnmi.
Run end to end test for dash and gnmi.

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111

Description for the changelog

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

@ganglyu ganglyu requested a review from qiluo-msft February 7, 2024 09:08
@ganglyu ganglyu requested a review from liuh-80 February 26, 2024 06:10
liuh-80
liuh-80 previously approved these changes Feb 26, 2024
@ganglyu ganglyu requested a review from qiluo-msft March 2, 2024 07:04
// Skip first elem
fullPath.Elem = elems[1:]
// Skip first two elem
// New GNMI path schema, /CONFIG_DB/localhost/PORT
Copy link
Collaborator

@qiluo-msft qiluo-msft Mar 5, 2024

Choose a reason for hiding this comment

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

localhost

This is more general design change, not directly related to multiple database. Let's use a separate PR. #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

return db_list, err
}

func GetDbSeparatorByDBKey(db_name string, dbkey swsscommon.SonicDBKey) (separator string, err error) {
Copy link
Collaborator

@qiluo-msft qiluo-msft Mar 5, 2024

Choose a reason for hiding this comment

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

GetDbSeparatorByDBKey

Many functions below are 1:1 mapping of swsscommon functions. Why not directly using swsscommon functions? What is the value of the extra wrapper? #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This wrapper can translate swsscommon exception to error and invoke swsscommon initialize API.

@ganglyu ganglyu merged commit 3fa77c6 into sonic-net:master Mar 18, 2024
5 checks passed
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