On Mon, Mar 06, 2017 at 07:41:44PM +0200, Lennert Buytenhek wrote:
> Hi!
>
> bgp_init() in proto/bgp/bgp.c does:
>
> P->accept_ra_types = c->secondary ? RA_ACCEPTED : RA_OPTIMAL;
>
> and then bgp_rx_open() in proto/bgp/packets.c does:
>
> if (p->add_path_tx)
> p->p.accept_ra_types = RA_ANY;
>
> As bgp_init() seems to only be called at configuration time, this
> means that if you have add path tx, and you have a peer that advertises
> rx add path and then reconnects without advertising rx add path, you
> will still have RA_ANY set on the sending side and not have the correct
> behavior of only sending the optimal route. (This is easy to verify.)
>
> Not entirely sure how you'd want to fix this, but perhaps replicating
> the RA_ACCEPTED / RA_OPTIONAL assignment in bgp_rx_open() (or just
> moving it there) would do the trick?
In other words, something like this?
(I didn't trace all the code paths, but I'm guessing it should be safe
to remove the ->accept_ra_types assignment from bgp_init() based on the
fact that we are overriding it in bgp_rx_open() and expecting that to
do the trick for the add_path_tx case -- if that's not sufficient then
it's not safe to do that reassignment from there to begin with.)
diff --git a/proto/bgp/packets.c b/proto/bgp/packets.c
index d100b7d..a566d11 100644
--- a/proto/bgp/packets.c
+++ b/proto/bgp/packets.c
@@ -1039,6 +1039,8 @@ bgp_rx_open(struct bgp_conn *conn, byte *pkt, uint len)
if (p->add_path_tx)
p->p.accept_ra_types = RA_ANY;
+ else
+ p->p.accept_ra_types = p->cf->secondary ? RA_ACCEPTED : RA_OPTIMAL;
DBG("BGP: Hold timer set to %d, keepalive to %d, AS to %d, ID to %x, AS4
session to %d\n", conn->hold_time, conn->keepalive_time, p->remote_as,
p->remote_id, p->as4_session);
