Skip to content

[database]: Update the DPU database service configuration #17562

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 8 commits into from
Apr 9, 2024
Merged

[database]: Update the DPU database service configuration #17562

merged 8 commits into from
Apr 9, 2024

Conversation

Pterosaur
Copy link
Contributor

@Pterosaur Pterosaur commented Dec 19, 2023 •
edited
Loading

Why I did it

According to the HLD: sonic-net/SONiC#1534, update the DPU database configuration.

Work item tracking
  • Microsoft ADO (number only):

How I did it

  1. Add the remote redis configuration in database_config.json.j2
  2. Rename the database_global.json.j2
  3. Determine the remote DB port according to the midplane IP
  4. Get the DPU information from platform.json

How to verify it

Check them locally

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

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205
  • 202211
  • 202305

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)

prgeor
prgeor reviewed Dec 19, 2023
View reviewed changes
dockers/docker-database/database_config.json.j2 Outdated
@@ -102,23 +111,23 @@
"DPU_APPL_DB" : {
"id" : 15,
"separator": ":",
"instance" : "redis",
"instance" : {% if include_remote_db%} "remote_redis" {% else %} "redis" {% endif %},
Copy link
Contributor

Choose a reason for hiding this comment

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

@Pterosaur which design doc are you following? I am not sure, if the DPU_

will be hosted on the NPU side or DPU side?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add the design doc to the description and that has been reviewed by community.

prgeor
prgeor reviewed Dec 19, 2023
View reviewed changes
dockers/docker-database/docker-database-init.sh Outdated
# Determine the DB PORT from midplane IP
IFS=. read -r a b c d <<< $midplane_ip
midplane_ip_num=$((a * 256 ** 3 + b * 256 ** 2 + c * 256 + d))
export REMOTE_DB_PORT=$((6381 + $midplane_ip_num - $midplane_base_ip_num))
Copy link
Contributor

@prgeor prgeor Dec 19, 2023 •
edited by qiluo-msft
Loading

Choose a reason for hiding this comment

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

@Pterosaur you can get the ip from the "eth0-midplane" interface. Use the last digit (1 for DPU1, 2 for DPU 2, and so on)

REMOTE_DB_PORT = 6380 + <last digit of the IP>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good suggestion, done.

@Pterosaur Pterosaur changed the title Add remote database logic for dpu [database]: Update the DPU database service configuration Apr 3, 2024
@Pterosaur
Refactor remote db
9b7d70c 
Signed-off-by: Ze Gan <[email protected]>
@Pterosaur Pterosaur mentioned this pull request Apr 3, 2024
Merged
@Pterosaur Pterosaur marked this pull request as ready for review April 3, 2024 10:18
@Pterosaur Pterosaur requested a review from lguohan as a code owner April 3, 2024 10:18
@Pterosaur Pterosaur requested a review from ganglyu April 3, 2024 10:18
@Pterosaur Pterosaur marked this pull request as draft April 3, 2024 10:18
@Pterosaur Pterosaur marked this pull request as ready for review April 3, 2024 13:42
@Pterosaur Pterosaur requested a review from prgeor April 7, 2024 03:14
@ganglyu
Copy link
Contributor

ganglyu commented Apr 8, 2024

For gnmi and other containers, the /var/run/redis directory is mounted by default, but the /var/run/redisdpux is not mounted. And then we can't access database configuration for dpu.

@Pterosaur Pterosaur force-pushed the dpu_remote_db branch 2 times, most recently from 7db4a4d to 9864aec Compare April 8, 2024 06:59
@Pterosaur
Copy link
Contributor Author

For gnmi and other containers, the /var/run/redis directory is mounted by default, but the /var/run/redisdpux is not mounted. And then we can't access database configuration for dpu.

Done

qiluo-msft
qiluo-msft reviewed Apr 9, 2024
View reviewed changes
dockers/docker-database/database_config.json.j2 Outdated
@@ -102,23 +111,23 @@
"DPU_APPL_DB" : {
"id" : 15,
"separator": ":",
"instance" : "redis",
"instance" : {% if include_remote_db%} "remote_redis" {% else %} "redis" {% endif %},
Copy link
Collaborator

@qiluo-msft qiluo-msft Apr 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

%

Do you want to add a blank before %? #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

@Pterosaur
fix indent
10fd6e5 
Signed-off-by: Ze Gan <[email protected]>
@qiluo-msft qiluo-msft merged commit 5e217a6 into sonic-net:master Apr 9, 2024
19 checks passed
@oleksandrivantsiv oleksandrivantsiv mentioned this pull request Jul 16, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@qiluo-msft qiluo-msft qiluo-msft approved these changes

@ganglyu ganglyu ganglyu approved these changes

@lguohan lguohan Awaiting requested review from lguohan lguohan is a code owner

@prgeor prgeor Awaiting requested review from prgeor

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Pterosaur @ganglyu @qiluo-msft @prgeor