bgpd, tests: bgp_evpn_rt5, add test with match evpn vni command #17652

pguibert6WIND · 2024-12-16T15:03:45Z

Add a test that ensures that the 'match evpn vni' command works with bgp evpn rt5 updates.

ton31337 · 2024-12-17T07:23:08Z

tests/topotests/bgp_evpn_rt5/r3/zebra.conf

@@ -0,0 +1,23 @@
+log stdout


Not needed.

ton31337 · 2024-12-17T07:23:15Z

tests/topotests/bgp_evpn_rt5/r3/zebra.conf

@@ -0,0 +1,23 @@
+log stdout
+
+hostname r3


Not needed.

ton31337 · 2024-12-17T07:23:18Z

tests/topotests/bgp_evpn_rt5/r3/zebra.conf

+log stdout
+
+hostname r3
+password zebra


Not needed.

ton31337 · 2024-12-17T07:23:24Z

tests/topotests/bgp_evpn_rt5/r3/zebra.conf

+! debug zebra kernel
+! debug zebra dplane
+! debug zebra rib
+log stdout


Not needed.

Jafaral

Please use unified config, frr.conf

pguibert6WIND · 2024-12-18T12:32:56Z

Please use unified config, frr.conf

This is not as simple, but let s try. With unified config, test does not pass. Without unified config, test passes.
What is observed is that ipv4 routes from r1-vrf-101 are not exported as l2vpn evpn entries, when it does not work.

ton31337 · 2025-01-02T11:15:25Z

Can we fix frrbot issues?

pguibert6WIND · 2025-01-02T15:21:21Z

Can we fix frrbot issues?

it should be ok now

pguibert6WIND · 2025-01-03T07:29:30Z

ci:rerun unrelated

ton31337 · 2025-01-03T07:42:40Z

tests/topotests/bgp_evpn_rt5/r1/frr.conf

@@ -1,15 +1,42 @@
+log stdout


Please drop it...

It's still here...

it seems I lost the modif . done again.

ton31337 · 2025-01-03T07:42:44Z

tests/topotests/bgp_evpn_rt5/r1/frr.conf

 ! debug bgp neighbor-events
 ! debug bgp updates
 ! debug bgp zebra
+log stdout


Please drop it...

Still here.

it seems I lost the modif . done again.

ton31337 · 2025-01-03T07:43:01Z

tests/topotests/bgp_evpn_rt5/r2/frr.conf

@@ -1,6 +1,24 @@
+log stdout


Please drop it...

Still here.

it seems I lost the modif . done again.

ton31337 · 2025-01-03T07:43:05Z

tests/topotests/bgp_evpn_rt5/r2/frr.conf

+log stdout
+
+hostname r2
+password zebra


Please drop it...

Still here.

it seems I lost the modif . done again.

pguibert6WIND · 2025-01-06T07:23:15Z

ci:rerun unrelated

pguibert6WIND · 2025-01-07T13:52:30Z

ci:rerun ospf metric still not working and it is unrelated. someone should really look at ospf metric.

pguibert6WIND · 2025-01-08T08:15:15Z

Please use unified config, frr.conf

@jafar, it is done now

pguibert6WIND · 2025-01-08T08:16:36Z

ci:rerun still unrelated ospf metric

Jafaral · 2025-01-08T20:27:05Z

@pguibert6WIND , the test test_ospf_metric_propagation test_all_links_up has rarely failed in recent months. The fact it is not failing in any other PR but it is failing again and again here means it might be related. This test uses BGP for route leaking. We expect to see a route that is supposed to be propagated through different nodes, crossing multiple vrfs using BGP, that route isn't there anymore.

pguibert6WIND · 2025-01-09T09:44:18Z

@pguibert6WIND , the test test_ospf_metric_propagation test_all_links_up has rarely failed in recent months. The fact it is not failing in any other PR but it is failing again and again here means it might be related. This test uses BGP for route leaking. We expect to see a route that is supposed to be propagated through different nodes, crossing multiple vrfs using BGP, that route isn't there anymore.

