Support multiple database for DPU #174
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ganglyu
force-pushed
the
support_dpu_database
branch
from
bd02b15 to
f310689
Compare
qiluo-msft
reviewed
Dec 12, 2023
gnmi_server/server_test.go
Outdated
|
|
||
| path, _ := os.Getwd() | ||
| path = filepath.Dir(path) | ||
|
|
||
| var cmd *exec.Cmd | ||
| cmd = exec.Command("bash", "-c", "cd "+path+" && "+"pytest") | ||
| cmd = exec.Command("bash", "-c", "cd "+path+" && "+"pytest -m 'not dpu'") |
qiluo-msft
reviewed
Dec 12, 2023
gnmi_server/server_test.go
Outdated
| @@ -3861,6 +3861,41 @@ print('%s') | |||
| s.s.Stop() | |||
| } | |||
|
|
|||
| // Test DPU configuration with multiple databases | |||
There was a problem hiding this comment.
also generalize the PR title?
qiluo-msft
reviewed
Dec 12, 2023
| @@ -168,7 +185,8 @@ func GetDbTcpAddr(db_name string, ns string) string { | |||
| return hostname + ":" + strconv.Itoa(port) | |||
| } | |||
|
|
|||
| func DbGetNamespaceAndConfigFile(ns_to_cfgfile_map map[string]string) { | |||
| func DbParseConfigFile(name_to_cfgfile_map map[string]string) { | |||
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why I did it
sonic-net/sonic-buildimage#17161
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
How I did it
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.
We are trying to modify DPU_APPL_DB with GNMI, but the database is not accessible by swsscommon. This is because DPU_APPL_DB is a new database defined in /var/run/redisdpu0/sonic-db/database_config.json. As a workaround, we use GNMI to modify APPL_DB instead, but we need to update swsscommon to support the new database configuration.
I add a new element in GNMI path to support database instance.
The second element is used for target database instance, and it can be localhost, asic0, asic1, ..., dpu0, dpu1, and so on.
If SONiC device is not multi-asic or multi-dpu, this second element should be localhost.
This design is not backward compatible.
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)
Description for the changelog
Link to config_db schema for YANG module changes
A picture of a cute animal (not mandatory but encouraged)