[dpu]: Add DPU database service #17161

Pterosaur · 2023-11-14T04:20:15Z

Sub PRs:
sonic-net/sonic-host-services#84
#17191

Why I did it

According to the design, the database instances of DPU will be kept in the NPU host.

Work item tracking

Microsoft ADO (number only): 25072889

How I did it

To follow the multiple ASIC design, I assume a new platform environment variable NUM_DPU will be defined in the /usr/share/sonic/device/$PLATFORM/platform_env.conf. Based on this number, NPU host will launch a corresponding number of instances for the DPU database.

How to verify it

Defined NUM_DPU in the platform_env.conf
Declare a new field in the config_db for database feature.
remove the existing databse service by 'docker rm -f database' and reboot OS.

We should get multiple database services and global_config as following.

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

Tested branch (Please provide the tested image version)

Description for the changelog

Link to config_db schema for YANG module changes

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

Signed-off-by: Ze Gan <[email protected]>

Pterosaur · 2023-11-15T00:54:44Z

/azpw run

mssonicbld · 2023-11-15T00:54:47Z

/AzurePipelines run

azure-pipelines · 2023-11-15T00:54:52Z

You have several pipelines (over 10) configured to build pull requests in this repository. Specify which pipelines you would like to run by using /azp run [pipelines] command. You can specify multiple pipelines using a comma separated list.

Pterosaur · 2023-11-15T00:59:03Z

/azpw run Azure.sonic-buildimage

mssonicbld · 2023-11-15T00:59:05Z

/AzurePipelines run Azure.sonic-buildimage

azure-pipelines · 2023-11-15T00:59:16Z

Azure Pipelines successfully started running 1 pipeline(s).

ganglyu · 2023-11-15T08:30:40Z

dockers/docker-database/database_config.json.j2

