Skip to content

Commit b283847

Browse files
authored
[Bgpcfgd] Add dependency callback logic in SRv6 Manager to handle out-of-order table processing (sonic-net#794)
<!-- Please make sure you've read and understood our contributing guidelines: https://github.com/Azure/SONiC/blob/gh-pages/CONTRIBUTING.md failure_prs.log skip_prs.log Make sure all your commits include a signature generated with `git commit -s` ** If this is a bug fix, make sure your description includes "fixes #xxxx", or "closes #xxxx" or "resolves #xxxx" Please provide the following information: --> #### Why I did it Previously, if the SRv6 manager received a config update that added a SRV6_MY_SID entry before its corresponding LOCATOR entry added, the SRv6 manager would not process the SRV6_MY_SID entry and print an ERR syslog. However, during testing, NVIDIA team found that it is possible to have such a scenario in normal operations, especially during config reload. So, we decide to add a dependency-based caching mechanism to handle out-of-order table processing. This is the first use case where we will dynamically add/remove the dependency, so we also define a new unsubscribe function for the directory class. The reason why we need to remove the dependency in managers_srv6 is because the on_deps_change function will only process the cache when all dependencies are satisfied. If a locator is removed and the corresponding dependency is not removed, then the on_deps_change will never get to the satisfied condition. Fix sonic-net#21826 (Issue) ##### Work item tracking - Microsoft ADO **(number only)**: #### How I did it #### How to verify it <!-- If PR needs to be backported, then the PR must be tested against the base branch and the earliest backport release branch and provide tested image version on these two branches. For example, if the PR is requested for master, 202211 and 202012, then the requester needs to provide test results on master and 202012. --> #### Which release branch to backport (provide reason below if selected) <!-- - Note we only backport fixes to a release branch, *not* features! - Please also provide a reason for the backporting below. - e.g. - [x] 202006 --> - [ ] 201811 - [ ] 201911 - [ ] 202006 - [ ] 202012 - [ ] 202106 - [ ] 202111 - [ ] 202205 - [ ] 202211 - [ ] 202305 - [x] 202412 #### Tested branch (Please provide the tested image version) <!-- - Please provide tested image version - e.g. - [x] 20201231.100 --> - [x] 202412 #### Description for the changelog <!-- Write a short (one line) summary that describes the changes in this pull request for inclusion in the changelog: --> <!-- Ensure to add label/tag for the feature raised. example - PR#2174 under sonic-utilities repo. where, Generic Config and Update feature has been labelled as GCU. --> #### Link to config_db schema for YANG module changes <!-- Provide a link to config_db schema for the table for which YANG model is defined Link should point to correct section on https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-yang-models/doc/Configuration.md --> #### A picture of a cute animal (not mandatory but encouraged)
1 parent 7ecf9d7 commit b283847

3 files changed

Lines changed: 43 additions & 11 deletions

File tree

src/sonic-bgpcfgd/bgpcfgd/directory.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,4 +156,11 @@ def subscribe(self, deps, handler):
156156
"""
157157
for db, table, path in deps:
158158
slot = self.get_slot_name(db, table)
159-
self.notify[slot][path].append(handler)
159+
self.notify[slot][path].append(handler)
160+
161+
def unsubscribe(self, deps):
162+
for db, table, path in deps:
163+
slot = self.get_slot_name(db, table)
164+
if slot in self.notify:
165+
if path in self.notify[slot]:
166+
del self.notify[slot][path]

src/sonic-bgpcfgd/bgpcfgd/managers_srv6.py

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ def __init__(self, common_objs, db, table):
2222
"""
2323
super(SRv6Mgr, self).__init__(
2424
common_objs,
25-
[],
25+
set(),
2626
db,
2727
table,
2828
)
@@ -59,9 +59,11 @@ def sids_set_handler(self, key, data):
5959
prefix_len = int(ip_prefix.split("/")[1])
6060

6161
if not self.directory.path_exist(self.db_name, "SRV6_MY_LOCATORS", locator_name):
62-
log_err("Found a SRv6 SID config entry with a locator that does not exist: {} | {}".format(key, data))
62+
log_warn("Found a SRv6 SID config entry with a locator that does not exist yet: {} | {}".format(key, data))
63+
self.deps.add((self.db_name, "SRV6_MY_LOCATORS", locator_name))
64+
self.directory.subscribe([(self.db_name, "SRV6_MY_LOCATORS", locator_name)], self.on_deps_change)
6365
return False
64-
66+
6567
locator = self.directory.get(self.db_name, "SRV6_MY_LOCATORS", locator_name)
6668
if locator.block_len + locator.node_len > prefix_len:
6769
log_err("Found a SRv6 SID config entry with an invalid prefix length {} | {}".format(key, data))
@@ -70,11 +72,11 @@ def sids_set_handler(self, key, data):
7072
if 'action' not in data:
7173
log_err("Found a SRv6 SID config entry that does not specify action: {} | {}".format(key, data))
7274
return False
73-
75+
7476
if data['action'] not in supported_SRv6_behaviors:
7577
log_err("Found a SRv6 SID config entry associated with unsupported action: {} | {}".format(key, data))
7678
return False
77-
79+
7880
sid = SID(locator_name, ip_prefix, data) # the information in data will be parsed into SID's attributes
7981

8082
cmd_list = ['segment-routing', 'srv6', 'static-sids']
@@ -102,6 +104,9 @@ def locators_del_handler(self, key):
102104
self.cfg_mgr.push_list(cmd_list)
103105
log_debug("{} SRv6 static configuration {}|{} is scheduled for updates. {}".format(self.db_name, self.table_name, key, str(cmd_list)))
104106
self.directory.remove(self.db_name, self.table_name, key)
107+
if (self.db_name, "SRV6_MY_LOCATORS", locator_name) in self.deps:
108+
self.deps.remove((self.db_name, "SRV6_MY_LOCATORS", locator_name))
109+
self.directory.unsubscribe([(self.db_name, "SRV6_MY_LOCATORS", locator_name)])
105110

106111
def sids_del_handler(self, key):
107112
locator_name = key.split("|")[0]
@@ -134,7 +139,7 @@ class SID:
134139
def __init__(self, locator, ip_prefix, data):
135140
self.locator_name = locator
136141
self.ip_prefix = ip_prefix
137-
142+
138143
self.action = data['action']
139144
self.decap_vrf = data['decap_vrf'] if 'decap_vrf' in data else DEFAULT_VRF
140145
self.adj = data['adj'].split(',') if 'adj' in data else []

src/sonic-bgpcfgd/tests/test_srv6.py

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
from unittest.mock import MagicMock, patch
1+
from unittest.mock import MagicMock
2+
import time
23

34
from bgpcfgd.directory import Directory
45
from bgpcfgd.template import TemplateFabric
@@ -110,7 +111,7 @@ def test_uDT46_add_vrf1():
110111
def test_uN_del():
111112
loc_mgr, sid_mgr = constructor()
112113
assert loc_mgr.set_handler("loc1", {'prefix': 'fcbb:bbbb:1::'})
113-
114+
114115
# add uN function first
115116
assert sid_mgr.set_handler("loc1|FCBB:BBBB:1::/48", {
116117
'action': 'uN'
@@ -130,7 +131,7 @@ def test_uN_del():
130131
def test_uDT46_del_vrf1():
131132
loc_mgr, sid_mgr = constructor()
132133
assert loc_mgr.set_handler("loc1", {'prefix': 'fcbb:bbbb:1::'})
133-
134+
134135
# add a uN action first to make the uDT46 action not the last function
135136
assert sid_mgr.set_handler("loc1|FCBB:BBBB:1::/48", {
136137
'action': 'uN'
@@ -162,4 +163,23 @@ def test_invalid_add():
162163
'action': 'uN'
163164
}), expected_ret=False, expected_cmds=[])
164165

165-
assert not sid_mgr.directory.path_exist(sid_mgr.db_name, sid_mgr.table_name, "loc2|fcbb:bbbb:21:f1::\\64")
166+
assert not sid_mgr.directory.path_exist(sid_mgr.db_name, sid_mgr.table_name, "loc2|fcbb:bbbb:21:f1::\\64")
167+
168+
def test_out_of_order_add():
169+
loc_mgr, sid_mgr = constructor()
170+
loc_mgr.cfg_mgr.push_list = MagicMock()
171+
sid_mgr.cfg_mgr.push_list = MagicMock()
172+
173+
# add the sid first
174+
sid_mgr.handler(op='SET', key="loc2|FCBB:BBBB:21::/48", data={'action': 'uN'})
175+
176+
# verify that the sid is not added
177+
assert not sid_mgr.directory.path_exist(sid_mgr.db_name, sid_mgr.table_name, "loc2|fcbb:bbbb:21::\\48")
178+
179+
# add the locator
180+
loc_mgr.handler(op='SET', key="loc2", data={'prefix': 'fcbb:bbbb:21::'})
181+
182+
time.sleep(3)
183+
184+
# verify that the sid is added after locator config was there
185+
assert sid_mgr.directory.path_exist(sid_mgr.db_name, sid_mgr.table_name, "loc2|fcbb:bbbb:21::\\48")

0 commit comments

Comments
 (0)