OMG, I really was thinking it was unrelated. thanks to this, I found out an issue related to the import command. Now it should be ok.
thanks.

Jafaral

My comment has been addressed Thank you.

ton31337 · 2025-01-10T07:41:48Z

tests/topotests/bgp_evpn_rt5/r1/frr.conf

+log stdout
+
+hostname r1
+password zebra


Let's drop this.

it seems I lost the modif . done again.

Add a test that ensures that the 'match evpn vni' command works with bgp evpn rt5 updates. Signed-off-by: Philippe Guibert <[email protected]>

When not configuring a route distinguisher, neither route-target, the derived rd settings differ if config load applies with frr.conf or not. For instance, the forged rd with frr.conf: > # show bgp l2vpn evpn json > "192.168.101.41:3":{ > "rd":"192.168.101.41:3", and without: > "192.168.101.41:2":{ > "rd":"192.168.101.41:2", The defined rts also are impacted. Temporay fix this by using an hardset configuration for all route distinguisher and route target of the setups. Signed-off-by: Philippe Guibert <[email protected]>

Replace the various per-daemon config files with a unique frr.conf configuration file. Signed-off-by: Philippe Guibert <[email protected]>

Some bgp evpn memory contexts are not freed at the end of the bgp process. > ================================================================= > ==1208677==ERROR: LeakSanitizer: detected memory leaks > > Direct leak of 96 byte(s) in 2 object(s) allocated from: > #0 0x7f93ad4b4a57 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154 > FRRouting#1 0x7f93ace77233 in qcalloc lib/memory.c:106 > FRRouting#2 0x563bb68f4df1 in process_type5_route bgpd/bgp_evpn.c:5084 > FRRouting#3 0x563bb68fb663 in bgp_nlri_parse_evpn bgpd/bgp_evpn.c:6302 > FRRouting#4 0x563bb69ea2a9 in bgp_nlri_parse bgpd/bgp_packet.c:347 > FRRouting#5 0x563bb69f7716 in bgp_update_receive bgpd/bgp_packet.c:2482 > FRRouting#6 0x563bb6a04d3b in bgp_process_packet bgpd/bgp_packet.c:4091 > FRRouting#7 0x7f93acf8082d in event_call lib/event.c:1996 > FRRouting#8 0x7f93ace48931 in frr_run lib/libfrr.c:1232 > FRRouting#9 0x563bb6880ae1 in main bgpd/bgp_main.c:557 > FRRouting#10 0x7f93ac829d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 Actually, the bgp evpn context may noy be used if adj rib in is unused. This may lead to memory leaks. Fix this by freeing the context in those corner cases. Signed-off-by: Philippe Guibert <[email protected]>

When running the bgp_evpn_rt5 setup with unified config, memory leak about a non deleted BGP instance happens. > root@ubuntu2204hwe:~/frr/tests/topotests/bgp_evpn_rt5# cat /tmp/topotests/bgp_evpn_rt5.test_bgp_evpn/r1.asan.bgpd.1164105 > > ================================================================= > ==1164105==ERROR: LeakSanitizer: detected memory leaks > > Indirect leak of 12496 byte(s) in 1 object(s) allocated from: > #0 0x7f358eeb4a57 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154 > FRRouting#1 0x7f358e877233 in qcalloc lib/memory.c:106 > FRRouting#2 0x55d06c95680a in bgp_create bgpd/bgpd.c:3405 > FRRouting#3 0x55d06c95a7b3 in bgp_get bgpd/bgpd.c:3805 > FRRouting#4 0x55d06c87a9b5 in bgp_get_vty bgpd/bgp_vty.c:603 > FRRouting#5 0x55d06c68dc71 in bgp_evpn_local_l3vni_add bgpd/bgp_evpn.c:7032 > FRRouting#6 0x55d06c92989b in bgp_zebra_process_local_l3vni bgpd/bgp_zebra.c:3204 > FRRouting#7 0x7f358e9e3feb in zclient_read lib/zclient.c:4626 > FRRouting#8 0x7f358e98082d in event_call lib/event.c:1996 > FRRouting#9 0x7f358e848931 in frr_run lib/libfrr.c:1232 > FRRouting#10 0x55d06c60eae1 in main bgpd/bgp_main.c:557 > FRRouting#11 0x7f358e229d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 Actually, a BGP VRF Instance is created in auto mode when creating the global BGP instance for the L3 VNI. And again, an other BGP VRF instance is created. Fix this by ensuring that a non existing BGP instance is not present. If it is present, and with auto mode or in hidden mode, then override the AS value. Fixes: f153b9a ("bgpd: Ignore auto created VRF BGP instances") Signed-off-by: Philippe Guibert <[email protected]>

