[cmaster-next] [PATCH] bgpd: Fix crashes when no default bgp instance is configured.

[Martin Winter]

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
>>