[cmaster-next] [PATCH] bgpd: Fix crashes when no default bgp instance is configured.
Martin Winter
mwinter at opensourcerouting.org
Thu Dec 1 18:02:09 EST 2016
On 1 Dec 2016, at 12:17, Lou Berger wrote:
> Sigh -- been one of those days - Haven't had time to try to get Martin's
> tests running -
>
> Martin,
> What extra lines are you referring to?
See https://ci1.netdef.org/browse/QUAGGA-CMASTERNEXT66-TOPOU1604-8
On a “show ip bgp” we get the extra line “type=bgp, subtype=0”, i.e.:
10.101.3.0/24 172.16.1.1 100 0 65001 i
type=bgp, subtype=0
For me they look like forgotten debug info. (type “bgp” is something I
assume on “show ip bgp” and subtype=0 means nothing to me)
- Martin
> On 12/1/2016 3:08 PM, Martin Winter wrote:
>> On 1 Dec 2016, at 6:15, Donald Sharp wrote:
>>> Lou -
>>>
>>> This fixes the crashes that are found by the new topotest testing
>>> suite that Martin has created. As stated in the commit note, I have
>>> done no work to ensure that vnc is going to work correctly without a
>>> default bgp instance. We just no longer crash with these tests.
>>> Additionally I see lots of unguarded debugs in the vnc code. I
>>> believe these need to be cleaned up as well in some manner, thoughts?
>>>
>>> Martin -
>>>
>>> The bgp tests still fail, from what appears to be display output
>>> change. How would you like to proceed here? Send a patch to fix this
>>> issue to the topotests github doohickey?
>> I brought up this question before. I can fix the test to ignore these
>> extra lines, but I’m not sure this is the right way to do it.
>>
>> Maybe Lou (or someone else) can explain the need/use of this extra
>> information and if this is intended or something forgotten from
>> development
>> and should be removed.
>> (I worry about many other users doing screen scraping on this output)
>>
>> - Martin
>>
>>
>>> On Thu, Dec 1, 2016 at 9:11 AM, Donald Sharp
>>> <sharpd at cumulusnetworks.com> wrote:
>>>> The vnc code assumes that bgp must have a default instance.
>>>> This code change checks to make sure that we do before
>>>> proceeding. It makes no assurances that vnc will behave
>>>> correctly without a default instance.
>>>>
>>>> Signed-off-by: Donald Sharp <sharpd at cumulusnetworks.com>
>>>> ---
>>>> bgpd/rfapi/rfapi_import.c | 3 ++-
>>>> bgpd/rfapi/vnc_import_bgp.c | 17 +++++++++++++++--
>>>> 2 files changed, 17 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/bgpd/rfapi/rfapi_import.c b/bgpd/rfapi/rfapi_import.c
>>>> index 8783024..77da4f9 100644
>>>> --- a/bgpd/rfapi/rfapi_import.c
>>>> +++ b/bgpd/rfapi/rfapi_import.c
>>>> @@ -4414,7 +4414,8 @@ rfapiProcessPeerDown (struct peer *peer)
>>>> */
>>>>
>>>> bgp = bgp_get_default (); /* assume 1 instance for now */
>>>> - assert (bgp);
>>>> + if (!bgp)
>>>> + return;
>>>>
>>>> h = bgp->rfapi;
>>>> assert (h);
>>>> diff --git a/bgpd/rfapi/vnc_import_bgp.c
>>>> b/bgpd/rfapi/vnc_import_bgp.c
>>>> index 4215ce2..dc2640a 100644
>>>> --- a/bgpd/rfapi/vnc_import_bgp.c
>>>> +++ b/bgpd/rfapi/vnc_import_bgp.c
>>>> @@ -208,12 +208,17 @@ prefix_bag_free (void *pb)
>>>> static void
>>>> print_rhn_list (const char *tag1, const char *tag2)
>>>> {
>>>> - struct bgp *bgp = bgp_get_default ();
>>>> - struct skiplist *sl = bgp->rfapi->resolve_nve_nexthop;
>>>> + struct bgp *bgp;
>>>> + struct skiplist *sl;
>>>> struct skiplistnode *p;
>>>> struct prefix_bag *pb;
>>>> int count = 0;
>>>>
>>>> + bgp = bgp_get_default ();
>>>> + if (!bgp)
>>>> + return;
>>>> +
>>>> + sl = bgp->frapi->resolve_nve_nexthop;
>>>> if (!sl)
>>>> {
>>>> zlog_debug ("%s: %s: RHN List is empty", (tag1 ? tag1 : ""),
>>>> @@ -251,6 +256,8 @@ vnc_rhnck (char *tag)
>>>> struct skiplistnode *p;
>>>>
>>>> bgp = bgp_get_default ();
>>>> + if (!bgp)
>>>> + return;
>>>> sl = bgp->rfapi->resolve_nve_nexthop;
>>>>
>>>> if (!sl)
>>>> @@ -1798,6 +1805,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 +2002,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.7.4
>>>>
More information about the dev
mailing list