@@ -2,7 +2,7 @@
    "INSTANCES": {
        "redis":{
            "hostname" : "{{HOST_IP}}",
-            "port" : 6379,
+            "port" : {{REDIS_PORT}},
            "unix_socket_path" : "/var/run/redis{{NAMESPACE_ID}}/redis.sock",


DPU should use different unix socket path.

Signed-off-by: Ze Gan <[email protected]>

ganglyu · 2023-11-15T08:41:05Z

Why do we have /var/run/redisdpu0/sonic-db/database_global.json?

ganglyu · 2023-11-15T08:43:55Z

Please create a separate PR for sonic yang models

Pterosaur · 2023-11-15T12:22:49Z

Why do we have /var/run/redisdpu0/sonic-db/database_global.json?

Removed.

Signed-off-by: Ze Gan <[email protected]>

Pterosaur · 2023-11-16T04:26:34Z

/azpw run Azure.sonic-buildimage

mssonicbld · 2023-11-16T04:26:37Z

/AzurePipelines run Azure.sonic-buildimage

azure-pipelines · 2023-11-16T04:26:48Z

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Ze Gan <[email protected]>

lguohan · 2023-11-16T17:18:09Z

src/sonic-py-common/sonic_py_common/device_info.py

+
+
+def get_num_dpus():
+    num_dpus = 0


let's add todo here. we should use platform api to get the dpu number instead of rely on the platform env config. @prgeor .

r12f · 2023-11-16T22:24:00Z

dockers/docker-database/Dockerfile.j2

@@ -32,7 +32,8 @@ RUN apt-get clean -y                                  && \
             s/^# unixsocket/unixsocket/;                \
             s/redis-server.sock/redis.sock/g;           \
             s/^client-output-buffer-limit pubsub [0-9]+mb [0-9]+mb [0-9]+/client-output-buffer-limit pubsub 0 0 0/; \
-             s/^notify-keyspace-events ""$/notify-keyspace-events AKE/ \
+             s/^notify-keyspace-events ""$/notify-keyspace-events AKE/; \
+             s/^databases [0-9]+$/databases 100/ \


is this change expected? do we need 100 databases or it is changed for testing but later forget to remove

The default value of databases is 16 which isn't enough for new databases of DPU.

r12f · 2023-11-16T22:25:54Z

dockers/docker-database/database_config.json.j2

@@ -94,6 +95,30 @@
            "separator": ":",
            "instance" : "redis"
        }
+{% if DATABASE_TYPE is defined and DATABASE_TYPE == "dpudb" %}
+        ,
+        "DPU_APPL_DB" : {


maybe i missed some update, so just checking - have we decided to use DPU_APPL_DB or DASH_APPL_DB and etc.

Yes, I want to introduce new databases like, DPU_APPL_DB, which follows chassis design. I feel it's clearer to keep the underlay and overlay objects in different databases.

r12f · 2023-11-16T22:26:35Z

dockers/docker-database/database_global.json.j2

+{% for dpu in range(dpu_cnt) %}
+        {
+            "database_type" : "dpudb",
+            "include" : "../../redisdpu{{dpu}}/sonic-db/database_config.json"


nit: i wonder if we could use "redis-dpu{{dpu}}"

and we can do it in another change, since this change is already approved.

It's not easy. Because the database folder consists of the prefix "redis" and suffix "$DEV" without the dash "-". This convention has been used in the multi-ASIC scenario.

judyjoseph · 2023-11-17T00:25:37Z

dockers/docker-database/database_config.json.j2

+            "port" : {{REDIS_PORT}},
+            "unix_socket_path" : "/var/run/redis{{DEV}}/redis.sock",
+            "persistence_for_warm_boot" : "yes",
+            "database_type": "{{DATABASE_TYPE}}"


should we add a check to add the field "database_type" if DATABASE_TYPE is defined?

judyjoseph · 2023-11-17T01:12:36Z

dockers/docker-database/docker-database-init.sh

@@ -59,7 +71,7 @@ if [[ $DATABASE_TYPE == "chassisdb" ]]; then
 fi

 # copy/generate the database_global.json file if this is global database service in multi asic platform.
-if [[ $NAMESPACE_ID == "" ]] && [[ $NAMESPACE_COUNT -gt 1 ]]
+if [[ $DATABASE_TYPE == "" ]] && [[ $NAMESPACE_COUNT -gt 1 || $NUM_DPU -gt 1 ]]


Why is this check $NAMESPACE_ID == "" changed here to use $DATABASE_TYPE ? This check originally meant we are working in the host namespace where namespace is ""

In the DPU scenario, all dpu databases are running in the host namespace which means the namespace_id is also empty to the dpu database instance. So, I use database_type == "" to identify the host db instance.

From what I understand in case of dpu we set the DATABASE_TYPE as below, and for all other cases like single_asic/multi-asic DATABASE_TYPE will be ""

The earlier check below was applicable only to global db which is in host namespace in case of multi-asic/single asic

if [[ $NAMESPACE_ID == "" ]] && [[ $NAMESPACE_COUNT -gt 1 ]]

So now with this change $NAMESPACE_ID == "" --------> $DATABASE_TYPE == "" , for multi-asic platforms it will never go to the else part as DATABASE_TYPE is always "" ?.

You are right, I will fix it.

Fix it in PR: #17221 Please help to check it.

Taking a look.

Signed-off-by: Ze Gan <[email protected]>

… in multi asic platform (#17221) Fix bug: #17161 (comment) multi-asic platforms it will never go to the else part as DATABASE_TYPE is always "" Microsoft ADO (number only): 25072889 Move the checker NAMESPACE_ID == "" back Signed-off-by: Ze Gan <[email protected]>

Dpu database

8bc0e30

Signed-off-by: Ze Gan <[email protected]>

Pterosaur requested a review from lguohan as a code owner November 14, 2023 04:20

Pterosaur mentioned this pull request Nov 14, 2023

[featured]: Add database services for DPU sonic-net/sonic-host-services#84

Merged

Add yang module

9d7e6aa

Signed-off-by: Ze Gan <[email protected]>

Pterosaur requested a review from qiluo-msft as a code owner November 14, 2023 04:31

Pterosaur requested a review from r12f November 14, 2023 04:39

Pterosaur requested a review from judyjoseph November 15, 2023 01:08

ganglyu reviewed Nov 15, 2023

View reviewed changes

Fix bugs

307b8c8

Signed-off-by: Ze Gan <[email protected]>

Pterosaur force-pushed the dpu_database branch 2 times, most recently from a8cceca to 87a9b2c Compare November 15, 2023 12:04

Pterosaur closed this Nov 15, 2023

Pterosaur reopened this Nov 15, 2023

Pterosaur force-pushed the dpu_database branch from 87a9b2c to 330ac8a Compare November 15, 2023 14:08

Fix comments

7aa84c9

Signed-off-by: Ze Gan <[email protected]>

Pterosaur force-pushed the dpu_database branch from 330ac8a to 7aa84c9 Compare November 16, 2023 01:55

Remove yang

d8ea2bd

Signed-off-by: Ze Gan <[email protected]>

ganglyu approved these changes Nov 16, 2023

View reviewed changes

Increase databasenum

b309999

Signed-off-by: Ze Gan <[email protected]>

lguohan reviewed Nov 16, 2023

View reviewed changes

lguohan previously approved these changes Nov 16, 2023

View reviewed changes

r12f reviewed Nov 16, 2023

View reviewed changes

r12f approved these changes Nov 16, 2023

View reviewed changes

judyjoseph reviewed Nov 17, 2023

View reviewed changes

Pterosaur dismissed lguohan’s stale review via df3c296 November 17, 2023 09:06

Fix comment

d26601f

Signed-off-by: Ze Gan <[email protected]>

Pterosaur force-pushed the dpu_database branch from df3c296 to d26601f Compare November 17, 2023 09:59

Pterosaur requested review from judyjoseph and lguohan November 17, 2023 13:50

lguohan merged commit 9f08f88 into sonic-net:master Nov 17, 2023
20 checks passed

Pterosaur mentioned this pull request Nov 18, 2023

[docker-database-init.sh]:Fix wrong creating of database_global.json in multi asic platform #17221

Merged

11 tasks

Pterosaur mentioned this pull request Dec 9, 2023

[doc]: Init Smartswitch database High Level Design sonic-net/SONiC#1534

Merged

ganglyu mentioned this pull request Dec 9, 2023

Support multiple database for DPU sonic-net/sonic-gnmi#174

Closed

6 tasks

Pterosaur mentioned this pull request Feb 25, 2024

[smartswitch] Update IP assignment flow sonic-net/SONiC#1586

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[dpu]: Add DPU database service #17161

[dpu]: Add DPU database service #17161

Pterosaur commented Nov 14, 2023 •

edited

Loading

Pterosaur commented Nov 15, 2023

mssonicbld commented Nov 15, 2023

azure-pipelines bot commented Nov 15, 2023

Pterosaur commented Nov 15, 2023

mssonicbld commented Nov 15, 2023

azure-pipelines bot commented Nov 15, 2023

ganglyu Nov 15, 2023

Pterosaur Nov 15, 2023

ganglyu commented Nov 15, 2023

ganglyu commented Nov 15, 2023

Pterosaur commented Nov 15, 2023

Pterosaur commented Nov 16, 2023

mssonicbld commented Nov 16, 2023

azure-pipelines bot commented Nov 16, 2023

lguohan Nov 16, 2023

Pterosaur Nov 17, 2023

r12f Nov 16, 2023

Pterosaur Nov 17, 2023

r12f Nov 16, 2023

Pterosaur Nov 17, 2023

r12f Nov 16, 2023

r12f Nov 16, 2023

Pterosaur Nov 17, 2023

judyjoseph Nov 17, 2023

Pterosaur Nov 17, 2023

judyjoseph Nov 17, 2023 •

edited

Loading

Pterosaur Nov 17, 2023

judyjoseph Nov 17, 2023

Pterosaur Nov 18, 2023

Pterosaur Nov 18, 2023

judyjoseph Nov 20, 2023

[dpu]: Add DPU database service #17161

[dpu]: Add DPU database service #17161

Conversation

Pterosaur commented Nov 14, 2023 • edited Loading

Why I did it

Work item tracking

How I did it

How to verify it

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

Tested branch (Please provide the tested image version)

Description for the changelog

Link to config_db schema for YANG module changes

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

Pterosaur commented Nov 15, 2023

mssonicbld commented Nov 15, 2023

azure-pipelines bot commented Nov 15, 2023

Pterosaur commented Nov 15, 2023

mssonicbld commented Nov 15, 2023

azure-pipelines bot commented Nov 15, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ganglyu commented Nov 15, 2023

ganglyu commented Nov 15, 2023

Pterosaur commented Nov 15, 2023

Pterosaur commented Nov 16, 2023

mssonicbld commented Nov 16, 2023

azure-pipelines bot commented Nov 16, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

judyjoseph Nov 17, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Pterosaur commented Nov 14, 2023 •

edited

Loading

judyjoseph Nov 17, 2023 •

edited

Loading