There is a rather annoying memory leak when a peer goes down.
In peer_down() most data is correctly removed but the Adj-RIB-Out was not
properly flushed and so those entries lingered around.
Also 'bgpctl show rib out nei $FOO' will blow up the rde because of a
use-after-free access of the peer id.

This flushes the Adj-RIB-Out explicitly, then cleans the pending UPDATES
and finally cleans the Adj-RIB-In (causing new route decisions) after that
all routes of that peer have gone and all is fine.

This order of doing things works because the peer is set to state DOWN
early on and so no more updates are generated for this session.

This works well in my case.
-- 
:wq Claudio

Index: rde.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
retrieving revision 1.450
diff -u -p -r1.450 rde.c
--- rde.c       28 Nov 2018 08:32:27 -0000      1.450
+++ rde.c       29 Nov 2018 10:38:13 -0000
@@ -3371,6 +3371,20 @@ peer_up(u_int32_t id, struct session_up 
        }
 }
 
+static void
+peer_adjout_flush_upcall(struct rib_entry *re, void *arg)
+{
+       struct rde_peer *peer = arg;
+       struct prefix *p, *np;
+
+       LIST_FOREACH_SAFE(p, &re->prefix_h, rib_l, np) {
+               if (peer != prefix_peer(p))
+                       continue;
+               prefix_destroy(p);
+               break;  /* optimization, only one match per peer possible */
+       }
+}
+
 void
 peer_down(u_int32_t id)
 {
@@ -3383,10 +3397,16 @@ peer_down(u_int32_t id)
        }
        peer->remote_bgpid = 0;
        peer->state = PEER_DOWN;
-       up_down(peer);
        /* stop all pending dumps which may depend on this peer */
        rib_dump_terminate(peer->loc_rib_id, peer, rde_up_dump_upcall);
 
+       /* flush Adj-RIB-Out for this peer */
+       if (rib_dump_new(RIB_ADJ_OUT, AID_UNSPEC, 0, peer,
+           peer_adjout_flush_upcall, NULL, NULL) == -1)
+               fatal("%s: rib_dump_new", __func__);
+
+       up_down(peer);
+
        peer_flush(peer, AID_UNSPEC, 0);
 
        peer->prefix_cnt = 0;
@@ -3438,6 +3458,7 @@ peer_flush_upcall(struct rib_entry *re, 
 
                prefix_destroy(p);
                peer->prefix_cnt--;
+               break;  /* optimization, only one match per peer possible */
        }
 }
 

Reply via email to