From 094dcc3cdac19d3da65b38effc45aa88d960909f Mon Sep 17 00:00:00 2001 From: Francois Dumontet Date: Tue, 23 Apr 2024 11:16:24 +0200 Subject: [PATCH 01/28] bgpd: fix "bgp as-pah access-list" with "set aspath exclude" set/unset issues whith the following config router bgp 65001 no bgp ebgp-requires-policy neighbor 192.168.1.2 remote-as external neighbor 192.168.1.2 timers 3 10 ! address-family ipv4 unicast neighbor 192.168.1.2 route-map r2 in exit-address-family exit ! bgp as-path access-list FIRST seq 5 permit ^65 bgp as-path access-list SECOND seq 5 permit 2$ ! route-map r2 permit 6 match ip address prefix-list p2 set as-path exclude as-path-access-list SECOND exit ! route-map r2 permit 10 match ip address prefix-list p1 set as-path exclude 65003 exit ! route-map r2 permit 20 match ip address prefix-list p3 set as-path exclude all exit making some no bgp as-path access-list SECOND permit 2$ bgp as-path access-list SECOND permit 3$ clear bgp * no bgp as-path access-list SECOND permit 3$ bgp as-path access-list SECOND permit 2$ clear bgp * will induce some crashes thus we rework the links between aslists and aspath_exclude Signed-off-by: Francois Dumontet --- bgpd/bgp_aspath.c | 47 ++++++++++++++++++++++++++++++++++++ bgpd/bgp_aspath.h | 7 ++++++ bgpd/bgp_filter.c | 41 +++++++++++++++++++------------ bgpd/bgp_filter.h | 13 +++++----- bgpd/bgp_routemap.c | 59 ++++++++++++++++++--------------------------- 5 files changed, 108 insertions(+), 59 deletions(-) diff --git a/bgpd/bgp_aspath.c b/bgpd/bgp_aspath.c index 9dcd0ad1d621..4c1615a5c625 100644 --- a/bgpd/bgp_aspath.c +++ b/bgpd/bgp_aspath.c @@ -77,6 +77,9 @@ static struct hash *ashash; /* Stream for SNMP. See aspath_snmp_pathseg */ static struct stream *snmp_stream; +/* as-path orphan exclude list */ +static struct as_list_list_head as_exclude_list_orphan; + /* Callers are required to initialize the memory */ static as_t *assegment_data_new(int num) { @@ -1558,6 +1561,38 @@ struct aspath *aspath_prepend(struct aspath *as1, struct aspath *as2) /* Not reached */ } +/* insert aspath exclude in head of orphan exclude list*/ +void as_exclude_set_orphan(struct aspath_exclude *ase) +{ + ase->exclude_aspath_acl = NULL; + as_list_list_add_head(&as_exclude_list_orphan, ase); +} + +void as_exclude_remove_orphan(struct aspath_exclude *ase) +{ + if (as_list_list_count(&as_exclude_list_orphan)) + as_list_list_del(&as_exclude_list_orphan, ase); +} + +/* currently provide only one exclude, not a list */ +struct aspath_exclude *as_exclude_lookup_orphan(const char *acl_name) +{ + struct aspath_exclude *ase = NULL; + char *name = NULL; + + frr_each (as_list_list, &as_exclude_list_orphan, ase) { + if (ase->exclude_aspath_acl_name) { + name = ase->exclude_aspath_acl_name; + if (!strcmp(name, acl_name)) + break; + } + } + if (ase) + as_exclude_remove_orphan(ase); + + return ase; +} + /* Iterate over AS_PATH segments and wipe all occurrences of the * listed AS numbers. Hence some segments may lose some or even * all data on the way, the operation is implemented as a smarter @@ -2236,14 +2271,26 @@ void aspath_init(void) { ashash = hash_create_size(32768, aspath_key_make, aspath_cmp, "BGP AS Path"); + + as_list_list_init(&as_exclude_list_orphan); } void aspath_finish(void) { + struct aspath_exclude *ase; + hash_clean_and_free(&ashash, (void (*)(void *))aspath_free); if (snmp_stream) stream_free(snmp_stream); + + while ((ase = as_list_list_pop(&as_exclude_list_orphan))) { + aspath_free(ase->aspath); + if (ase->exclude_aspath_acl_name) + XFREE(MTYPE_TMP, ase->exclude_aspath_acl_name); + XFREE(MTYPE_ROUTE_MAP_COMPILED, ase); + } + as_list_list_fini(&as_exclude_list_orphan); } /* return and as path value */ diff --git a/bgpd/bgp_aspath.h b/bgpd/bgp_aspath.h index 2a831c3a5510..f7e57fd66dda 100644 --- a/bgpd/bgp_aspath.h +++ b/bgpd/bgp_aspath.h @@ -9,6 +9,7 @@ #include "lib/json.h" #include "bgpd/bgp_route.h" #include "bgpd/bgp_filter.h" +#include /* AS path segment type. */ #define AS_SET 1 @@ -67,11 +68,14 @@ struct aspath { /* `set as-path exclude ASn' */ struct aspath_exclude { + struct as_list_list_item exclude_list; struct aspath *aspath; bool exclude_all; char *exclude_aspath_acl_name; struct as_list *exclude_aspath_acl; }; +DECLARE_DLIST(as_list_list, struct aspath_exclude, exclude_list); + /* Prototypes. */ extern void aspath_init(void); @@ -83,6 +87,9 @@ extern struct aspath *aspath_parse(struct stream *s, size_t length, extern struct aspath *aspath_dup(struct aspath *aspath); extern struct aspath *aspath_aggregate(struct aspath *as1, struct aspath *as2); extern struct aspath *aspath_prepend(struct aspath *as1, struct aspath *as2); +extern void as_exclude_set_orphan(struct aspath_exclude *ase); +extern void as_exclude_remove_orphan(struct aspath_exclude *ase); +extern struct aspath_exclude *as_exclude_lookup_orphan(const char *acl_name); extern struct aspath *aspath_filter_exclude(struct aspath *source, struct aspath *exclude_list); extern struct aspath *aspath_filter_exclude_all(struct aspath *source); diff --git a/bgpd/bgp_filter.c b/bgpd/bgp_filter.c index a85117965aaa..002f054f5e41 100644 --- a/bgpd/bgp_filter.c +++ b/bgpd/bgp_filter.c @@ -16,7 +16,7 @@ #include "bgpd/bgp_aspath.h" #include "bgpd/bgp_regex.h" -/* List of AS filter list. */ +/* List of AS list. */ struct as_list_list { struct as_list *head; struct as_list *tail; @@ -205,14 +205,6 @@ static struct as_list *as_list_new(void) static void as_list_free(struct as_list *aslist) { - struct aspath_exclude_list *cur_bp = aslist->exclude_list; - struct aspath_exclude_list *next_bp = NULL; - - while (cur_bp) { - next_bp = cur_bp->next; - XFREE(MTYPE_ROUTE_MAP_COMPILED, cur_bp); - cur_bp = next_bp; - } XFREE (MTYPE_AS_STR, aslist->name); XFREE (MTYPE_AS_LIST, aslist); @@ -299,7 +291,6 @@ static void as_list_delete(struct as_list *aslist) { struct as_list_list *list; struct as_filter *filter, *next; - struct aspath_exclude_list *cur_bp; for (filter = aslist->head; filter; filter = next) { next = filter->next; @@ -318,12 +309,6 @@ static void as_list_delete(struct as_list *aslist) else list->head = aslist->next; - cur_bp = aslist->exclude_list; - while (cur_bp) { - cur_bp->bp_as_excl->exclude_aspath_acl = NULL; - cur_bp = cur_bp->next; - } - as_list_free(aslist); } @@ -431,6 +416,7 @@ DEFUN(as_path, bgp_as_path_cmd, enum as_filter_type type; struct as_filter *asfilter; struct as_list *aslist; + struct aspath_exclude *ase; regex_t *regex; char *regstr; int64_t seqnum = ASPATH_SEQ_NUMBER_AUTO; @@ -482,6 +468,22 @@ DEFUN(as_path, bgp_as_path_cmd, else as_list_filter_add(aslist, asfilter); + /* init the exclude rule list*/ + as_list_list_init(&aslist->exclude_rule); + + /* get aspath orphan exclude that are using this acl */ + ase = as_exclude_lookup_orphan(alname); + if (ase) { + as_list_list_add_head(&aslist->exclude_rule, ase); + /* set reverse pointer */ + ase->exclude_aspath_acl = aslist; + /* set list of aspath excludes using that acl */ + while ((ase = as_exclude_lookup_orphan(alname))) { + as_list_list_add_head(&aslist->exclude_rule, ase); + ase->exclude_aspath_acl = aslist; + } + } + return CMD_SUCCESS; } @@ -502,6 +504,7 @@ DEFUN(no_as_path, no_bgp_as_path_cmd, enum as_filter_type type; struct as_filter *asfilter; struct as_list *aslist; + struct aspath_exclude *ase; char *regstr; regex_t *regex; @@ -556,6 +559,12 @@ DEFUN(no_as_path, no_bgp_as_path_cmd, XFREE(MTYPE_TMP, regstr); + /* put aspath exclude list into orphan */ + if (as_list_list_count(&aslist->exclude_rule)) + while ((ase = as_list_list_pop(&aslist->exclude_rule))) + as_exclude_set_orphan(ase); + + as_list_list_fini(&aslist->exclude_rule); as_list_filter_delete(aslist, asfilter); return CMD_SUCCESS; diff --git a/bgpd/bgp_filter.h b/bgpd/bgp_filter.h index 2d9f07ce84a3..77a3f3c2f727 100644 --- a/bgpd/bgp_filter.h +++ b/bgpd/bgp_filter.h @@ -6,11 +6,12 @@ #ifndef _QUAGGA_BGP_FILTER_H #define _QUAGGA_BGP_FILTER_H +#include + #define ASPATH_SEQ_NUMBER_AUTO -1 enum as_filter_type { AS_FILTER_DENY, AS_FILTER_PERMIT }; - /* Element of AS path filter. */ struct as_filter { struct as_filter *next; @@ -25,11 +26,7 @@ struct as_filter { int64_t seq; }; -struct aspath_exclude_list { - struct aspath_exclude_list *next; - struct aspath_exclude *bp_as_excl; -}; - +PREDECL_DLIST(as_list_list); /* AS path filter list. */ struct as_list { char *name; @@ -39,7 +36,9 @@ struct as_list { struct as_filter *head; struct as_filter *tail; - struct aspath_exclude_list *exclude_list; + + /* Changes in AS path */ + struct as_list_list_head exclude_rule; }; diff --git a/bgpd/bgp_routemap.c b/bgpd/bgp_routemap.c index df5c2557bd26..f938869b1c48 100644 --- a/bgpd/bgp_routemap.c +++ b/bgpd/bgp_routemap.c @@ -2325,7 +2325,7 @@ static const struct route_map_rule_cmd route_set_aspath_prepend_cmd = { static void *route_aspath_exclude_compile(const char *arg) { struct aspath_exclude *ase; - struct aspath_exclude_list *ael; + struct as_list *aux_aslist; const char *str = arg; static const char asp_acl[] = "as-path-access-list"; @@ -2337,44 +2337,37 @@ static void *route_aspath_exclude_compile(const char *arg) while (*str == ' ') str++; ase->exclude_aspath_acl_name = XSTRDUP(MTYPE_TMP, str); - ase->exclude_aspath_acl = as_list_lookup(str); + aux_aslist = as_list_lookup(str); + if (!aux_aslist) + /* new orphan filter */ + as_exclude_set_orphan(ase); + else + as_list_list_add_head(&aux_aslist->exclude_rule, ase); + + ase->exclude_aspath_acl = aux_aslist; } else ase->aspath = aspath_str2aspath(str, bgp_get_asnotation(NULL)); - if (ase->exclude_aspath_acl) { - ael = XCALLOC(MTYPE_ROUTE_MAP_COMPILED, - sizeof(struct aspath_exclude_list)); - ael->bp_as_excl = ase; - ael->next = ase->exclude_aspath_acl->exclude_list; - ase->exclude_aspath_acl->exclude_list = ael; - } - return ase; } static void route_aspath_exclude_free(void *rule) { struct aspath_exclude *ase = rule; - struct aspath_exclude_list *cur_ael = NULL; - struct aspath_exclude_list *prev_ael = NULL; + struct as_list *acl; + + /* manage references to that rule*/ + if (ase->exclude_aspath_acl) { + acl = ase->exclude_aspath_acl; + as_list_list_del(&acl->exclude_rule, ase); + } else { + /* no ref to acl, this aspath exclude is orphan */ + as_exclude_remove_orphan(ase); + } aspath_free(ase->aspath); if (ase->exclude_aspath_acl_name) XFREE(MTYPE_TMP, ase->exclude_aspath_acl_name); - if (ase->exclude_aspath_acl) - cur_ael = ase->exclude_aspath_acl->exclude_list; - while (cur_ael) { - if (cur_ael->bp_as_excl == ase) { - if (prev_ael) - prev_ael->next = cur_ael->next; - else - ase->exclude_aspath_acl->exclude_list = NULL; - XFREE(MTYPE_ROUTE_MAP_COMPILED, cur_ael); - break; - } - prev_ael = cur_ael; - cur_ael = cur_ael->next; - } XFREE(MTYPE_ROUTE_MAP_COMPILED, ase); } @@ -2409,16 +2402,10 @@ route_set_aspath_exclude(void *rule, const struct prefix *dummy, void *object) else if (ase->exclude_all) path->attr->aspath = aspath_filter_exclude_all(new_path); - else if (ase->exclude_aspath_acl_name) { - if (!ase->exclude_aspath_acl) - ase->exclude_aspath_acl = - as_list_lookup(ase->exclude_aspath_acl_name); - if (ase->exclude_aspath_acl) - path->attr->aspath = - aspath_filter_exclude_acl(new_path, - ase->exclude_aspath_acl); - } - + else if (ase->exclude_aspath_acl) + path->attr->aspath = + aspath_filter_exclude_acl(new_path, + ase->exclude_aspath_acl); return RMAP_OKAY; } From 0df2e149970beff39915d0095614d56d5859f3ff Mon Sep 17 00:00:00 2001 From: Francois Dumontet Date: Wed, 24 Apr 2024 14:34:48 +0200 Subject: [PATCH 02/28] tests: improve tests for aspath exclude and bgp access list add some match in route map rules add some set unset bgp access path list add another prefix for better tests discrimination update expected results Signed-off-by: Francois Dumontet --- .../bgp_set_aspath_exclude/r1/bgpd.conf | 9 +++ .../bgp_set_aspath_exclude/r3/zebra.conf | 1 + .../test_bgp_set_aspath_exclude.py | 70 +++++++++++++++---- 3 files changed, 67 insertions(+), 13 deletions(-) diff --git a/tests/topotests/bgp_set_aspath_exclude/r1/bgpd.conf b/tests/topotests/bgp_set_aspath_exclude/r1/bgpd.conf index 9bef24f93154..c70b4934a079 100644 --- a/tests/topotests/bgp_set_aspath_exclude/r1/bgpd.conf +++ b/tests/topotests/bgp_set_aspath_exclude/r1/bgpd.conf @@ -8,10 +8,19 @@ router bgp 65001 exit-address-family ! ip prefix-list p1 seq 5 permit 172.16.255.31/32 +ip prefix-list p2 seq 5 permit 172.16.255.32/32 +ip prefix-list p3 seq 5 permit 172.16.255.30/32 ! +bgp as-path access-list FIRST permit ^65 +bgp as-path access-list SECOND permit 2$ + +route-map r2 permit 6 + match ip address prefix-list p2 + set as-path exclude as-path-access-list SECOND route-map r2 permit 10 match ip address prefix-list p1 set as-path exclude 65003 route-map r2 permit 20 + match ip address prefix-list p3 set as-path exclude all ! diff --git a/tests/topotests/bgp_set_aspath_exclude/r3/zebra.conf b/tests/topotests/bgp_set_aspath_exclude/r3/zebra.conf index 3fa6c644844c..56893158a403 100644 --- a/tests/topotests/bgp_set_aspath_exclude/r3/zebra.conf +++ b/tests/topotests/bgp_set_aspath_exclude/r3/zebra.conf @@ -1,5 +1,6 @@ ! int lo + ip address 172.16.255.30/32 ip address 172.16.255.31/32 ip address 172.16.255.32/32 ! diff --git a/tests/topotests/bgp_set_aspath_exclude/test_bgp_set_aspath_exclude.py b/tests/topotests/bgp_set_aspath_exclude/test_bgp_set_aspath_exclude.py index d373a749fe87..85e7b9676d0e 100644 --- a/tests/topotests/bgp_set_aspath_exclude/test_bgp_set_aspath_exclude.py +++ b/tests/topotests/bgp_set_aspath_exclude/test_bgp_set_aspath_exclude.py @@ -64,29 +64,33 @@ def teardown_module(mod): expected_1 = { "routes": { + "172.16.255.30/32": [{"path": ""}], "172.16.255.31/32": [{"path": "65002"}], - "172.16.255.32/32": [{"path": ""}], + "172.16.255.32/32": [{"path": "65003"}], } } expected_2 = { "routes": { - "172.16.255.31/32": [{"path": ""}], + "172.16.255.30/32": [{"path": ""}], + "172.16.255.31/32": [{"path": "65002"}], "172.16.255.32/32": [{"path": ""}], } } expected_3 = { "routes": { - "172.16.255.31/32": [{"path": "65003"}], - "172.16.255.32/32": [{"path": "65003"}], + "172.16.255.30/32": [{"path": ""}], + "172.16.255.31/32": [{"path": "65002"}], + "172.16.255.32/32": [{"path": "65002 65003"}], } } expected_4 = { "routes": { - "172.16.255.31/32": [{"path": "65002 65003"}], - "172.16.255.32/32": [{"path": "65002 65003"}], + "172.16.255.30/32": [{"path": ""}], + "172.16.255.31/32": [{"path": "65002"}], + "172.16.255.32/32": [{"path": "65002"}], } } @@ -117,34 +121,42 @@ def test_bgp_set_aspath_exclude_access_list(): rname = "r1" r1 = tgen.gears[rname] + # tgen.mininet_cli() r1.vtysh_cmd( """ conf bgp as-path access-list FIRST permit ^65 route-map r2 permit 6 + no set as-path exclude as-path-access-list SECOND set as-path exclude as-path-access-list FIRST """ ) + # tgen.mininet_cli() + r1.vtysh_cmd( + """ +clear bgp * + """ + ) test_func = functools.partial(bgp_converge, tgen.gears["r1"], expected_2) _, result = topotest.run_and_expect(test_func, None, count=30, wait=0.5) - assert result is None, "Failed overriding incoming AS-PATH with regex 1 route-map" + assert result is None, "Failed change of exclude rule in route map" r1.vtysh_cmd( """ conf - bgp as-path access-list SECOND permit 2 route-map r2 permit 6 + no set as-path exclude as-path-access-list FIRST set as-path exclude as-path-access-list SECOND """ ) # tgen.mininet_cli() - test_func = functools.partial(bgp_converge, tgen.gears["r1"], expected_3) + test_func = functools.partial(bgp_converge, tgen.gears["r1"], expected_1) _, result = topotest.run_and_expect(test_func, None, count=30, wait=0.5) - assert result is None, "Failed overriding incoming AS-PATH with regex 2 route-map" + assert result is None, "Failed reverting exclude rule in route map" def test_no_bgp_set_aspath_exclude_access_list(): @@ -159,15 +171,28 @@ def test_no_bgp_set_aspath_exclude_access_list(): r1.vtysh_cmd( """ conf - no bgp as-path access-list SECOND permit 2 + no bgp as-path access-list SECOND permit 2$ + """ + ) + + r1.vtysh_cmd( + """ +clear bgp * """ ) test_func = functools.partial(bgp_converge, tgen.gears["r1"], expected_3) _, result = topotest.run_and_expect(test_func, None, count=30, wait=0.5) - assert result is None, "Failed removing bgp as-path access-list" + assert result is None, "Failed to removing current accesslist" + # tgen.mininet_cli() + r1.vtysh_cmd( + """ +conf + bgp as-path access-list SECOND permit 3$ + """ + ) r1.vtysh_cmd( """ clear bgp * @@ -177,7 +202,26 @@ def test_no_bgp_set_aspath_exclude_access_list(): test_func = functools.partial(bgp_converge, tgen.gears["r1"], expected_4) _, result = topotest.run_and_expect(test_func, None, count=30, wait=0.5) - assert result is None, "Failed to renegotiate with peers" + assert result is None, "Failed to renegotiate with peers 2" + + r1.vtysh_cmd( + """ +conf + route-map r2 permit 6 + no set as-path exclude as-path-access-list SECOND + """ + ) + + r1.vtysh_cmd( + """ +clear bgp * + """ + ) + + test_func = functools.partial(bgp_converge, tgen.gears["r1"], expected_3) + _, result = topotest.run_and_expect(test_func, None, count=30, wait=0.5) + + assert result is None, "Failed to renegotiate with peers 2" if __name__ == "__main__": From 5d7af51c4f7980507135babd94d392ca179c1bf7 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Wed, 12 Jun 2024 22:54:45 +0300 Subject: [PATCH 03/28] bgpd: Check if we have really enough data before doing memcpy for software version If we receive CAPABILITY message (software-version), we SHOULD check if we really have enough data before doing memcpy(), that could also lead to buffer overflow. (data + len > end) is not enough, because after this check we do data++ and later memcpy(..., data, len). That means we have one more byte. Hit this through fuzzing by ``` 0 0xaaaaaadf872c in __asan_memcpy (/home/ubuntu/frr-public/frr_public_private-libfuzzer/bgpd/.libs/bgpd+0x35872c) (BuildId: 9c6e455d0d9a20f5a4d2f035b443f50add9564d7) 1 0xaaaaab06bfbc in bgp_dynamic_capability_software_version /home/ubuntu/frr-public/frr_public_private-libfuzzer/bgpd/bgp_packet.c:3713:3 2 0xaaaaab05ccb4 in bgp_capability_msg_parse /home/ubuntu/frr-public/frr_public_private-libfuzzer/bgpd/bgp_packet.c:3839:4 3 0xaaaaab05c074 in bgp_capability_receive /home/ubuntu/frr-public/frr_public_private-libfuzzer/bgpd/bgp_packet.c:3980:9 4 0xaaaaab05e48c in bgp_process_packet /home/ubuntu/frr-public/frr_public_private-libfuzzer/bgpd/bgp_packet.c:4109:11 5 0xaaaaaae36150 in LLVMFuzzerTestOneInput /home/ubuntu/frr-public/frr_public_private-libfuzzer/bgpd/bgp_main.c:582:3 ``` Hit this again by Iggy \m/ Reported-by: Iggy Frankovic Signed-off-by: Donatas Abraitis --- bgpd/bgp_packet.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c index 3f38790cbda3..5d9789432646 100644 --- a/bgpd/bgp_packet.c +++ b/bgpd/bgp_packet.c @@ -3694,7 +3694,7 @@ static void bgp_dynamic_capability_software_version(uint8_t *pnt, int action, char soft_version[BGP_MAX_SOFT_VERSION + 1] = {}; if (action == CAPABILITY_ACTION_SET) { - if (data + len > end) { + if (data + len + 1 > end) { zlog_err("%pBP: Received invalid Software Version capability length %d", peer, len); return; From b685ab5e1bdec0848502c20e9596b9716b236639 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Thu, 13 Jun 2024 08:12:10 +0300 Subject: [PATCH 04/28] bgpd: Check if we have really enough data before doing memcpy for FQDN capability We advance data pointer (data++), but we do memcpy() with the length that is 1-byte over, which is technically heap overflow. ``` ==411461==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x50600011da1a at pc 0xc4f45a9786f0 bp 0xffffed1e2740 sp 0xffffed1e1f30 READ of size 4 at 0x50600011da1a thread T0 0 0xc4f45a9786ec in __asan_memcpy (/home/ubuntu/frr-public/frr_public_private-libfuzzer/bgpd/.libs/bgpd+0x3586ec) (BuildId: e794c5f796eee20c8973d7efb9bf5735e54d44cd) 1 0xc4f45abf15f8 in bgp_dynamic_capability_fqdn /home/ubuntu/frr-public/frr_public_private-libfuzzer/bgpd/bgp_packet.c:3457:4 2 0xc4f45abdd408 in bgp_capability_msg_parse /home/ubuntu/frr-public/frr_public_private-libfuzzer/bgpd/bgp_packet.c:3911:4 3 0xc4f45abdbeb4 in bgp_capability_receive /home/ubuntu/frr-public/frr_public_private-libfuzzer/bgpd/bgp_packet.c:3980:9 4 0xc4f45abde2cc in bgp_process_packet /home/ubuntu/frr-public/frr_public_private-libfuzzer/bgpd/bgp_packet.c:4109:11 5 0xc4f45a9b6110 in LLVMFuzzerTestOneInput /home/ubuntu/frr-public/frr_public_private-libfuzzer/bgpd/bgp_main.c:582:3 ``` Found by fuzzing. Reported-by: Iggy Frankovic Signed-off-by: Donatas Abraitis --- bgpd/bgp_packet.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c index 3f38790cbda3..e54766aa3958 100644 --- a/bgpd/bgp_packet.c +++ b/bgpd/bgp_packet.c @@ -3437,7 +3437,7 @@ static void bgp_dynamic_capability_fqdn(uint8_t *pnt, int action, } len = *data; - if (data + len > end) { + if (data + len + 1 > end) { zlog_err("%pBP: Received invalid FQDN capability length (host name) %d", peer, hdr->length); return; @@ -3468,7 +3468,7 @@ static void bgp_dynamic_capability_fqdn(uint8_t *pnt, int action, /* domainname */ len = *data; - if (data + len > end) { + if (data + len + 1 > end) { zlog_err("%pBP: Received invalid FQDN capability length (domain name) %d", peer, len); return; From 34b209f0ae2caca0d1ebcde9d4095375ac31b562 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Thu, 13 Jun 2024 08:43:21 +0300 Subject: [PATCH 05/28] bgpd: Adjust the length of tunnel encap sub-tlv by sub-tlv type Fixes: 79563af564ad0fe5b9c8d95bf080d570f87b1859 ("bgpd: Get 1 or 2 octets for Sub-TLV length (Tunnel Encap attr)") Signed-off-by: Donatas Abraitis --- bgpd/bgp_attr.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index da4701d069db..9f377589e960 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -2728,10 +2728,13 @@ static int bgp_attr_encap(struct bgp_attr_parser_args *args) if (BGP_ATTR_ENCAP == type) { subtype = stream_getc(BGP_INPUT(peer)); - sublength = (subtype < 128) - ? stream_getc(BGP_INPUT(peer)) - : stream_getw(BGP_INPUT(peer)); - length -= 2; + if (subtype < 128) { + sublength = stream_getc(BGP_INPUT(peer)); + length -= 2; + } else { + sublength = stream_getw(BGP_INPUT(peer)); + length -= 3; + } #ifdef ENABLE_BGP_VNC } else { subtype = stream_getw(BGP_INPUT(peer)); From 9929486d6bdb28469a5b626a17d5bc9991c83ce3 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Thu, 13 Jun 2024 09:00:21 +0300 Subject: [PATCH 06/28] bgpd: Check if we have real stream data for tunnel encapsulation sub-tlvs When the packet is malformed it can use whatever values it wants. Let's check what the real data we have in a stream instead of relying on malformed values. Reported-by: Iggy Frankovic Signed-off-by: Donatas Abraitis --- bgpd/bgp_attr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index 9f377589e960..18c7b13535d0 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -2721,7 +2721,7 @@ static int bgp_attr_encap(struct bgp_attr_parser_args *args) } } - while (length >= 4) { + while (STREAM_READABLE(BGP_INPUT(peer)) >= 4) { uint16_t subtype = 0; uint16_t sublength = 0; struct bgp_attr_encap_subtlv *tlv; From 686d477242890c575024962188dd50c06160f2a3 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Fri, 14 Jun 2024 10:30:31 +0300 Subject: [PATCH 07/28] docker: Use Alpine Linux 3.20 Signed-off-by: Donatas Abraitis --- docker/alpine/Dockerfile | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docker/alpine/Dockerfile b/docker/alpine/Dockerfile index 1cff06feee21..3f811455b394 100644 --- a/docker/alpine/Dockerfile +++ b/docker/alpine/Dockerfile @@ -1,7 +1,7 @@ # syntax=docker/dockerfile:1 # Create a basic stage set up to build APKs -FROM alpine:3.19 as alpine-builder +FROM alpine:3.20 as alpine-builder RUN apk add \ --update-cache \ abuild \ @@ -24,7 +24,7 @@ RUN cd /src/libyang \ && abuild -r -P /pkgs/apk # This stage builds a dist tarball from the source -FROM alpine:3.19 as source-builder +FROM alpine:3.20 as source-builder RUN mkdir -p /src/alpine /pkgs/apk COPY alpine/APKBUILD.in /src/alpine COPY --from=alpine-apk-builder-libyang /pkgs/apk/src /pkgs/apk @@ -57,7 +57,7 @@ RUN cd /dist \ && abuild -r -P /pkgs/apk # This stage installs frr from the apk -FROM alpine:3.19 +FROM alpine:3.20 RUN mkdir -p /pkgs/apk COPY --from=alpine-apk-builder /pkgs/apk/ /pkgs/apk/ RUN apk add \ From 2876847d500b7b734fa290e6dc9aa151de9f9a48 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Fri, 14 Jun 2024 10:35:27 +0300 Subject: [PATCH 08/28] docker: Do not require checks for libyang Alpine Linux package build Since Alpine Linux 3.20, we get this: >>> WARNING: libyang: APKBUILD does not run any tests! Alpine policy will soon require that packages have any relevant testsuites run during the build process. To fix, either define a check() function, or declare !check in $options to indicate the package does not have a testsuite. Signed-off-by: Donatas Abraitis --- docker/alpine/libyang/APKBUILD | 1 + 1 file changed, 1 insertion(+) diff --git a/docker/alpine/libyang/APKBUILD b/docker/alpine/libyang/APKBUILD index 6973fd62d865..7979ee1b1ed2 100755 --- a/docker/alpine/libyang/APKBUILD +++ b/docker/alpine/libyang/APKBUILD @@ -11,6 +11,7 @@ makedepends="bison cmake cmocka-dev flex pcre2-dev" checkdepends="expect grep shunit2" subpackages="$pkgname-dev $pkgname-doc" source="$pkgname-$pkgver.tar.gz::https://github.com/CESNET/libyang/archive/v$pkgver.tar.gz" +options="!check" # secfixes: # 1.0.215-r1: From 0314adac4b906ac6c9abcb702f9fd799b89d93f2 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Fri, 14 Jun 2024 11:37:23 +0300 Subject: [PATCH 09/28] docker: Set ABUILD_APK_INDEX_OPTS for libyang In build() stage of abuild, it does `apk index ...` where libyang* packages are unsigned. We don't sign them here, and thus we need to specify `--allow-untrusted`. Signed-off-by: Donatas Abraitis --- docker/alpine/libyang/APKBUILD | 1 + 1 file changed, 1 insertion(+) diff --git a/docker/alpine/libyang/APKBUILD b/docker/alpine/libyang/APKBUILD index 7979ee1b1ed2..d8cd4d918a53 100755 --- a/docker/alpine/libyang/APKBUILD +++ b/docker/alpine/libyang/APKBUILD @@ -22,6 +22,7 @@ options="!check" # - CVE-2021-28906 build() { + export ABUILD_APK_INDEX_OPTS="--allow-untrusted" if [ "$CBUILD" != "$CHOST" ]; then CMAKE_CROSSOPTS="-DCMAKE_SYSTEM_NAME=Linux -DCMAKE_HOST_SYSTEM_NAME=Linux" fi From 09db258157576999975a9d95add4ae327f7f3a5c Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Fri, 14 Jun 2024 16:33:32 +0300 Subject: [PATCH 10/28] docker: Set ABUILD_APK_INDEX_OPTS for frr build In build() stage of abuild, it does `apk index ...` where frr* packages are unsigned. We don't sign them here, and thus we need to specify `--allow-untrusted`. Signed-off-by: Donatas Abraitis --- alpine/APKBUILD.in | 2 ++ 1 file changed, 2 insertions(+) diff --git a/alpine/APKBUILD.in b/alpine/APKBUILD.in index 2cb3feec157f..855b5859039c 100644 --- a/alpine/APKBUILD.in +++ b/alpine/APKBUILD.in @@ -33,6 +33,8 @@ _libdir=/usr/lib _user=frr build() { + export ABUILD_APK_INDEX_OPTS="--allow-untrusted" + cd "$builddir" ./configure \ From c25c7e929d550c2faca3af74a29593b8c0b75db3 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Wed, 19 Jun 2024 14:09:00 +0300 Subject: [PATCH 11/28] bgpd: Set last reset reason to admin shutdown if it was manually Before this patch, we always printed the last reason "Waiting for OPEN", but if it's a manual shutdown, then we technically are not waiting for OPEN. Signed-off-by: Donatas Abraitis --- bgpd/bgpd.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index 869d2b455214..0ad97166331a 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -4829,6 +4829,8 @@ void bgp_shutdown_enable(struct bgp *bgp, const char *msg) /* iterate through peers of BGP instance */ for (ALL_LIST_ELEMENTS_RO(bgp->peer, node, peer)) { + peer->last_reset = PEER_DOWN_USER_SHUTDOWN; + /* continue, if peer is already in administrative shutdown. */ if (CHECK_FLAG(peer->flags, PEER_FLAG_SHUTDOWN)) continue; @@ -4883,8 +4885,10 @@ void bgp_shutdown_disable(struct bgp *bgp) /* clear the BGP instances shutdown flag */ UNSET_FLAG(bgp->flags, BGP_FLAG_SHUTDOWN); - for (ALL_LIST_ELEMENTS_RO(bgp->peer, node, peer)) + for (ALL_LIST_ELEMENTS_RO(bgp->peer, node, peer)) { bgp_timer_set(peer->connection); + peer->last_reset = PEER_DOWN_WAITING_OPEN; + } } /* Change specified peer flag. */ @@ -4956,6 +4960,10 @@ static int peer_flag_modify(struct peer *peer, uint64_t flag, int set) bgp_zebra_terminate_radv(peer->bgp, peer); } + if (flag == PEER_FLAG_SHUTDOWN) + peer->last_reset = set ? PEER_DOWN_USER_SHUTDOWN + : PEER_DOWN_WAITING_OPEN; + /* Execute flag action on peer. */ if (action.type == peer_change_reset) peer_flag_modify_action(peer, flag); @@ -4991,6 +4999,10 @@ static int peer_flag_modify(struct peer *peer, uint64_t flag, int set) set ? bgp_zebra_initiate_radv(member->bgp, member) : bgp_zebra_terminate_radv(member->bgp, member); + if (flag == PEER_FLAG_SHUTDOWN) + member->last_reset = set ? PEER_DOWN_USER_SHUTDOWN + : PEER_DOWN_WAITING_OPEN; + /* Execute flag action on peer-group member. */ if (action.type == peer_change_reset) peer_flag_modify_action(member, flag); From b5bd626a82b2541bee8e3120139e19ba05e444c8 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Wed, 19 Jun 2024 14:32:16 +0300 Subject: [PATCH 12/28] bgpd: Remove redundant whitespace before printing the reason of the failed peer Before: ``` Neighbor EstdCnt DropCnt ResetTime Reason 127.0.0.1 0 0 never Waiting for peer OPEN (n/a) ``` After: ``` Neighbor EstdCnt DropCnt ResetTime Reason 127.0.0.1 0 0 never Waiting for peer OPEN (n/a) ``` Signed-off-by: Donatas Abraitis --- bgpd/bgp_vty.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index fbe1db9d2a00..230fedf4ece8 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -11661,10 +11661,9 @@ static void bgp_show_peer_reset(struct vty * vty, struct peer *peer, BGP_NOTIFY_CEASE_HARD_RESET) : ""); } else { - vty_out(vty, " %s (%s)\n", + vty_out(vty, " %s (%s)\n", peer_down_str[(int)peer->last_reset], - peer->soft_version ? peer->soft_version - : "n/a"); + peer->soft_version ? peer->soft_version : "n/a"); } } } From f18cb4fb58cd443fad02ed991387b301885aaaea Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Wed, 8 May 2024 19:27:22 +0200 Subject: [PATCH 13/28] pimd: make clang-format slightly less annoying The YANG module list is really better as 1-item-per-line. Signed-off-by: David Lamparter --- pimd/pim6_main.c | 2 +- pimd/pim_main.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pimd/pim6_main.c b/pimd/pim6_main.c index 5ce6985c4520..24443404eb5e 100644 --- a/pimd/pim6_main.c +++ b/pimd/pim6_main.c @@ -94,6 +94,7 @@ struct frr_signal_t pim6d_signals[] = { }, }; +/* clang-format off */ static const struct frr_yang_module_info *const pim6d_yang_modules[] = { &frr_filter_info, &frr_interface_info, @@ -105,7 +106,6 @@ static const struct frr_yang_module_info *const pim6d_yang_modules[] = { &frr_gmp_info, }; -/* clang-format off */ FRR_DAEMON_INFO(pim6d, PIM6, .vty_port = PIM6D_VTY_PORT, .proghelp = "Protocol Independent Multicast (RFC7761) for IPv6", diff --git a/pimd/pim_main.c b/pimd/pim_main.c index 400db396c289..8f2ce0bed335 100644 --- a/pimd/pim_main.c +++ b/pimd/pim_main.c @@ -59,6 +59,7 @@ struct zebra_privs_t pimd_privs = { .cap_num_p = array_size(_caps_p), .cap_num_i = 0}; +/* clang-format off */ static const struct frr_yang_module_info *const pimd_yang_modules[] = { &frr_filter_info, &frr_interface_info, @@ -70,7 +71,6 @@ static const struct frr_yang_module_info *const pimd_yang_modules[] = { &frr_gmp_info, }; -/* clang-format off */ FRR_DAEMON_INFO(pimd, PIM, .vty_port = PIMD_VTY_PORT, .proghelp = "Implementation of the PIM routing protocol.", From 1f223ae1eed80f2552a279ee34464edf363bba6f Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Mon, 11 Jul 2022 21:58:27 +0200 Subject: [PATCH 14/28] pimd: add debugs for i-am-RP state This proved helpful in debugging the Candidate-RP code. Signed-off-by: David Lamparter --- pimd/pim_rp.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pimd/pim_rp.c b/pimd/pim_rp.c index d8d25712a30f..b0fb8a509af2 100644 --- a/pimd/pim_rp.c +++ b/pimd/pim_rp.c @@ -543,6 +543,9 @@ int pim_rp_new(struct pim_instance *pim, pim_addr rp_addr, struct prefix group, pim_zebra_update_all_interfaces(pim); pim_rp_check_interfaces(pim, rp_all); + if (rp_all->i_am_rp && PIM_DEBUG_PIM_NHT_RP) + zlog_debug("new RP %pPA for %pFX is ourselves", + &rp_all->rp.rpf_addr, &rp_all->group); pim_rp_refresh_group_to_rp_mapping(pim); pim_find_or_track_nexthop(pim, nht_p, NULL, rp_all, NULL); @@ -634,6 +637,9 @@ int pim_rp_new(struct pim_instance *pim, pim_addr rp_addr, struct prefix group, pim_zebra_update_all_interfaces(pim); pim_rp_check_interfaces(pim, rp_info); + if (rp_info->i_am_rp && PIM_DEBUG_PIM_NHT_RP) + zlog_debug("new RP %pPA for %pFX is ourselves", + &rp_info->rp.rpf_addr, &rp_info->group); pim_rp_refresh_group_to_rp_mapping(pim); /* Register addr with Zebra NHT */ From 18b82f64c9538b6fd3b9f61c86dfff8cbdc8c6b6 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Wed, 8 May 2024 19:25:55 +0200 Subject: [PATCH 15/28] pimd: add prefix_* and IANA_AFI v4/v6 defines The Candidate-RP code is about to use these. Signed-off-by: David Lamparter --- pimd/pim_addr.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pimd/pim_addr.h b/pimd/pim_addr.h index ecba739a5a4e..7b0c3f0350e2 100644 --- a/pimd/pim_addr.h +++ b/pimd/pim_addr.h @@ -14,11 +14,13 @@ #if PIM_IPV == 4 typedef struct in_addr pim_addr; +typedef struct prefix_ipv4 prefix_pim; #define PIM_ADDRSTRLEN INET_ADDRSTRLEN #define PIM_AF AF_INET #define PIM_AFI AFI_IP #define PIM_PROTO_REG IPPROTO_RAW +#define PIM_IANA_AFI IANA_AFI_IPV4 #define PIM_IPADDR IPADDR_V4 #define ipaddr_pim ipaddr_v4 #define PIM_MAX_BITLEN IPV4_MAX_BITLEN @@ -44,11 +46,13 @@ union pimprefixconstptr { #else typedef struct in6_addr pim_addr; +typedef struct prefix_ipv6 prefix_pim; #define PIM_ADDRSTRLEN INET6_ADDRSTRLEN #define PIM_AF AF_INET6 #define PIM_AFI AFI_IP6 #define PIM_PROTO_REG IPPROTO_PIM +#define PIM_IANA_AFI IANA_AFI_IPV6 #define PIM_IPADDR IPADDR_V6 #define ipaddr_pim ipaddr_v6 #define PIM_MAX_BITLEN IPV6_MAX_BITLEN From cca9bc193edfebbd93fbdd76bd4c3b8310f007d6 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Sat, 9 Jul 2022 18:47:43 +0200 Subject: [PATCH 16/28] pimd: allow sending packets without pinned iface The upcoming Candidate-RP code needs to send PIM packets that go through normal unicast routing, without forcing a specific output interface. Allow passing in NULL ifp to do that. Signed-off-by: David Lamparter --- pimd/pim_pim.c | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/pimd/pim_pim.c b/pimd/pim_pim.c index 1bc265b138f6..6a7e8924f2f6 100644 --- a/pimd/pim_pim.c +++ b/pimd/pim_pim.c @@ -636,17 +636,15 @@ static int pim_msg_send_frame(pim_addr src, pim_addr dst, ifindex_t ifindex, int pim_msg_send(int fd, pim_addr src, pim_addr dst, uint8_t *pim_msg, int pim_msg_size, struct interface *ifp) { - struct pim_interface *pim_ifp; - + if (ifp) { + struct pim_interface *pim_ifp = ifp->info; - pim_ifp = ifp->info; - - if (pim_ifp->pim_passive_enable) { - if (PIM_DEBUG_PIM_PACKETS) - zlog_debug( - "skip sending PIM message on passive interface %s", - ifp->name); - return 0; + if (pim_ifp->pim_passive_enable) { + if (PIM_DEBUG_PIM_PACKETS) + zlog_debug("skip sending PIM message on passive interface %s", + ifp->name); + return 0; + } } #if PIM_IPV == 4 @@ -710,7 +708,7 @@ int pim_msg_send(int fd, pim_addr src, pim_addr dst, uint8_t *pim_msg, if (PIM_DEBUG_PIM_PACKETS) zlog_debug("%s: to %pPA on %s: msg_size=%d checksum=%x", - __func__, &dst, ifp->name, pim_msg_size, + __func__, &dst, ifp ? ifp->name : "*", pim_msg_size, header->checksum); if (PIM_DEBUG_PIM_PACKETDUMP_SEND) { @@ -718,7 +716,7 @@ int pim_msg_send(int fd, pim_addr src, pim_addr dst, uint8_t *pim_msg, } pim_msg_send_frame(fd, (char *)buffer, sendlen, (struct sockaddr *)&to, - tolen, ifp->name); + tolen, ifp ? ifp->name : "*"); return 0; #else @@ -727,7 +725,7 @@ int pim_msg_send(int fd, pim_addr src, pim_addr dst, uint8_t *pim_msg, iovector[0].iov_base = pim_msg; iovector[0].iov_len = pim_msg_size; - pim_msg_send_frame(src, dst, ifp->ifindex, &iovector[0], fd); + pim_msg_send_frame(src, dst, ifp ? ifp->ifindex : 0, &iovector[0], fd); return 0; #endif From b9a02da83b65fa7066ef17f71a32f5aa0648e28e Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Fri, 25 Jun 2021 11:42:38 +0200 Subject: [PATCH 17/28] pimd: prepare NHT for tracking BSM C-RPs For BSMs, we should track which of the RP candidates in the BSM message are actually available, before trying to use them (which also puts them in NHT for that). This applies for both BSRs as well as BSM receivers. Signed-off-by: David Lamparter --- pimd/pim_bsm.c | 5 +++++ pimd/pim_nht.c | 47 ++++++++++++++++++++++++++++++++++++++++------- pimd/pim_nht.h | 7 +++++++ 3 files changed, 52 insertions(+), 7 deletions(-) diff --git a/pimd/pim_bsm.c b/pimd/pim_bsm.c index df9161943d37..2d451718a983 100644 --- a/pimd/pim_bsm.c +++ b/pimd/pim_bsm.c @@ -1451,3 +1451,8 @@ int pim_bsm_process(struct interface *ifp, pim_sgaddr *sg, uint8_t *buf, return 0; } + +void pim_crp_nht_update(struct pim_instance *pim, struct pim_nexthop_cache *pnc) +{ + /* stub for Candidate-RP */ +} diff --git a/pimd/pim_nht.c b/pimd/pim_nht.c index 32cdf4bf8289..57dcff3b4710 100644 --- a/pimd/pim_nht.c +++ b/pimd/pim_nht.c @@ -161,18 +161,27 @@ void pim_nht_bsr_add(struct pim_instance *pim, pim_addr addr) pnc->bsr_count++; } +bool pim_nht_candrp_add(struct pim_instance *pim, pim_addr addr) +{ + struct pim_nexthop_cache *pnc; + + pnc = pim_nht_get(pim, addr); + + pnc->candrp_count++; + return CHECK_FLAG(pnc->flags, PIM_NEXTHOP_VALID); +} + static void pim_nht_drop_maybe(struct pim_instance *pim, struct pim_nexthop_cache *pnc) { if (PIM_DEBUG_PIM_NHT) - zlog_debug( - "%s: NHT %pPA(%s) rp_list count:%d upstream count:%ld BSR count:%u", - __func__, &pnc->rpf.rpf_addr, pim->vrf->name, - pnc->rp_list->count, pnc->upstream_hash->count, - pnc->bsr_count); + zlog_debug("%s: NHT %pPA(%s) rp_list count:%d upstream count:%ld BSR count:%u Cand-RP count:%u", + __func__, &pnc->rpf.rpf_addr, pim->vrf->name, + pnc->rp_list->count, pnc->upstream_hash->count, + pnc->bsr_count, pnc->candrp_count); - if (pnc->rp_list->count == 0 && pnc->upstream_hash->count == 0 - && pnc->bsr_count == 0) { + if (pnc->rp_list->count == 0 && pnc->upstream_hash->count == 0 && + pnc->bsr_count == 0 && pnc->candrp_count == 0) { struct zclient *zclient = pim_zebra_zclient_get(); pim_sendmsg_zebra_rnh(pim, zclient, pnc, @@ -258,6 +267,27 @@ void pim_nht_bsr_del(struct pim_instance *pim, pim_addr addr) pim_nht_drop_maybe(pim, pnc); } +void pim_nht_candrp_del(struct pim_instance *pim, pim_addr addr) +{ + struct pim_nexthop_cache *pnc = NULL; + struct pim_nexthop_cache lookup; + + lookup.rpf.rpf_addr = addr; + + pnc = hash_lookup(pim->rpf_hash, &lookup); + + if (!pnc) { + zlog_warn("attempting to delete nonexistent NHT C-RP entry %pPA", + &addr); + return; + } + + assertf(pnc->candrp_count > 0, "addr=%pPA", &addr); + pnc->candrp_count--; + + pim_nht_drop_maybe(pim, pnc); +} + bool pim_nht_bsr_rpf_check(struct pim_instance *pim, pim_addr bsr_addr, struct interface *src_ifp, pim_addr src_ip) { @@ -900,6 +930,9 @@ void pim_nexthop_update(struct vrf *vrf, struct prefix *match, pim_update_rp_nh(pim, pnc); if (pnc->upstream_hash->count) pim_update_upstream_nh(pim, pnc); + + if (pnc->candrp_count) + pim_crp_nht_update(pim, pnc); } int pim_ecmp_nexthop_lookup(struct pim_instance *pim, diff --git a/pimd/pim_nht.h b/pimd/pim_nht.h index a1feb76e3b73..e74b375dc6d2 100644 --- a/pimd/pim_nht.h +++ b/pimd/pim_nht.h @@ -38,6 +38,7 @@ struct pim_nexthop_cache { * same BSR */ uint32_t bsr_count; + uint32_t candrp_count; }; struct pnc_hash_walk_data { @@ -71,4 +72,10 @@ void pim_nht_bsr_del(struct pim_instance *pim, pim_addr bsr_addr); bool pim_nht_bsr_rpf_check(struct pim_instance *pim, pim_addr bsr_addr, struct interface *src_ifp, pim_addr src_ip); void pim_upstream_nh_if_update(struct pim_instance *pim, struct interface *ifp); + +/* wrappers for usage with Candidate RPs in BSMs */ +bool pim_nht_candrp_add(struct pim_instance *pim, pim_addr addr); +void pim_nht_candrp_del(struct pim_instance *pim, pim_addr addr); +void pim_crp_nht_update(struct pim_instance *pim, struct pim_nexthop_cache *pnc); + #endif From c494702929d15141e8f3fd4a1d9f85095076a4b7 Mon Sep 17 00:00:00 2001 From: Acee Lindem Date: Fri, 31 May 2024 14:08:04 +0000 Subject: [PATCH 18/28] ospfd: Improve OSPF neighbor retransmission list granularity and precision The current OSPF neighbor retransmission operates on a single per-neighbor periodic timer that sends all LSAs on the list when it expires. Additionally, since it skips the first retransmission of received LSAs so that at least the retransmission interval (resulting in a delay of between the retransmission interval and twice the interval. In environments where the links are lossy on P2MP networks with "delay-reflood" configured (which relies on neighbor retransmission in partial meshs), the implementation is sub-optimal (to say the least). This commit reimplements OSPF neighbor retransmission as follows: 1. A new data structure making use the application managed typesafe.h doubly linked list implements an OSPF LSA list where each node includes a timestamp. 2. The existing neighbor LS retransmission LSDB data structure is augmented with a pointer to the list node on the LSA list to faciliate O(1) removal when the LSA is acknowledged. 3. The neighbor LS retransmission timer is set to the expiration timer of the LSA at the top of the list. 4. When the timer expires, LSAs are retransmitted that within the window of the current time and a small delta (50 milli-secs default). The LSAs that are retransmited are given an updated retransmission time and moved to the end of the LSA list. 5. Configuration is added to set the "retransmission-window" to a value other than 50 milliseconds. 6. Neighbor and interface LSA retransmission counters are added to provide insight into the lossiness of the links. However, these will increment quickly on non-fully meshed P2MP networks with "delay-reflood" configured. 7. Added a topotest to exercise the implementation on a non-fully meshed P2MP network with "delay-reflood" configured. The alternative was to use existing mechanisms to instroduce loss but these seem less determistic in a topotest. Signed-off-by: Acee Lindem --- doc/developer/ospf-ls-retrans.rst | 69 +++++++++ doc/developer/ospf.rst | 1 + doc/user/ospfd.rst | 12 +- lib/libospf.h | 1 + ospfd/ospf_flood.c | 146 +++++++++++++++--- ospfd/ospf_flood.h | 22 +++ ospfd/ospf_interface.c | 5 + ospfd/ospf_interface.h | 3 + ospfd/ospf_lsdb.c | 53 +++++++ ospfd/ospf_lsdb.h | 23 +++ ospfd/ospf_memory.c | 2 + ospfd/ospf_memory.h | 2 + ospfd/ospf_neighbor.c | 8 +- ospfd/ospf_neighbor.h | 7 +- ospfd/ospf_nsm.c | 6 +- ospfd/ospf_packet.c | 91 ++++++----- ospfd/ospf_packet.h | 2 +- ospfd/ospf_vty.c | 140 +++++++++++++---- .../r1/show_ip_ospf_interface.ref | 2 + tests/topotests/ospf_p2mp/r1/frr-p2mp.conf | 6 + tests/topotests/ospf_p2mp/r2/frr-p2mp.conf | 6 + tests/topotests/ospf_p2mp/r3/frr-p2mp.conf | 6 + tests/topotests/ospf_p2mp/r4/frr-p2mp.conf | 6 + .../ospf_p2mp/test_ospf_p2mp_broadcast.py | 112 ++++++++++++-- 24 files changed, 620 insertions(+), 111 deletions(-) create mode 100644 doc/developer/ospf-ls-retrans.rst diff --git a/doc/developer/ospf-ls-retrans.rst b/doc/developer/ospf-ls-retrans.rst new file mode 100644 index 000000000000..230d7a1c5d95 --- /dev/null +++ b/doc/developer/ospf-ls-retrans.rst @@ -0,0 +1,69 @@ +OSPF Neighor Retransmission List +================================ + +Overview +-------- + +OSPF neighbor link-state retransmission lists are implemented using +both a sparse Link State Database (LSDB) and a doubly-linked list. +Rather than previous per-neighbor periodic timer, a per-neighbor +timer is set to the expiration time of the next scheduled LSA +retransmission. + +Sparse Link State Database (LSDB) +--------------------------------- + +When an explicit or implied acknowledgment is recieved from a +neighbor in 2-way state or higher, the acknowledge LSA must be +removed from the neighbor's link state retransmission list. In order +to do this efficiently, a sparse LSDB is utilized. LSDB entries also +include a pointer to the corresponding list entry so that it may be +efficiently removed from the doubly-linked list. + +The sparse LSDB is implemented using the OSPF functions is +ospf_lsdb.[c,h]. OSPF LSDBs are implemented as an array of route +tables (lib/table.[c,h]). What is unique of the LS Retransmission +list LSDB is that each entry also has a pointer into the doubly-linked +list to facilitate fast deletions. + +Doubly-Linked List +------------------ + +In addition to the sparse LSDB, LSAs on a neighbor LS retransmission +list are also maintained in a linked-list order chronologically +with the LSA scheduled for the next retransmission at the head of +the list. + +The doubly-link list is implemented using the dlist macros in +lib/typesafe.h. + +LSA LS Retransmission List Addition +------------------------------------ + +When an LSA is added to a neighbor retransmission list, it is +added to both the sparse LSDB and the doubly-linked list with a pointer +in the LSDB route-table node to the list entry. The LSA is added to +the tail of the list with the expiration time set to the current time +with the retransmission interval added. If the neighbor retransmission +timer is not set, it is set to expire at the time of the newly added +LSA. + +LSA LS Retransmission List Deletion +----------------------------------- + +When an LSA is deleted from a neighbor retransmission list, it is +deleted from eboth the sparse LSDB and the doubly-linked list with the +pointer the LSDB route-table node used to efficiently delete the entry +from the list. If the LSA at the head of the list was removed, then +the neighbor retransmission timer is reset to the expiration of the +LSA at the head of the list or canceled if the list is empty. + +Neighbor LS Retransmission List Expiration +------------------------------------------ + +When the neighbor retransmission timer expires, the LSA at the top of +list and any in a configured window (e.g., 50 milliseconds) are +retransmitted. The LSAs that have been retransmitted are removed from +the list and readded to the tail of the list with a new expiration time +which is retransmit-interval seconds in the future. + diff --git a/doc/developer/ospf.rst b/doc/developer/ospf.rst index 837a0bd18598..da4802533c03 100644 --- a/doc/developer/ospf.rst +++ b/doc/developer/ospf.rst @@ -8,6 +8,7 @@ OSPFD :maxdepth: 2 ospf-api + ospf-ls-retrans ospf-sr cspf diff --git a/doc/user/ospfd.rst b/doc/user/ospfd.rst index 70c15e73deb0..b80adba7f046 100644 --- a/doc/user/ospfd.rst +++ b/doc/user/ospfd.rst @@ -738,7 +738,17 @@ Interfaces retransmitting Database Description and Link State Request packets. The default value is 5 seconds. -.. clicmd:: ip ospf transmit-delay (1-65535) [A.B.C.D] +.. clicmd:: ip ospf retransmit-window (20-1000) + + + Set number of milliseconds in the window for neighbor LSA retransmission. + When a neighbor Link State (LS) retransmission timer expires, LSAs scheduled + to be retransmitted within the number of milliseconds configured are + retransmitted to the neighbor. Any expiring after the window will be + retransmitted the next time the neighbor LS retransmission timer expires. + The default is 50 milliseconds. + + .. clicmd:: ip ospf transmit-delay (1-65535) [A.B.C.D] Set number of seconds for InfTransDelay value. LSAs' age should be diff --git a/lib/libospf.h b/lib/libospf.h index 0ac490a00ece..f2dc5d61d9eb 100644 --- a/lib/libospf.h +++ b/lib/libospf.h @@ -58,6 +58,7 @@ extern "C" { #define OSPF_HELLO_DELAY_DEFAULT 10 #define OSPF_ROUTER_PRIORITY_DEFAULT 1 #define OSPF_RETRANSMIT_INTERVAL_DEFAULT 5 +#define OSPF_RETRANSMIT_WINDOW_DEFAULT 50 /* milliseconds */ #define OSPF_TRANSMIT_DELAY_DEFAULT 1 #define OSPF_DEFAULT_BANDWIDTH 10000 /* Mbps */ diff --git a/ospfd/ospf_flood.c b/ospfd/ospf_flood.c index e15871ac81cd..e9797ce935a7 100644 --- a/ospfd/ospf_flood.c +++ b/ospfd/ospf_flood.c @@ -1015,7 +1015,7 @@ void ospf_ls_request_delete(struct ospf_neighbor *nbr, struct ospf_lsa *lsa) ospf_lsdb_delete(&nbr->ls_req, lsa); } -/* Remove all LSA from neighbor's ls-requenst list. */ +/* Remove all LSAs from neighbor's ls-request list. */ void ospf_ls_request_delete_all(struct ospf_neighbor *nbr) { ospf_lsa_unlock(&nbr->ls_req_last); @@ -1061,58 +1061,114 @@ int ospf_ls_retransmit_isempty(struct ospf_neighbor *nbr) /* Add LSA to be retransmitted to neighbor's ls-retransmit list. */ void ospf_ls_retransmit_add(struct ospf_neighbor *nbr, struct ospf_lsa *lsa) { - struct ospf_lsa *old; + struct ospf_lsdb_linked_node *ls_rxmt_node; + struct ospf_lsa_list_entry *ls_rxmt_list_entry; + struct ospf_lsa *old = NULL; + bool rxmt_head_replaced = false; - old = ospf_ls_retransmit_lookup(nbr, lsa); + ls_rxmt_node = ospf_lsdb_linked_lookup(&nbr->ls_rxmt, lsa); + if (ls_rxmt_node) + old = ls_rxmt_node->info; if (ospf_lsa_more_recent(old, lsa) < 0) { if (old) { old->retransmit_counter--; + if (ls_rxmt_node->lsa_list_entry == + ospf_lsa_list_first(&nbr->ls_rxmt_list)) + rxmt_head_replaced = true; + ospf_lsa_list_del(&nbr->ls_rxmt_list, + ls_rxmt_node->lsa_list_entry); + XFREE(MTYPE_OSPF_LSA_LIST, ls_rxmt_node->lsa_list_entry); + ospf_lsdb_delete(&nbr->ls_rxmt, old); if (IS_DEBUG_OSPF(lsa, LSA_FLOODING)) - zlog_debug("RXmtL(%lu)--, NBR(%pI4(%s)), LSA[%s]", + zlog_debug("RXmtL(%lu) NBR(%pI4(%s)) Old Delete LSA[%s] on Add", ospf_ls_retransmit_count(nbr), &nbr->router_id, ospf_get_name(nbr->oi->ospf), - dump_lsa_key(old)); - ospf_lsdb_delete(&nbr->ls_rxmt, old); + dump_lsa_key(lsa)); + ospf_lsa_unlock(&old); } lsa->retransmit_counter++; + ls_rxmt_list_entry = XCALLOC(MTYPE_OSPF_LSA_LIST, + sizeof(struct ospf_lsa_list_entry)); + /* + * Set the LSA retransmission time for the neighbor; + */ + monotime(&ls_rxmt_list_entry->list_entry_time); + ls_rxmt_list_entry->list_entry_time.tv_sec += nbr->v_ls_rxmt; + + /* + * Add the LSA to the neighbor retransmission list. + */ + ls_rxmt_list_entry->lsa = ospf_lsa_lock(lsa); + ospf_lsa_list_add_tail(&nbr->ls_rxmt_list, ls_rxmt_list_entry); + ospf_lsdb_add(&nbr->ls_rxmt, lsa); + /* - * We cannot make use of the newly introduced callback function - * "lsdb->new_lsa_hook" to replace debug output below, just - * because - * it seems no simple and smart way to pass neighbor information - * to - * the common function "ospf_lsdb_add()" -- endo. + * Look up the newly added node and set the list pointer. */ + ls_rxmt_node = ospf_lsdb_linked_lookup(&nbr->ls_rxmt, lsa); + ls_rxmt_node->lsa_list_entry = ls_rxmt_list_entry; + if (IS_DEBUG_OSPF(lsa, LSA_FLOODING)) - zlog_debug("RXmtL(%lu)++, NBR(%pI4(%s)), LSA[%s]", + zlog_debug("RXmtL(%lu) NBR(%pI4(%s)) Add LSA[%s] retrans at (%ld/%ld)", ospf_ls_retransmit_count(nbr), - &nbr->router_id, - ospf_get_name(nbr->oi->ospf), - dump_lsa_key(lsa)); - ospf_lsdb_add(&nbr->ls_rxmt, lsa); + &nbr->router_id, ospf_get_name(nbr->oi->ospf), + dump_lsa_key(lsa), + (long)ls_rxmt_list_entry->list_entry_time + .tv_sec, + (long)ls_rxmt_list_entry->list_entry_time + .tv_usec); + /* + * Reset the neighbor LSA retransmission timer if isn't currently + * running or the LSA at the head of the list was updated. + */ + if (!nbr->t_ls_rxmt || rxmt_head_replaced) + ospf_ls_retransmit_set_timer(nbr); } } /* Remove LSA from neibghbor's ls-retransmit list. */ void ospf_ls_retransmit_delete(struct ospf_neighbor *nbr, struct ospf_lsa *lsa) { - if (ospf_ls_retransmit_lookup(nbr, lsa)) { + struct ospf_lsdb_linked_node *ls_rxmt_node; + + ls_rxmt_node = ospf_lsdb_linked_lookup(&nbr->ls_rxmt, lsa); + + if (ls_rxmt_node) { + bool rxmt_timer_reset; + + if (ls_rxmt_node->lsa_list_entry == + ospf_lsa_list_first(&nbr->ls_rxmt_list)) + rxmt_timer_reset = true; + else + rxmt_timer_reset = false; + lsa->retransmit_counter--; - if (IS_DEBUG_OSPF(lsa, LSA_FLOODING)) /* -- endo. */ - zlog_debug("RXmtL(%lu)--, NBR(%pI4(%s)), LSA[%s]", + ospf_lsa_list_del(&nbr->ls_rxmt_list, + ls_rxmt_node->lsa_list_entry); + XFREE(MTYPE_OSPF_LSA_LIST, ls_rxmt_node->lsa_list_entry); + ospf_lsdb_delete(&nbr->ls_rxmt, lsa); + if (IS_DEBUG_OSPF(lsa, LSA_FLOODING)) + zlog_debug("RXmtL(%lu) NBR(%pI4(%s)) Delete LSA[%s]", ospf_ls_retransmit_count(nbr), - &nbr->router_id, - ospf_get_name(nbr->oi->ospf), + &nbr->router_id, ospf_get_name(nbr->oi->ospf), dump_lsa_key(lsa)); - ospf_lsdb_delete(&nbr->ls_rxmt, lsa); + ospf_lsa_unlock(&lsa); + + /* + * If the LS retransmission entry at the head of the list was + * deleted, reset the timer. + */ + if (rxmt_timer_reset) + ospf_ls_retransmit_set_timer(nbr); } } /* Clear neighbor's ls-retransmit list. */ void ospf_ls_retransmit_clear(struct ospf_neighbor *nbr) { + struct ospf_lsa_list_entry *ls_rxmt_list_entry; struct ospf_lsdb *lsdb; int i; @@ -1128,10 +1184,54 @@ void ospf_ls_retransmit_clear(struct ospf_neighbor *nbr) ospf_ls_retransmit_delete(nbr, lsa); } + frr_each_safe (ospf_lsa_list, &nbr->ls_rxmt_list, ls_rxmt_list_entry) { + ospf_lsa_list_del(&nbr->ls_rxmt_list, ls_rxmt_list_entry); + ospf_lsa_unlock(&ls_rxmt_list_entry->lsa); + XFREE(MTYPE_OSPF_LSA_LIST, ls_rxmt_list_entry); + } + ospf_lsa_unlock(&nbr->ls_req_last); nbr->ls_req_last = NULL; } +/* + * Set the neighbor's ls-retransmit timer based on the next + * LSA retransmit time. + */ +void ospf_ls_retransmit_set_timer(struct ospf_neighbor *nbr) +{ + struct ospf_lsa_list_entry *ls_rxmt_list_entry; + + if (nbr->t_ls_rxmt) + EVENT_OFF(nbr->t_ls_rxmt); + + ls_rxmt_list_entry = ospf_lsa_list_first(&nbr->ls_rxmt_list); + if (ls_rxmt_list_entry) { + struct timeval current_time, delay; + unsigned long delay_milliseconds; + + monotime(¤t_time); + if (timercmp(¤t_time, + &ls_rxmt_list_entry->list_entry_time, >=)) + delay_milliseconds = 10; + else { + timersub(&ls_rxmt_list_entry->list_entry_time, + ¤t_time, &delay); + delay_milliseconds = (delay.tv_sec * 1000) + + (delay.tv_usec / 1000); + } + + event_add_timer_msec(master, ospf_ls_rxmt_timer, nbr, + delay_milliseconds, &nbr->t_ls_rxmt); + if (IS_DEBUG_OSPF(lsa, LSA_FLOODING)) + zlog_debug("RXmtL(%lu) NBR(%pI4(%s)) retrans timer set in %ld msecs - Head LSA(%s)", + ospf_ls_retransmit_count(nbr), + &nbr->router_id, ospf_get_name(nbr->oi->ospf), + delay_milliseconds, + dump_lsa_key(ls_rxmt_list_entry->lsa)); + } +} + /* Lookup LSA from neighbor's ls-retransmit list. */ struct ospf_lsa *ospf_ls_retransmit_lookup(struct ospf_neighbor *nbr, struct ospf_lsa *lsa) diff --git a/ospfd/ospf_flood.h b/ospfd/ospf_flood.h index 3757400d0cec..d9d953735148 100644 --- a/ospfd/ospf_flood.h +++ b/ospfd/ospf_flood.h @@ -7,6 +7,26 @@ #ifndef _ZEBRA_OSPF_FLOOD_H #define _ZEBRA_OSPF_FLOOD_H +/* + * OSPF Temporal LSA List + */ +PREDECL_DLIST(ospf_lsa_list); + +struct ospf_lsa_list_entry { + /* Linkage for LSA List */ + struct ospf_lsa_list_item list_linkage; + + /* + * Time associated with the list entry. For example, for a neigbhor + * link retransmission list, this is the retransmission time. + */ + struct timeval list_entry_time; + + struct ospf_lsa *lsa; +}; + +DECLARE_DLIST(ospf_lsa_list, struct ospf_lsa_list_entry, list_linkage); + extern int ospf_flood(struct ospf *, struct ospf_neighbor *, struct ospf_lsa *, struct ospf_lsa *); extern int ospf_flood_through(struct ospf *, struct ospf_neighbor *, @@ -36,6 +56,8 @@ extern void ospf_ls_retransmit_add(struct ospf_neighbor *, struct ospf_lsa *); extern void ospf_ls_retransmit_delete(struct ospf_neighbor *, struct ospf_lsa *); extern void ospf_ls_retransmit_clear(struct ospf_neighbor *); +extern void ospf_ls_retransmit_set_timer(struct ospf_neighbor *nbr); + extern struct ospf_lsa *ospf_ls_retransmit_lookup(struct ospf_neighbor *, struct ospf_lsa *); extern void ospf_ls_retransmit_delete_nbr_area(struct ospf_area *, diff --git a/ospfd/ospf_interface.c b/ospfd/ospf_interface.c index 11ac7af7c9ed..803c36861dcd 100644 --- a/ospfd/ospf_interface.c +++ b/ospfd/ospf_interface.c @@ -542,6 +542,7 @@ static struct ospf_if_params *ospf_new_if_params(void) UNSET_IF_PARAM(oip, output_cost_cmd); UNSET_IF_PARAM(oip, transmit_delay); UNSET_IF_PARAM(oip, retransmit_interval); + UNSET_IF_PARAM(oip, retransmit_window); UNSET_IF_PARAM(oip, passive_interface); UNSET_IF_PARAM(oip, v_hello); UNSET_IF_PARAM(oip, fast_hello); @@ -599,6 +600,7 @@ void ospf_free_if_params(struct interface *ifp, struct in_addr addr) if (!OSPF_IF_PARAM_CONFIGURED(oip, output_cost_cmd) && !OSPF_IF_PARAM_CONFIGURED(oip, transmit_delay) && !OSPF_IF_PARAM_CONFIGURED(oip, retransmit_interval) && + !OSPF_IF_PARAM_CONFIGURED(oip, retransmit_window) && !OSPF_IF_PARAM_CONFIGURED(oip, passive_interface) && !OSPF_IF_PARAM_CONFIGURED(oip, v_hello) && !OSPF_IF_PARAM_CONFIGURED(oip, fast_hello) && @@ -695,6 +697,9 @@ int ospf_if_new_hook(struct interface *ifp) IF_DEF_PARAMS(ifp)->retransmit_interval = OSPF_RETRANSMIT_INTERVAL_DEFAULT; + SET_IF_PARAM(IF_DEF_PARAMS(ifp), retransmit_window); + IF_DEF_PARAMS(ifp)->retransmit_window = OSPF_RETRANSMIT_WINDOW_DEFAULT; + SET_IF_PARAM(IF_DEF_PARAMS(ifp), priority); IF_DEF_PARAMS(ifp)->priority = OSPF_ROUTER_PRIORITY_DEFAULT; diff --git a/ospfd/ospf_interface.h b/ospfd/ospf_interface.h index 45d0b7943a24..a944847b5d9b 100644 --- a/ospfd/ospf_interface.h +++ b/ospfd/ospf_interface.h @@ -47,6 +47,8 @@ struct ospf_if_params { output_cost_cmd); /* Command Interface Output Cost */ DECLARE_IF_PARAM(uint32_t, retransmit_interval); /* Retransmission Interval */ + DECLARE_IF_PARAM(uint32_t, + retransmit_window); /* Retransmission Window */ DECLARE_IF_PARAM(uint8_t, passive_interface); /* OSPF Interface is passive: no sending or receiving (no need to @@ -296,6 +298,7 @@ struct ospf_interface { uint32_t ls_ack_out; /* LS Ack message output count. */ uint32_t discarded; /* discarded input count by error. */ uint32_t state_change; /* Number of status change. */ + uint32_t ls_rxmt_lsa; /* Number of LSAs retransmitted. */ uint32_t full_nbrs; diff --git a/ospfd/ospf_lsdb.c b/ospfd/ospf_lsdb.c index 0111c4924ec2..d1b3eb0d354b 100644 --- a/ospfd/ospf_lsdb.c +++ b/ospfd/ospf_lsdb.c @@ -34,6 +34,59 @@ void ospf_lsdb_init(struct ospf_lsdb *lsdb) lsdb->type[i].db = route_table_init(); } +static struct route_node * +ospf_lsdb_linked_node_create(route_table_delegate_t *delegate, + struct route_table *table) +{ + struct ospf_lsdb_linked_node *node; + + node = XCALLOC(MTYPE_OSPF_LSDB_NODE, + sizeof(struct ospf_lsdb_linked_node)); + + return (struct route_node *)node; +} + +static void ospf_lsdb_linked_node_destroy(route_table_delegate_t *delegate, + struct route_table *table, + struct route_node *node) +{ + struct ospf_lsdb_linked_node *lsdb_linked_node = + (struct ospf_lsdb_linked_node *)node; + + XFREE(MTYPE_OSPF_LSDB_NODE, lsdb_linked_node); +} + +static route_table_delegate_t ospf_lsdb_linked_table_delegate = { + .create_node = ospf_lsdb_linked_node_create, + .destroy_node = ospf_lsdb_linked_node_destroy, +}; + +void ospf_lsdb_linked_init(struct ospf_lsdb *lsdb) +{ + int i; + + for (i = OSPF_MIN_LSA; i < OSPF_MAX_LSA; i++) + lsdb->type[i].db = route_table_init_with_delegate( + &ospf_lsdb_linked_table_delegate); +} + +struct ospf_lsdb_linked_node *ospf_lsdb_linked_lookup(struct ospf_lsdb *lsdb, + struct ospf_lsa *lsa) +{ + struct ospf_lsdb_linked_node *lsdb_linked_node; + struct route_table *table; + struct prefix_ls lp; + + table = lsdb->type[lsa->data->type].db; + ls_prefix_set(&lp, lsa); + lsdb_linked_node = (struct ospf_lsdb_linked_node *) + route_node_lookup(table, (struct prefix *)&lp); + if (lsdb_linked_node) + route_unlock_node((struct route_node *)lsdb_linked_node); + + return lsdb_linked_node; +} + void ospf_lsdb_free(struct ospf_lsdb *lsdb) { ospf_lsdb_cleanup(lsdb); diff --git a/ospfd/ospf_lsdb.h b/ospfd/ospf_lsdb.h index bf295ca8306a..e5e3be8baa0b 100644 --- a/ospfd/ospf_lsdb.h +++ b/ospfd/ospf_lsdb.h @@ -7,6 +7,9 @@ #ifndef _ZEBRA_OSPF_LSDB_H #define _ZEBRA_OSPF_LSDB_H +#include "prefix.h" +#include "table.h" + /* OSPF LSDB structure. */ struct ospf_lsdb { struct { @@ -43,9 +46,29 @@ struct ospf_lsdb { #define AREA_LSDB(A,T) ((A)->lsdb->type[(T)].db) #define AS_LSDB(O,T) ((O)->lsdb->type[(T)].db) +/* + * Alternate route node structure for LSDB nodes linked to + * list elements. + */ +struct ospf_lsdb_linked_node { + /* + * Caution these must be the very first fields + */ + ROUTE_NODE_FIELDS + + /* + * List entry on an LSA list, e.g., a neighbor + * retransmission list. + */ + struct ospf_lsa_list_entry *lsa_list_entry; +}; + /* OSPF LSDB related functions. */ extern struct ospf_lsdb *ospf_lsdb_new(void); extern void ospf_lsdb_init(struct ospf_lsdb *); +extern void ospf_lsdb_linked_init(struct ospf_lsdb *lsdb); +extern struct ospf_lsdb_linked_node * +ospf_lsdb_linked_lookup(struct ospf_lsdb *lsdb, struct ospf_lsa *lsa); extern void ospf_lsdb_free(struct ospf_lsdb *); extern void ospf_lsdb_cleanup(struct ospf_lsdb *); extern void ls_prefix_set(struct prefix_ls *lp, struct ospf_lsa *lsa); diff --git a/ospfd/ospf_memory.c b/ospfd/ospf_memory.c index 9854c8cae80b..478af323d30b 100644 --- a/ospfd/ospf_memory.c +++ b/ospfd/ospf_memory.c @@ -45,3 +45,5 @@ DEFINE_MTYPE(OSPFD, OSPF_GR_HELPER, "OSPF Graceful Restart Helper"); DEFINE_MTYPE(OSPFD, OSPF_EXTERNAL_RT_AGGR, "OSPF External Route Summarisation"); DEFINE_MTYPE(OSPFD, OSPF_P_SPACE, "OSPF TI-LFA P-Space"); DEFINE_MTYPE(OSPFD, OSPF_Q_SPACE, "OSPF TI-LFA Q-Space"); +DEFINE_MTYPE(OSPFD, OSPF_LSA_LIST, "OSPF LSA List"); +DEFINE_MTYPE(OSPFD, OSPF_LSDB_NODE, "OSPF LSDB Linked Node"); diff --git a/ospfd/ospf_memory.h b/ospfd/ospf_memory.h index d11b69abb01b..e2139b517b17 100644 --- a/ospfd/ospf_memory.h +++ b/ospfd/ospf_memory.h @@ -44,5 +44,7 @@ DECLARE_MTYPE(OSPF_GR_HELPER); DECLARE_MTYPE(OSPF_EXTERNAL_RT_AGGR); DECLARE_MTYPE(OSPF_P_SPACE); DECLARE_MTYPE(OSPF_Q_SPACE); +DECLARE_MTYPE(OSPF_LSA_LIST); +DECLARE_MTYPE(OSPF_LSDB_NODE); #endif /* _QUAGGA_OSPF_MEMORY_H */ diff --git a/ospfd/ospf_neighbor.c b/ospfd/ospf_neighbor.c index d47d5816057a..2514fc0ab3b9 100644 --- a/ospfd/ospf_neighbor.c +++ b/ospfd/ospf_neighbor.c @@ -68,7 +68,7 @@ struct ospf_neighbor *ospf_nbr_new(struct ospf_interface *oi) nbr->v_inactivity = OSPF_IF_PARAM(oi, v_wait); nbr->v_db_desc = OSPF_IF_PARAM(oi, retransmit_interval); nbr->v_ls_req = OSPF_IF_PARAM(oi, retransmit_interval); - nbr->v_ls_upd = OSPF_IF_PARAM(oi, retransmit_interval); + nbr->v_ls_rxmt = OSPF_IF_PARAM(oi, retransmit_interval); nbr->priority = -1; /* DD flags. */ @@ -80,8 +80,10 @@ struct ospf_neighbor *ospf_nbr_new(struct ospf_interface *oi) nbr->nbr_nbma = NULL; ospf_lsdb_init(&nbr->db_sum); - ospf_lsdb_init(&nbr->ls_rxmt); + + ospf_lsdb_linked_init(&nbr->ls_rxmt); ospf_lsdb_init(&nbr->ls_req); + ospf_lsa_list_init(&nbr->ls_rxmt_list); nbr->crypt_seqnum = 0; @@ -128,7 +130,7 @@ void ospf_nbr_free(struct ospf_neighbor *nbr) EVENT_OFF(nbr->t_inactivity); EVENT_OFF(nbr->t_db_desc); EVENT_OFF(nbr->t_ls_req); - EVENT_OFF(nbr->t_ls_upd); + EVENT_OFF(nbr->t_ls_rxmt); /* Cancel all events. */ /* Thread lookup cost would be negligible. */ event_cancel_event(master, nbr); diff --git a/ospfd/ospf_neighbor.h b/ospfd/ospf_neighbor.h index 07d095f03d1b..0e041f9e6df2 100644 --- a/ospfd/ospf_neighbor.h +++ b/ospfd/ospf_neighbor.h @@ -9,6 +9,7 @@ #include #include +#include /* Neighbor Data Structure */ struct ospf_neighbor { @@ -44,6 +45,7 @@ struct ospf_neighbor { /* LSA data. */ struct ospf_lsdb ls_rxmt; + struct ospf_lsa_list_head ls_rxmt_list; struct ospf_lsdb db_sum; struct ospf_lsdb ls_req; struct ospf_lsa *ls_req_last; @@ -54,13 +56,13 @@ struct ospf_neighbor { uint32_t v_inactivity; uint32_t v_db_desc; uint32_t v_ls_req; - uint32_t v_ls_upd; + uint32_t v_ls_rxmt; /* Threads. */ struct event *t_inactivity; struct event *t_db_desc; struct event *t_ls_req; - struct event *t_ls_upd; + struct event *t_ls_rxmt; struct event *t_hello_reply; /* NBMA configured neighbour */ @@ -71,6 +73,7 @@ struct ospf_neighbor { struct timeval ts_last_regress; /* last regressive NSM change */ const char *last_regress_str; /* Event which last regressed NSM */ uint32_t state_change; /* NSM state change counter */ + uint32_t ls_rxmt_lsa; /* Number of LSAs retransmited. */ /* BFD information */ struct bfd_session_params *bfd_session; diff --git a/ospfd/ospf_nsm.c b/ospfd/ospf_nsm.c index c466ddcc6f90..079a1fa55ed3 100644 --- a/ospfd/ospf_nsm.c +++ b/ospfd/ospf_nsm.c @@ -112,18 +112,16 @@ static void nsm_timer_set(struct ospf_neighbor *nbr) case NSM_Init: case NSM_TwoWay: EVENT_OFF(nbr->t_db_desc); - EVENT_OFF(nbr->t_ls_upd); + EVENT_OFF(nbr->t_ls_rxmt); EVENT_OFF(nbr->t_ls_req); break; case NSM_ExStart: OSPF_NSM_TIMER_ON(nbr->t_db_desc, ospf_db_desc_timer, nbr->v_db_desc); - EVENT_OFF(nbr->t_ls_upd); + EVENT_OFF(nbr->t_ls_rxmt); EVENT_OFF(nbr->t_ls_req); break; case NSM_Exchange: - OSPF_NSM_TIMER_ON(nbr->t_ls_upd, ospf_ls_upd_timer, - nbr->v_ls_upd); if (!IS_SET_DD_MS(nbr->dd_flags)) EVENT_OFF(nbr->t_db_desc); break; diff --git a/ospfd/ospf_packet.c b/ospfd/ospf_packet.c index 87aaccad920b..86f877b6214e 100644 --- a/ospfd/ospf_packet.c +++ b/ospfd/ospf_packet.c @@ -292,54 +292,66 @@ void ospf_ls_req_event(struct ospf_neighbor *nbr) event_add_event(master, ospf_ls_req_timer, nbr, 0, &nbr->t_ls_req); } -/* Cyclic timer function. Fist registered in ospf_nbr_new () in - ospf_neighbor.c */ -void ospf_ls_upd_timer(struct event *thread) +/* + * OSPF neighbor link state retransmission timer handler. Unicast + * unacknowledged LSAs to the neigbhors. + */ +void ospf_ls_rxmt_timer(struct event *thread) { struct ospf_neighbor *nbr; + int retransmit_interval, retransmit_window, rxmt_lsa_count = 0; nbr = EVENT_ARG(thread); - nbr->t_ls_upd = NULL; + nbr->t_ls_rxmt = NULL; + retransmit_interval = nbr->v_ls_rxmt; + retransmit_window = OSPF_IF_PARAM(nbr->oi, retransmit_window); /* Send Link State Update. */ if (ospf_ls_retransmit_count(nbr) > 0) { + struct ospf_lsa_list_entry *ls_rxmt_list_entry; + struct timeval current_time, latest_rxmt_time, next_rxmt_time; + struct timeval rxmt_interval = { retransmit_interval, 0 }; + struct timeval rxmt_window; struct list *update; - struct ospf_lsdb *lsdb; - int i; - int retransmit_interval; - retransmit_interval = - OSPF_IF_PARAM(nbr->oi, retransmit_interval); + /* + * Set the retransmission window based on the configured value + * in milliseconds. + */ + rxmt_window.tv_sec = retransmit_window / 1000; + rxmt_window.tv_usec = (retransmit_window % 1000) * 1000; + + /* + * Calculate the latest retransmit time for LSAs transmited in + * this timer pass by adding the retransmission window to the + * current time. Calculate the next retransmission time by adding + * the retransmit interval to the current time. + */ + monotime(¤t_time); + timeradd(¤t_time, &rxmt_window, &latest_rxmt_time); + timeradd(¤t_time, &rxmt_interval, &next_rxmt_time); - lsdb = &nbr->ls_rxmt; update = list_new(); + while ((ls_rxmt_list_entry = + ospf_lsa_list_first(&nbr->ls_rxmt_list))) { + if (timercmp(&ls_rxmt_list_entry->list_entry_time, + &latest_rxmt_time, >)) + break; - for (i = OSPF_MIN_LSA; i < OSPF_MAX_LSA; i++) { - struct route_table *table = lsdb->type[i].db; - struct route_node *rn; - - for (rn = route_top(table); rn; rn = route_next(rn)) { - struct ospf_lsa *lsa; - - if ((lsa = rn->info) != NULL) { - /* Don't retransmit an LSA if we - received it within - the last RxmtInterval seconds - this - is to allow the - neighbour a chance to acknowledge the - LSA as it may - have ben just received before the - retransmit timer - fired. This is a small tweak to what - is in the RFC, - but it will cut out out a lot of - retransmit traffic - - MAG */ - if (monotime_since(&lsa->tv_recv, NULL) - >= retransmit_interval * 1000000LL) - listnode_add(update, rn->info); - } - } + listnode_add(update, ls_rxmt_list_entry->lsa); + rxmt_lsa_count++; + + /* + * Set the next retransmit time for the LSA and move it + * to the end of the neighbor's retransmission list. + */ + ls_rxmt_list_entry->list_entry_time = next_rxmt_time; + ospf_lsa_list_del(&nbr->ls_rxmt_list, + ls_rxmt_list_entry); + ospf_lsa_list_add_tail(&nbr->ls_rxmt_list, + ls_rxmt_list_entry); + nbr->ls_rxmt_lsa++; + nbr->oi->ls_rxmt_lsa++; } if (listcount(update) > 0) @@ -348,8 +360,13 @@ void ospf_ls_upd_timer(struct event *thread) list_delete(&update); } + if (IS_DEBUG_OSPF_EVENT) + zlog_debug("RXmtL(%lu) NBR(%pI4(%s)) timer event - sent %u LSAs", + ospf_ls_retransmit_count(nbr), &nbr->router_id, + ospf_get_name(nbr->oi->ospf), rxmt_lsa_count); + /* Set LS Update retransmission timer. */ - OSPF_NSM_TIMER_ON(nbr->t_ls_upd, ospf_ls_upd_timer, nbr->v_ls_upd); + ospf_ls_retransmit_set_timer(nbr); } void ospf_ls_ack_timer(struct event *thread) diff --git a/ospfd/ospf_packet.h b/ospfd/ospf_packet.h index 234738979e95..2c9dba6c8819 100644 --- a/ospfd/ospf_packet.h +++ b/ospfd/ospf_packet.h @@ -140,7 +140,7 @@ extern void ospf_ls_ack_send_delayed(struct ospf_interface *); extern void ospf_ls_retransmit(struct ospf_interface *, struct ospf_lsa *); extern void ospf_ls_req_event(struct ospf_neighbor *); -extern void ospf_ls_upd_timer(struct event *thread); +extern void ospf_ls_rxmt_timer(struct event *thread); extern void ospf_ls_ack_timer(struct event *thread); extern void ospf_poll_timer(struct event *thread); extern void ospf_hello_reply_timer(struct event *thread); diff --git a/ospfd/ospf_vty.c b/ospfd/ospf_vty.c index 3a11b2123210..7a7a684dd6f6 100644 --- a/ospfd/ospf_vty.c +++ b/ospfd/ospf_vty.c @@ -815,6 +815,7 @@ struct ospf_vl_config_data { int del_keychain; int hello_interval; /* Obvious what these are... */ int retransmit_interval; + int retransmit_window; int transmit_delay; int dead_interval; }; @@ -957,6 +958,12 @@ static int ospf_vl_set_timers(struct ospf_vl_data *vl_data, vl_config->retransmit_interval; } + if (vl_config->retransmit_window) { + SET_IF_PARAM(IF_DEF_PARAMS(ifp), retransmit_window); + IF_DEF_PARAMS(ifp)->retransmit_window = + vl_config->retransmit_window; + } + if (vl_config->transmit_delay) { SET_IF_PARAM(IF_DEF_PARAMS(ifp), transmit_delay); IF_DEF_PARAMS(ifp)->transmit_delay = vl_config->transmit_delay; @@ -1012,14 +1019,16 @@ static int ospf_vl_set(struct ospf *ospf, struct ospf_vl_config_data *vl_config) "Use null authentication\n" \ "Use message-digest authentication\n" -#define VLINK_HELPSTR_TIME_PARAM \ - "Time between HELLO packets\n" \ - "Seconds\n" \ - "Time between retransmitting lost link state advertisements\n" \ - "Seconds\n" \ - "Link state transmit delay\n" \ - "Seconds\n" \ - "Interval time after which a neighbor is declared down\n" \ +#define VLINK_HELPSTR_TIME_PARAM \ + "Time between HELLO packets\n" \ + "Seconds\n" \ + "Time between retransmitting lost link state advertisements\n" \ + "Seconds\n" \ + "Window for LSA retransmit - Retransmit LSAs expiring in this window\n" \ + "Milliseconds\n" \ + "Link state transmit delay\n" \ + "Seconds\n" \ + "Interval time after which a neighbor is declared down\n" \ "Seconds\n" #define VLINK_HELPSTR_AUTH_SIMPLE \ @@ -1204,7 +1213,7 @@ DEFUN (no_ospf_area_vlink, DEFUN (ospf_area_vlink_intervals, ospf_area_vlink_intervals_cmd, - "area virtual-link A.B.C.D {hello-interval (1-65535)|retransmit-interval (1-65535)|transmit-delay (1-65535)|dead-interval (1-65535)}", + "area virtual-link A.B.C.D {hello-interval (1-65535)|retransmit-interval (1-65535)|retransmit-window (20-10000)|transmit-delay (1-65535)|dead-interval (1-65535)}", VLINK_HELPSTR_IPADDR VLINK_HELPSTR_TIME_PARAM) { @@ -1236,6 +1245,9 @@ DEFUN (ospf_area_vlink_intervals, else if (strmatch(argv[idx]->text, "retransmit-interval")) vl_config.retransmit_interval = strtol(argv[++idx]->arg, NULL, 10); + else if (strmatch(argv[idx]->text, "retransmit-window")) + vl_config.retransmit_window = strtol(argv[++idx]->arg, + NULL, 10); else if (strmatch(argv[idx]->text, "transmit-delay")) vl_config.transmit_delay = strtol(argv[++idx]->arg, NULL, 10); @@ -1250,7 +1262,7 @@ DEFUN (ospf_area_vlink_intervals, DEFUN (no_ospf_area_vlink_intervals, no_ospf_area_vlink_intervals_cmd, - "no area virtual-link A.B.C.D {hello-interval (1-65535)|retransmit-interval (1-65535)|transmit-delay (1-65535)|dead-interval (1-65535)}", + "no area virtual-link A.B.C.D {hello-interval (1-65535)|retransmit-interval (1-65535)|retransmit-window (20-1000)|transmit-delay (1-65535)|dead-interval (1-65535)}", NO_STR VLINK_HELPSTR_IPADDR VLINK_HELPSTR_TIME_PARAM) @@ -1282,6 +1294,9 @@ DEFUN (no_ospf_area_vlink_intervals, else if (strmatch(argv[idx]->text, "retransmit-interval")) vl_config.retransmit_interval = OSPF_RETRANSMIT_INTERVAL_DEFAULT; + else if (strmatch(argv[idx]->text, "retransmit-window")) + vl_config.retransmit_window = + OSPF_RETRANSMIT_WINDOW_DEFAULT; else if (strmatch(argv[idx]->text, "transmit-delay")) vl_config.transmit_delay = OSPF_TRANSMIT_DELAY_DEFAULT; else if (strmatch(argv[idx]->text, "dead-interval")) @@ -3846,6 +3861,10 @@ static void show_ip_ospf_interface_sub(struct vty *vty, struct ospf *ospf, json_object_int_add( json_interface_sub, "timerRetransmitSecs", OSPF_IF_PARAM(oi, retransmit_interval)); + json_object_int_add(json_interface_sub, + "timerRetransmitWindowMsecs", + OSPF_IF_PARAM(oi, + retransmit_window)); } else { vty_out(vty, " Timer intervals configured,"); vty_out(vty, " Hello "); @@ -3964,6 +3983,16 @@ static void show_ip_ospf_interface_sub(struct vty *vty, struct ospf *ospf, "nbrFilterPrefixList", "N/A"); } + + /* Non-Traffic interface counters + */ + if (use_json) + json_object_int_add(json_interface_sub, + "lsaRetransmissions", + oi->ls_rxmt_lsa); + else + vty_out(vty, " LSA retransmissions: %u\n", + oi->ls_rxmt_lsa); } } @@ -5177,12 +5206,20 @@ static void show_ip_ospf_neighbor_detail_sub(struct vty *vty, lookup_msg(ospf_ism_state_msg, ospf_nbr_ism_state(nbr), NULL)); } + /* Show state changes. */ if (use_json) json_object_int_add(json_neigh, "stateChangeCounter", nbr->state_change); else - vty_out(vty, " %d state changes\n", nbr->state_change); + vty_out(vty, " %d state changes\n", nbr->state_change); + + /* Show LSA retransmissions. */ + if (use_json) + json_object_int_add(json_neigh, "lsaRetransmissions", + nbr->ls_rxmt_lsa); + else + vty_out(vty, " %u LSA retransmissions\n", nbr->ls_rxmt_lsa); if (nbr->ts_last_progress.tv_sec || nbr->ts_last_progress.tv_usec) { struct timeval res; @@ -5231,7 +5268,7 @@ static void show_ip_ospf_neighbor_detail_sub(struct vty *vty, if (DR(oi).s_addr == INADDR_ANY) { if (!use_json) vty_out(vty, - " No designated router on this network\n"); + " No designated router on this network\n"); } else { nbr_dr = ospf_nbr_lookup_by_addr(oi->nbrs, &DR(oi)); if (nbr_dr) { @@ -5250,14 +5287,14 @@ static void show_ip_ospf_neighbor_detail_sub(struct vty *vty, if (nbr_bdr == NULL) { if (!use_json) vty_out(vty, - " No backup designated router on this network\n"); + " No backup designated router on this network\n"); } else { if (use_json) json_object_string_addf(json_neigh, "routerDesignatedBackupId", "%pI4", &nbr_bdr->router_id); else - vty_out(vty, " BDR is %pI4\n", &nbr_bdr->router_id); + vty_out(vty, " BDR is %pI4\n", &nbr_bdr->router_id); } /* Show options. */ @@ -5347,7 +5384,7 @@ static void show_ip_ospf_neighbor_detail_sub(struct vty *vty, /* Show Link State Update Retransmission thread. */ if (use_json) { - if (nbr->t_ls_upd != NULL) + if (nbr->t_ls_rxmt != NULL) json_object_string_add( json_neigh, "threadLinkStateUpdateRetransmission", @@ -5355,7 +5392,7 @@ static void show_ip_ospf_neighbor_detail_sub(struct vty *vty, } else vty_out(vty, " Thread Link State Update Retransmission %s\n\n", - nbr->t_ls_upd != NULL ? "on" : "off"); + nbr->t_ls_rxmt != NULL ? "on" : "off"); if (!use_json) { vty_out(vty, " Graceful restart Helper info:\n"); @@ -7993,7 +8030,7 @@ static void ospf_nbr_timer_update(struct ospf_interface *oi) nbr->v_inactivity = OSPF_IF_PARAM(oi, v_wait); nbr->v_db_desc = OSPF_IF_PARAM(oi, retransmit_interval); nbr->v_ls_req = OSPF_IF_PARAM(oi, retransmit_interval); - nbr->v_ls_upd = OSPF_IF_PARAM(oi, retransmit_interval); + nbr->v_ls_rxmt = OSPF_IF_PARAM(oi, retransmit_interval); } } @@ -8728,6 +8765,40 @@ DEFUN_HIDDEN (no_ospf_retransmit_interval, return no_ip_ospf_retransmit_interval(self, vty, argc, argv); } +DEFPY(ip_ospf_retransmit_window, ip_ospf_retransmit_window_addr_cmd, + "[no] ip ospf retransmit-window ![(20-1000)]$retransmit-window [A.B.C.D]$ip_addr", NO_STR + "IP Information\n" + "OSPF interface commands\n" + "Window for LSA retransmit - Retransmit LSAs expiring in this window\n" + "Milliseconds\n" + "Address of interface\n") +{ + VTY_DECLVAR_CONTEXT(interface, ifp); + struct ospf_if_params *params; + + params = IF_DEF_PARAMS(ifp); + + if (ip_addr.s_addr != INADDR_ANY) { + params = ospf_get_if_params(ifp, ip_addr); + ospf_if_update_params(ifp, ip_addr); + } + + if (no) { + UNSET_IF_PARAM(params, retransmit_window); + params->retransmit_window = OSPF_RETRANSMIT_WINDOW_DEFAULT; + } else { + SET_IF_PARAM(params, retransmit_window); + params->retransmit_window = retransmit_window; + } + + /* + * There is nothing to do when the retransmit-window changes, any + * change will take effect the next time the interface LSA retransmision + * timer expires. + */ + return CMD_SUCCESS; +} + DEFPY (ip_ospf_gr_hdelay, ip_ospf_gr_hdelay_cmd, "ip ospf graceful-restart hello-delay (1-1800)", @@ -12210,6 +12281,17 @@ static int config_write_interface_one(struct vty *vty, struct vrf *vrf) vty_out(vty, "\n"); } + /* Retransmit Window print. */ + if (OSPF_IF_PARAM_CONFIGURED(params, retransmit_window) && + params->retransmit_window != + OSPF_RETRANSMIT_WINDOW_DEFAULT) { + vty_out(vty, " ip ospf retransmit-window %u", + params->retransmit_window); + if (params != IF_DEF_PARAMS(ifp) && rn) + vty_out(vty, " %pI4", &rn->p.u.prefix4); + vty_out(vty, "\n"); + } + /* Transmit Delay print. */ if (OSPF_IF_PARAM_CONFIGURED(params, transmit_delay) && params->transmit_delay @@ -12567,19 +12649,22 @@ static int config_write_virtual_link(struct vty *vty, struct ospf *ospf) oi = vl_data->vl_oi; /* timers */ - if (OSPF_IF_PARAM(oi, v_hello) - != OSPF_HELLO_INTERVAL_DEFAULT - || OSPF_IF_PARAM(oi, v_wait) - != OSPF_ROUTER_DEAD_INTERVAL_DEFAULT - || OSPF_IF_PARAM(oi, retransmit_interval) - != OSPF_RETRANSMIT_INTERVAL_DEFAULT - || OSPF_IF_PARAM(oi, transmit_delay) - != OSPF_TRANSMIT_DELAY_DEFAULT) + if (OSPF_IF_PARAM(oi, v_hello) != + OSPF_HELLO_INTERVAL_DEFAULT || + OSPF_IF_PARAM(oi, v_wait) != + OSPF_ROUTER_DEAD_INTERVAL_DEFAULT || + OSPF_IF_PARAM(oi, retransmit_interval) != + OSPF_RETRANSMIT_INTERVAL_DEFAULT || + OSPF_IF_PARAM(oi, retransmit_window) != + OSPF_RETRANSMIT_WINDOW_DEFAULT || + OSPF_IF_PARAM(oi, transmit_delay) != + OSPF_TRANSMIT_DELAY_DEFAULT) vty_out(vty, - " area %s virtual-link %pI4 hello-interval %d retransmit-interval %d transmit-delay %d dead-interval %d\n", + " area %s virtual-link %pI4 hello-interval %d retransmit-interval %d retransmit-window %d transmit-delay %d dead-interval %d\n", buf, &vl_data->vl_peer, OSPF_IF_PARAM(oi, v_hello), OSPF_IF_PARAM(oi, retransmit_interval), + OSPF_IF_PARAM(oi, retransmit_window), OSPF_IF_PARAM(oi, transmit_delay), OSPF_IF_PARAM(oi, v_wait)); else @@ -13112,6 +13197,9 @@ static void ospf_vty_if_init(void) install_element(INTERFACE_NODE, &no_ip_ospf_retransmit_interval_addr_cmd); + /* "ip ospf retransmit-window" commands. */ + install_element(INTERFACE_NODE, &ip_ospf_retransmit_window_addr_cmd); + /* "ip ospf transmit-delay" commands. */ install_element(INTERFACE_NODE, &ip_ospf_transmit_delay_addr_cmd); install_element(INTERFACE_NODE, &no_ip_ospf_transmit_delay_addr_cmd); diff --git a/tests/topotests/all_protocol_startup/r1/show_ip_ospf_interface.ref b/tests/topotests/all_protocol_startup/r1/show_ip_ospf_interface.ref index f52b51d9d8b6..e4e329011163 100644 --- a/tests/topotests/all_protocol_startup/r1/show_ip_ospf_interface.ref +++ b/tests/topotests/all_protocol_startup/r1/show_ip_ospf_interface.ref @@ -11,6 +11,7 @@ r1-eth0 is up Hello due in XX.XXXs Neighbor Count is 0, Adjacent neighbor count is 0 Graceful Restart hello delay: 10s + LSA retransmissions: 0 r1-eth3 is up ifindex X, MTU 1500 bytes, BW XX Mbit Internet Address 192.168.3.1/26, Broadcast 192.168.3.63, Area 0.0.0.0 @@ -24,3 +25,4 @@ r1-eth3 is up Hello due in XX.XXXs Neighbor Count is 0, Adjacent neighbor count is 0 Graceful Restart hello delay: 10s + LSA retransmissions: 0 diff --git a/tests/topotests/ospf_p2mp/r1/frr-p2mp.conf b/tests/topotests/ospf_p2mp/r1/frr-p2mp.conf index cb4538c0e342..89f255bb44ea 100644 --- a/tests/topotests/ospf_p2mp/r1/frr-p2mp.conf +++ b/tests/topotests/ospf_p2mp/r1/frr-p2mp.conf @@ -1,4 +1,10 @@ ! +!log file ospfd.log debug +! debug ospf event +! debug ospf client +! debug ospf lsa +! debug ospf packet all + hostname r1 password zebra log file /tmp/r1-frr.log diff --git a/tests/topotests/ospf_p2mp/r2/frr-p2mp.conf b/tests/topotests/ospf_p2mp/r2/frr-p2mp.conf index 0ca8aec3bf38..429330987e06 100644 --- a/tests/topotests/ospf_p2mp/r2/frr-p2mp.conf +++ b/tests/topotests/ospf_p2mp/r2/frr-p2mp.conf @@ -1,4 +1,10 @@ ! +!log file ospfd.log debug +! debug ospf event +! debug ospf client +! debug ospf lsa +! debug ospf packet all +! hostname r2 password zebra log file /tmp/r1-frr.log diff --git a/tests/topotests/ospf_p2mp/r3/frr-p2mp.conf b/tests/topotests/ospf_p2mp/r3/frr-p2mp.conf index 41ea70d4439c..eada78450e81 100644 --- a/tests/topotests/ospf_p2mp/r3/frr-p2mp.conf +++ b/tests/topotests/ospf_p2mp/r3/frr-p2mp.conf @@ -1,4 +1,10 @@ ! +!log file ospfd.log debug +! debug ospf event +! debug ospf client +! debug ospf lsa +! debug ospf packet all +! hostname r3 password zebra log file /tmp/r1-frr.log diff --git a/tests/topotests/ospf_p2mp/r4/frr-p2mp.conf b/tests/topotests/ospf_p2mp/r4/frr-p2mp.conf index 21fa9c72f9a9..3146ea095762 100644 --- a/tests/topotests/ospf_p2mp/r4/frr-p2mp.conf +++ b/tests/topotests/ospf_p2mp/r4/frr-p2mp.conf @@ -1,4 +1,10 @@ ! +!log file ospfd.log debug +! debug ospf event +! debug ospf client +! debug ospf lsa +! debug ospf packet all +! hostname r4 password zebra log file /tmp/r1-frr.log diff --git a/tests/topotests/ospf_p2mp/test_ospf_p2mp_broadcast.py b/tests/topotests/ospf_p2mp/test_ospf_p2mp_broadcast.py index d52c8147fe48..455c737f0d71 100644 --- a/tests/topotests/ospf_p2mp/test_ospf_p2mp_broadcast.py +++ b/tests/topotests/ospf_p2mp/test_ospf_p2mp_broadcast.py @@ -9,6 +9,7 @@ import os import sys +from time import sleep from functools import partial import pytest @@ -113,7 +114,9 @@ def teardown_module(): tgen.stop_topology() -def verify_p2mp_interface(tgen, router, nbr_cnt, nbr_adj_cnt, nbr_filter): +def verify_p2mp_interface( + tgen, router, nbr_cnt, nbr_adj_cnt, delay_reflood, nbr_filter +): "Verify the P2MP Configuration and interface settings" topo_router = tgen.gears[router] @@ -147,7 +150,7 @@ def verify_p2mp_interface(tgen, router, nbr_cnt, nbr_adj_cnt, nbr_filter): "nbrCount": nbr_cnt, "nbrAdjacentCount": nbr_adj_cnt, "prefixSuppression": False, - "p2mpDelayReflood": False, + "p2mpDelayReflood": delay_reflood, "nbrFilterPrefixList": nbr_filter, } } @@ -280,7 +283,7 @@ def test_p2mp_broadcast_interface(): pytest.skip("Skipped because of router(s) failure") step("Verify router r1 interface r1-eth0 p2mp configuration") - verify_p2mp_interface(tgen, "r1", 3, 3, "N/A") + verify_p2mp_interface(tgen, "r1", 3, 3, False, "N/A") step("Verify router r1 p2mp interface r1-eth0 neighbors") verify_p2mp_neighbor( @@ -305,7 +308,7 @@ def test_p2mp_broadcast_interface(): step("Verify router r1 interface r1-eth0 p2mp configuration application") r1.vtysh_cmd("conf t\ninterface r1-eth0\nip ospf network point-to-multipoint") - verify_p2mp_interface(tgen, "r1", 3, 3, "N/A") + verify_p2mp_interface(tgen, "r1", 3, 3, False, "N/A") step("Verify restablishment of r1-eth0 p2mp neighbors") verify_p2mp_neighbor( @@ -324,14 +327,14 @@ def test_p2mp_broadcast_interface(): verify_p2mp_route(tgen, "r1", "10.1.4.0/24", 24, "10.1.0.4", "r1-eth0") -def test_p2mp_broadcast_neighbor_filter(): +def p2mp_broadcast_neighbor_filter_common(delay_reflood): tgen = get_topogen() if tgen.routers_have_failure(): pytest.skip("Skipped because of router(s) failure") step("Verify router r1 interface r1-eth0 p2mp configuration") - verify_p2mp_interface(tgen, "r1", 3, 3, "N/A") + verify_p2mp_interface(tgen, "r1", 3, 3, delay_reflood, "N/A") step("Verify router r1 p2mp interface r1-eth0 neighbors") verify_p2mp_neighbor( @@ -362,7 +365,7 @@ def test_p2mp_broadcast_neighbor_filter(): assert neighbor_filter_cfg == " ip ospf neighbor-filter nbr-filter", assertmsg step("Verify non-existent neighbor-filter is not applied to r1 interfaces") - verify_p2mp_interface(tgen, "r1", 3, 3, "N/A") + verify_p2mp_interface(tgen, "r1", 3, 3, delay_reflood, "N/A") step("Add nbr-filter prefix-list configuration to r1") r1.vtysh_cmd("conf t\nip prefix-list nbr-filter seq 200 permit any") @@ -370,7 +373,7 @@ def test_p2mp_broadcast_neighbor_filter(): step( "Verify neighbor-filter is now applied to r1 interface and neighbors still adjacent" ) - verify_p2mp_interface(tgen, "r1", 3, 3, "nbr-filter") + verify_p2mp_interface(tgen, "r1", 3, 3, delay_reflood, "nbr-filter") step("Add nbr-filter prefix-list configuration to block r4") r1.vtysh_cmd("conf t\nip prefix-list nbr-filter seq 10 deny 10.1.0.4/32") @@ -378,7 +381,7 @@ def test_p2mp_broadcast_neighbor_filter(): step( "Verify neighbor-filter is now applied to r1 interface and r4 is no longer adjacent" ) - verify_p2mp_interface(tgen, "r1", 2, 2, "nbr-filter") + verify_p2mp_interface(tgen, "r1", 2, 2, delay_reflood, "nbr-filter") verify_p2mp_neighbor_missing(tgen, "r1", "4.4.4.4") step("Verify route to r4 subnet is now through r2") @@ -390,7 +393,7 @@ def test_p2mp_broadcast_neighbor_filter(): step( "Verify neighbor-filter is now applied to r1 interface and r2 is no longer adjacent" ) - verify_p2mp_interface(tgen, "r1", 1, 1, "nbr-filter") + verify_p2mp_interface(tgen, "r1", 1, 1, delay_reflood, "nbr-filter") verify_p2mp_neighbor_missing(tgen, "r1", "2.2.2.2") step("Verify route to r4 and r2 subnet are now through r3") @@ -406,24 +409,105 @@ def test_p2mp_broadcast_neighbor_filter(): assert rc, assertmsg step("Verify interface neighbor-filter is removed and neighbors present") - verify_p2mp_interface(tgen, "r1", 3, 3, "N/A") + verify_p2mp_interface(tgen, "r1", 3, 3, delay_reflood, "N/A") step("Add neighbor filter configuration and verify neighbors are filtered") r1.vtysh_cmd("conf t\ninterface r1-eth0\nip ospf neighbor-filter nbr-filter") - verify_p2mp_interface(tgen, "r1", 1, 1, "nbr-filter") + verify_p2mp_interface(tgen, "r1", 1, 1, delay_reflood, "nbr-filter") verify_p2mp_neighbor_missing(tgen, "r1", "2.2.2.2") verify_p2mp_neighbor_missing(tgen, "r1", "4.4.4.4") step("Remove nbr-filter prefix-list configuration to block r2 and verify neighbor") r1.vtysh_cmd("conf t\nno ip prefix-list nbr-filter seq 20") - verify_p2mp_interface(tgen, "r1", 2, 2, "nbr-filter") + verify_p2mp_interface(tgen, "r1", 2, 2, delay_reflood, "nbr-filter") verify_p2mp_neighbor( tgen, "r1", "2.2.2.2", "Full/DROther", "10.1.0.2", "r1-eth0:10.1.0.1" ) step("Delete nbr-filter prefix-list and verify neighbors are present") r1.vtysh_cmd("conf t\nno ip prefix-list nbr-filter") - verify_p2mp_interface(tgen, "r1", 3, 3, "N/A") + verify_p2mp_interface(tgen, "r1", 3, 3, delay_reflood, "N/A") + + +def test_p2mp_broadcast_neighbor_filter(): + p2mp_broadcast_neighbor_filter_common(False) + + +def test_p2mp_broadcast_neighbor_filter_delay_reflood(): + tgen = get_topogen() + + if tgen.routers_have_failure(): + pytest.skip("Skipped because of router(s) failure") + + step("Modify router r1 interface r1-eth0 p2mp delay-reflood configuration") + r1 = tgen.gears["r1"] + r1.vtysh_cmd( + "conf t\ninterface r1-eth0\nip ospf network point-to-multipoint delay-reflood" + ) + verify_p2mp_interface(tgen, "r1", 3, 3, True, "N/A") + + step("Modify router r2 interface r2-eth0 p2mp delay-reflood configuration") + r2 = tgen.gears["r2"] + r2.vtysh_cmd( + "conf t\ninterface r2-eth0\nip ospf network point-to-multipoint delay-reflood" + ) + + step("Modify router r3 interface r3-eth0 p2mp delay-reflood configuration") + r3 = tgen.gears["r3"] + r3.vtysh_cmd( + "conf t\ninterface r3-eth0\nip ospf network point-to-multipoint delay-reflood" + ) + + step("Modify router r4 interface r4-eth0 p2mp delay-reflood configuration") + r4 = tgen.gears["r4"] + r4.vtysh_cmd( + "conf t\ninterface r4-eth0\nip ospf network point-to-multipoint delay-reflood" + ) + + p2mp_broadcast_neighbor_filter_common(True) + + step("Recreate a partial P2MP mesh with neighbor filters") + step("Add nbr-filter prefix-list configuration to block r4") + r1.vtysh_cmd("conf t\nip prefix-list nbr-filter seq 30 permit any") + r1.vtysh_cmd("conf t\nip prefix-list nbr-filter seq 10 deny 10.1.0.3/32") + r1.vtysh_cmd("conf t\nip prefix-list nbr-filter seq 20 deny 10.1.0.4/32") + r1.vtysh_cmd("conf t\ninterface r1-eth0\nip ospf neighbor-filter nbr-filter") + + r2.vtysh_cmd("conf t\nip prefix-list nbr-filter seq 30 permit any") + r2.vtysh_cmd("conf t\nip prefix-list nbr-filter seq 10 deny 10.1.0.4/32") + r2.vtysh_cmd("conf t\ninterface r2-eth0\nip ospf neighbor-filter nbr-filter") + + r3.vtysh_cmd("conf t\nip prefix-list nbr-filter seq 30 permit any") + r3.vtysh_cmd("conf t\nip prefix-list nbr-filter seq 10 deny 10.1.0.1/32") + r3.vtysh_cmd("conf t\ninterface r3-eth0\nip ospf neighbor-filter nbr-filter") + + r4.vtysh_cmd("conf t\nip prefix-list nbr-filter seq 30 permit any") + r4.vtysh_cmd("conf t\nip prefix-list nbr-filter seq 10 deny 10.1.0.1/32") + r4.vtysh_cmd("conf t\nip prefix-list nbr-filter seq 20 deny 10.1.0.2/32") + r4.vtysh_cmd("conf t\ninterface r4-eth0\nip ospf neighbor-filter nbr-filter") + + step( + "Add redistribution and spaced static routes to r1 to test delay flood retransmission" + ) + r1.vtysh_cmd("conf t\nrouter ospf\nredistribute static") + r1.vtysh_cmd("conf t\nip route 20.1.1.1/32 null0") + sleep(1) + r1.vtysh_cmd("conf t\nip route 20.1.1.2/32 null0") + sleep(1) + r1.vtysh_cmd("conf t\nip route 20.1.1.3/32 null0") + sleep(1) + r1.vtysh_cmd("conf t\nip route 20.1.1.4/32 null0") + sleep(1) + r1.vtysh_cmd("conf t\nip route 20.1.1.5/32 null0") + sleep(1) + + step( + "Verify the routes are installed on r1 with delay-reflood in P2MP partial mesh" + ) + verify_p2mp_route(tgen, "r4", "20.1.1.1/32", 32, "10.1.0.3", "r4-eth0") + verify_p2mp_route(tgen, "r4", "20.1.1.2/32", 32, "10.1.0.3", "r4-eth0") + verify_p2mp_route(tgen, "r4", "20.1.1.3/32", 32, "10.1.0.3", "r4-eth0") + verify_p2mp_route(tgen, "r4", "20.1.1.4/32", 32, "10.1.0.3", "r4-eth0") def test_memory_leak(): From 56c16ee529b546058c8d1fabbb701d8ed2fded75 Mon Sep 17 00:00:00 2001 From: Sindhu Parvathi Gopinathan Date: Wed, 19 Jun 2024 07:35:31 -0700 Subject: [PATCH 19/28] zebra: clear evpn dup-addr return error-msg when there is no vni clear evpn dup-addr cli returns error-msg for below conditions, - If evpn is not enabled & - If there is no VNI exists. supported command: ``` clear evpn dup-addr vni ``` Ticket: #3495573 Testing: bharat# clear evpn dup-addr vni all Error type: validation Error description: % EVPN not enabled bharat# clear evpn dup-addr vni 20 Error type: validation Error description: % VNI 20 does not exist Signed-off-by: Sindhu Parvathi Gopinathan's Signed-off-by: Chirag Shah --- zebra/zebra_nb_rpcs.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/zebra/zebra_nb_rpcs.c b/zebra/zebra_nb_rpcs.c index 938193df2f2a..744ba620f22e 100644 --- a/zebra/zebra_nb_rpcs.c +++ b/zebra/zebra_nb_rpcs.c @@ -12,6 +12,8 @@ #include "zebra/zebra_router.h" #include "zebra/zebra_vrf.h" #include "zebra/zebra_vxlan.h" +#include "zebra/zebra_vxlan_if.h" +#include "zebra/zebra_evpn.h" /* * XPath: /frr-zebra:clear-evpn-dup-addr @@ -21,6 +23,11 @@ int clear_evpn_dup_addr_rpc(struct nb_cb_rpc_args *args) struct zebra_vrf *zvrf; int ret = NB_OK; + if (!is_evpn_enabled()) { + snprintf(args->errmsg, args->errmsg_len, + "%% EVPN not enabled\n"); + return NB_ERR_VALIDATION; + } zvrf = zebra_vrf_get_evpn(); if (yang_dnode_exists(args->input, "all-vnis")) { @@ -30,6 +37,12 @@ int clear_evpn_dup_addr_rpc(struct nb_cb_rpc_args *args) struct ipaddr host_ip = {.ipa_type = IPADDR_NONE}; struct ethaddr mac; + if (!zebra_evpn_lookup(vni)) { + snprintf(args->errmsg, args->errmsg_len, + "%% VNI %u does not exist\n", vni); + return NB_ERR_VALIDATION; + } + if (yang_dnode_exists(args->input, "mac-addr")) { yang_dnode_get_mac(&mac, args->input, "mac-addr"); ret = zebra_vxlan_clear_dup_detect_vni_mac(zvrf, vni, From f9d4d1626d47944715eb522af38909cc90a1d53c Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Fri, 21 Jun 2024 11:32:05 +0200 Subject: [PATCH 20/28] tools/checkpatch: accept pim_* typedefs pimd uses "v4/v6 dispatch" typedefs. Don't complain about those in checkpatch. Signed-off-by: David Lamparter --- tools/checkpatch.pl | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/checkpatch.pl b/tools/checkpatch.pl index f52f1a161649..3e7abeda3962 100755 --- a/tools/checkpatch.pl +++ b/tools/checkpatch.pl @@ -4668,6 +4668,7 @@ sub process { # check for new typedefs, only function parameters and sparse annotations # make sense. if ($line =~ /\btypedef\s/ && + $line !~ /\btypedef.*\s(pim_[^\s]+|[^\s]+_pim)\s*;/ && $line !~ /\btypedef\s+$Type\s*\(\s*\*?$Ident\s*\)\s*\(/ && $line !~ /\btypedef\s+$Type\s+$Ident\s*\(/ && $line !~ /\b$typeTypedefs\b/ && From c06fb90b63ccf2c4d1220f4d8e4b423bc4fc6a40 Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Fri, 21 Jun 2024 10:59:43 +0200 Subject: [PATCH 21/28] isisd: fix display crash srv6 sid structure in json Fix a crash when doing "show isis database detail json" in isis_srv6_topo1 topotest. > #0 raise (sig=) at ../sysdeps/unix/sysv/linux/raise.c:50 > #1 0x00007fad89524e2c in core_handler (signo=6, siginfo=0x7ffe86a4b8b0, context=0x7ffe86a4b780) at lib/sigevent.c:258 > #2 > #3 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50 > #4 0x00007fad8904e537 in __GI_abort () at abort.c:79 > #5 0x00007fad8904e40f in __assert_fail_base (fmt=0x7fad891c5688 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", assertion=0x7fad8a3e70e8 "json_object_get_type(jso) == json_type_object", > file=0x7fad8a3e7064 "./json_object.c", line=590, function=) at assert.c:92 > #6 0x00007fad8905d662 in __GI___assert_fail (assertion=0x7fad8a3e70e8 "json_object_get_type(jso) == json_type_object", file=0x7fad8a3e7064 "./json_object.c", line=590, > function=0x7fad8a3e7440 "json_object_object_add_ex") at assert.c:101 > #7 0x00007fad8a3dfe93 in json_object_object_add_ex () from /lib/x86_64-linux-gnu/libjson-c.so.5 > #8 0x000055708e3f8f7f in format_subsubtlv_srv6_sid_structure (sid_struct=0x602000172b70, buf=0x0, json=0x6040000a21d0, indent=6) at isisd/isis_tlvs.c:2880 > #9 0x000055708e3f9acb in isis_format_subsubtlvs (subsubtlvs=0x602000172b50, buf=0x0, json=0x6040000a21d0, indent=6) at isisd/isis_tlvs.c:3022 > #10 0x000055708e3eefb0 in format_item_ext_subtlvs (exts=0x614000047440, buf=0x0, json=0x6040000a2190, indent=2, mtid=2) at isisd/isis_tlvs.c:1313 > #11 0x000055708e3fd599 in format_item_extended_reach (mtid=2, i=0x60300015aed0, buf=0x0, json=0x6040000a1bd0, indent=0) at isisd/isis_tlvs.c:3763 > #12 0x000055708e40d46a in format_item (mtid=2, context=ISIS_CONTEXT_LSP, type=ISIS_TLV_MT_REACH, i=0x60300015aed0, buf=0x0, json=0x6040000a1bd0, indent=0) at isisd/isis_tlvs.c:6789 > #13 0x000055708e40d4fc in format_items_ (mtid=2, context=ISIS_CONTEXT_LSP, type=ISIS_TLV_MT_REACH, items=0x60600021d160, buf=0x0, json=0x6040000a1bd0, indent=0) at isisd/isis_tlvs.c:6804 > #14 0x000055708e40edbc in format_mt_items (context=ISIS_CONTEXT_LSP, type=ISIS_TLV_MT_REACH, m=0x6180000845d8, buf=0x0, json=0x6040000a1bd0, indent=0) at isisd/isis_tlvs.c:7147 > #15 0x000055708e4111e9 in format_tlvs (tlvs=0x618000084480, buf=0x0, json=0x6040000a1bd0, indent=0) at isisd/isis_tlvs.c:7572 > #16 0x000055708e4114ce in isis_format_tlvs (tlvs=0x618000084480, json=0x6040000a1bd0) at isisd/isis_tlvs.c:7613 > #17 0x000055708e36f167 in lsp_print_detail (lsp=0x612000058b40, vty=0x0, json=0x6040000a1bd0, dynhost=1 '\001', isis=0x60d00001f800) at isisd/isis_lsp.c:785 > #18 0x000055708e36f31f in lsp_print_all (vty=0x0, json=0x6040000a0490, head=0x61f000005488, detail=1 '\001', dynhost=1 '\001', isis=0x60d00001f800) at isisd/isis_lsp.c:820 > #19 0x000055708e4379fc in show_isis_database_lspdb_json (json=0x6040000a0450, area=0x61f000005480, level=0, lspdb=0x61f000005488, sysid_str=0x0, ui_level=1) at isisd/isisd.c:2683 > #20 0x000055708e437ef9 in show_isis_database_json (json=0x6040000a0310, sysid_str=0x0, ui_level=1, isis=0x60d00001f800) at isisd/isisd.c:2754 > #21 0x000055708e438357 in show_isis_database_common (vty=0x62e000060400, json=0x6040000a0310, sysid_str=0x0, ui_level=1, isis=0x60d00001f800) at isisd/isisd.c:2788 > #22 0x000055708e438591 in show_isis_database (vty=0x62e000060400, json=0x6040000a0310, sysid_str=0x0, ui_level=1, vrf_name=0x7fad89806300 "default", all_vrf=false) > at isisd/isisd.c:2825 > #23 0x000055708e43891d in show_database (self=0x55708e5519c0 , vty=0x62e000060400, argc=5, argv=0x6040000a02d0) at isisd/isisd.c:2855 > #24 0x00007fad893a9767 in cmd_execute_command_real (vline=0x60300015f220, vty=0x62e000060400, cmd=0x0, up_level=0) at lib/command.c:1002 > #25 0x00007fad893a9adc in cmd_execute_command (vline=0x60300015f220, vty=0x62e000060400, cmd=0x0, vtysh=0) at lib/command.c:1061 > #26 0x00007fad893aa728 in cmd_execute (vty=0x62e000060400, cmd=0x621000025900 "show isis database detail json ", matched=0x0, vtysh=0) at lib/command.c:1227 Note that prior to 2e670cd779, there was no crash but only the last "srv6-sid-structure" was displayed. A "srv6-sid-structure" should be displayed for each "sid". This commit also fix this. Was: > "srv6-lan-endx-sid": [ > { > "sid": "fc00:0:1:1::", > "weight": 0, > "algorithm": "SPF", > "neighbor-id": "0000.0000.0002" > }, > { > "sid": "fc00:0:1:2::", > "weight": 0, > "algorithm": "SPF", > "neighbor-id": "0000.0000.0003" > } > ], > "srv6-sid-structure": { > "loc-block-len": 32, > "loc-node-len": 16, > "func-len": 16, > "arg-len": 0 > }, Now (srv6-sid-structure are identical but they are not always): > "srv6-lan-endx-sid": [ > { > "sid": "fc00:0:1:1::", > "algorithm": "SPF", > "neighbor-id": "0000.0000.0002", > "srv6-sid-structure": { > "loc-block-len": 32, > "loc-node-len": 16, > "func-len": 8, > "arg-len": 0 > }, > }, > { > "sid": "fc00:0:1:2::", > "algorithm": "SPF", > "neighbor-id": "0000.0000.0003", > "srv6-sid-structure": { > "loc-block-len": 32, > "loc-node-len": 16, > "func-len": 16, > "arg-len": 0 > }, > } > ], Fixes: 2e670cd779 ("isisd: fix display of srv6 subsubtlvs") Fixes: 648a158802 ("isisd: Add SRv6 End.X SID to Sub-TLV format func") Signed-off-by: Louis Scalbert --- isisd/isis_tlvs.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/isisd/isis_tlvs.c b/isisd/isis_tlvs.c index 3bb8a4824698..1c345501525f 100644 --- a/isisd/isis_tlvs.c +++ b/isisd/isis_tlvs.c @@ -1311,8 +1311,7 @@ static void format_item_ext_subtlvs(struct isis_ext_subtlvs *exts, json_object_array_add(arr_adj_json, flags_json); if (adj->subsubtlvs) isis_format_subsubtlvs(adj->subsubtlvs, - NULL, - arr_adj_json, + NULL, flags_json, indent + 4); } /* end old deprecated key format */ @@ -1350,8 +1349,7 @@ static void format_item_ext_subtlvs(struct isis_ext_subtlvs *exts, json_object_array_add(arr_adj_json, flags_json); if (adj->subsubtlvs) isis_format_subsubtlvs(adj->subsubtlvs, - NULL, - arr_adj_json, + NULL, flags_json, indent + 4); } } else @@ -1432,8 +1430,7 @@ static void format_item_ext_subtlvs(struct isis_ext_subtlvs *exts, json_object_array_add(arr_adj_json, flags_json); if (lan->subsubtlvs) isis_format_subsubtlvs(lan->subsubtlvs, - NULL, - arr_adj_json, + NULL, flags_json, indent + 4); } /* end old deprecated key format */ @@ -1477,8 +1474,7 @@ static void format_item_ext_subtlvs(struct isis_ext_subtlvs *exts, json_object_array_add(arr_adj_json, flags_json); if (lan->subsubtlvs) isis_format_subsubtlvs(lan->subsubtlvs, - NULL, - arr_adj_json, + NULL, flags_json, indent + 4); } } else From fdb89ab51a83cd4cb1c4ce3068eba5e3a431954b Mon Sep 17 00:00:00 2001 From: Carmine Scarpitta Date: Fri, 21 Jun 2024 17:01:49 +0200 Subject: [PATCH 22/28] zebra: Remove dead SRv6 code At line 1736, `alloc_mode` is set to `SRV6_SID_ALLOC_MODE_EXPLICIT` or `SRV6_SID_ALLOC_MODE_DYNAMIC` depending on the `sid_value` variable. There will never be a case where alloc_mode will be `SRV6_SID_ALLOC_MODE_MAX` or `SRV6_SID_ALLOC_MODE_UNSPEC`. Let's replace the `switch(alloc_mode) {...}` with an if-else. Fixes CID 1594015. ** CID 1594015: (DEADCODE) /zebra/zebra_srv6.c: 1782 in get_srv6_sid() /zebra/zebra_srv6.c: 1781 in get_srv6_sid() ________________________________________________________________________________________________________ *** CID 1594015: (DEADCODE) /zebra/zebra_srv6.c: 1782 in get_srv6_sid() 1776 } 1777 1778 ret = get_srv6_sid_dynamic(sid, ctx, locator); 1779 1780 break; 1781 case SRV6_SID_ALLOC_MODE_MAX: CID 1594015: (DEADCODE) Execution cannot reach this statement: "case SRV6_SID_ALLOC_MODE_UN...". 1782 case SRV6_SID_ALLOC_MODE_UNSPEC: 1783 default: 1784 flog_err(EC_ZEBRA_SM_CANNOT_ASSIGN_SID, 1785 "%s: SRv6 Manager: Unrecognized alloc mode %u", 1786 __func__, alloc_mode); 1787 /* We should never arrive here */ /zebra/zebra_srv6.c: 1781 in get_srv6_sid() 1775 return -1; 1776 } 1777 1778 ret = get_srv6_sid_dynamic(sid, ctx, locator); 1779 1780 break; CID 1594015: (DEADCODE) Execution cannot reach this statement: "case SRV6_SID_ALLOC_MODE_MAX:". 1781 case SRV6_SID_ALLOC_MODE_MAX: 1782 case SRV6_SID_ALLOC_MODE_UNSPEC: 1783 default: 1784 flog_err(EC_ZEBRA_SM_CANNOT_ASSIGN_SID, 1785 "%s: SRv6 Manager: Unrecognized alloc mode %u", 1786 __func__, alloc_mode); Signed-off-by: Carmine Scarpitta --- zebra/zebra_srv6.c | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/zebra/zebra_srv6.c b/zebra/zebra_srv6.c index 0ca77a49740c..2c1f86a3fc3c 100644 --- a/zebra/zebra_srv6.c +++ b/zebra/zebra_srv6.c @@ -1742,8 +1742,7 @@ int get_srv6_sid(struct zebra_srv6_sid **sid, struct srv6_sid_ctx *ctx, __func__, srv6_sid_ctx2str(buf, sizeof(buf), ctx), sid_value, srv6_sid_alloc_mode2str(alloc_mode)); - switch (alloc_mode) { - case SRV6_SID_ALLOC_MODE_EXPLICIT: + if (alloc_mode == SRV6_SID_ALLOC_MODE_EXPLICIT) { /* * Explicit SID allocation: allocate a specific SID value */ @@ -1755,9 +1754,7 @@ int get_srv6_sid(struct zebra_srv6_sid **sid, struct srv6_sid_ctx *ctx, } ret = get_srv6_sid_explicit(sid, ctx, sid_value); - - break; - case SRV6_SID_ALLOC_MODE_DYNAMIC: + } else { /* * Dynamic SID allocation: allocate any available SID value */ @@ -1776,16 +1773,6 @@ int get_srv6_sid(struct zebra_srv6_sid **sid, struct srv6_sid_ctx *ctx, } ret = get_srv6_sid_dynamic(sid, ctx, locator); - - break; - case SRV6_SID_ALLOC_MODE_MAX: - case SRV6_SID_ALLOC_MODE_UNSPEC: - default: - flog_err(EC_ZEBRA_SM_CANNOT_ASSIGN_SID, - "%s: SRv6 Manager: Unrecognized alloc mode %u", - __func__, alloc_mode); - /* We should never arrive here */ - assert(0); } return ret; From 375a02d2a30cd7a06b568187e23226ad5d083c87 Mon Sep 17 00:00:00 2001 From: Carmine Scarpitta Date: Fri, 21 Jun 2024 17:41:34 +0200 Subject: [PATCH 23/28] zebra: Fix wrong variable used in `for` loop The `for` loop starting at line 1848 searches the `func_allocated` array for a pointer that points to a specific `sid_wide_func` element. The loop should iterate over all the elements of the `func_allocated` array and dereference each element to see if it is the one we are looking for. Currently, the loop is using the wrong variable to iterate over the array. Let's fix this issue by using the correct variable in the loop. Fixes CID 1594014 Fixes CID 1594016 ** CID 1594014: Null pointer dereferences (FORWARD_NULL) /zebra/zebra_srv6.c: 1860 in release_srv6_sid_func_explicit() ________________________________________________________________________________________________________ *** CID 1594014: Null pointer dereferences (FORWARD_NULL) /zebra/zebra_srv6.c: 1860 in release_srv6_sid_func_explicit() 1854 1855 /* Lookup SID function in the functions allocated list of EWLIB range */ 1856 for (ALL_LIST_ELEMENTS_RO(block->u.usid 1857 .wide_lib[sid_func] 1858 .func_allocated, 1859 node, sid_func_ptr)) CID 1594014: Null pointer dereferences (FORWARD_NULL) Dereferencing null pointer "sid_wide_func_ptr". 1860 if (*sid_wide_func_ptr == sid_wide_func) 1861 break; 1862 1863 /* Ensure that the SID function is allocated */ 1864 if (!sid_wide_func_ptr) { 1865 zlog_warn("%s: failed to release wide SID function %u, function is not allocated", ** CID 1594016: Possible Control flow issues (DEADCODE) /zebra/zebra_srv6.c: 1871 in release_srv6_sid_func_explicit() ________________________________________________________________________________________________________ *** CID 1594016: Possible Control flow issues (DEADCODE) /zebra/zebra_srv6.c: 1871 in release_srv6_sid_func_explicit() 1865 zlog_warn("%s: failed to release wide SID function %u, function is not allocated", 1866 __func__, sid_wide_func); 1867 return -1; 1868 } 1869 1870 /* Release the SID function from the EWLIB range */ CID 1594016: Possible Control flow issues (DEADCODE) Execution cannot reach this statement: "listnode_delete(block->u.us...". 1871 listnode_delete(block->u.usid.wide_lib[sid_func] 1872 .func_allocated, 1873 sid_wide_func_ptr); 1874 zebra_srv6_sid_func_free(sid_wide_func_ptr); 1875 } else { 1876 zlog_warn("%s: function %u is outside ELIB [%u/%u] and EWLIB alloc ranges [%u/%u]", Signed-off-by: Carmine Scarpitta --- zebra/zebra_srv6.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zebra/zebra_srv6.c b/zebra/zebra_srv6.c index 2c1f86a3fc3c..be335a5dedf2 100644 --- a/zebra/zebra_srv6.c +++ b/zebra/zebra_srv6.c @@ -1843,7 +1843,7 @@ static bool release_srv6_sid_func_explicit(struct zebra_srv6_sid_block *block, for (ALL_LIST_ELEMENTS_RO(block->u.usid .wide_lib[sid_func] .func_allocated, - node, sid_func_ptr)) + node, sid_wide_func_ptr)) if (*sid_wide_func_ptr == sid_wide_func) break; From df97a9d13318f15c59bb055b90529e9e8378a619 Mon Sep 17 00:00:00 2001 From: Carmine Scarpitta Date: Fri, 21 Jun 2024 17:47:46 +0200 Subject: [PATCH 24/28] zebra: Fix NULL pointer dereference The `locator` pointer is dereferenced before ensuring it is not NULL. Fix the issue by checking that the pointer is not NULL before dereferencing it. Fixes 1594013 ** CID 1594013: Null pointer dereferences (REVERSE_INULL) /zebra/zebra_srv6.c: 961 in zebra_srv6_sid_compose() ________________________________________________________________________________________________________ *** CID 1594013: Null pointer dereferences (REVERSE_INULL) /zebra/zebra_srv6.c: 961 in zebra_srv6_sid_compose() 955 struct srv6_locator *locator, 956 uint32_t sid_func) 957 { 958 uint8_t offset, func_len; 959 struct srv6_sid_format *format = locator->sid_format; 960 CID 1594013: Null pointer dereferences (REVERSE_INULL) Null-checking "locator" suggests that it may be null, but it has already been dereferenced on all paths leading to the check. 961 if (!sid_value || !locator) 962 return false; 963 964 if (format) { 965 offset = format->block_len + format->node_len; 966 func_len = format->function_len; Signed-off-by: Carmine Scarpitta --- zebra/zebra_srv6.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/zebra/zebra_srv6.c b/zebra/zebra_srv6.c index be335a5dedf2..e82b781c6fea 100644 --- a/zebra/zebra_srv6.c +++ b/zebra/zebra_srv6.c @@ -956,11 +956,12 @@ static bool zebra_srv6_sid_compose(struct in6_addr *sid_value, uint32_t sid_func) { uint8_t offset, func_len; - struct srv6_sid_format *format = locator->sid_format; + struct srv6_sid_format *format; if (!sid_value || !locator) return false; + format = locator->sid_format; if (format) { offset = format->block_len + format->node_len; func_len = format->function_len; From c554b5d90a40d16423a015849b76d56045fb7702 Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Fri, 21 Jun 2024 11:17:32 +0200 Subject: [PATCH 25/28] isisd: remane flags_json variable The variable flags_json was incorrectly named, leading to confusion and causing the bug fixed in the previous commit. Rename the variable to refer to SRv6 End SID instead. Cosmetic change. Signed-off-by: Louis Scalbert --- isisd/isis_tlvs.c | 373 ++++++++++++++++++++++++---------------------- 1 file changed, 195 insertions(+), 178 deletions(-) diff --git a/isisd/isis_tlvs.c b/isisd/isis_tlvs.c index 1c345501525f..4ea384ef23bf 100644 --- a/isisd/isis_tlvs.c +++ b/isisd/isis_tlvs.c @@ -1010,7 +1010,7 @@ static void format_item_ext_subtlvs(struct isis_ext_subtlvs *exts, struct isis_adj_sid *adj; if (json) { - struct json_object *arr_adj_json, *flags_json; + struct json_object *arr_adj_json, *adj_sid_json; #if CONFDATE > 20240916 CPP_NOTICE("remove deprecated key format with -") @@ -1022,42 +1022,37 @@ static void format_item_ext_subtlvs(struct isis_ext_subtlvs *exts, adj; adj = adj->next) { snprintfrr(cnt_buf, sizeof(cnt_buf), "%d", adj->sid); - flags_json = json_object_new_object(); - json_object_int_add(flags_json, "sid", + adj_sid_json = json_object_new_object(); + json_object_int_add(adj_sid_json, "sid", adj->sid); - json_object_int_add(flags_json, "weight", + json_object_int_add(adj_sid_json, "weight", adj->weight); - json_object_string_add( - flags_json, "flag-f", - adj->flags & EXT_SUBTLV_LINK_ADJ_SID_FFLG - ? "1" - : "0"); - json_object_string_add( - flags_json, "flag-b", - adj->flags & EXT_SUBTLV_LINK_ADJ_SID_BFLG - ? "1" - : "0"); - json_object_string_add( - flags_json, "flag-v", - adj->flags & EXT_SUBTLV_LINK_ADJ_SID_VFLG - ? "1" - : "0"); - json_object_string_add( - flags_json, "flag-l", - adj->flags & EXT_SUBTLV_LINK_ADJ_SID_LFLG - ? "1" - : "0"); - json_object_string_add( - flags_json, "flag-s", - adj->flags & EXT_SUBTLV_LINK_ADJ_SID_SFLG - ? "1" - : "0"); - json_object_string_add( - flags_json, "flag-p", - adj->flags & EXT_SUBTLV_LINK_ADJ_SID_PFLG - ? "1" - : "0"); - json_object_array_add(arr_adj_json, flags_json); + json_object_string_add(adj_sid_json, "flag-f", + adj->flags & EXT_SUBTLV_LINK_ADJ_SID_FFLG + ? "1" + : "0"); + json_object_string_add(adj_sid_json, "flag-b", + adj->flags & EXT_SUBTLV_LINK_ADJ_SID_BFLG + ? "1" + : "0"); + json_object_string_add(adj_sid_json, "flag-v", + adj->flags & EXT_SUBTLV_LINK_ADJ_SID_VFLG + ? "1" + : "0"); + json_object_string_add(adj_sid_json, "flag-l", + adj->flags & EXT_SUBTLV_LINK_ADJ_SID_LFLG + ? "1" + : "0"); + json_object_string_add(adj_sid_json, "flag-s", + adj->flags & EXT_SUBTLV_LINK_ADJ_SID_SFLG + ? "1" + : "0"); + json_object_string_add(adj_sid_json, "flag-p", + adj->flags & EXT_SUBTLV_LINK_ADJ_SID_PFLG + ? "1" + : "0"); + json_object_array_add(arr_adj_json, + adj_sid_json); } /* end old deprecated key format */ @@ -1067,35 +1062,37 @@ static void format_item_ext_subtlvs(struct isis_ext_subtlvs *exts, adj; adj = adj->next) { snprintfrr(cnt_buf, sizeof(cnt_buf), "%d", adj->sid); - flags_json = json_object_new_object(); - json_object_int_add(flags_json, "sid", adj->sid); - json_object_int_add(flags_json, "weight", + adj_sid_json = json_object_new_object(); + json_object_int_add(adj_sid_json, "sid", + adj->sid); + json_object_int_add(adj_sid_json, "weight", adj->weight); - json_object_boolean_add(flags_json, "flagF", + json_object_boolean_add(adj_sid_json, "flagF", adj->flags & EXT_SUBTLV_LINK_ADJ_SID_FFLG ? true : false); - json_object_boolean_add(flags_json, "flagB", + json_object_boolean_add(adj_sid_json, "flagB", adj->flags & EXT_SUBTLV_LINK_ADJ_SID_BFLG ? true : false); - json_object_boolean_add(flags_json, "flagV", + json_object_boolean_add(adj_sid_json, "flagV", adj->flags & EXT_SUBTLV_LINK_ADJ_SID_VFLG ? true : false); - json_object_boolean_add(flags_json, "flagL", + json_object_boolean_add(adj_sid_json, "flagL", adj->flags & EXT_SUBTLV_LINK_ADJ_SID_LFLG ? true : false); - json_object_boolean_add(flags_json, "flagS", + json_object_boolean_add(adj_sid_json, "flagS", adj->flags & EXT_SUBTLV_LINK_ADJ_SID_SFLG ? true : false); - json_object_boolean_add(flags_json, "flagP", + json_object_boolean_add(adj_sid_json, "flagP", adj->flags & EXT_SUBTLV_LINK_ADJ_SID_PFLG ? true : false); - json_object_array_add(arr_adj_json, flags_json); + json_object_array_add(arr_adj_json, + adj_sid_json); } } else for (adj = (struct isis_adj_sid *)exts->adj_sid.head; @@ -1128,7 +1125,7 @@ static void format_item_ext_subtlvs(struct isis_ext_subtlvs *exts, if (IS_SUBTLV(exts, EXT_LAN_ADJ_SID)) { struct isis_lan_adj_sid *lan; if (json) { - struct json_object *arr_adj_json, *flags_json; + struct json_object *arr_adj_json, *lan_adj_json; #if CONFDATE > 20240916 CPP_NOTICE("remove deprecated key format with -") @@ -1147,42 +1144,37 @@ static void format_item_ext_subtlvs(struct isis_ext_subtlvs *exts, continue; snprintfrr(cnt_buf, sizeof(cnt_buf), "%d", lan->sid); - flags_json = json_object_new_object(); - json_object_int_add(flags_json, "sid", + lan_adj_json = json_object_new_object(); + json_object_int_add(lan_adj_json, "sid", lan->sid); - json_object_int_add(flags_json, "weight", + json_object_int_add(lan_adj_json, "weight", lan->weight); - json_object_string_add( - flags_json, "flag-f", - lan->flags & EXT_SUBTLV_LINK_ADJ_SID_FFLG - ? "1" - : "0"); - json_object_string_add( - flags_json, "flag-b", - lan->flags & EXT_SUBTLV_LINK_ADJ_SID_BFLG - ? "1" - : "0"); - json_object_string_add( - flags_json, "flag-v", - lan->flags & EXT_SUBTLV_LINK_ADJ_SID_VFLG - ? "1" - : "0"); - json_object_string_add( - flags_json, "flag-l", - lan->flags & EXT_SUBTLV_LINK_ADJ_SID_LFLG - ? "1" - : "0"); - json_object_string_add( - flags_json, "flag-s", - lan->flags & EXT_SUBTLV_LINK_ADJ_SID_SFLG - ? "1" - : "0"); - json_object_string_add( - flags_json, "flag-p", - lan->flags & EXT_SUBTLV_LINK_ADJ_SID_PFLG - ? "1" - : "0"); - json_object_array_add(arr_adj_json, flags_json); + json_object_string_add(lan_adj_json, "flag-f", + lan->flags & EXT_SUBTLV_LINK_ADJ_SID_FFLG + ? "1" + : "0"); + json_object_string_add(lan_adj_json, "flag-b", + lan->flags & EXT_SUBTLV_LINK_ADJ_SID_BFLG + ? "1" + : "0"); + json_object_string_add(lan_adj_json, "flag-v", + lan->flags & EXT_SUBTLV_LINK_ADJ_SID_VFLG + ? "1" + : "0"); + json_object_string_add(lan_adj_json, "flag-l", + lan->flags & EXT_SUBTLV_LINK_ADJ_SID_LFLG + ? "1" + : "0"); + json_object_string_add(lan_adj_json, "flag-s", + lan->flags & EXT_SUBTLV_LINK_ADJ_SID_SFLG + ? "1" + : "0"); + json_object_string_add(lan_adj_json, "flag-p", + lan->flags & EXT_SUBTLV_LINK_ADJ_SID_PFLG + ? "1" + : "0"); + json_object_array_add(arr_adj_json, + lan_adj_json); } /* end old deprecated key format */ @@ -1197,35 +1189,37 @@ static void format_item_ext_subtlvs(struct isis_ext_subtlvs *exts, continue; snprintfrr(cnt_buf, sizeof(cnt_buf), "%d", lan->sid); - flags_json = json_object_new_object(); - json_object_int_add(flags_json, "sid", lan->sid); - json_object_int_add(flags_json, "weight", + lan_adj_json = json_object_new_object(); + json_object_int_add(lan_adj_json, "sid", + lan->sid); + json_object_int_add(lan_adj_json, "weight", lan->weight); - json_object_boolean_add(flags_json, "flagF", + json_object_boolean_add(lan_adj_json, "flagF", lan->flags & EXT_SUBTLV_LINK_ADJ_SID_FFLG ? true : false); - json_object_boolean_add(flags_json, "flagB", + json_object_boolean_add(lan_adj_json, "flagB", lan->flags & EXT_SUBTLV_LINK_ADJ_SID_BFLG ? true : false); - json_object_boolean_add(flags_json, "flagV", + json_object_boolean_add(lan_adj_json, "flagV", lan->flags & EXT_SUBTLV_LINK_ADJ_SID_VFLG ? true : false); - json_object_boolean_add(flags_json, "flagL", + json_object_boolean_add(lan_adj_json, "flagL", lan->flags & EXT_SUBTLV_LINK_ADJ_SID_LFLG ? true : false); - json_object_boolean_add(flags_json, "flagS", + json_object_boolean_add(lan_adj_json, "flagS", lan->flags & EXT_SUBTLV_LINK_ADJ_SID_SFLG ? true : false); - json_object_boolean_add(flags_json, "flagP", + json_object_boolean_add(lan_adj_json, "flagP", lan->flags & EXT_SUBTLV_LINK_ADJ_SID_PFLG ? true : false); - json_object_array_add(arr_adj_json, flags_json); + json_object_array_add(arr_adj_json, + lan_adj_json); } } else @@ -1268,7 +1262,7 @@ static void format_item_ext_subtlvs(struct isis_ext_subtlvs *exts, struct isis_srv6_endx_sid_subtlv *adj; if (json) { - struct json_object *arr_adj_json, *flags_json; + struct json_object *arr_adj_json, *srv6_endx_sid_json; #if CONFDATE > 20240916 CPP_NOTICE("remove deprecated key format with -") @@ -1282,36 +1276,41 @@ static void format_item_ext_subtlvs(struct isis_ext_subtlvs *exts, adj; adj = adj->next) { snprintfrr(cnt_buf, sizeof(cnt_buf), "%pI6", &adj->sid); - flags_json = json_object_new_object(); - json_object_string_addf(flags_json, "sid", - "%pI6", &adj->sid); - json_object_string_add( - flags_json, "algorithm", - sr_algorithm_string(adj->algorithm)); - json_object_int_add(flags_json, "weight", - adj->weight); - json_object_string_add( - flags_json, "behavior", - seg6local_action2str(adj->behavior)); - json_object_string_add( - flags_json, "flag-b", - adj->flags & EXT_SUBTLV_LINK_SRV6_ENDX_SID_BFLG - ? "1" - : "0"); - json_object_string_add( - flags_json, "flag-s", - adj->flags & EXT_SUBTLV_LINK_SRV6_ENDX_SID_SFLG - ? "1" - : "0"); - json_object_string_add( - flags_json, "flag-p", - adj->flags & EXT_SUBTLV_LINK_SRV6_ENDX_SID_PFLG - ? "1" - : "0"); - json_object_array_add(arr_adj_json, flags_json); + srv6_endx_sid_json = json_object_new_object(); + json_object_string_addf(srv6_endx_sid_json, + "sid", "%pI6", + &adj->sid); + json_object_string_add(srv6_endx_sid_json, + "algorithm", + sr_algorithm_string( + adj->algorithm)); + json_object_int_add(srv6_endx_sid_json, + "weight", adj->weight); + json_object_string_add(srv6_endx_sid_json, + "behavior", + seg6local_action2str( + adj->behavior)); + json_object_string_add(srv6_endx_sid_json, + "flag-b", + adj->flags & EXT_SUBTLV_LINK_SRV6_ENDX_SID_BFLG + ? "1" + : "0"); + json_object_string_add(srv6_endx_sid_json, + "flag-s", + adj->flags & EXT_SUBTLV_LINK_SRV6_ENDX_SID_SFLG + ? "1" + : "0"); + json_object_string_add(srv6_endx_sid_json, + "flag-p", + adj->flags & EXT_SUBTLV_LINK_SRV6_ENDX_SID_PFLG + ? "1" + : "0"); + json_object_array_add(arr_adj_json, + srv6_endx_sid_json); if (adj->subsubtlvs) isis_format_subsubtlvs(adj->subsubtlvs, - NULL, flags_json, + NULL, + srv6_endx_sid_json, indent + 4); } /* end old deprecated key format */ @@ -1323,33 +1322,38 @@ static void format_item_ext_subtlvs(struct isis_ext_subtlvs *exts, adj; adj = adj->next) { snprintfrr(cnt_buf, sizeof(cnt_buf), "%pI6", &adj->sid); - flags_json = json_object_new_object(); - json_object_string_addf(flags_json, "sid", - "%pI6", &adj->sid); - json_object_string_add(flags_json, "algorithm", + srv6_endx_sid_json = json_object_new_object(); + json_object_string_addf(srv6_endx_sid_json, + "sid", "%pI6", + &adj->sid); + json_object_string_add(srv6_endx_sid_json, + "algorithm", sr_algorithm_string( adj->algorithm)); - json_object_int_add(flags_json, "weight", - adj->weight); - json_object_string_add(flags_json, "behavior", + json_object_int_add(srv6_endx_sid_json, + "weight", adj->weight); + json_object_string_add(srv6_endx_sid_json, + "behavior", seg6local_action2str( adj->behavior)); json_object_boolean_add( - flags_json, "flagB", + srv6_endx_sid_json, "flagB", !!(adj->flags & EXT_SUBTLV_LINK_SRV6_ENDX_SID_BFLG)); json_object_boolean_add( - flags_json, "flagS", + srv6_endx_sid_json, "flagS", !!(adj->flags & EXT_SUBTLV_LINK_SRV6_ENDX_SID_SFLG)); json_object_boolean_add( - flags_json, "flagP", + srv6_endx_sid_json, "flagP", !!(adj->flags & EXT_SUBTLV_LINK_SRV6_ENDX_SID_PFLG)); - json_object_array_add(arr_adj_json, flags_json); + json_object_array_add(arr_adj_json, + srv6_endx_sid_json); if (adj->subsubtlvs) isis_format_subsubtlvs(adj->subsubtlvs, - NULL, flags_json, + NULL, + srv6_endx_sid_json, indent + 4); } } else @@ -1382,7 +1386,8 @@ static void format_item_ext_subtlvs(struct isis_ext_subtlvs *exts, if (IS_SUBTLV(exts, EXT_SRV6_LAN_ENDX_SID)) { struct isis_srv6_lan_endx_sid_subtlv *lan; if (json) { - struct json_object *arr_adj_json, *flags_json; + struct json_object *arr_adj_json, + *srv6_lan_endx_sid_json; #if CONFDATE > 20240916 CPP_NOTICE("remove deprecated key format with -") @@ -1396,41 +1401,47 @@ static void format_item_ext_subtlvs(struct isis_ext_subtlvs *exts, lan; lan = lan->next) { snprintfrr(cnt_buf, sizeof(cnt_buf), "%pI6", &lan->sid); - flags_json = json_object_new_object(); - json_object_string_addf(flags_json, "sid", - "%pI6", &lan->sid); - json_object_int_add(flags_json, "weight", - lan->weight); - json_object_string_add( - flags_json, "algorithm", - sr_algorithm_string(lan->algorithm)); - json_object_int_add(flags_json, "weight", - lan->weight); - json_object_string_add( - flags_json, "behavior", - seg6local_action2str(lan->behavior)); - json_object_string_add( - flags_json, "flag-b", - lan->flags & EXT_SUBTLV_LINK_SRV6_ENDX_SID_BFLG - ? "1" - : "0"); - json_object_string_add( - flags_json, "flag-s", - lan->flags & EXT_SUBTLV_LINK_SRV6_ENDX_SID_SFLG - ? "1" - : "0"); - json_object_string_add( - flags_json, "flag-p", - lan->flags & EXT_SUBTLV_LINK_SRV6_ENDX_SID_PFLG - ? "1" - : "0"); - json_object_string_addf(flags_json, + srv6_lan_endx_sid_json = + json_object_new_object(); + json_object_string_addf(srv6_lan_endx_sid_json, + "sid", "%pI6", + &lan->sid); + json_object_int_add(srv6_lan_endx_sid_json, + "weight", lan->weight); + json_object_string_add(srv6_lan_endx_sid_json, + "algorithm", + sr_algorithm_string( + lan->algorithm)); + json_object_int_add(srv6_lan_endx_sid_json, + "weight", lan->weight); + json_object_string_add(srv6_lan_endx_sid_json, + "behavior", + seg6local_action2str( + lan->behavior)); + json_object_string_add(srv6_lan_endx_sid_json, + "flag-b", + lan->flags & EXT_SUBTLV_LINK_SRV6_ENDX_SID_BFLG + ? "1" + : "0"); + json_object_string_add(srv6_lan_endx_sid_json, + "flag-s", + lan->flags & EXT_SUBTLV_LINK_SRV6_ENDX_SID_SFLG + ? "1" + : "0"); + json_object_string_add(srv6_lan_endx_sid_json, + "flag-p", + lan->flags & EXT_SUBTLV_LINK_SRV6_ENDX_SID_PFLG + ? "1" + : "0"); + json_object_string_addf(srv6_lan_endx_sid_json, "neighbor-id", "%pSY", lan->neighbor_id); - json_object_array_add(arr_adj_json, flags_json); + json_object_array_add(arr_adj_json, + srv6_lan_endx_sid_json); if (lan->subsubtlvs) isis_format_subsubtlvs(lan->subsubtlvs, - NULL, flags_json, + NULL, + srv6_lan_endx_sid_json, indent + 4); } /* end old deprecated key format */ @@ -1443,38 +1454,44 @@ static void format_item_ext_subtlvs(struct isis_ext_subtlvs *exts, lan; lan = lan->next) { snprintfrr(cnt_buf, sizeof(cnt_buf), "%pI6", &lan->sid); - flags_json = json_object_new_object(); - json_object_string_addf(flags_json, "sid", - "%pI6", &lan->sid); - json_object_int_add(flags_json, "weight", - lan->weight); - json_object_string_add(flags_json, "algorithm", + srv6_lan_endx_sid_json = + json_object_new_object(); + json_object_string_addf(srv6_lan_endx_sid_json, + "sid", "%pI6", + &lan->sid); + json_object_int_add(srv6_lan_endx_sid_json, + "weight", lan->weight); + json_object_string_add(srv6_lan_endx_sid_json, + "algorithm", sr_algorithm_string( lan->algorithm)); - json_object_int_add(flags_json, "weight", - lan->weight); - json_object_string_add(flags_json, "behavior", + json_object_int_add(srv6_lan_endx_sid_json, + "weight", lan->weight); + json_object_string_add(srv6_lan_endx_sid_json, + "behavior", seg6local_action2str( lan->behavior)); json_object_boolean_add( - flags_json, "flagB", + srv6_lan_endx_sid_json, "flagB", !!(lan->flags & EXT_SUBTLV_LINK_SRV6_ENDX_SID_BFLG)); json_object_boolean_add( - flags_json, "flagS", + srv6_lan_endx_sid_json, "flagS", !!(lan->flags & EXT_SUBTLV_LINK_SRV6_ENDX_SID_SFLG)); json_object_boolean_add( - flags_json, "flagP", + srv6_lan_endx_sid_json, "flagP", !!(lan->flags & EXT_SUBTLV_LINK_SRV6_ENDX_SID_PFLG)); - json_object_string_addf(flags_json, + json_object_string_addf(srv6_lan_endx_sid_json, "neighbor-id", "%pSY", lan->neighbor_id); - json_object_array_add(arr_adj_json, flags_json); + json_object_array_add(arr_adj_json, + srv6_lan_endx_sid_json); if (lan->subsubtlvs) isis_format_subsubtlvs(lan->subsubtlvs, - NULL, flags_json, + NULL, + srv6_lan_endx_sid_json, indent + 4); } } else From 3e82cf29286407eb1b088c33a5deb4b673574a74 Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Fri, 21 Jun 2024 11:18:59 +0200 Subject: [PATCH 26/28] isisd: fix neighbor id json key d5879267aa ("isisd: fix show database json format") renamed JSON keys to a standard format but forgot to rename the neighbor-id key. Fixes: d5879267aa ("isisd: fix show database json format") Signed-off-by: Louis Scalbert --- isisd/isis_tlvs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/isisd/isis_tlvs.c b/isisd/isis_tlvs.c index 4ea384ef23bf..8405ad089c6d 100644 --- a/isisd/isis_tlvs.c +++ b/isisd/isis_tlvs.c @@ -1484,7 +1484,7 @@ static void format_item_ext_subtlvs(struct isis_ext_subtlvs *exts, !!(lan->flags & EXT_SUBTLV_LINK_SRV6_ENDX_SID_PFLG)); json_object_string_addf(srv6_lan_endx_sid_json, - "neighbor-id", "%pSY", + "neighborID", "%pSY", lan->neighbor_id); json_object_array_add(arr_adj_json, srv6_lan_endx_sid_json); From cd2e872e65c0512b2d57a92517622efe21cf306e Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Mon, 24 Jun 2024 13:38:37 +0200 Subject: [PATCH 27/28] isisd: fix srv6 endx sid key name srv6EndSID is actually srv6EndXSID. Fixes: d5879267aa ("isisd: fix show database json format") Signed-off-by: Louis Scalbert --- isisd/isis_tlvs.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/isisd/isis_tlvs.c b/isisd/isis_tlvs.c index 8405ad089c6d..c7f45b246948 100644 --- a/isisd/isis_tlvs.c +++ b/isisd/isis_tlvs.c @@ -1316,7 +1316,8 @@ static void format_item_ext_subtlvs(struct isis_ext_subtlvs *exts, /* end old deprecated key format */ arr_adj_json = json_object_new_array(); - json_object_object_add(json, "srv6EndSID", arr_adj_json); + json_object_object_add(json, "srv6EndXSID", + arr_adj_json); for (adj = (struct isis_srv6_endx_sid_subtlv *) exts->srv6_endx_sid.head; adj; adj = adj->next) { From 3b98ddf5018cf7526b50c15018cbaf71a38fa752 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Mon, 24 Jun 2024 20:16:16 +0300 Subject: [PATCH 28/28] bgpd: Relax OAD (One-Administration-Domain) for RFC8212 RFC 8212 defines leak prevention for eBGP peers, but BGP-OAD defines a new peering type One Administrative Domain (OAD), where multiple ASNs could be used inside a single administrative domain. OAD allows sending non-transitive attributes, so this prevention should be relaxed too. Signed-off-by: Donatas Abraitis --- bgpd/bgp_route.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 4dcb22234ac5..9c3602311f77 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -6333,7 +6333,7 @@ void bgp_set_stale_route(struct peer *peer, afi_t afi, safi_t safi) bool bgp_outbound_policy_exists(struct peer *peer, struct bgp_filter *filter) { - if (peer->sort == BGP_PEER_IBGP) + if (peer->sort == BGP_PEER_IBGP || peer->sub_sort == BGP_PEER_EBGP_OAD) return true; if (peer->sort == BGP_PEER_EBGP && @@ -6346,7 +6346,7 @@ bool bgp_outbound_policy_exists(struct peer *peer, struct bgp_filter *filter) bool bgp_inbound_policy_exists(struct peer *peer, struct bgp_filter *filter) { - if (peer->sort == BGP_PEER_IBGP) + if (peer->sort == BGP_PEER_IBGP || peer->sub_sort == BGP_PEER_EBGP_OAD) return true; if (peer->sort == BGP_PEER_EBGP