matthew/sonic-buildimage
Archived
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[BGP] support BGP pending FIB suppression (#12853)

Browse Source 
Signed-off-by: Stepan Blyschak stepanb@nvidia.com

DEPENDS: #12852

Why I did it
To support BGP pending FIB suppression.

How I did it
I backported patches from FRR 8.4 feature that allows communicating ASIC route status back to FRR.
Also, added a new field in DEVICE_METADATA YANG model table. Added UT for YANG model changes.

How to verify it
Run on the switch.
This commit is contained in:
Stepan Blyshchak 2023-04-20 14:56:13 +03:00 committed by GitHub
parent b40896b3ba
commit 04099f075d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
42 changed files with 1660 additions and 56 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
dockers/docker-fpm-frr/frr/bgpd/bgpd.main.conf.j2
View File

@ -66,6 +66,7 @@ router bgp {{ DEVICE_METADATA['localhost']['bgp_asn'] }}
!
{% block bgp_init %}
bgp log-neighbor-changes
bgp suppress-fib-pending
no bgp default ipv4-unicast
no bgp ebgp-requires-policy
{% if (DEVICE_METADATA is defined) and ('localhost' in DEVICE_METADATA) and ('subtype' in DEVICE_METADATA['localhost']) and (DEVICE_METADATA['localhost']['subtype'].lower() == 'dualtor') %}

2
dockers/docker-fpm-frr/frr/supervisord/supervisord.conf.j2
View File

@ -30,7 +30,7 @@ stderr_logfile=syslog
dependent_startup=true
[program:zebra]
command=/usr/lib/frr/zebra -A 127.0.0.1 -s 90000000 -M dplane_fpm_nl -M snmp
command=/usr/lib/frr/zebra -A 127.0.0.1 -s 90000000 -M dplane_fpm_nl -M snmp --asic-offload=notify_on_offload
priority=4
autostart=false
autorestart=false

2
platform/vs/docker-sonic-vs/supervisord.conf.j2
View File

@ -164,7 +164,7 @@ environment=ASAN_OPTIONS="log_path=/var/log/asan/teammgrd-asan.log{{ asan_extra_
{% endif %}
[program:zebra]
command=/usr/lib/frr/zebra -A 127.0.0.1 -s 90000000 -M dplane_fpm_nl
command=/usr/lib/frr/zebra -A 127.0.0.1 -s 90000000 -M dplane_fpm_nl --asic-offload=notify_on_offload
priority=13
autostart=false
autorestart=false

5
src/sonic-bgpcfgd/bgpcfgd/managers_bgp.py
View File

@ -304,10 +304,11 @@ class BGPPeerMgrBase(Manager):
:return: True if no errors, False if there are errors
"""
bgp_asn = self.directory.get_slot("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME)["localhost"]["bgp_asn"]
enable_bgp_suppress_fib_pending_cmd = 'bgp suppress-fib-pending'
if vrf == 'default':
cmd = ('router bgp %s\n' % bgp_asn) + cmd
cmd = ('router bgp %s\n %s\n' % (bgp_asn, enable_bgp_suppress_fib_pending_cmd)) + cmd
else:
cmd = ('router bgp %s vrf %s\n' % (bgp_asn, vrf)) + cmd
cmd = ('router bgp %s vrf %s\n %s\n' % (bgp_asn, vrf, enable_bgp_suppress_fib_pending_cmd)) + cmd
self.cfg_mgr.push(cmd)
return True

1
src/sonic-bgpcfgd/tests/data/sonic-cfggen/bgpd.conf.j2/all.conf
View File

@ -55,6 +55,7 @@ route-map HIDE_INTERNAL permit 20
router bgp 55555
!
bgp log-neighbor-changes
bgp suppress-fib-pending
no bgp default ipv4-unicast
no bgp ebgp-requires-policy
!

1
src/sonic-bgpcfgd/tests/data/sonic-cfggen/bgpd.main.conf.j2/all.conf
View File

@ -34,6 +34,7 @@ route-map HIDE_INTERNAL permit 10
router bgp 55555
!
bgp log-neighbor-changes
bgp suppress-fib-pending
no bgp default ipv4-unicast
no bgp ebgp-requires-policy
!

1
src/sonic-bgpcfgd/tests/data/sonic-cfggen/bgpd.main.conf.j2/base.conf
View File

@ -12,6 +12,7 @@ ip prefix-list PL_LoopbackV4 permit 55.55.55.55/32
router bgp 55555
!
bgp log-neighbor-changes
bgp suppress-fib-pending
no bgp default ipv4-unicast
no bgp ebgp-requires-policy
!

1
src/sonic-bgpcfgd/tests/data/sonic-cfggen/bgpd.main.conf.j2/defaults.conf
View File

@ -34,6 +34,7 @@ route-map HIDE_INTERNAL permit 10
router bgp 55555
!
bgp log-neighbor-changes
bgp suppress-fib-pending
no bgp default ipv4-unicast
no bgp ebgp-requires-policy
!

1
src/sonic-bgpcfgd/tests/data/sonic-cfggen/bgpd.main.conf.j2/ipv6_lo.conf
View File

@ -14,6 +14,7 @@ ipv6 prefix-list PL_LoopbackV6 permit fc00::1/128
router bgp 55555
!
bgp log-neighbor-changes
bgp suppress-fib-pending
no bgp default ipv4-unicast
no bgp ebgp-requires-policy
!

1
src/sonic-bgpcfgd/tests/data/sonic-cfggen/bgpd.main.conf.j2/packet_chassis.conf
View File

@ -34,6 +34,7 @@ route-map HIDE_INTERNAL permit 10
router bgp 55555
!
bgp log-neighbor-changes
bgp suppress-fib-pending
no bgp default ipv4-unicast
no bgp ebgp-requires-policy
!

1
src/sonic-bgpcfgd/tests/data/sonic-cfggen/bgpd.main.conf.j2/voq_chassis.conf
View File

@ -34,6 +34,7 @@ route-map HIDE_INTERNAL permit 10
router bgp 55555
!
bgp log-neighbor-changes
bgp suppress-fib-pending
no bgp default ipv4-unicast
no bgp ebgp-requires-policy
!

1
src/sonic-bgpcfgd/tests/data/sonic-cfggen/frr.conf.j2/all.conf
View File

@ -70,6 +70,7 @@ route-map HIDE_INTERNAL permit 10
router bgp 55555
!
bgp log-neighbor-changes
bgp suppress-fib-pending
no bgp default ipv4-unicast
no bgp ebgp-requires-policy
!

1
src/sonic-config-engine/tests/sample_output/py2/bgpd_frr.conf
View File

@ -42,6 +42,7 @@ ip prefix-list LOCAL_VLAN_IPV4_PREFIX seq 10 permit 192.168.0.0/27
router bgp 65100
!
bgp log-neighbor-changes
bgp suppress-fib-pending
no bgp default ipv4-unicast
no bgp ebgp-requires-policy
!

1
src/sonic-config-engine/tests/sample_output/py2/bgpd_frr_backend_asic.conf
View File

@ -53,6 +53,7 @@ route-map HIDE_INTERNAL permit 10
router bgp 65100
!
bgp log-neighbor-changes
bgp suppress-fib-pending
no bgp default ipv4-unicast
no bgp ebgp-requires-policy
!

1
src/sonic-config-engine/tests/sample_output/py2/bgpd_frr_dualtor.conf
View File

@ -42,6 +42,7 @@ ip prefix-list LOCAL_VLAN_IPV4_PREFIX seq 10 permit 192.168.0.0/27
router bgp 65100
!
bgp log-neighbor-changes
bgp suppress-fib-pending
no bgp default ipv4-unicast
no bgp ebgp-requires-policy
coalesce-time 10000

1
src/sonic-config-engine/tests/sample_output/py2/bgpd_frr_frontend_asic.conf
View File

@ -53,6 +53,7 @@ route-map HIDE_INTERNAL permit 10
router bgp 65100
!
bgp log-neighbor-changes
bgp suppress-fib-pending
no bgp default ipv4-unicast
no bgp ebgp-requires-policy
!

1
src/sonic-config-engine/tests/sample_output/py2/frr.conf
View File

@ -61,6 +61,7 @@ ip prefix-list LOCAL_VLAN_IPV4_PREFIX seq 10 permit 192.168.0.0/27
router bgp 65100
!
bgp log-neighbor-changes
bgp suppress-fib-pending
no bgp default ipv4-unicast
no bgp ebgp-requires-policy
!

1
src/sonic-config-engine/tests/sample_output/py2/t2-chassis-fe-bgpd.conf
View File

@ -58,6 +58,7 @@ ip prefix-list PL_LoopbackV4 permit 4.0.0.0/32
router bgp 4000
!
bgp log-neighbor-changes
bgp suppress-fib-pending
no bgp default ipv4-unicast
no bgp ebgp-requires-policy
!

1
src/sonic-config-engine/tests/sample_output/py3/bgpd_frr.conf
View File

@ -42,6 +42,7 @@ ip prefix-list LOCAL_VLAN_IPV4_PREFIX seq 10 permit 192.168.200.0/27
router bgp 65100
!
bgp log-neighbor-changes
bgp suppress-fib-pending
no bgp default ipv4-unicast
no bgp ebgp-requires-policy
!

1
src/sonic-config-engine/tests/sample_output/py3/bgpd_frr_backend_asic.conf
View File

@ -53,6 +53,7 @@ route-map HIDE_INTERNAL permit 10
router bgp 65100
!
bgp log-neighbor-changes
bgp suppress-fib-pending
no bgp default ipv4-unicast
no bgp ebgp-requires-policy
!

1
src/sonic-config-engine/tests/sample_output/py3/bgpd_frr_dualtor.conf
View File

@ -42,6 +42,7 @@ ip prefix-list LOCAL_VLAN_IPV4_PREFIX seq 10 permit 192.168.200.0/27
router bgp 65100
!
bgp log-neighbor-changes
bgp suppress-fib-pending
no bgp default ipv4-unicast
no bgp ebgp-requires-policy
coalesce-time 10000

1
src/sonic-config-engine/tests/sample_output/py3/bgpd_frr_frontend_asic.conf
View File

@ -53,6 +53,7 @@ route-map HIDE_INTERNAL permit 10
router bgp 65100
!
bgp log-neighbor-changes
bgp suppress-fib-pending
no bgp default ipv4-unicast
no bgp ebgp-requires-policy
!

1
src/sonic-config-engine/tests/sample_output/py3/frr.conf
View File

@ -61,6 +61,7 @@ ip prefix-list LOCAL_VLAN_IPV4_PREFIX seq 10 permit 192.168.200.0/27
router bgp 65100
!
bgp log-neighbor-changes
bgp suppress-fib-pending
no bgp default ipv4-unicast
no bgp ebgp-requires-policy
!

1
src/sonic-config-engine/tests/sample_output/py3/t2-chassis-fe-bgpd.conf
View File

@ -58,6 +58,7 @@ ip prefix-list PL_LoopbackV4 permit 4.0.0.0/32
router bgp 4000
!
bgp log-neighbor-changes
bgp suppress-fib-pending
no bgp default ipv4-unicast
no bgp ebgp-requires-policy
!

48
src/sonic-frr/patch/0003-Use-vrf_id-for-vrf-not-tabled_id.patch
View File

@ -1,48 +0,0 @@
From 5dba497fb3810f9e5cb4b23bec151ec44d8dcec4 Mon Sep 17 00:00:00 2001
From: Stepan Blyschak <stepanb@nvidia.com>
Date: Mon, 16 Jan 2023 11:24:16 +0000
Subject: [PATCH] Use vrf_id for vrf, not tabled_id
Signed-off-by: Stepan Blyschak <stepanb@nvidia.com>
---
zebra/rt_netlink.c | 24 ++++++++++++++++++------
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/zebra/rt_netlink.c b/zebra/rt_netlink.c
index 24c01b7f5..d4567990e 100644
--- a/zebra/rt_netlink.c
+++ b/zebra/rt_netlink.c
@@ -1969,12 +1969,24 @@ ssize_t netlink_route_multipath_msg_encode(int cmd,
#endif
/* Table corresponding to this route. */
table_id = dplane_ctx_get_table(ctx);
- if (table_id < 256)
- req->r.rtm_table = table_id;
- else {
- req->r.rtm_table = RT_TABLE_UNSPEC;
- if (!nl_attr_put32(&req->n, datalen, RTA_TABLE, table_id))
- return 0;
+ if (!fpm) {
+ if (table_id < 256)
+ req->r.rtm_table = table_id;
+ else {
+ req->r.rtm_table = RT_TABLE_UNSPEC;
+ if (!nl_attr_put32(&req->n, datalen, RTA_TABLE, table_id))
+ return 0;
+ }
+ } else {
+ /* Put vrf if_index instead of table id */
+ vrf_id_t vrf = dplane_ctx_get_vrf(ctx);
+ if (vrf < 256)
+ req->r.rtm_table = vrf;
+ else {
+ req->r.rtm_table = RT_TABLE_UNSPEC;
+ if (!nl_attr_put32(&req->n, datalen, RTA_TABLE, vrf))
+ return 0;
+ }
}
if (IS_ZEBRA_DEBUG_KERNEL)
--
2.30.2

190
src/sonic-frr/patch/0014-bgpd-Allow-network-XXX-to-work-with-bgp-suppress-fib.patch Normal file
View File

@ -0,0 +1,190 @@
From 2ce65073a1c2c1a225488287eb886a76149db0a4 Mon Sep 17 00:00:00 2001
From: Donald Sharp <sharpd@nvidia.com>
Date: Wed, 12 Oct 2022 14:53:21 -0400
Subject: [PATCH 04/11] 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>
(cherry picked from commit 4801fc4670020406fc609dedabc7482d88e3b656)
---
bgpd/bgp_route.h | 20 ++++++++++++++--
.../bgp_suppress_fib/r1/bgp_ipv4_allowas.json | 2 +-
.../bgp_suppress_fib/r2/bgp_ipv4_allowas.json | 2 +-
tests/topotests/bgp_suppress_fib/r2/bgpd.conf | 2 ++
.../topotests/bgp_suppress_fib/r2/zebra.conf | 3 +++
.../bgp_suppress_fib/r3/v4_route3.json | 23 +++++++++++++++++++
.../bgp_suppress_fib/test_bgp_suppress_fib.py | 14 ++++++++---
7 files changed, 59 insertions(+), 7 deletions(-)
create mode 100644 tests/topotests/bgp_suppress_fib/r3/v4_route3.json
diff --git a/bgpd/bgp_route.h b/bgpd/bgp_route.h
index a8ec2dc90..003a6cb79 100644
--- a/bgpd/bgp_route.h
+++ b/bgpd/bgp_route.h
@@ -595,19 +595,35 @@ static inline bool bgp_check_advertise(struct bgp *bgp, struct bgp_dest *dest)
*/
static inline bool bgp_check_withdrawal(struct bgp *bgp, struct bgp_dest *dest)
{
- struct bgp_path_info *pi;
+ struct bgp_path_info *pi, *selected = NULL;
if (!BGP_SUPPRESS_FIB_ENABLED(bgp))
return false;
for (pi = bgp_dest_get_bgp_path_info(dest); pi; pi = pi->next) {
- if (CHECK_FLAG(pi->flags, BGP_PATH_SELECTED))
+ if (CHECK_FLAG(pi->flags, BGP_PATH_SELECTED)) {
+ selected = pi;
continue;
+ }
if (pi->sub_type != BGP_ROUTE_NORMAL)
return true;
}
+ /*
+ * pi is selected and bgp is dealing with a static route
+ * ( ie a network statement of some sort ). FIB installed
+ * is irrelevant
+ *
+ * I am not sure what the above for loop is wanted in this
+ * manner at this point. But I do know that if I have
+ * a static route that is selected and it's the one
+ * being checked for should I withdrawal we do not
+ * want to withdraw the route on installation :)
+ */
+ if (selected && selected->sub_type == BGP_ROUTE_STATIC)
+ return false;
+
if (CHECK_FLAG(dest->flags, BGP_NODE_FIB_INSTALLED))
return false;
diff --git a/tests/topotests/bgp_suppress_fib/r1/bgp_ipv4_allowas.json b/tests/topotests/bgp_suppress_fib/r1/bgp_ipv4_allowas.json
index bc4d0f479..1a5ede276 100644
--- a/tests/topotests/bgp_suppress_fib/r1/bgp_ipv4_allowas.json
+++ b/tests/topotests/bgp_suppress_fib/r1/bgp_ipv4_allowas.json
@@ -32,7 +32,7 @@
],
"peer":{
"peerId":"10.0.0.2",
- "routerId":"10.0.0.9",
+ "routerId":"60.0.0.1",
"type":"external"
}
}
diff --git a/tests/topotests/bgp_suppress_fib/r2/bgp_ipv4_allowas.json b/tests/topotests/bgp_suppress_fib/r2/bgp_ipv4_allowas.json
index 16561ce83..4a35abfd6 100644
--- a/tests/topotests/bgp_suppress_fib/r2/bgp_ipv4_allowas.json
+++ b/tests/topotests/bgp_suppress_fib/r2/bgp_ipv4_allowas.json
@@ -61,7 +61,7 @@
],
"peer":{
"peerId":"0.0.0.0",
- "routerId":"10.0.0.9"
+ "routerId":"60.0.0.1"
}
}
]
diff --git a/tests/topotests/bgp_suppress_fib/r2/bgpd.conf b/tests/topotests/bgp_suppress_fib/r2/bgpd.conf
index ebef2012a..010e86aad 100644
--- a/tests/topotests/bgp_suppress_fib/r2/bgpd.conf
+++ b/tests/topotests/bgp_suppress_fib/r2/bgpd.conf
@@ -7,3 +7,5 @@ router bgp 2
bgp suppress-fib-pending
neighbor 10.0.0.1 remote-as 1
neighbor 10.0.0.10 remote-as 3
+ address-family ipv4 uni
+ network 60.0.0.0/24
\ No newline at end of file
diff --git a/tests/topotests/bgp_suppress_fib/r2/zebra.conf b/tests/topotests/bgp_suppress_fib/r2/zebra.conf
index 443fffc70..6e8bce045 100644
--- a/tests/topotests/bgp_suppress_fib/r2/zebra.conf
+++ b/tests/topotests/bgp_suppress_fib/r2/zebra.conf
@@ -1,4 +1,7 @@
!
+interface lo
+ ip address 60.0.0.1/24
+!
interface r2-eth0
ip address 10.0.0.2/30
!
diff --git a/tests/topotests/bgp_suppress_fib/r3/v4_route3.json b/tests/topotests/bgp_suppress_fib/r3/v4_route3.json
new file mode 100644
index 000000000..ab8c3aa5e
--- /dev/null
+++ b/tests/topotests/bgp_suppress_fib/r3/v4_route3.json
@@ -0,0 +1,23 @@
+{
+ "60.0.0.0/24":[
+ {
+ "prefix":"60.0.0.0/24",
+ "protocol":"bgp",
+ "selected":true,
+ "destSelected":true,
+ "distance":20,
+ "metric":0,
+ "installed":true,
+ "table":254,
+ "nexthops":[
+ {
+ "fib":true,
+ "ip":"10.0.0.9",
+ "afi":"ipv4",
+ "interfaceName":"r3-eth0",
+ "active":true
+ }
+ ]
+ }
+ ]
+}
diff --git a/tests/topotests/bgp_suppress_fib/test_bgp_suppress_fib.py b/tests/topotests/bgp_suppress_fib/test_bgp_suppress_fib.py
index 2c87d9d7b..96a294cae 100644
--- a/tests/topotests/bgp_suppress_fib/test_bgp_suppress_fib.py
+++ b/tests/topotests/bgp_suppress_fib/test_bgp_suppress_fib.py
@@ -84,8 +84,6 @@ def test_bgp_route():
r3 = tgen.gears["r3"]
- sleep(5)
-
json_file = "{}/r3/v4_route.json".format(CWD)
expected = json.loads(open(json_file).read())
@@ -95,7 +93,7 @@ def test_bgp_route():
"show ip route 40.0.0.0 json",
expected,
)
- _, result = topotest.run_and_expect(test_func, None, count=2, wait=0.5)
+ _, result = topotest.run_and_expect(test_func, None, count=20, wait=0.5)
assertmsg = '"r3" JSON output mismatches'
assert result is None, assertmsg
@@ -112,6 +110,16 @@ def test_bgp_route():
assertmsg = '"r3" JSON output mismatches'
assert result is None, assertmsg
+ json_file = "{}/r3/v4_route3.json".format(CWD)
+ expected = json.loads(open(json_file).read())
+
+ test_func = partial(
+ topotest.router_json_cmp,
+ r3,
+ "show ip route 10.0.0.3 json",
+ expected,
+ )
+ _, result = topotest.run_and_expect(test_func, None, count=3, wait=0.5)
def test_bgp_better_admin_won():
"A better Admin distance protocol may come along and knock us out"
--
2.30.2

27
src/sonic-frr/patch/0015-zebra-Return-statements-do-not-use-paranthesis.patch Executable file
View File

@ -0,0 +1,27 @@
From 07a41dcd82ff41b406d0011554d70ce79da68df5 Mon Sep 17 00:00:00 2001
From: Donald Sharp <sharpd@nvidia.com>
Date: Wed, 5 Oct 2022 10:26:07 -0400
Subject: [PATCH 1/8] zebra: Return statements do not use paranthesis
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
---
zebra/zebra_rib.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/zebra/zebra_rib.c b/zebra/zebra_rib.c
index cf1baf077..472e48925 100644
--- a/zebra/zebra_rib.c
+++ b/zebra/zebra_rib.c
@@ -1403,8 +1403,7 @@ static bool rib_route_match_ctx(const struct route_entry *re,
}
done:
-
- return (result);
+ return result;
}
static void zebra_rib_fixup_system(struct route_node *rn)
--
2.30.2

205
src/sonic-frr/patch/0016-zebra-Add-zrouter.asic_notification_nexthop_control.patch Executable file
View File

@ -0,0 +1,205 @@
From 01e267d98c63ee4464f96fead11dda9ee0ad911b Mon Sep 17 00:00:00 2001
From: Donald Sharp <sharpd@nvidia.com>
Date: Wed, 5 Oct 2022 10:04:11 -0400
Subject: [PATCH 2/8] zebra: Add `zrouter.asic_notification_nexthop_control`
Volta submitted notification changes for the dplane that had a
special use case for their system. Volta is no more, the code
is not being actively developed and from talking with ex-Volta
employees there is no current plans to even maintain this code.
Wrap the special handling of nexthops that their asic-dataplane
did in a bit of code to isolate it and allow for future removal,
as that I do not actually believe anyone else is using this code.
Add a CPP_NOTICE several years into the future that will tell us
to remove the code. If someone starts using it then they will
have to notice this variable to set it and hopefully they will
see my CPP_NOTICE to come talk to us. If this is being used then
we can just remove this wrapper.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
---
zebra/zebra_rib.c | 95 +++++++++++++++++++++++++-------------------
zebra/zebra_router.c | 11 +++++
zebra/zebra_router.h | 8 ++++
zebra/zebra_vty.c | 9 +++++
4 files changed, 83 insertions(+), 40 deletions(-)
diff --git a/zebra/zebra_rib.c b/zebra/zebra_rib.c
index 472e48925..abd6e07f9 100644
--- a/zebra/zebra_rib.c
+++ b/zebra/zebra_rib.c
@@ -2215,55 +2215,70 @@ static void rib_process_dplane_notify(struct zebra_dplane_ctx *ctx)
/* Various fib transitions: changed nexthops; from installed to
* not-installed; or not-installed to installed.
*/
- if (start_count > 0 && end_count > 0) {
- if (debug_p)
- zlog_debug(
- "%s(%u:%u):%pRN applied nexthop changes from dplane notification",
- VRF_LOGNAME(vrf), dplane_ctx_get_vrf(ctx),
- dplane_ctx_get_table(ctx), rn);
+ if (zrouter.asic_notification_nexthop_control) {
+ if (start_count > 0 && end_count > 0) {
+ if (debug_p)
+ zlog_debug(
+ "%s(%u:%u):%pRN applied nexthop changes from dplane notification",
+ VRF_LOGNAME(vrf),
+ dplane_ctx_get_vrf(ctx),
+ dplane_ctx_get_table(ctx), rn);
- /* Changed nexthops - update kernel/others */
- dplane_route_notif_update(rn, re,
- DPLANE_OP_ROUTE_UPDATE, ctx);
+ /* Changed nexthops - update kernel/others */
+ dplane_route_notif_update(rn, re,
+ DPLANE_OP_ROUTE_UPDATE, ctx);
- } else if (start_count == 0 && end_count > 0) {
- if (debug_p)
- zlog_debug(
- "%s(%u:%u):%pRN installed transition from dplane notification",
- VRF_LOGNAME(vrf), dplane_ctx_get_vrf(ctx),
- dplane_ctx_get_table(ctx), rn);
+ } else if (start_count == 0 && end_count > 0) {
+ if (debug_p)
+ zlog_debug(
+ "%s(%u:%u):%pRN installed transition from dplane notification",
+ VRF_LOGNAME(vrf),
+ dplane_ctx_get_vrf(ctx),
+ dplane_ctx_get_table(ctx), rn);
- /* We expect this to be the selected route, so we want
- * to tell others about this transition.
- */
- SET_FLAG(re->status, ROUTE_ENTRY_INSTALLED);
+ /* We expect this to be the selected route, so we want
+ * to tell others about this transition.
+ */
+ SET_FLAG(re->status, ROUTE_ENTRY_INSTALLED);
- /* Changed nexthops - update kernel/others */
- dplane_route_notif_update(rn, re, DPLANE_OP_ROUTE_UPDATE, ctx);
+ /* Changed nexthops - update kernel/others */
+ dplane_route_notif_update(rn, re,
+ DPLANE_OP_ROUTE_UPDATE, ctx);
- /* Redistribute, lsp, and nht update */
- redistribute_update(rn, re, NULL);
+ /* Redistribute, lsp, and nht update */
+ redistribute_update(rn, re, NULL);
- } else if (start_count > 0 && end_count == 0) {
- if (debug_p)
- zlog_debug(
- "%s(%u:%u):%pRN un-installed transition from dplane notification",
- VRF_LOGNAME(vrf), dplane_ctx_get_vrf(ctx),
- dplane_ctx_get_table(ctx), rn);
+ } else if (start_count > 0 && end_count == 0) {
+ if (debug_p)
+ zlog_debug(
+ "%s(%u:%u):%pRN un-installed transition from dplane notification",
+ VRF_LOGNAME(vrf),
+ dplane_ctx_get_vrf(ctx),
+ dplane_ctx_get_table(ctx), rn);
- /* Transition from _something_ installed to _nothing_
- * installed.
- */
- /* We expect this to be the selected route, so we want
- * to tell others about this transistion.
- */
- UNSET_FLAG(re->status, ROUTE_ENTRY_INSTALLED);
+ /* Transition from _something_ installed to _nothing_
+ * installed.
+ */
+ /* We expect this to be the selected route, so we want
+ * to tell others about this transistion.
+ */
+ UNSET_FLAG(re->status, ROUTE_ENTRY_INSTALLED);
- /* Changed nexthops - update kernel/others */
- dplane_route_notif_update(rn, re, DPLANE_OP_ROUTE_DELETE, ctx);
+ /* Changed nexthops - update kernel/others */
+ dplane_route_notif_update(rn, re,
+ DPLANE_OP_ROUTE_DELETE, ctx);
- /* Redistribute, lsp, and nht update */
- redistribute_delete(rn, re, NULL);
+ /* Redistribute, lsp, and nht update */
+ redistribute_delete(rn, re, NULL);
+ }
+ }
+
+ if (!zebra_router_notify_on_ack()) {
+ if (CHECK_FLAG(re->flags, ZEBRA_FLAG_OFFLOADED))
+ zsend_route_notify_owner_ctx(ctx, ZAPI_ROUTE_INSTALLED);
+ if (CHECK_FLAG(re->flags, ZEBRA_FLAG_OFFLOAD_FAILED))
+ zsend_route_notify_owner_ctx(ctx,
+ ZAPI_ROUTE_FAIL_INSTALL);
}
/* Make any changes visible for lsp and nexthop-tracking processing */
diff --git a/zebra/zebra_router.c b/zebra/zebra_router.c
index 92a3b9424..d47517bbb 100644
--- a/zebra/zebra_router.c
+++ b/zebra/zebra_router.c
@@ -304,6 +304,17 @@ void zebra_router_init(bool asic_offload, bool notify_on_ack)
zrouter.asic_offloaded = asic_offload;
zrouter.notify_on_ack = notify_on_ack;
+ /*
+ * If you start using asic_notification_nexthop_control
+ * come talk to the FRR community about what you are doing
+ * We would like to know.
+ */
+#if CONFDATE > 20251231
+ CPP_NOTICE(
+ "Remove zrouter.asic_notification_nexthop_control as that it's not being maintained or used");
+#endif
+ zrouter.asic_notification_nexthop_control = false;
+
#ifdef HAVE_SCRIPTING
zebra_script_init();
#endif
diff --git a/zebra/zebra_router.h b/zebra/zebra_router.h
index c0eab9cd6..583bd0038 100644
--- a/zebra/zebra_router.h
+++ b/zebra/zebra_router.h
@@ -209,6 +209,14 @@ struct zebra_router {
*/
bool asic_offloaded;
bool notify_on_ack;
+
+ /*
+ * If the asic is notifying us about successful nexthop
+ * allocation/control. Some developers have made their
+ * asic take control of how many nexthops/ecmp they can
+ * have and will report what is successfull or not
+ */
+ bool asic_notification_nexthop_control;
};
#define GRACEFUL_RESTART_TIME 60
diff --git a/zebra/zebra_vty.c b/zebra/zebra_vty.c
index 0a3d676a9..a7eb09df0 100644
--- a/zebra/zebra_vty.c
+++ b/zebra/zebra_vty.c
@@ -3972,6 +3972,15 @@ DEFUN (show_zebra,
if (zrouter.asic_offloaded)
vty_out(vty, "Asic Offload is being used\n");
+ /*
+ * Do not display this unless someone is actually using it
+ *
+ * Why this distinction? I think this is effectively dead code
+ * and should not be exposed. Maybe someone proves me wrong.
+ */
+ if (zrouter.asic_notification_nexthop_control)
+ vty_out(vty, "ASIC offload and nexthop control is being used");
+
vty_out(vty,
" Route Route Neighbor LSP LSP\n");
vty_out(vty,
--
2.30.2

56
src/sonic-frr/patch/0017-zebra-Re-arrange-fpm_read-to-reduce-code-duplication.patch Executable file
View File

@ -0,0 +1,56 @@
From 88ba9ad950d461847a159b0f4899375ecd23d4b9 Mon Sep 17 00:00:00 2001
From: Donald Sharp <sharpd@nvidia.com>
Date: Wed, 5 Oct 2022 11:28:43 -0400
Subject: [PATCH 3/8] zebra: Re-arrange fpm_read to reduce code duplication
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
---
zebra/dplane_fpm_nl.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/zebra/dplane_fpm_nl.c b/zebra/dplane_fpm_nl.c
index 3b02128c9..8d41f3b07 100644
--- a/zebra/dplane_fpm_nl.c
+++ b/zebra/dplane_fpm_nl.c
@@ -466,13 +466,6 @@ static int fpm_read(struct thread *t)
/* Let's ignore the input at the moment. */
rv = stream_read_try(fnc->ibuf, fnc->socket,
STREAM_WRITEABLE(fnc->ibuf));
- /* We've got an interruption. */
- if (rv == -2) {
- /* Schedule next read. */
- thread_add_read(fnc->fthread->master, fpm_read, fnc,
- fnc->socket, &fnc->t_read);
- return 0;
- }
if (rv == 0) {
atomic_fetch_add_explicit(&fnc->counters.connection_closes, 1,
memory_order_relaxed);
@@ -491,15 +484,21 @@ static int fpm_read(struct thread *t)
FPM_RECONNECT(fnc);
return 0;
}
+
+ /* Schedule the next read */
+ thread_add_read(fnc->fthread->master, fpm_read, fnc, fnc->socket,
+ &fnc->t_read);
+
+ /* We've got an interruption. */
+ if (rv == -2)
+ return 0;
+
stream_reset(fnc->ibuf);
/* Account all bytes read. */
atomic_fetch_add_explicit(&fnc->counters.bytes_read, rv,
memory_order_relaxed);
- thread_add_read(fnc->fthread->master, fpm_read, fnc, fnc->socket,
- &fnc->t_read);
-
return 0;
}
--
2.30.2

78
src/sonic-frr/patch/0018-zebra-Add-dplane_ctx_get-set_flags.patch Executable file
View File

@ -0,0 +1,78 @@
From 77d430e04359417d6fca8226e256ee2845fbbc58 Mon Sep 17 00:00:00 2001
From: Donald Sharp <sharpd@nvidia.com>
Date: Mon, 3 Oct 2022 15:28:48 -0400
Subject: [PATCH 4/8] zebra: Add dplane_ctx_get|set_flags
Zebra needs the ability to pass this data around.
Add it to the dplanes ability to pass.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
zebra: Add a dplane_ctx_set_flags
The dplane_ctx_set_flags call is missing, we will need it. Add it.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
---
zebra/zebra_dplane.c | 17 +++++++++++++++++
zebra/zebra_dplane.h | 2 ++
2 files changed, 19 insertions(+)
diff --git a/zebra/zebra_dplane.c b/zebra/zebra_dplane.c
index 656ebcf3b..5d94e1a0b 100644
--- a/zebra/zebra_dplane.c
+++ b/zebra/zebra_dplane.c
@@ -132,6 +132,8 @@ struct dplane_route_info {
uint32_t zd_mtu;
uint32_t zd_nexthop_mtu;
+ uint32_t zd_flags;
+
/* Nexthop hash entry info */
struct dplane_nexthop_info nhe;
@@ -1323,6 +1325,20 @@ uint16_t dplane_ctx_get_old_instance(const struct zebra_dplane_ctx *ctx)
return ctx->u.rinfo.zd_old_instance;
}
+uint32_t dplane_ctx_get_flags(const struct zebra_dplane_ctx *ctx)
+{
+ DPLANE_CTX_VALID(ctx);
+
+ return ctx->u.rinfo.zd_flags;
+}
+
+void dplane_ctx_set_flags(struct zebra_dplane_ctx *ctx, uint32_t flags)
+{
+ DPLANE_CTX_VALID(ctx);
+
+ ctx->u.rinfo.zd_flags = flags;
+}
+
uint32_t dplane_ctx_get_metric(const struct zebra_dplane_ctx *ctx)
{
DPLANE_CTX_VALID(ctx);
@@ -2358,6 +2374,7 @@ int dplane_ctx_route_init(struct zebra_dplane_ctx *ctx, enum dplane_op_e op,
ctx->zd_table_id = re->table;
+ ctx->u.rinfo.zd_flags = re->flags;
ctx->u.rinfo.zd_metric = re->metric;
ctx->u.rinfo.zd_old_metric = re->metric;
ctx->zd_vrf_id = re->vrf_id;
diff --git a/zebra/zebra_dplane.h b/zebra/zebra_dplane.h
index 1d5518138..3210137c0 100644
--- a/zebra/zebra_dplane.h
+++ b/zebra/zebra_dplane.h
@@ -343,6 +343,8 @@ route_tag_t dplane_ctx_get_old_tag(const struct zebra_dplane_ctx *ctx);
uint16_t dplane_ctx_get_instance(const struct zebra_dplane_ctx *ctx);
void dplane_ctx_set_instance(struct zebra_dplane_ctx *ctx, uint16_t instance);
uint16_t dplane_ctx_get_old_instance(const struct zebra_dplane_ctx *ctx);
+uint32_t dplane_ctx_get_flags(const struct zebra_dplane_ctx *ctx);
+void dplane_ctx_set_flags(struct zebra_dplane_ctx *ctx, uint32_t flags);
uint32_t dplane_ctx_get_metric(const struct zebra_dplane_ctx *ctx);
uint32_t dplane_ctx_get_old_metric(const struct zebra_dplane_ctx *ctx);
uint32_t dplane_ctx_get_mtu(const struct zebra_dplane_ctx *ctx);
--
2.30.2

135
src/sonic-frr/patch/0019-zebra-Rearrange-dplane_ctx_route_init.patch Executable file
View File

@ -0,0 +1,135 @@
From 1ecc146b19ea78a34fc5cba08d55aca72527d7c3 Mon Sep 17 00:00:00 2001
From: Donald Sharp <sharpd@nvidia.com>
Date: Mon, 3 Oct 2022 13:22:22 -0400
Subject: [PATCH 5/8] zebra: Rearrange dplane_ctx_route_init
In order for a future commit to abstract the dplane_ctx_route_init
so that the kernel can use it, let's move some stuff around
and add a dplane_ctx_route_init_basic that can be used by multiple
different paths
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
create a dplane_ctx_route_init_basic so it can be used
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
---
zebra/zebra_dplane.c | 64 +++++++++++++++++++++++++++++---------------
zebra/zebra_dplane.h | 6 +++++
2 files changed, 49 insertions(+), 21 deletions(-)
diff --git a/zebra/zebra_dplane.c b/zebra/zebra_dplane.c
index 5d94e1a0b..05f9c2e08 100644
--- a/zebra/zebra_dplane.c
+++ b/zebra/zebra_dplane.c
@@ -2334,25 +2334,16 @@ static int dplane_ctx_ns_init(struct zebra_dplane_ctx *ctx,
return AOK;
}
-/*
- * Initialize a context block for a route update from zebra data structs.
- */
-int dplane_ctx_route_init(struct zebra_dplane_ctx *ctx, enum dplane_op_e op,
- struct route_node *rn, struct route_entry *re)
+int dplane_ctx_route_init_basic(struct zebra_dplane_ctx *ctx,
+ enum dplane_op_e op, struct route_entry *re,
+ const struct prefix *p,
+ const struct prefix *src_p, afi_t afi,
+ safi_t safi)
{
int ret = EINVAL;
- const struct route_table *table = NULL;
- const struct rib_table_info *info;
- const struct prefix *p, *src_p;
- struct zebra_ns *zns;
- struct zebra_vrf *zvrf;
- struct nexthop *nexthop;
- struct zebra_l3vni *zl3vni;
- const struct interface *ifp;
- struct dplane_intf_extra *if_extra;
- if (!ctx || !rn || !re)
- goto done;
+ if (!ctx || !re)
+ return ret;
TAILQ_INIT(&ctx->u.rinfo.intf_extra_q);
@@ -2362,9 +2353,6 @@ int dplane_ctx_route_init(struct zebra_dplane_ctx *ctx, enum dplane_op_e op,
ctx->u.rinfo.zd_type = re->type;
ctx->u.rinfo.zd_old_type = re->type;
- /* Prefixes: dest, and optional source */
- srcdest_rnode_prefixes(rn, &p, &src_p);
-
prefix_copy(&(ctx->u.rinfo.zd_dest), p);
if (src_p)
@@ -2385,11 +2373,45 @@ int dplane_ctx_route_init(struct zebra_dplane_ctx *ctx, enum dplane_op_e op,
ctx->u.rinfo.zd_old_tag = re->tag;
ctx->u.rinfo.zd_distance = re->distance;
+ ctx->u.rinfo.zd_afi = afi;
+ ctx->u.rinfo.zd_safi = safi;
+
+ return AOK;
+}
+
+/*
+ * Initialize a context block for a route update from zebra data structs.
+ */
+int dplane_ctx_route_init(struct zebra_dplane_ctx *ctx, enum dplane_op_e op,
+ struct route_node *rn, struct route_entry *re)
+{
+ int ret = EINVAL;
+ const struct route_table *table = NULL;
+ const struct rib_table_info *info;
+ const struct prefix *p, *src_p;
+ struct zebra_ns *zns;
+ struct zebra_vrf *zvrf;
+ struct nexthop *nexthop;
+ struct zebra_l3vni *zl3vni;
+ const struct interface *ifp;
+ struct dplane_intf_extra *if_extra;
+
+ if (!ctx || !rn || !re)
+ return ret;
+
+ /*
+ * Let's grab the data from the route_node
+ * so that we can call a helper function
+ */
+
+ /* Prefixes: dest, and optional source */
+ srcdest_rnode_prefixes(rn, &p, &src_p);
table = srcdest_rnode_table(rn);
info = table->info;
- ctx->u.rinfo.zd_afi = info->afi;
- ctx->u.rinfo.zd_safi = info->safi;
+ if (dplane_ctx_route_init_basic(ctx, op, re, p, src_p, info->afi,
+ info->safi) != AOK)
+ return ret;
/* Copy nexthops; recursive info is included too */
copy_nexthops(&(ctx->u.rinfo.zd_ng.nexthop),
diff --git a/zebra/zebra_dplane.h b/zebra/zebra_dplane.h
index 3210137c0..9d7502828 100644
--- a/zebra/zebra_dplane.h
+++ b/zebra/zebra_dplane.h
@@ -774,6 +774,12 @@ dplane_pbr_ipset_entry_delete(struct zebra_pbr_ipset_entry *ipset);
int dplane_ctx_route_init(struct zebra_dplane_ctx *ctx, enum dplane_op_e op,
struct route_node *rn, struct route_entry *re);
+int dplane_ctx_route_init_basic(struct zebra_dplane_ctx *ctx,
+ enum dplane_op_e op, struct route_entry *re,
+ const struct prefix *p,
+ const struct prefix *src_p, afi_t afi,
+ safi_t safi);
+
/* Encode next hop information into data plane context. */
int dplane_ctx_nexthop_init(struct zebra_dplane_ctx *ctx, enum dplane_op_e op,
struct nhg_hash_entry *nhe);
--
2.30.2

155
src/sonic-frr/patch/0020-zebra-Add-ctx-to-netlink-message-parsing.patch Executable file
View File

@ -0,0 +1,155 @@
From aebf87ce244c5189cec686ddf1278ae343807a53 Mon Sep 17 00:00:00 2001
From: Donald Sharp <sharpd@nvidia.com>
Date: Tue, 4 Oct 2022 15:41:36 -0400
Subject: [PATCH 6/8] zebra: Add ctx to netlink message parsing
Add the initial step of passing in a dplane context
to reading route netlink messages. This code
will be run in two contexts:
a) The normal pthread for reading netlink messages from
the kernel
b) The dplane_fpm_nl pthread.
The goal of this commit is too just allow a) to work
b) will be filled in in the future. Effectively
everything should still be working as it should
pre this change. We will just possibly allow
the passing of the context around( but not used )
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
---
zebra/rt_netlink.c | 31 +++++++++++++++++++++++--------
zebra/rt_netlink.h | 4 ++++
zebra/zebra_dplane.c | 11 +++++++++++
zebra/zebra_dplane.h | 10 ++++++++++
4 files changed, 48 insertions(+), 8 deletions(-)
diff --git a/zebra/rt_netlink.c b/zebra/rt_netlink.c
index 24c01b7f5..b2df2cd8a 100644
--- a/zebra/rt_netlink.c
+++ b/zebra/rt_netlink.c
@@ -667,8 +667,9 @@ static uint8_t parse_multipath_nexthops_unicast(ns_id_t ns_id,
}
/* Looking up routing table by netlink interface. */
-static int netlink_route_change_read_unicast(struct nlmsghdr *h, ns_id_t ns_id,
- int startup)
+int netlink_route_change_read_unicast_internal(struct nlmsghdr *h,
+ ns_id_t ns_id, int startup,
+ struct zebra_dplane_ctx *ctx)
{
int len;
struct rtmsg *rtm;
@@ -740,9 +741,8 @@ static int netlink_route_change_read_unicast(struct nlmsghdr *h, ns_id_t ns_id,
selfroute = is_selfroute(rtm->rtm_protocol);
- if (!startup && selfroute
- && h->nlmsg_type == RTM_NEWROUTE
- && !zrouter.asic_offloaded) {
+ if (!startup && selfroute && h->nlmsg_type == RTM_NEWROUTE &&
+ !zrouter.asic_offloaded && !ctx) {
if (IS_ZEBRA_DEBUG_KERNEL)
zlog_debug("Route type: %d Received that we think we have originated, ignoring",
rtm->rtm_protocol);
@@ -963,12 +963,20 @@ static int netlink_route_change_read_unicast(struct nlmsghdr *h, ns_id_t ns_id,
}
if (nhe_id || ng)
- rib_add_multipath(afi, SAFI_UNICAST, &p,
- &src_p, re, ng);
+ dplane_rib_add_multipath(afi, SAFI_UNICAST, &p,
+ &src_p, re, ng,
+ startup, ctx);
else
XFREE(MTYPE_RE, re);
}
} else {
+ if (ctx) {
+ zlog_err(
+ "%s: %pFX RTM_DELROUTE received but received a context as well",
+ __func__, &p);
+ return 0;
+ }
+
if (nhe_id) {
rib_delete(afi, SAFI_UNICAST, vrf_id, proto, 0, flags,
&p, &src_p, NULL, nhe_id, table, metric,
@@ -993,7 +1001,14 @@ static int netlink_route_change_read_unicast(struct nlmsghdr *h, ns_id_t ns_id,
}
}
- return 0;
+ return 1;
+}
+
+static int netlink_route_change_read_unicast(struct nlmsghdr *h, ns_id_t ns_id,
+ int startup)
+{
+ return netlink_route_change_read_unicast_internal(h, ns_id, startup,
+ NULL);
}
static struct mcast_route_data *mroute = NULL;
diff --git a/zebra/rt_netlink.h b/zebra/rt_netlink.h
index 93c06e555..da044c2c4 100644
--- a/zebra/rt_netlink.h
+++ b/zebra/rt_netlink.h
@@ -122,6 +122,10 @@ netlink_put_lsp_update_msg(struct nl_batch *bth, struct zebra_dplane_ctx *ctx);
extern enum netlink_msg_status
netlink_put_pw_update_msg(struct nl_batch *bth, struct zebra_dplane_ctx *ctx);
+int netlink_route_change_read_unicast_internal(struct nlmsghdr *h,
+ ns_id_t ns_id, int startup,
+ struct zebra_dplane_ctx *ctx);
+
#ifdef NETLINK_DEBUG
const char *nlmsg_type2str(uint16_t type);
const char *af_type2str(int type);
diff --git a/zebra/zebra_dplane.c b/zebra/zebra_dplane.c
index 05f9c2e08..5048bf7dc 100644
--- a/zebra/zebra_dplane.c
+++ b/zebra/zebra_dplane.c
@@ -5320,6 +5320,17 @@ kernel_dplane_process_ipset_entry(struct zebra_dplane_provider *prov,
dplane_provider_enqueue_out_ctx(prov, ctx);
}
+void dplane_rib_add_multipath(afi_t afi, safi_t safi, struct prefix *p,
+ struct prefix_ipv6 *src_p, struct route_entry *re,
+ struct nexthop_group *ng, int startup,
+ struct zebra_dplane_ctx *ctx)
+{
+ if (!ctx)
+ rib_add_multipath(afi, safi, p, src_p, re, ng);
+ else {
+ }
+}
+
/*
* Kernel provider callback
*/
diff --git a/zebra/zebra_dplane.h b/zebra/zebra_dplane.h
index 9d7502828..4732a1628 100644
--- a/zebra/zebra_dplane.h
+++ b/zebra/zebra_dplane.h
@@ -938,6 +938,16 @@ void zebra_dplane_pre_finish(void);
void zebra_dplane_finish(void);
void zebra_dplane_shutdown(void);
+/*
+ * decision point for sending a routing update through the old
+ * straight to zebra master pthread or through the dplane to
+ * the master pthread for handling
+ */
+void dplane_rib_add_multipath(afi_t afi, safi_t safi, struct prefix *p,
+ struct prefix_ipv6 *src_p, struct route_entry *re,
+ struct nexthop_group *ng, int startup,
+ struct zebra_dplane_ctx *ctx);
+
#ifdef __cplusplus
}
#endif
--
2.30.2

289
src/sonic-frr/patch/0021-zebra-Read-from-the-dplane_fpm_nl-a-route-update.patch Executable file
View File

@ -0,0 +1,289 @@
From 95fac14ae8076ef4f5749aa3b761a7603b8c05cc Mon Sep 17 00:00:00 2001
From: Donald Sharp <sharpd@nvidia.com>
Date: Fri, 7 Oct 2022 08:02:44 -0400
Subject: [PATCH 7/8] zebra: Read from the dplane_fpm_nl a route update
Read from the fpm dplane a route update that will
include status about whether or not the asic was
successfull in offloading the route.
Have this data passed up to zebra for processing and disseminate
this data as appropriate.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
---
doc/developer/fpm.rst | 16 ++++++
zebra/dplane_fpm_nl.c | 122 +++++++++++++++++++++++++++++++++++++++++-
zebra/zebra_dplane.c | 10 ++--
zebra/zebra_dplane.h | 2 +-
zebra/zebra_rib.c | 16 +++++-
5 files changed, 158 insertions(+), 8 deletions(-)
diff --git a/doc/developer/fpm.rst b/doc/developer/fpm.rst
index 984986913..56d33671d 100644
--- a/doc/developer/fpm.rst
+++ b/doc/developer/fpm.rst
@@ -101,3 +101,19 @@ Data
^^^^
The netlink or protobuf message payload.
+
+
+Route Status Notification from ASIC
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+The dplane_fpm_nl has the ability to read route netlink messages
+from the underlying fpm implementation that can tell zebra
+whether or not the route has been Offloaded/Failed or Trapped.
+The end developer must send the data up the same socket that has
+been created to listen for FPM messages from Zebra. The data sent
+must have a Frame Header with Version set to 1, Message Type set to 1
+and an appropriate message Length. The message data must contain
+a RTM_NEWROUTE netlink message that sends the prefix and nexthops
+associated with the route. Finally rtm_flags must contain
+RTM_F_OFFLOAD, RTM_F_TRAP and or RTM_F_OFFLOAD_FAILED to signify
+what has happened to the route in the ASIC.
diff --git a/zebra/dplane_fpm_nl.c b/zebra/dplane_fpm_nl.c
index 8d41f3b07..893ee5845 100644
--- a/zebra/dplane_fpm_nl.c
+++ b/zebra/dplane_fpm_nl.c
@@ -51,6 +51,7 @@
#include "zebra/kernel_netlink.h"
#include "zebra/rt_netlink.h"
#include "zebra/debug.h"
+#include "fpm/fpm.h"
#define SOUTHBOUND_DEFAULT_ADDR INADDR_LOOPBACK
#define SOUTHBOUND_DEFAULT_PORT 2620
@@ -461,7 +462,13 @@ static void fpm_reconnect(struct fpm_nl_ctx *fnc)
static int fpm_read(struct thread *t)
{
struct fpm_nl_ctx *fnc = THREAD_ARG(t);
+ fpm_msg_hdr_t fpm;
ssize_t rv;
+ char buf[65535];
+ struct nlmsghdr *hdr;
+ struct zebra_dplane_ctx *ctx;
+ size_t available_bytes;
+ size_t hdr_available_bytes;
/* Let's ignore the input at the moment. */
rv = stream_read_try(fnc->ibuf, fnc->socket,
@@ -493,12 +500,123 @@ static int fpm_read(struct thread *t)
if (rv == -2)
return 0;
- stream_reset(fnc->ibuf);
-
/* Account all bytes read. */
atomic_fetch_add_explicit(&fnc->counters.bytes_read, rv,
memory_order_relaxed);
+ available_bytes = STREAM_READABLE(fnc->ibuf);
+ while (available_bytes) {
+ if (available_bytes < (ssize_t)FPM_MSG_HDR_LEN) {
+ stream_pulldown(fnc->ibuf);
+ return 0;
+ }
+
+ fpm.version = stream_getc(fnc->ibuf);
+ fpm.msg_type = stream_getc(fnc->ibuf);
+ fpm.msg_len = stream_getw(fnc->ibuf);
+
+ if (fpm.version != FPM_PROTO_VERSION &&
+ fpm.msg_type != FPM_MSG_TYPE_NETLINK) {
+ stream_reset(fnc->ibuf);
+ zlog_warn(
+ "%s: Received version/msg_type %u/%u, expected 1/1",
+ __func__, fpm.version, fpm.msg_type);
+
+ FPM_RECONNECT(fnc);
+ return 0;
+ }
+
+ /*
+ * If the passed in length doesn't even fill in the header
+ * something is wrong and reset.
+ */
+ if (fpm.msg_len < FPM_MSG_HDR_LEN) {
+ zlog_warn(
+ "%s: Received message length: %u that does not even fill the FPM header",
+ __func__, fpm.msg_len);
+ FPM_RECONNECT(fnc);
+ return 0;
+ }
+
+ /*
+ * If we have not received the whole payload, reset the stream
+ * back to the beginning of the header and move it to the
+ * top.
+ */
+ if (fpm.msg_len > available_bytes) {
+ stream_rewind_getp(fnc->ibuf, FPM_MSG_HDR_LEN);
+ stream_pulldown(fnc->ibuf);
+ return 0;
+ }
+
+ available_bytes -= FPM_MSG_HDR_LEN;
+
+ /*
+ * Place the data from the stream into a buffer
+ */
+ hdr = (struct nlmsghdr *)buf;
+ stream_get(buf, fnc->ibuf, fpm.msg_len - FPM_MSG_HDR_LEN);
+ hdr_available_bytes = fpm.msg_len - FPM_MSG_HDR_LEN;
+ available_bytes -= hdr_available_bytes;
+
+ /* Sanity check: must be at least header size. */
+ if (hdr->nlmsg_len < sizeof(*hdr)) {
+ zlog_warn(
+ "%s: [seq=%u] invalid message length %u (< %zu)",
+ __func__, hdr->nlmsg_seq, hdr->nlmsg_len,
+ sizeof(*hdr));
+ continue;
+ }
+
+ if (hdr->nlmsg_len > fpm.msg_len) {
+ zlog_warn(
+ "%s: Received a inner header length of %u that is greater than the fpm total length of %u",
+ __func__, hdr->nlmsg_len, fpm.msg_len);
+ FPM_RECONNECT(fnc);
+ }
+
+ /* Not enough bytes available. */
+ if (hdr->nlmsg_len > hdr_available_bytes) {
+ zlog_warn(
+ "%s: [seq=%u] invalid message length %u (> %zu)",
+ __func__, hdr->nlmsg_seq, hdr->nlmsg_len,
+ available_bytes);
+ continue;
+ }
+
+ if (!(hdr->nlmsg_flags & NLM_F_REQUEST)) {
+ if (IS_ZEBRA_DEBUG_FPM)
+ zlog_debug(
+ "%s: [seq=%u] not a request, skipping",
+ __func__, hdr->nlmsg_seq);
+
+ /*
+ * This request is a bust, go to the next one
+ */
+ continue;
+ }
+
+ switch (hdr->nlmsg_type) {
+ case RTM_NEWROUTE:
+ ctx = dplane_ctx_alloc();
+ dplane_ctx_set_op(ctx, DPLANE_OP_ROUTE_NOTIFY);
+ if (netlink_route_change_read_unicast_internal(
+ hdr, 0, false, ctx) != 1) {
+ dplane_ctx_fini(&ctx);
+ stream_pulldown(fnc->ibuf);
+ return 0;
+ }
+ break;
+ default:
+ if (IS_ZEBRA_DEBUG_FPM)
+ zlog_debug(
+ "%s: Received message type %u which is not currently handled",
+ __func__, hdr->nlmsg_type);
+ break;
+ }
+ }
+
+ stream_reset(fnc->ibuf);
return 0;
}
diff --git a/zebra/zebra_dplane.c b/zebra/zebra_dplane.c
index 5048bf7dc..336056abb 100644
--- a/zebra/zebra_dplane.c
+++ b/zebra/zebra_dplane.c
@@ -2337,7 +2337,7 @@ static int dplane_ctx_ns_init(struct zebra_dplane_ctx *ctx,
int dplane_ctx_route_init_basic(struct zebra_dplane_ctx *ctx,
enum dplane_op_e op, struct route_entry *re,
const struct prefix *p,
- const struct prefix *src_p, afi_t afi,
+ const struct prefix_ipv6 *src_p, afi_t afi,
safi_t safi)
{
int ret = EINVAL;
@@ -2388,7 +2388,8 @@ int dplane_ctx_route_init(struct zebra_dplane_ctx *ctx, enum dplane_op_e op,
int ret = EINVAL;
const struct route_table *table = NULL;
const struct rib_table_info *info;
- const struct prefix *p, *src_p;
+ const struct prefix *p;
+ const struct prefix_ipv6 *src_p;
struct zebra_ns *zns;
struct zebra_vrf *zvrf;
struct nexthop *nexthop;
@@ -2405,7 +2406,7 @@ int dplane_ctx_route_init(struct zebra_dplane_ctx *ctx, enum dplane_op_e op,
*/
/* Prefixes: dest, and optional source */
- srcdest_rnode_prefixes(rn, &p, &src_p);
+ srcdest_rnode_prefixes(rn, &p, (const struct prefix **)&src_p);
table = srcdest_rnode_table(rn);
info = table->info;
@@ -5328,6 +5329,9 @@ void dplane_rib_add_multipath(afi_t afi, safi_t safi, struct prefix *p,
if (!ctx)
rib_add_multipath(afi, safi, p, src_p, re, ng);
else {
+ dplane_ctx_route_init_basic(ctx, dplane_ctx_get_op(ctx), re, p,
+ src_p, afi, safi);
+ dplane_provider_enqueue_to_zebra(ctx);
}
}
diff --git a/zebra/zebra_dplane.h b/zebra/zebra_dplane.h
index 4732a1628..c3fe3ba40 100644
--- a/zebra/zebra_dplane.h
+++ b/zebra/zebra_dplane.h
@@ -777,7 +777,7 @@ int dplane_ctx_route_init(struct zebra_dplane_ctx *ctx, enum dplane_op_e op,
int dplane_ctx_route_init_basic(struct zebra_dplane_ctx *ctx,
enum dplane_op_e op, struct route_entry *re,
const struct prefix *p,
- const struct prefix *src_p, afi_t afi,
+ const struct prefix_ipv6 *src_p, afi_t afi,
safi_t safi);
/* Encode next hop information into data plane context. */
diff --git a/zebra/zebra_rib.c b/zebra/zebra_rib.c
index abd6e07f9..50f19adcc 100644
--- a/zebra/zebra_rib.c
+++ b/zebra/zebra_rib.c
@@ -2138,8 +2138,7 @@ static void rib_process_dplane_notify(struct zebra_dplane_ctx *ctx)
}
/* Ensure we clear the QUEUED flag */
- if (!zrouter.asic_offloaded)
- UNSET_FLAG(re->status, ROUTE_ENTRY_QUEUED);
+ UNSET_FLAG(re->status, ROUTE_ENTRY_QUEUED);
/* Is this a notification that ... matters? We mostly care about
* the route that is currently selected for installation; we may also
@@ -2182,6 +2181,19 @@ static void rib_process_dplane_notify(struct zebra_dplane_ctx *ctx)
dplane_ctx_get_type(ctx)));
}
goto done;
+ } else {
+ uint32_t flags = dplane_ctx_get_flags(ctx);
+
+ if (CHECK_FLAG(flags, ZEBRA_FLAG_OFFLOADED)) {
+ UNSET_FLAG(re->flags, ZEBRA_FLAG_OFFLOAD_FAILED);
+ SET_FLAG(re->flags, ZEBRA_FLAG_OFFLOADED);
+ }
+ if (CHECK_FLAG(flags, ZEBRA_FLAG_OFFLOAD_FAILED)) {
+ UNSET_FLAG(re->flags, ZEBRA_FLAG_OFFLOADED);
+ SET_FLAG(re->flags, ZEBRA_FLAG_OFFLOAD_FAILED);
+ }
+ if (CHECK_FLAG(flags, ZEBRA_FLAG_TRAPPED))
+ SET_FLAG(re->flags, ZEBRA_FLAG_TRAPPED);
}
/* We'll want to determine whether the installation status of the
--
2.30.2

100
src/sonic-frr/patch/0022-zebra-Fix-code-because-missing-backport.patch Executable file
View File

@ -0,0 +1,100 @@
From 0104413e893f3e2d41da101b6788aea240e5a21a Mon Sep 17 00:00:00 2001
From: Donald Sharp <sharpd@nvidia.com>
Date: Thu, 15 Dec 2022 10:52:38 -0500
Subject: [PATCH 8/8] zebra: Fix code because missing backport
The commit:
commit 07fd1f7e9420efd480e85a9ab72b88fff274f791
Author: Donald Sharp <sharpd@nvidia.com>
Date: Tue Aug 9 20:07:09 2022 -0400
zebra: use rib_add_multipath in rt_netlink.c
The new route code path was using a combination of
both rib_add() and rib_add_multipath() let's clean
it up some to use rib_add_multipath()
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Abstracted the calling of rib_add and rib_add_multipath in
netlink_route_change_read_unicast to just calling rib_add_multipath
one time. Since this commit was not backported the netlink_route_change_read_unicast
function had an additional rib_add that is not going through the new dplane
code to make a decision point about how to send up the message.
Make it so the non multipath case uses the new decision point.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
---
zebra/rt_netlink.c | 41 ++++++++++++++++++++++++-----------------
1 file changed, 24 insertions(+), 17 deletions(-)
diff --git a/zebra/rt_netlink.c b/zebra/rt_netlink.c
index b2df2cd8a..76402561e 100644
--- a/zebra/rt_netlink.c
+++ b/zebra/rt_netlink.c
@@ -909,37 +909,44 @@ int netlink_route_change_read_unicast_internal(struct nlmsghdr *h,
afi = AFI_IP6;
if (h->nlmsg_type == RTM_NEWROUTE) {
+ struct route_entry *re;
+ struct nexthop_group *ng = NULL;
+
+ re = XCALLOC(MTYPE_RE, sizeof(struct route_entry));
+ re->type = proto;
+ re->distance = distance;
+ re->flags = flags;
+ re->metric = metric;
+ re->mtu = mtu;
+ re->vrf_id = vrf_id;
+ re->table = table;
+ re->uptime = monotime(NULL);
+ re->tag = tag;
+ re->nhe_id = nhe_id;
+
+ if (!nhe_id)
+ ng = nexthop_group_new();
if (!tb[RTA_MULTIPATH]) {
- struct nexthop nh = {0};
+ struct nexthop *nexthop, nh = {0};
if (!nhe_id) {
nh = parse_nexthop_unicast(
ns_id, rtm, tb, bh_type, index, prefsrc,
gate, afi, vrf_id);
+
+ nexthop = nexthop_new();
+ *nexthop = nh;
+ nexthop_group_add_sorted(ng, nexthop);
}
- rib_add(afi, SAFI_UNICAST, vrf_id, proto, 0, flags, &p,
- &src_p, &nh, nhe_id, table, metric, mtu,
- distance, tag);
+ dplane_rib_add_multipath(afi, SAFI_UNICAST, &p, &src_p,
+ re, ng, startup, ctx);
} else {
/* This is a multipath route */
- struct route_entry *re;
struct nexthop_group *ng = NULL;
struct rtnexthop *rtnh =
(struct rtnexthop *)RTA_DATA(tb[RTA_MULTIPATH]);
- re = XCALLOC(MTYPE_RE, sizeof(struct route_entry));
- re->type = proto;
- re->distance = distance;
- re->flags = flags;
- re->metric = metric;
- re->mtu = mtu;
- re->vrf_id = vrf_id;
- re->table = table;
- re->uptime = monotime(NULL);
- re->tag = tag;
- re->nhe_id = nhe_id;
-
if (!nhe_id) {
uint8_t nhop_num;
--
2.30.2

113
src/sonic-frr/patch/0023-Use-vrf_id-for-vrf-not-tabled_id.patch Normal file
View File

@ -0,0 +1,113 @@
From 349e3f758860be0077b69919c39764d3486ec44a Mon Sep 17 00:00:00 2001
From: Stepan Blyschak <stepanb@nvidia.com>
Date: Mon, 16 Jan 2023 11:45:19 +0000
Subject: [PATCH] use vrf id instead of table id
Signed-off-by: Stepan Blyschak <stepanb@nvidia.com>
---
zebra/rt_netlink.c | 74 ++++++++++++++++++++++++++++++++++++++--------
1 file changed, 61 insertions(+), 13 deletions(-)
diff --git a/zebra/rt_netlink.c b/zebra/rt_netlink.c
index 29137a09f..e7b808754 100644
--- a/zebra/rt_netlink.c
+++ b/zebra/rt_netlink.c
@@ -388,6 +388,30 @@ vrf_id_t vrf_lookup_by_table(uint32_t table_id, ns_id_t ns_id)
return VRF_DEFAULT;
}
+static uint32_t table_lookup_by_vrf(vrf_id_t vrf_id, ns_id_t ns_id)
+{
+ struct vrf *vrf;
+ struct zebra_vrf *zvrf;
+
+ RB_FOREACH (vrf, vrf_id_head, &vrfs_by_id) {
+ zvrf = vrf->info;
+ if (zvrf == NULL)
+ continue;
+ /* case vrf with netns : match the netnsid */
+ if (vrf_is_backend_netns()) {
+ if (ns_id == zvrf_id(zvrf))
+ return zvrf->table_id;
+ } else {
+ /* VRF is VRF_BACKEND_VRF_LITE */
+ if (zvrf_id(zvrf) != vrf_id)
+ continue;
+ return zvrf->table_id;
+ }
+ }
+
+ return RT_TABLE_UNSPEC;
+}
+
/**
* @parse_encap_mpls() - Parses encapsulated mpls attributes
* @tb: Pointer to rtattr to look for nested items in.
@@ -754,14 +778,26 @@ int netlink_route_change_read_unicast_internal(struct nlmsghdr *h,
if (rtm->rtm_family == AF_MPLS)
return 0;
- /* Table corresponding to route. */
- if (tb[RTA_TABLE])
- table = *(int *)RTA_DATA(tb[RTA_TABLE]);
- else
- table = rtm->rtm_table;
+ if (!ctx) {
+ /* Table corresponding to route. */
+ if (tb[RTA_TABLE])
+ table = *(int *)RTA_DATA(tb[RTA_TABLE]);
+ else
+ table = rtm->rtm_table;
+
+ /* Map to VRF */
+ vrf_id = vrf_lookup_by_table(table, ns_id);
+ } else {
+ /* With FPM, rtm_table contains vrf id, see netlink_route_multipath_msg_encode */
+ if (tb[RTA_TABLE])
+ vrf_id = *(int *)RTA_DATA(tb[RTA_TABLE]);
+ else
+ vrf_id = rtm->rtm_table;
+
+ /* Map to table */
+ table = table_lookup_by_vrf(vrf_id, ns_id);
+ }
- /* Map to VRF */
- vrf_id = vrf_lookup_by_table(table, ns_id);
if (vrf_id == VRF_DEFAULT) {
if (!is_zebra_valid_kernel_table(table)
&& !is_zebra_main_routing_table(table))
@@ -2004,12 +2040,24 @@ ssize_t netlink_route_multipath_msg_encode(int cmd,
#endif
/* Table corresponding to this route. */
table_id = dplane_ctx_get_table(ctx);
- if (table_id < 256)
- req->r.rtm_table = table_id;
- else {
- req->r.rtm_table = RT_TABLE_UNSPEC;
- if (!nl_attr_put32(&req->n, datalen, RTA_TABLE, table_id))
- return 0;
+ if (!fpm) {
+ if (table_id < 256)
+ req->r.rtm_table = table_id;
+ else {
+ req->r.rtm_table = RT_TABLE_UNSPEC;
+ if (!nl_attr_put32(&req->n, datalen, RTA_TABLE, table_id))
+ return 0;
+ }
+ } else {
+ /* Put vrf if_index instead of table id */
+ vrf_id_t vrf = dplane_ctx_get_vrf(ctx);
+ if (vrf < 256)
+ req->r.rtm_table = vrf;
+ else {
+ req->r.rtm_table = RT_TABLE_UNSPEC;
+ if (!nl_attr_put32(&req->n, datalen, RTA_TABLE, vrf))
+ return 0;
+ }
}
if (IS_ZEBRA_DEBUG_KERNEL)
--
2.30.2

31
src/sonic-frr/patch/0024-zebra-continue-fpm-read-when-we-decide-a-netlink-message-is-not-needed.patch Normal file
View File

@ -0,0 +1,31 @@
From c0275ab189d619c2f688a383e5e83183f02eaf6f Mon Sep 17 00:00:00 2001
From: Donald Sharp <sharpd@nvidia.com>
Date: Tue, 10 Jan 2023 08:36:50 -0500
Subject: [PATCH] zebra: Continue fpm_read when we decide a netlink message is
not needed
When FRR receives a netlink message that it decides to stop parsing
it returns a 0 ( instead of a -1 ). Just make the dplane continue
reading other data instead of aborting the read.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
---
zebra/dplane_fpm_nl.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/zebra/dplane_fpm_nl.c b/zebra/dplane_fpm_nl.c
index a31e79212..9d9ea7283 100644
--- a/zebra/dplane_fpm_nl.c
+++ b/zebra/dplane_fpm_nl.c
@@ -605,7 +610,10 @@ static void fpm_read(struct thread *t)
hdr, 0, false, ctx) != 1) {
dplane_ctx_fini(&ctx);
stream_pulldown(fnc->ibuf);
- return 0;
+ /*
+ * Let's continue to read other messages
+ * Even if we ignore this one.
+ */
}
break;
default:

49
src/sonic-frr/patch/0025-zebra-Send-nht-resolved-entry-up-to-concerned-protoc.patch Normal file
View File

@ -0,0 +1,49 @@
From 49c14222cb0d3cb60b32340976b4e42e5644a563 Mon Sep 17 00:00:00 2001
From: Stepan Blyschak <stepanb@nvidia.com>
Date: Tue, 31 Jan 2023 10:24:30 +0000
Subject: [PATCH] zebra: Send nht resolved entry up to concerned protocols in
all cases
There existed the idea, from Volta, that a nexthop group would not have
the same nexthops installed -vs- what FRR actually sent down. The
dplane would notify you.
With the addition of 06525c4
the code was put behind a bit of a wall controlled the usage
of it.
The flag ROUTE_ENTRY_USE_FIB_NHG flag was being used
to control which set was being sent up to concerned parties
in nexthop tracking. Put this flag behind the wall and
do not necessarily set it when we receive a data plane
notification about a route being installed or not.
Fixes: #12706
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
---
zebra/zebra_rib.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/zebra/zebra_rib.c b/zebra/zebra_rib.c
index 50f19adcc..a8dbe4470 100644
--- a/zebra/zebra_rib.c
+++ b/zebra/zebra_rib.c
@@ -1666,9 +1666,12 @@ no_nexthops:
ctxnhg->nexthop != NULL ? "" : " (empty)");
/* Set the flag about the dedicated fib list */
- SET_FLAG(re->status, ROUTE_ENTRY_USE_FIB_NHG);
- if (ctxnhg->nexthop)
- copy_nexthops(&(re->fib_ng.nexthop), ctxnhg->nexthop, NULL);
+ if (zrouter.asic_notification_nexthop_control) {
+ SET_FLAG(re->status, ROUTE_ENTRY_USE_FIB_NHG);
+ if (ctxnhg->nexthop)
+ copy_nexthops(&(re->fib_ng.nexthop), ctxnhg->nexthop,
+ NULL);
+ }
check_backups:
--
2.30.2

137
src/sonic-frr/patch/0026-bgpd-Ensure-suppress-fib-pending-works-with-network-.patch Normal file
View File

@ -0,0 +1,137 @@
From 5a1d3c9e24791ebdb7e375a02afe9c5f6146c488 Mon Sep 17 00:00:00 2001
From: Donald Sharp <sharpd@nvidia.com>
Date: Wed, 22 Mar 2023 11:35:28 -0400
Subject: [PATCH] bgpd: Ensure suppress-fib-pending works with network
statements
The flag for telling BGP that a route is expected to be installed
first before notifying a peer was always being set upon receipt
of a path that could be accepted as bestpath. This is not correct:
imagine that you have a peer sending you a route and you have a
network statement that covers the same route. Irrelevant if the
network statement would win the flag on the dest was being set
in bgp_update. Thus you could get into a situation where
the network statement path wins but since the flag is set on
the node, it will never be announced to a peer.
Let's just move the setting of the flag into bgp_zebra_announce
and _withdraw. In _announce set the flag to TRUE when suppress-fib
is enabled. In _withdraw just always unset the flag as that a withdrawal
does not need to wait for rib removal before announcing. This will
cover the case when a network statement is added after the route has
been learned from a peer.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
---
bgpd/bgp_route.c | 22 -------------------
bgpd/bgp_zebra.c | 14 ++++++++++++
.../bgp_suppress_fib/test_bgp_suppress_fib.py | 14 ++++++++++++
3 files changed, 28 insertions(+), 22 deletions(-)
diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index d4b5066c6..c01e2fd2c 100644
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -2994,11 +2994,6 @@ static void bgp_process_main_one(struct bgp *bgp, struct bgp_dest *dest,
if (bgp_fibupd_safi(safi)
&& !bgp_option_check(BGP_OPT_NO_FIB)) {
- if (BGP_SUPPRESS_FIB_ENABLED(bgp)
- && new_select->sub_type == BGP_ROUTE_NORMAL)
- SET_FLAG(dest->flags,
- BGP_NODE_FIB_INSTALL_PENDING);
-
if (new_select->type == ZEBRA_ROUTE_BGP
&& (new_select->sub_type == BGP_ROUTE_NORMAL
|| new_select->sub_type
@@ -3104,10 +3099,6 @@ static void bgp_process_main_one(struct bgp *bgp, struct bgp_dest *dest,
|| new_select->sub_type == BGP_ROUTE_AGGREGATE
|| new_select->sub_type == BGP_ROUTE_IMPORTED)) {
- if (BGP_SUPPRESS_FIB_ENABLED(bgp))
- SET_FLAG(dest->flags,
- BGP_NODE_FIB_INSTALL_PENDING);
-
/* if this is an evpn imported type-5 prefix,
* we need to withdraw the route first to clear
* the nh neigh and the RMAC entry.
@@ -3953,19 +3944,6 @@ int bgp_update(struct peer *peer, const struct prefix *p, uint32_t addpath_id,
goto filtered;
}
- /* The flag BGP_NODE_FIB_INSTALL_PENDING is for the following
- * condition :
- * Suppress fib is enabled
- * BGP_OPT_NO_FIB is not enabled
- * Route type is BGP_ROUTE_NORMAL (peer learnt routes)
- * Route is being installed first time (BGP_NODE_FIB_INSTALLED not set)
- */
- if (bgp_fibupd_safi(safi) && BGP_SUPPRESS_FIB_ENABLED(bgp)
- && (sub_type == BGP_ROUTE_NORMAL)
- && (!bgp_option_check(BGP_OPT_NO_FIB))
- && (!CHECK_FLAG(dest->flags, BGP_NODE_FIB_INSTALLED)))
- SET_FLAG(dest->flags, BGP_NODE_FIB_INSTALL_PENDING);
-
/* If maximum prefix count is configured and current prefix
* count exeed it.
*/
diff --git a/bgpd/bgp_zebra.c b/bgpd/bgp_zebra.c
index 21912d143..71816813a 100644
--- a/bgpd/bgp_zebra.c
+++ b/bgpd/bgp_zebra.c
@@ -1269,6 +1269,14 @@ void bgp_zebra_announce(struct bgp_dest *dest, const struct prefix *p,
uint32_t nhg_id = 0;
bool is_add;
+ /*
+ * BGP is installing this route and bgp has been configured
+ * to suppress announcements until the route has been installed
+ * let's set the fact that we expect this route to be installed
+ */
+ if (BGP_SUPPRESS_FIB_ENABLED(bgp))
+ SET_FLAG(dest->flags, BGP_NODE_FIB_INSTALL_PENDING);
+
/* Don't try to install if we're not connected to Zebra or Zebra doesn't
* know of this instance.
*/
@@ -1681,6 +1689,12 @@ void bgp_zebra_withdraw(const struct prefix *p, struct bgp_path_info *info,
struct zapi_route api;
struct peer *peer;
+ /*
+ * If we are withdrawing the route, we don't need to have this
+ * flag set. So unset it.
+ */
+ UNSET_FLAG(info->net->flags, BGP_NODE_FIB_INSTALL_PENDING);
+
/* Don't try to install if we're not connected to Zebra or Zebra doesn't
* know of this instance.
*/
diff --git a/tests/topotests/bgp_suppress_fib/test_bgp_suppress_fib.py b/tests/topotests/bgp_suppress_fib/test_bgp_suppress_fib.py
index 96a294cae..f812079c7 100644
--- a/tests/topotests/bgp_suppress_fib/test_bgp_suppress_fib.py
+++ b/tests/topotests/bgp_suppress_fib/test_bgp_suppress_fib.py
@@ -229,6 +229,20 @@ def test_bgp_allow_as_in():
assertmsg = '"r2" 192.168.1.1/32 route should be gone'
assert result is None, assertmsg
+def test_local_vs_non_local():
+ tgen = get_topogen()
+
+ if tgen.routers_have_failure():
+ pytest.skip(tgen.errors)
+
+ r2 = tgen.gears["r2"]
+
+ output = json.loads(r2.vtysh_cmd("show bgp ipv4 uni 60.0.0.0/24 json"))
+ paths = output["paths"]
+ for i in range(len(paths)):
+ if "fibPending" in paths[i]:
+ assert(False), "Route 60.0.0.0/24 should not have fibPending"
+
if __name__ == "__main__":
args = ["-s"] + sys.argv[1:]
--
2.20.1

15
src/sonic-frr/patch/series
View File

@ -1,6 +1,5 @@
0001-Add-support-of-bgp-tcp-DSCP-value.patch
0002-Reduce-severity-of-Vty-connected-from-message.patch
0003-Use-vrf_id-for-vrf-not-tabled_id.patch
0004-Allow-BGP-attr-NEXT_HOP-to-be-0.0.0.0-due-to-allevia.patch
0005-nexthops-compare-vrf-only-if-ip-type.patch
0006-frr-remove-frr-log-outchannel-to-var-log-frr.log.patch
@ -13,3 +12,17 @@ cross-compile-changes.patch
0011-bgpd-enhanced-capability-is-always-turned-on-for-int.patch
0012-Ensure-ospf_apiclient_lsa_originate-cannot-accidently-write-into-stack.patch
0013-zebra-fix-dplane-fpm-nl-to-allow-for-fast-configuration.patch
0014-bgpd-Allow-network-XXX-to-work-with-bgp-suppress-fib.patch
0015-zebra-Return-statements-do-not-use-paranthesis.patch
0016-zebra-Add-zrouter.asic_notification_nexthop_control.patch
0017-zebra-Re-arrange-fpm_read-to-reduce-code-duplication.patch
0018-zebra-Add-dplane_ctx_get-set_flags.patch
0019-zebra-Rearrange-dplane_ctx_route_init.patch
0020-zebra-Add-ctx-to-netlink-message-parsing.patch
0021-zebra-Read-from-the-dplane_fpm_nl-a-route-update.patch
0022-zebra-Fix-code-because-missing-backport.patch
0023-Use-vrf_id-for-vrf-not-tabled_id.patch
0024-zebra-continue-fpm-read-when-we-decide-a-netlink-message-is-not-needed.patch
0025-zebra-Send-nht-resolved-entry-up-to-concerned-protoc.patch
0026-bgpd-Ensure-suppress-fib-pending-works-with-network-.patch

11
src/sonic-yang-models/tests/yang_model_tests/tests/device_metadata.json
View File

@ -125,6 +125,15 @@
},
"DEVICE_METADATA_ADVERTISE_LO_PREFIX_AS_128": {
"desc": "Verifying advertising lo prefix as /128."
},
"DEVICE_METADATA_SUPPRESS_PENDING_FIB_ENABLED": {
"desc": "Enable bgp-suppress-fib-pending"
},
"DEVICE_METADATA_SUPPRESS_PENDING_FIB_DISABLED": {
"desc": "Disable bgp-suppress-fib-pending"
},
"DEVICE_METADATA_SUPPRESS_PENDING_FIB_ENABLED_SYNCHRONOUS_MODE_DISABLED": {
"desc": "Enable bgp-suppress-fib-pending when synchronous mode is disabled",
"eStr": ["ASIC synchronous mode must be enabled in order to enable suppress FIB pending feature"]
}
}

30
src/sonic-yang-models/tests/yang_model_tests/tests_config/device_metadata.json
View File

@ -332,5 +332,35 @@
}
}
}
},
"DEVICE_METADATA_SUPPRESS_PENDING_FIB_ENABLED": {
"sonic-device_metadata:sonic-device_metadata": {
"sonic-device_metadata:DEVICE_METADATA": {
"sonic-device_metadata:localhost": {
"synchronous_mode": "enable",
"suppress-fib-pending": "enabled"
}
}
}
},
"DEVICE_METADATA_SUPPRESS_PENDING_FIB_DISABLED": {
"sonic-device_metadata:sonic-device_metadata": {
"sonic-device_metadata:DEVICE_METADATA": {
"sonic-device_metadata:localhost": {
"synchronous_mode": "disable",
"suppress-fib-pending": "disabled"
}
}
}
},
"DEVICE_METADATA_SUPPRESS_PENDING_FIB_ENABLED_SYNCHRONOUS_MODE_DISABLED": {
"sonic-device_metadata:sonic-device_metadata": {
"sonic-device_metadata:DEVICE_METADATA": {
"sonic-device_metadata:localhost": {
"synchronous_mode": "disable",
"suppress-fib-pending": "enabled"
}
}
}
}
}

13
src/sonic-yang-models/yang-models/sonic-device_metadata.yang
View File

@ -204,6 +204,19 @@ module sonic-device_metadata {
description "Advertise Loopback0 interface IPv6 /128 subnet address as it is with set to true.
By default SONiC advertises /128 subnet prefix in Loopback0 as /64 subnet route";
}
leaf suppress-fib-pending {
description "Enable BGP suppress FIB pending feature. BGP will wait for route FIB installation before announcing routes";
type enumeration {
enum enabled;
enum disabled;
}
default disabled;
must "((current() = 'disabled') or (current() = 'enabled' and ../synchronous_mode = 'enable'))" {
error-message "ASIC synchronous mode must be enabled in order to enable suppress FIB pending feature";
}
}
}
/* end of container localhost */
}