-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Add remote dpu db to bfdmon. #22705
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
base: master
Are you sure you want to change the base?
Add remote dpu db to bfdmon. #22705
Conversation
/azp run Azure.sonic-buildimage |
Azure Pipelines successfully started running 1 pipeline(s). |
src/sonic-bgpcfgd/bfdmon/bfdmon.py
Outdated
except: | ||
syslog.syslog(syslog.LOG_ERR, "*ERROR* Failed to read connection info for remote dpu database from: {}".format(cmd)) | ||
return (None, None, None) | ||
remote_db = swsscommon.DBConnector(swsscommon.DPU_STATE_DB, redis_hostname, int(redis_port), 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please investigate initializing DBConnector by dbname, which implements the functionality of this function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
src/sonic-bgpcfgd/bfdmon/bfdmon.py
Outdated
self.remote_status_table, \ | ||
self.remote_table = self.connect_to_remote_db() | ||
|
||
def connect_to_remote_db(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hi @r12f, this makes software bfd tied to smartswitch if we use dpu_state_db. And what' the purpose to check bfd session state from hamgrd? If it is to check the reachability from the npu to the dpu, should we check the bfd session from npu side?
/azp run Azure.sonic-buildimage |
Azure Pipelines successfully started running 1 pipeline(s). |
("v4_bfd_up_sessions", json.dumps(list(self.local_v4_peers))), | ||
("v6_bfd_up_sessions", json.dumps(list(self.local_v6_peers))) | ||
("v4_bfd_up_sessions", json.dumps(list(self.local_v4_peers)).strip("[]")), | ||
("v6_bfd_up_sessions", json.dumps(list(self.local_v6_peers)).strip("[]")) | ||
] | ||
self.table.set("", values) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to write into both STATE_DB/DPU_BFD_PROBE_STATE and DPU_STATE_DB/DASH_BFD_PROBE_STATE? The former is for DPU and the data format is for hamgrd to consume. Since hamgrd will use the latter. I see no need to have the first.
Why I did it
To bring DPU software BFD (bfdmon) database updates inline with https://github.com/sonic-net/SONiC/blob/4fc4f481e24414eb3f5c6c557842723480f6c9cd/doc/smart-switch/smart-switch-database-architecture/smart-switch-database-design.md#architecture-design. Currently the software BFD session info is stored in the local STATE_DB; adding these changes for software BFD session status to be sent to the remote DPU_STATE_DB.
Work item tracking
How I did it
Added connection to DPU_STATE_DB to update DASH_BFD_PROBE_STATE with DPU software BFD session info in bfdmon.py
How to verify it
Loaded on Smartswitch DPU and checked that the software BFD session entries are added/deleted in DPU_STATE_DB for the corresponding DPU in switch SONiC.
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)