Skip to content

Commit d9770a6

Browse files
committed
[202405][FRR] Fixing FRR to make route node lock
1 parent 50ddef8 commit d9770a6

File tree

2 files changed

+96
-0
lines changed

2 files changed

+96
-0
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
From 402665769dc5563c246003720e75c3d6244a88eb Mon Sep 17 00:00:00 2001
2+
From: "Barry Friedman (friedman)" <friedman@cisco.com>
3+
Date: Wed, 9 Oct 2024 18:01:00 +0000
4+
Subject: [PATCH 1/2] lib: make route_node lock atomic
5+
6+
Multiple threads walking a route table perform read-modify-write
7+
operations on the lock variable. This can result in a wrong lock count
8+
which wrongly triggers a route_node_delete(). Making the access to the
9+
lock variable atomic should ensure the lock is correct.
10+
11+
Signed-off-by: Barry Friedman (friedman) <friedman@cisco.com>
12+
---
13+
lib/table.h | 6 +++---
14+
1 file changed, 3 insertions(+), 3 deletions(-)
15+
16+
diff --git a/lib/table.h b/lib/table.h
17+
index 5dec69ee7e..89993dd279 100644
18+
--- a/lib/table.h
19+
+++ b/lib/table.h
20+
@@ -129,7 +129,7 @@ struct route_table {
21+
struct route_node *table_rdonly(link[2]); \
22+
\
23+
/* Lock of this radix */ \
24+
- unsigned int table_rdonly(lock); \
25+
+ _Atomic unsigned int table_rdonly(lock); \
26+
\
27+
struct rn_hash_node_item nodehash; \
28+
/* Each node of route. */ \
29+
@@ -244,7 +244,7 @@ extern void route_table_iter_cleanup(route_table_iter_t *iter);
30+
/* Lock node. */
31+
static inline struct route_node *route_lock_node(struct route_node *node)
32+
{
33+
- (*(unsigned *)&node->lock)++;
34+
+ atomic_fetch_add_explicit(&node->lock, 1, memory_order_relaxed);
35+
return node;
36+
}
37+
38+
@@ -252,7 +252,7 @@ static inline struct route_node *route_lock_node(struct route_node *node)
39+
static inline void route_unlock_node(struct route_node *node)
40+
{
41+
assert(node->lock > 0);
42+
- (*(unsigned *)&node->lock)--;
43+
+ atomic_fetch_sub_explicit(&node->lock, 1, memory_order_relaxed);
44+
45+
if (node->lock == 0)
46+
route_node_delete(node);
47+
--
48+
2.43.2
49+
50+
51+
From bf96a5a334ea1e953bf98a92f6a942335f49700b Mon Sep 17 00:00:00 2001
52+
From: "Barry Friedman (friedman)" <friedman@cisco.com>
53+
Date: Fri, 11 Oct 2024 22:02:51 +0000
54+
Subject: [PATCH 2/2] lib: Fix atomic node lock warnings with enable-dev-build
55+
56+
The route_node lock variable type adds a const qualifier when
57+
--enable-dev-build is configured. This is to generate warnings
58+
in case client code tries to modify this variable. As a result
59+
the functions route_lock_node() and route_unlock_node() need to
60+
cast away the const qualifier to prevent the compile warnings.
61+
When they _Atomic qualifier was added I wrongly removed the
62+
cast. This change adds it back to prevent the warnings.
63+
64+
Signed-off-by: Barry Friedman (friedman) <friedman@cisco.com>
65+
---
66+
lib/table.h | 6 ++++--
67+
1 file changed, 4 insertions(+), 2 deletions(-)
68+
69+
diff --git a/lib/table.h b/lib/table.h
70+
index 89993dd279..8023f8aa30 100644
71+
--- a/lib/table.h
72+
+++ b/lib/table.h
73+
@@ -244,7 +244,8 @@ extern void route_table_iter_cleanup(route_table_iter_t *iter);
74+
/* Lock node. */
75+
static inline struct route_node *route_lock_node(struct route_node *node)
76+
{
77+
- atomic_fetch_add_explicit(&node->lock, 1, memory_order_relaxed);
78+
+ atomic_fetch_add_explicit((_Atomic unsigned *)&node->lock, 1,
79+
+ memory_order_relaxed);
80+
return node;
81+
}
82+
83+
@@ -252,7 +253,8 @@ static inline struct route_node *route_lock_node(struct route_node *node)
84+
static inline void route_unlock_node(struct route_node *node)
85+
{
86+
assert(node->lock > 0);
87+
- atomic_fetch_sub_explicit(&node->lock, 1, memory_order_relaxed);
88+
+ atomic_fetch_sub_explicit((_Atomic unsigned *)&node->lock, 1,
89+
+ memory_order_relaxed);
90+
91+
if (node->lock == 0)
92+
route_node_delete(node);
93+
--
94+
2.43.2
95+

src/sonic-frr/patch/series

+1
Original file line numberDiff line numberDiff line change
@@ -50,3 +50,4 @@
5050
0050-bgpd-backpressure-Avoid-use-after-free.patch
5151
0051-bgpd-backpressure-fix-ret-value-evpn_route_select_in.patch
5252
0052-bgpd-backpressure-log-error-for-evpn-when-route-inst.patch
53+
0053-make-route-node-lock-atomic.patch

0 commit comments

Comments
 (0)