Skip to content

Commit 4801fc4

Browse files
donaldsharpdonaldsharp
committed
bgpd: Allow network XXX to work with bgp suppress-fib-pending
When bgp is using `bgp suppress-fib-pending` and the end operator is using network statements, bgp was not sending the network'ed prefix'es to it's peers. Fix this. Also update the test cases for bgp_suppress_fib to test this new corner case( I am sure that there are going to be others that will need to be added ). Fixes: #12112 Signed-off-by: Donald Sharp <sharpd@nvidia.com>
1 parent 8460069 commit 4801fc4

7 files changed

Lines changed: 59 additions & 7 deletions

File tree

‎bgpd/bgp_route.h‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -608,19 +608,35 @@ static inline bool bgp_check_advertise(struct bgp *bgp, struct bgp_dest *dest)
608608
*/
609609
static inline bool bgp_check_withdrawal(struct bgp *bgp, struct bgp_dest *dest)
610610
{
611-
struct bgp_path_info *pi;
611+
struct bgp_path_info *pi, *selected = NULL;
612612

613613
if (!BGP_SUPPRESS_FIB_ENABLED(bgp))
614614
return false;
615615

616616
for (pi = bgp_dest_get_bgp_path_info(dest); pi; pi = pi->next) {
617-
if (CHECK_FLAG(pi->flags, BGP_PATH_SELECTED))
617+
if (CHECK_FLAG(pi->flags, BGP_PATH_SELECTED)) {
618+
selected = pi;
618619
continue;
620+
}
619621

620622
if (pi->sub_type != BGP_ROUTE_NORMAL)
621623
return true;
622624
}
623625

626+
/*
627+
* pi is selected and bgp is dealing with a static route
628+
* ( ie a network statement of some sort ). FIB installed
629+
* is irrelevant
630+
*
631+
* I am not sure what the above for loop is wanted in this
632+
* manner at this point. But I do know that if I have
633+
* a static route that is selected and it's the one
634+
* being checked for should I withdrawal we do not
635+
* want to withdraw the route on installation :)
636+
*/
637+
if (selected && selected->sub_type == BGP_ROUTE_STATIC)
638+
return false;
639+
624640
if (CHECK_FLAG(dest->flags, BGP_NODE_FIB_INSTALLED))
625641
return false;
626642

‎tests/topotests/bgp_suppress_fib/r1/bgp_ipv4_allowas.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
],
3333
"peer":{
3434
"peerId":"10.0.0.2",
35-
"routerId":"10.0.0.9",
35+
"routerId":"60.0.0.1",
3636
"type":"external"
3737
}
3838
}

‎tests/topotests/bgp_suppress_fib/r2/bgp_ipv4_allowas.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@
6161
],
6262
"peer":{
6363
"peerId":"0.0.0.0",
64-
"routerId":"10.0.0.9"
64+
"routerId":"60.0.0.1"
6565
}
6666
}
6767
]

‎tests/topotests/bgp_suppress_fib/r2/bgpd.conf‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,5 @@ router bgp 2
77
bgp suppress-fib-pending
88
neighbor 10.0.0.1 remote-as 1
99
neighbor 10.0.0.10 remote-as 3
10+
address-family ipv4 uni
11+
network 60.0.0.0/24

‎tests/topotests/bgp_suppress_fib/r2/zebra.conf‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
11
!
2+
interface lo
3+
ip address 60.0.0.1/24
4+
!
25
interface r2-eth0
36
ip address 10.0.0.2/30
47
!

‎tests/topotests/bgp_suppress_fib/r3/v4_route3.json‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
{
2+
"60.0.0.0/24":[
3+
{
4+
"prefix":"60.0.0.0/24",
5+
"protocol":"bgp",
6+
"selected":true,
7+
"destSelected":true,
8+
"distance":20,
9+
"metric":0,
10+
"installed":true,
11+
"table":254,
12+
"nexthops":[
13+
{
14+
"fib":true,
15+
"ip":"10.0.0.9",
16+
"afi":"ipv4",
17+
"interfaceName":"r3-eth0",
18+
"active":true
19+
}
20+
]
21+
}
22+
]
23+
}

‎tests/topotests/bgp_suppress_fib/test_bgp_suppress_fib.py‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,8 +84,6 @@ def test_bgp_route():
8484

8585
r3 = tgen.gears["r3"]
8686

87-
sleep(5)
88-
8987
json_file = "{}/r3/v4_route.json".format(CWD)
9088
expected = json.loads(open(json_file).read())
9189

@@ -95,7 +93,7 @@ def test_bgp_route():
9593
"show ip route 40.0.0.0 json",
9694
expected,
9795
)
98-
_, result = topotest.run_and_expect(test_func, None, count=2, wait=0.5)
96+
_, result = topotest.run_and_expect(test_func, None, count=20, wait=0.5)
9997
assertmsg = '"r3" JSON output mismatches'
10098
assert result is None, assertmsg
10199

@@ -112,6 +110,16 @@ def test_bgp_route():
112110
assertmsg = '"r3" JSON output mismatches'
113111
assert result is None, assertmsg
114112

113+
json_file = "{}/r3/v4_route3.json".format(CWD)
114+
expected = json.loads(open(json_file).read())
115+
116+
test_func = partial(
117+
topotest.router_json_cmp,
118+
r3,
119+
"show ip route 10.0.0.3 json",
120+
expected,
121+
)
122+
_, result = topotest.run_and_expect(test_func, None, count=3, wait=0.5)
115123

116124
def test_bgp_better_admin_won():
117125
"A better Admin distance protocol may come along and knock us out"

0 commit comments

Comments
 (0)