The more the vrf green is referenced in the import bgp command, the more there are instances created. The below configuration shows that the vrf green is referenced twice, and two BGP instances of vrf green are created. The below configuration: > router bgp 99 > [..] > import vrf green > exit > router bgp 99 vrf blue > [..] > import vrf green > exit > router bgp 99 vrf green > [..] > exit > > r4# show bgp vrfs > Type Id routerId #PeersCfg #PeersEstb Name > L3-VNI RouterMAC Interface > DFLT 0 10.0.3.4 0 0 default > 0 00:00:00:00:00:00 unknown > VRF 5 10.0.40.4 0 0 blue > 0 00:00:00:00:00:00 unknown > VRF 6 0.0.0.0 0 0 green > 0 00:00:00:00:00:00 unknown > VRF 6 10.0.94.4 0 0 green > 0 00:00:00:00:00:00 unknown Fix this at import command, by looking at an already present bgp instance. Signed-off-by: Philippe Guibert <[email protected]>

Some static analyzer issues can be observed in BGP code: > In file included from ./lib/zebra.h:13, > from lib/event.c:8: > ./lib/compiler.h:222:26: note: '#pragma message: Remove `clear thread cpu` command' > 222 | #define CPP_NOTICE(text) _Pragma(CPP_STR(message text)) > | ^~~~~~~ > lib/event.c:433:1: note: in expansion of macro 'CPP_NOTICE' > 433 | CPP_NOTICE("Remove `clear thread cpu` command") > | ^~~~~~~~~~ > bgpd/bgp_vty.c:1592:5: warning: Access to field 'as_pretty' results in a dereference of a null pointer (loaded from variable 'bgp') [core.NullDereference] > 1592 | bgp->as_pretty); > | ^~~~~~~~~~~~~~ > bgpd/bgp_vty.c:1599:5: warning: Access to field 'as_pretty' results in a dereference of a null pointer (loaded from variable 'bgp') [core.NullDereference] > 1599 | bgp->as_pretty); > | ^~~~~~~~~~~~~~ > bgpd/bgp_vty.c:1612:7: warning: Access to field 'flags' results in a dereference of a null pointer (loaded from variable 'bgp') [core.NullDereference] > 1612 | IS_BGP_INSTANCE_HIDDEN(bgp)) { > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~ > ./bgpd/bgpd.h:2906:3: note: expanded from macro 'IS_BGP_INSTANCE_HIDDEN' > 2906 | (CHECK_FLAG(_bgp->flags, BGP_FLAG_INSTANCE_HIDDEN) && \ > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > ./lib/zebra.h:274:31: note: expanded from macro 'CHECK_FLAG' > 274 | #define CHECK_FLAG(V,F) ((V) & (F)) > | ^~~ > bgpd/bgp_vty.c:1614:4: warning: Access to field 'flags' results in a dereference of a null pointer (loaded from variable 'bgp') [core.NullDereference] > 1614 | UNSET_FLAG(bgp->flags, BGP_FLAG_INSTANCE_HIDDEN); > | ^ ~~~ > ./lib/zebra.h:276:34: note: expanded from macro 'UNSET_FLAG' > 276 | #define UNSET_FLAG(V,F) (V) &= ~(F) > | ~ ^ > 4 warnings generated. > Static Analysis warning summary compared to base: Fix those issues by protecting the bgp pointer. Signed-off-by: Philippe Guibert <[email protected]>

