[cmaster-next] [PATCH] bgpd: Fix multiple bgp view vnc crash
When configuring multiple bgp views without a default bgp session, the code will crash. This code change only fixes the crash from happening. I'm not entirely sure that the code will not misbehave with multiple bgp views and configuring vnc in this case. Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com> --- bgpd/rfapi/vnc_import_bgp.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/bgpd/rfapi/vnc_import_bgp.c b/bgpd/rfapi/vnc_import_bgp.c index 4215ce2..441bdee 100644 --- a/bgpd/rfapi/vnc_import_bgp.c +++ b/bgpd/rfapi/vnc_import_bgp.c @@ -1798,6 +1798,9 @@ vnc_import_bgp_exterior_add_route_it ( struct bgp *bgp_default = bgp_get_default (); afi_t afi = family2afi (prefix->family); + if (!bgp_default) + return; + h = bgp_default->rfapi; hc = bgp_default->rfapi_cfg; @@ -1992,6 +1995,9 @@ vnc_import_bgp_exterior_del_route ( afi_t afi = family2afi (prefix->family); struct bgp *bgp_default = bgp_get_default (); + if (!bgp_default) + return; + memset (&pfx_orig_nexthop, 0, sizeof (struct prefix)); /* keep valgrind happy */ h = bgp_default->rfapi; -- 2.5.5
Thanks! On 11/30/2016 9:27 AM, Donald Sharp wrote:
When configuring multiple bgp views without a default bgp session, the code will crash.
This code change only fixes the crash from happening. I'm not entirely sure that the code will not misbehave with multiple bgp views and configuring vnc in this case.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com> --- bgpd/rfapi/vnc_import_bgp.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/bgpd/rfapi/vnc_import_bgp.c b/bgpd/rfapi/vnc_import_bgp.c index 4215ce2..441bdee 100644 --- a/bgpd/rfapi/vnc_import_bgp.c +++ b/bgpd/rfapi/vnc_import_bgp.c @@ -1798,6 +1798,9 @@ vnc_import_bgp_exterior_add_route_it ( struct bgp *bgp_default = bgp_get_default (); afi_t afi = family2afi (prefix->family);
+ if (!bgp_default) + return; + h = bgp_default->rfapi; hc = bgp_default->rfapi_cfg;
@@ -1992,6 +1995,9 @@ vnc_import_bgp_exterior_del_route ( afi_t afi = family2afi (prefix->family); struct bgp *bgp_default = bgp_get_default ();
+ if (!bgp_default) + return; + memset (&pfx_orig_nexthop, 0, sizeof (struct prefix)); /* keep valgrind happy */
h = bgp_default->rfapi;
Lou - A quick glance at the vnc code gives me this: rfapi_import.c: bgp = bgp_get_default (); /* assume 1 instance for now */ rfapi_import.c: bgp = bgp_get_default (); /* assume 1 instance for now */ With this code change am I just setting up the tests to fail at a different time? Does vnc need to be fixed to allow it to be run without a default bgp instance running? donald On Wed, Nov 30, 2016 at 9:58 AM, Lou Berger <lberger@labn.net> wrote:
Thanks!
On 11/30/2016 9:27 AM, Donald Sharp wrote:
When configuring multiple bgp views without a default bgp session, the code will crash.
This code change only fixes the crash from happening. I'm not entirely sure that the code will not misbehave with multiple bgp views and configuring vnc in this case.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com> --- bgpd/rfapi/vnc_import_bgp.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/bgpd/rfapi/vnc_import_bgp.c b/bgpd/rfapi/vnc_import_bgp.c index 4215ce2..441bdee 100644 --- a/bgpd/rfapi/vnc_import_bgp.c +++ b/bgpd/rfapi/vnc_import_bgp.c @@ -1798,6 +1798,9 @@ vnc_import_bgp_exterior_add_route_it ( struct bgp *bgp_default = bgp_get_default (); afi_t afi = family2afi (prefix->family);
+ if (!bgp_default) + return; + h = bgp_default->rfapi; hc = bgp_default->rfapi_cfg;
@@ -1992,6 +1995,9 @@ vnc_import_bgp_exterior_del_route ( afi_t afi = family2afi (prefix->family); struct bgp *bgp_default = bgp_get_default ();
+ if (!bgp_default) + return; + memset (&pfx_orig_nexthop, 0, sizeof (struct prefix)); /* keep valgrind happy */
h = bgp_default->rfapi;
Bad luck, this doesn’t fix it. New traceback: [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1". Core was generated by `/usr/lib/quagga/bgpd -d'. Program terminated with signal SIGABRT, Aborted. #0 0x00007fef8cecb428 in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:54 #0 0x00007fef8cecb428 in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:54 #1 0x00007fef8cecd02a in __GI_abort () at abort.c:89 #2 0x00007fef8d7b52be in core_handler (signo=<optimized out>, siginfo=<optimized out>, context=<optimized out>) at sigevent.c:242 #3 <signal handler called> #4 vnc_rhnck (tag=tag@entry=0x7fffad18a1a8 "vnc_import_bgp_add_route: enter") at rfapi/vnc_import_bgp.c:254 #5 0x00000000004b02e5 in vnc_import_bgp_add_route (bgp=bgp@entry=0x229dcd0, prefix=prefix@entry=0x22ba000, info=info@entry=0x22ba670) at rfapi/vnc_import_bgp.c:2770 #6 0x000000000044a25b in bgp_process_main (wq=<optimized out>, data=<optimized out>) at bgp_route.c:1974 #7 0x00007fef8d7b5dad in work_queue_run (thread=0x7fffad18c4f8) at workqueue.c:308 #8 0x00007fef8d79e70e in thread_call (thread=thread@entry=0x7fffad18c4f8) at thread.c:1442 #9 0x000000000042c586 in main (argc=2, argv=0x7fffad18c698) at bgp_main.c:567 Here is the relevant code (with the patch) 244: #ifdef ENABLE_VNC_RHNCK 245: static void 246: vnc_rhnck (char *tag) 247: { 248: struct bgp *bgp; 249: struct skiplist *sl; 250: struct skiplistnode *p; 251: 252: bgp = bgp_get_default (); 253: 254: sl = bgp->rfapi->resolve_nve_nexthop; 255: 256: if (!sl) 257: return; The issue is in line 254 - before the fix. In this testcase we enter the function with bgp=0. - Martin On 30 Nov 2016, at 6:27, Donald Sharp wrote:
When configuring multiple bgp views without a default bgp session, the code will crash.
This code change only fixes the crash from happening. I'm not entirely sure that the code will not misbehave with multiple bgp views and configuring vnc in this case.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com> --- bgpd/rfapi/vnc_import_bgp.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/bgpd/rfapi/vnc_import_bgp.c b/bgpd/rfapi/vnc_import_bgp.c index 4215ce2..441bdee 100644 --- a/bgpd/rfapi/vnc_import_bgp.c +++ b/bgpd/rfapi/vnc_import_bgp.c @@ -1798,6 +1798,9 @@ vnc_import_bgp_exterior_add_route_it ( struct bgp *bgp_default = bgp_get_default (); afi_t afi = family2afi (prefix->family);
+ if (!bgp_default) + return; + h = bgp_default->rfapi; hc = bgp_default->rfapi_cfg;
@@ -1992,6 +1995,9 @@ vnc_import_bgp_exterior_del_route ( afi_t afi = family2afi (prefix->family); struct bgp *bgp_default = bgp_get_default ();
+ if (!bgp_default) + return; + memset (&pfx_orig_nexthop, 0, sizeof (struct prefix)); /* keep valgrind happy */
h = bgp_default->rfapi; -- 2.5.5
_______________________________________________ cmaster-next mailing list cmaster-next@lists.nox.tf https://lists.nox.tf/listinfo/cmaster-next
I'll see if I can duplicate your setup today.... Lou On November 30, 2016 10:33:18 PM "Martin Winter" <mwinter@opensourcerouting.org> wrote:
Bad luck, this doesn’t fix it.
New traceback:
[Thread debugging using libthread_db enabled] Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1". Core was generated by `/usr/lib/quagga/bgpd -d'. Program terminated with signal SIGABRT, Aborted. #0 0x00007fef8cecb428 in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:54 #0 0x00007fef8cecb428 in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:54 #1 0x00007fef8cecd02a in __GI_abort () at abort.c:89 #2 0x00007fef8d7b52be in core_handler (signo=<optimized out>, siginfo=<optimized out>, context=<optimized out>) at sigevent.c:242 #3 <signal handler called> #4 vnc_rhnck (tag=tag@entry=0x7fffad18a1a8 "vnc_import_bgp_add_route: enter") at rfapi/vnc_import_bgp.c:254 #5 0x00000000004b02e5 in vnc_import_bgp_add_route (bgp=bgp@entry=0x229dcd0, prefix=prefix@entry=0x22ba000, info=info@entry=0x22ba670) at rfapi/vnc_import_bgp.c:2770 #6 0x000000000044a25b in bgp_process_main (wq=<optimized out>, data=<optimized out>) at bgp_route.c:1974 #7 0x00007fef8d7b5dad in work_queue_run (thread=0x7fffad18c4f8) at workqueue.c:308 #8 0x00007fef8d79e70e in thread_call (thread=thread@entry=0x7fffad18c4f8) at thread.c:1442 #9 0x000000000042c586 in main (argc=2, argv=0x7fffad18c698) at bgp_main.c:567
Here is the relevant code (with the patch)
244: #ifdef ENABLE_VNC_RHNCK 245: static void 246: vnc_rhnck (char *tag) 247: { 248: struct bgp *bgp; 249: struct skiplist *sl; 250: struct skiplistnode *p; 251: 252: bgp = bgp_get_default (); 253: 254: sl = bgp->rfapi->resolve_nve_nexthop; 255: 256: if (!sl) 257: return;
The issue is in line 254 - before the fix. In this testcase we enter the function with bgp=0.
- Martin
On 30 Nov 2016, at 6:27, Donald Sharp wrote:
When configuring multiple bgp views without a default bgp session, the code will crash.
This code change only fixes the crash from happening. I'm not entirely sure that the code will not misbehave with multiple bgp views and configuring vnc in this case.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com> --- bgpd/rfapi/vnc_import_bgp.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/bgpd/rfapi/vnc_import_bgp.c b/bgpd/rfapi/vnc_import_bgp.c index 4215ce2..441bdee 100644 --- a/bgpd/rfapi/vnc_import_bgp.c +++ b/bgpd/rfapi/vnc_import_bgp.c @@ -1798,6 +1798,9 @@ vnc_import_bgp_exterior_add_route_it ( struct bgp *bgp_default = bgp_get_default (); afi_t afi = family2afi (prefix->family);
+ if (!bgp_default) + return; + h = bgp_default->rfapi; hc = bgp_default->rfapi_cfg;
@@ -1992,6 +1995,9 @@ vnc_import_bgp_exterior_del_route ( afi_t afi = family2afi (prefix->family); struct bgp *bgp_default = bgp_get_default ();
+ if (!bgp_default) + return; + memset (&pfx_orig_nexthop, 0, sizeof (struct prefix)); /* keep valgrind happy */
h = bgp_default->rfapi; -- 2.5.5
_______________________________________________ cmaster-next mailing list cmaster-next@lists.nox.tf https://lists.nox.tf/listinfo/cmaster-next
participants (3)
-
Donald Sharp -
Lou Berger -
Martin Winter