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

[Lou Berger]

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