github-actions bot added master size/L labels Dec 16, 2024

ton31337 reviewed Dec 17, 2024

View reviewed changes

pguibert6WIND force-pushed the topotest_bgp_evpn_rt5 branch from e9a6c39 to 7b0fd42 Compare December 17, 2024 08:03

Jafaral requested changes Dec 17, 2024

View reviewed changes

pguibert6WIND force-pushed the topotest_bgp_evpn_rt5 branch 3 times, most recently from 395eab3 to 5176098 Compare December 20, 2024 14:07

pguibert6WIND force-pushed the topotest_bgp_evpn_rt5 branch from 5176098 to efbcf87 Compare December 30, 2024 15:24

frrbot bot added bgp bugfix labels Dec 31, 2024

pguibert6WIND force-pushed the topotest_bgp_evpn_rt5 branch from 9f7b3a2 to 3ad2665 Compare January 2, 2025 10:03

pguibert6WIND force-pushed the topotest_bgp_evpn_rt5 branch from 3ad2665 to a9b554f Compare January 2, 2025 15:20

ton31337 reviewed Jan 3, 2025

View reviewed changes

pguibert6WIND force-pushed the topotest_bgp_evpn_rt5 branch from a9b554f to 6ef780c Compare January 3, 2025 15:58

pguibert6WIND force-pushed the topotest_bgp_evpn_rt5 branch from 6ef780c to 8ef1ca1 Compare January 7, 2025 08:31

Jafaral changed the title ~~topotests: bgp_evpn_rt5, add test with match evpn vni command~~ bgpd, tests: bgp_evpn_rt5, add test with match evpn vni command Jan 7, 2025

pguibert6WIND force-pushed the topotest_bgp_evpn_rt5 branch from 8ef1ca1 to ebb2223 Compare January 8, 2025 14:53

pguibert6WIND force-pushed the topotest_bgp_evpn_rt5 branch from ebb2223 to a912478 Compare January 9, 2025 09:43

Jafaral reviewed Jan 10, 2025

View reviewed changes

Jafaral approved these changes Jan 10, 2025

View reviewed changes

ton31337 reviewed Jan 10, 2025

View reviewed changes

pguibert6WIND added 7 commits January 10, 2025 10:01

topotests: bgp_evpn_rt5, add test with match evpn vni command

3ab3e01

Add a test that ensures that the 'match evpn vni' command works with bgp evpn rt5 updates. Signed-off-by: Philippe Guibert <[email protected]>

topotests: add unified configuration

5695104

Replace the various per-daemon config files with a unique frr.conf configuration file. Signed-off-by: Philippe Guibert <[email protected]>

pguibert6WIND force-pushed the topotest_bgp_evpn_rt5 branch from 793dd15 to 915d1cb Compare January 10, 2025 09:07

bgpd, tests: bgp_evpn_rt5, add test with match evpn vni command #17652

Are you sure you want to change the base?

bgpd, tests: bgp_evpn_rt5, add test with match evpn vni command #17652

Conversation

pguibert6WIND commented Dec 16, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jafaral left a comment

Choose a reason for hiding this comment

pguibert6WIND commented Dec 18, 2024 • edited Loading

ton31337 commented Jan 2, 2025

pguibert6WIND commented Jan 2, 2025

pguibert6WIND commented Jan 3, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pguibert6WIND commented Jan 6, 2025

pguibert6WIND commented Jan 7, 2025

pguibert6WIND commented Jan 8, 2025

pguibert6WIND commented Jan 8, 2025

Jafaral commented Jan 8, 2025

pguibert6WIND commented Jan 9, 2025

Jafaral left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pguibert6WIND commented Dec 18, 2024 •

edited

Loading