[PATCH,RFC 0/3] Secondary remote AS support for protocol BGP.
Hi! These patches implement a way of simplifying BGP topology migrations that involve changing AS numbers on one or both ends of BGP sessions. The first patch allows a BGP peer to connect to us using either of two different AS numbers specified in our local config. (See the commit message for that patch for more details.) The second patch fixes what I think is a reconfiguration bug for protocol BGP and is in the stack mainly because of patch three. The third patch avoids bouncing a BGP session when reconfiguring its BGP protocol if all we're changing is adding a secondary remote AS to the protocol or changing one of two remote AS numbers that is not currently being advertised by the peer. (Again, see the commit message for that patch for more details.) Any feedback appreciated! And even if you happen not to like patch 3, I hope that you'd still be willing to consider patch 1 (and 2). Cheers, Lennert
When carrying out a BGP topology migration such as an AS renumbering or an AS merge, which involves changing the local AS being reported by one or both ends of a BGP session, being able to avoid the need to reconfigure both peers at the same time can significantly reduce the complexity of such a migration. Also, if the peers are managed by different administrative entities, it might not even be possible to reconfigure both peers as closely together in time as you would like. One way to make this easier is to allow configuring two local AS numbers on a BGP session, one AS number to try first, and another AS number to fall back to if connecting with the preferred AS number gives us back a NOTIFICATION with OPEN message error "Bad Peer AS" from the peer. You can then add this option on your end of the BGP session, and then ask your peer to reconfigure their end, and the session will remain up in the time interval between your change and the peer's change. However, doing it this way adds quite some complexity to the BGP connection state machine. An easier way of achieving the same thing is to allow specifying two different AS numbers that we will accept from the peer, and let the BGP session establish if the peer sends us either one of those AS numbers as its local AS. You would then first reconfigure one or both peer(s) to accept a secondary remote AS number in addition to the one that is currently being used, and then reconfigure one or both peer(s) to change the local AS number being sent in the OPEN message. This is both easier to implement and avoids the unnecessary but unavoidable reconnection attempts that stem from the fact that we cannot know that we are going to be trying to OPEN the connection using a local AS that our peer does not expect before it sends us a "Bad Peer AS". This patch implements the latter option, and this functionality is enabled by specifying two different remote AS numbers for a peer in the neighbor statement. Instead of specifying: neighbor 1.2.3.4 as 12345; Specify this: neighbor 1.2.3.4 as 12345 as 23456; Which will allow peer 1.2.3.4 to connect to us using either AS 12345 or AS 23456. (bird's config grammar already allows specifying multiple remote AS numbers for a peer, and it will use the last AS number you specify this way if you do that. With this patch, it will use the last two AS numbers you specify.) --- proto/bgp/bgp.c | 17 +++++++++-------- proto/bgp/bgp.h | 2 +- proto/bgp/config.Y | 2 +- proto/bgp/packets.c | 8 ++++++-- 4 files changed, 17 insertions(+), 12 deletions(-) diff --git a/proto/bgp/bgp.c b/proto/bgp/bgp.c index 0f1c944..bceae4a 100644 --- a/proto/bgp/bgp.c +++ b/proto/bgp/bgp.c @@ -1278,8 +1278,6 @@ bgp_init(struct proto_config *C) p->cf = c; p->local_as = c->local_as; - p->remote_as = c->remote_as; - p->is_internal = (c->local_as == c->remote_as); p->rs_client = c->rs_client; p->rr_client = c->rr_client; p->igp_table = get_igp_table(c); @@ -1291,8 +1289,6 @@ bgp_init(struct proto_config *C) void bgp_check_config(struct bgp_config *c) { - int internal = (c->local_as == c->remote_as); - /* Do not check templates at all */ if (c->c.class == SYM_TEMPLATE) return; @@ -1300,7 +1296,12 @@ bgp_check_config(struct bgp_config *c) /* EBGP direct by default, IBGP multihop by default */ if (c->multihop < 0) - c->multihop = internal ? 64 : 0; + { + if (c->local_as != c->remote_as || (c->remote_as2 != 0 && c->local_as != c->remote_as2)) + c->multihop = 0; + else + c->multihop = 64; + } /* Different default for gw_mode */ if (!c->gw_mode) @@ -1330,13 +1331,13 @@ bgp_check_config(struct bgp_config *c) if (!ipa_is_link_local(c->remote_ip) != !c->iface) cf_error("Link-local address and interface scope must be used together"); - if (!(c->capabilities && c->enable_as4) && (c->remote_as > 0xFFFF)) + if (!(c->capabilities && c->enable_as4) && (c->remote_as > 0xFFFF || c->remote_as2 > 0xFFFF)) cf_error("Neighbor AS number out of range (AS4 not available)"); - if (!internal && c->rr_client) + if (c->rr_client && (c->local_as != c->remote_as || (c->remote_as2 != 0 && c->local_as != c->remote_as2))) cf_error("Only internal neighbor can be RR client"); - if (internal && c->rs_client) + if (c->rs_client && (c->local_as == c->remote_as || c->local_as == c->remote_as2)) cf_error("Only external neighbor can be RS client"); if (c->multihop && (c->gw_mode == GW_DIRECT)) diff --git a/proto/bgp/bgp.h b/proto/bgp/bgp.h index e47a0eb..0b8cb46 100644 --- a/proto/bgp/bgp.h +++ b/proto/bgp/bgp.h @@ -19,7 +19,7 @@ struct eattr; struct bgp_config { struct proto_config c; - u32 local_as, remote_as; + u32 local_as, remote_as, remote_as2; ip_addr remote_ip; ip_addr source_addr; /* Source address to use */ struct iface *iface; /* Interface for link-local addresses */ diff --git a/proto/bgp/config.Y b/proto/bgp/config.Y index 55c602f..8cf70c1 100644 --- a/proto/bgp/config.Y +++ b/proto/bgp/config.Y @@ -61,7 +61,7 @@ bgp_proto_start: proto_start BGP { bgp_nbr_opts: /* empty */ | bgp_nbr_opts PORT expr { BGP_CFG->remote_port = $3; if (($3<1) || ($3>65535)) cf_error("Invalid port number"); } - | bgp_nbr_opts AS expr { BGP_CFG->remote_as = $3; } + | bgp_nbr_opts AS expr { BGP_CFG->remote_as2 = BGP_CFG->remote_as; BGP_CFG->remote_as = $3; } ; bgp_proto: diff --git a/proto/bgp/packets.c b/proto/bgp/packets.c index d100b7d..e402288 100644 --- a/proto/bgp/packets.c +++ b/proto/bgp/packets.c @@ -966,7 +966,7 @@ bgp_rx_open(struct bgp_conn *conn, byte *pkt, uint len) if ((conn->advertised_as != base_as) && (base_as != AS_TRANS)) log(L_WARN "%s: Peer advertised inconsistent AS numbers", p->p.name); - if (conn->advertised_as != p->remote_as) + if (conn->advertised_as != p->cf->remote_as && conn->advertised_as != p->cf->remote_as2) { if (conn->peer_as4_support) { @@ -979,6 +979,9 @@ bgp_rx_open(struct bgp_conn *conn, byte *pkt, uint len) return; } + p->remote_as = conn->advertised_as; + p->is_internal = (p->local_as == p->remote_as); + /* Check the other connection */ other = (conn == &p->outgoing_conn) ? &p->incoming_conn : &p->outgoing_conn; switch (other->state) @@ -1550,7 +1553,8 @@ bgp_rx_notification(struct bgp_conn *conn, byte *pkt, uint len) /* Capabilities are not explicitly enabled or disabled, therefore heuristic is used */ && (conn->start_state == BSS_CONNECT) /* Failed connection attempt have used capabilities */ - && (p->cf->remote_as <= 0xFFFF)) + && (p->cf->remote_as <= 0xFFFF) + && (p->cf->remote_as2 <= 0xFFFF)) /* Not possible with disabled capabilities */ { /* We try connect without capabilities */ -- 2.9.3
On Thu, Mar 09, 2017 at 04:29:40PM +0200, Lennert Buytenhek wrote:
This patch implements the latter option, and this functionality is enabled by specifying two different remote AS numbers for a peer in the neighbor statement. Instead of specifying:
neighbor 1.2.3.4 as 12345;
Specify this:
neighbor 1.2.3.4 as 12345 as 23456;
I would prefer if the secondary as would either have a separate keyword, (e.g. neighbor 1.2.3.4 as 12345 secondary as 23456) or it was just plain positional (e.g. neighbor 1.2.3.4 as 12345 23456).
(bird's config grammar already allows specifying multiple remote AS numbers for a peer, and it will use the last AS number you specify this way if you do that. With this patch, it will use the last two AS numbers you specify.) --- proto/bgp/bgp.c | 17 +++++++++-------- proto/bgp/bgp.h | 2 +- proto/bgp/config.Y | 2 +- proto/bgp/packets.c | 8 ++++++-- 4 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/proto/bgp/bgp.c b/proto/bgp/bgp.c index 0f1c944..bceae4a 100644 --- a/proto/bgp/bgp.c +++ b/proto/bgp/bgp.c @@ -1278,8 +1278,6 @@ bgp_init(struct proto_config *C)
p->cf = c; p->local_as = c->local_as; - p->remote_as = c->remote_as; - p->is_internal = (c->local_as == c->remote_as); p->rs_client = c->rs_client; p->rr_client = c->rr_client; p->igp_table = get_igp_table(c); @@ -1291,8 +1289,6 @@ bgp_init(struct proto_config *C) void bgp_check_config(struct bgp_config *c) { - int internal = (c->local_as == c->remote_as); -
I don't like changes in bgp_check_config(). I think that if you set remote_as as EBGP, while remote_as2 as IBGP, you could end up with inconsistent settings. IMHO allowing remote_as2 only in EBGP makes sense, as in IBGP remote ASN must be the same as local ASN. So we could make internal / is_internal fixed, as it was. -- Elen sila lumenn' omentielvo Ondrej 'Santiago' Zajicek (email: santiago@crfreenet.org) OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net) "To err is human -- to blame it on a computer is even more so."
On Thu, Mar 09, 2017 at 05:53:51PM +0100, Ondrej Zajicek wrote:
This patch implements the latter option, and this functionality is enabled by specifying two different remote AS numbers for a peer in the neighbor statement. Instead of specifying:
neighbor 1.2.3.4 as 12345;
Specify this:
neighbor 1.2.3.4 as 12345 as 23456;
I would prefer if the secondary as would either have a separate keyword, (e.g. neighbor 1.2.3.4 as 12345 secondary as 23456) or it was just plain positional (e.g. neighbor 1.2.3.4 as 12345 23456).
Right, okay. There isn't really a difference in the code between the "primary" and the "secondary" AS (currently they are remote_as and remote_as2, and remote_as2 isn't always set, but the two variables are entirely interchangeable if they are both set), so I would prefer the latter option. I'll change the patch accordingly.
(bird's config grammar already allows specifying multiple remote AS numbers for a peer, and it will use the last AS number you specify this way if you do that. With this patch, it will use the last two AS numbers you specify.) --- proto/bgp/bgp.c | 17 +++++++++-------- proto/bgp/bgp.h | 2 +- proto/bgp/config.Y | 2 +- proto/bgp/packets.c | 8 ++++++-- 4 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/proto/bgp/bgp.c b/proto/bgp/bgp.c index 0f1c944..bceae4a 100644 --- a/proto/bgp/bgp.c +++ b/proto/bgp/bgp.c @@ -1278,8 +1278,6 @@ bgp_init(struct proto_config *C)
p->cf = c; p->local_as = c->local_as; - p->remote_as = c->remote_as; - p->is_internal = (c->local_as == c->remote_as); p->rs_client = c->rs_client; p->rr_client = c->rr_client; p->igp_table = get_igp_table(c); @@ -1291,8 +1289,6 @@ bgp_init(struct proto_config *C) void bgp_check_config(struct bgp_config *c) { - int internal = (c->local_as == c->remote_as); -
I don't like changes in bgp_check_config(). I think that if you set remote_as as EBGP, while remote_as2 as IBGP, you could end up with inconsistent settings. IMHO allowing remote_as2 only in EBGP makes sense, as in IBGP remote ASN must be the same as local ASN. So we could make internal / is_internal fixed, as it was.
My main use case for this patch is actually to migrate a data center network from iBGP to eBGP, with a setup that is vaguely similar to the one described in rfc7938. I tried to be careful to keep the behavior in bgp_check_config() consistent, and whenever iBGP versus eBGP matters, I tried to err on the side of caution, e.g.: * If multihop isn't set explicitly, it would default to 64 for iBGP or 0 for eBGP, and with this patch, it will default to 0 if either remote_as or remote_as2 indicate eBGP, because you don't want to default it to 64 if there is the potential that the session will establish as eBGP. * If rr_client is configured, both remote_as and remote_as2 must indicate eBGP (or remote_as must indicate eBGP and remote_as2 must not be set). * If rs_client is configured, both remote_as and remote_as2 must indicate iBGP (or, much more likely, remote_as must indicate iBGP and remote_as2 must not be set). I accordingly moved the is_internal assignment into bgp_rx_open(), because only when we receive the remote's OPEN can we know whether this session is actually going to be iBGP or eBGP. I'm sitting on a bunch more patches related to this, for example a per-peer toggle to consider routes from an eBGP peer as iBGP routes as far as the RFC 4271 9.1.2.2. d) check (Prefer external peers) is concerned, and an extension to 'path metric' to skip leading private ASes in the AS_PATH when comparing path lengths, et cetera, but I am not actually a fan of using eBGP in the data center. The main reason why people do this, as far as I can see, is that using eBGP gives you a superior route redistribution and loop detection model, and IMHO, iBGP should have had some intra-AS variant of AS_PATH with router IDs instead of AS numbers (called, say, INTRA_AS_PATH) -- and then we wouldn't have needed ugly hacks like route reflectors either. That said, these patches are invaluable to us, but they might not all make a lot of sense to merge upstream. We carry a lot more bird patches internally, for example a patch to do ADD_PATHS only for a subset of prefixes, more fine-grained control over how to merge multipath BGP routes, and we have our own way to inject MPLS routes (!), and my goal was to try a bit harder than we traditionally have to get things upstream, but if you think that something doesn't make sense to support in the upstream version of bird, then I totally understand that, as we do have some pretty specific use cases. Cheers, Lennert
On Fri, Mar 10, 2017 at 05:19:11PM +0200, Lennert Buytenhek wrote:
* If rr_client is configured, both remote_as and remote_as2 must indicate eBGP (or remote_as must indicate eBGP and remote_as2 must not be set).
* If rs_client is configured, both remote_as and remote_as2 must indicate iBGP (or, much more likely, remote_as must indicate iBGP and remote_as2 must not be set).
(rr_client / rs_client should be the other way around here!)
I don't know how I got on this email list. Please remove me. ________________________________ From: Bird-users <bird-users-bounces@network.cz> on behalf of Lennert Buytenhek <buytenh@wantstofly.org> Sent: Friday, March 10, 2017 8:39:37 AM To: Ondrej Zajicek Cc: bird-users@network.cz; David Barroso Subject: Re: [PATCH,RFC 1/3] BGP: Implement secondary remote AS number support. On Fri, Mar 10, 2017 at 05:19:11PM +0200, Lennert Buytenhek wrote:
* If rr_client is configured, both remote_as and remote_as2 must indicate eBGP (or remote_as must indicate eBGP and remote_as2 must not be set).
* If rs_client is configured, both remote_as and remote_as2 must indicate iBGP (or, much more likely, remote_as must indicate iBGP and remote_as2 must not be set).
(rr_client / rs_client should be the other way around here!)
On Fri, Mar 10, 2017 at 05:19:11PM +0200, Lennert Buytenhek wrote:
On Thu, Mar 09, 2017 at 05:53:51PM +0100, Ondrej Zajicek wrote:
This patch implements the latter option, and this functionality is enabled by specifying two different remote AS numbers for a peer in the neighbor statement. Instead of specifying:
neighbor 1.2.3.4 as 12345;
Specify this:
neighbor 1.2.3.4 as 12345 as 23456;
I would prefer if the secondary as would either have a separate keyword, (e.g. neighbor 1.2.3.4 as 12345 secondary as 23456) or it was just plain positional (e.g. neighbor 1.2.3.4 as 12345 23456).
Right, okay. There isn't really a difference in the code between the "primary" and the "secondary" AS
That is true, i thought about local AS.
@@ -1291,8 +1289,6 @@ bgp_init(struct proto_config *C) void bgp_check_config(struct bgp_config *c) { - int internal = (c->local_as == c->remote_as); -
I don't like changes in bgp_check_config(). I think that if you set remote_as as EBGP, while remote_as2 as IBGP, you could end up with inconsistent settings. IMHO allowing remote_as2 only in EBGP makes sense, as in IBGP remote ASN must be the same as local ASN. So we could make internal / is_internal fixed, as it was.
My main use case for this patch is actually to migrate a data center network from iBGP to eBGP, with a setup that is vaguely similar to the one described in rfc7938.
Well, that seems like too specific use case. Although you carefully modified bgp_check_config(), it makes changes to it harder, esp. in 2.0 branch when there is more branching related to which kind of BGP session is configured. But the idea of allowing a different remote ASNs in EBGP make sense to me. Perhaps instead of having two remote ASNs, there could be variant to allow any.
I'm sitting on a bunch more patches related to this, for example a per-peer toggle to consider routes from an eBGP peer as iBGP routes as far as the RFC 4271 9.1.2.2. d) check (Prefer external peers) is concerned, and an extension to 'path metric' to skip leading private ASes in the AS_PATH when comparing path lengths, et cetera, but I am not actually a fan of using eBGP in the data center. The main reason why people do this, as far as I can see, is that using eBGP gives you a superior route redistribution and loop detection model, and IMHO, iBGP should have had some intra-AS variant of AS_PATH with router IDs instead of AS numbers (called, say, INTRA_AS_PATH)
That is essentially BGP confederations (RFC 5065). We already have that in 2.0 branch [*], but we do not plan to backport it to 1.6 branch. Or perhaps just setting every router as route reflector / separate RR cluster and use CLUSTER_LIST as your INTRA_AS_PATH. The main question here is that whether you want forwarding paths to be congruent with BGP propagation paths, which is done in EBGP, but not in IBGP nor in BGP confederations. Althought that could be probably forced by resetting NEXT_HOP attribute (next hop self option). [*] https://gitlab.labs.nic.cz/labs/bird/commit/5509e17d0c1b4e75d5911864f75ba119...
That said, these patches are invaluable to us, but they might not all make a lot of sense to merge upstream. We carry a lot more bird patches internally, for example a patch to do ADD_PATHS only for a subset of prefixes, more fine-grained control over how to merge multipath BGP routes, and we have our own way to inject MPLS routes (!), and my goal was to try a bit harder than we traditionally have to get things upstream,
Great. I am glad to see moving patches to upstream, although i am sometimes a bit slow to react to them. Perhaps you could first send us a list of patches/changes (or just raw / unsanitized patches) so we could response which ones we see as worth of merging to upstream. -- Elen sila lumenn' omentielvo Ondrej 'Santiago' Zajicek (email: santiago@crfreenet.org) OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net) "To err is human -- to blame it on a computer is even more so."
Since the area between 'struct proto_config c' and 'char *password' in 'struct bgp_config' is compared using memcmp() in bgp_reconfigure(), move ->check_link and ->bfd so that they are part of that area. Also add a note to 'struct bgp_config' to avoid this tripping up people in the future. --- proto/bgp/bgp.h | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/proto/bgp/bgp.h b/proto/bgp/bgp.h index 0b8cb46..0b69333 100644 --- a/proto/bgp/bgp.h +++ b/proto/bgp/bgp.h @@ -19,6 +19,8 @@ struct eattr; struct bgp_config { struct proto_config c; + + /* start of config vars compared using memcmp() in bgp_reconfigure() */ u32 local_as, remote_as, remote_as2; ip_addr remote_ip; ip_addr source_addr; /* Source address to use */ @@ -62,11 +64,12 @@ struct bgp_config { unsigned error_delay_time_min; /* Time to wait after an error is detected */ unsigned error_delay_time_max; unsigned disable_after_error; /* Disable the protocol when error is detected */ + int check_link; /* Use iface link state for liveness detection */ + int bfd; /* Use BFD for liveness detection */ + /* end of config vars compared using memcmp() in bgp_reconfigure() */ char *password; /* Password used for MD5 authentication */ struct rtable_config *igp_table; /* Table used for recursive next hop lookups */ - int check_link; /* Use iface link state for liveness detection */ - int bfd; /* Use BFD for liveness detection */ }; #define MLL_SELF 1 -- 2.9.3
On Thu, Mar 09, 2017 at 04:30:24PM +0200, Lennert Buytenhek wrote:
Since the area between 'struct proto_config c' and 'char *password' in 'struct bgp_config' is compared using memcmp() in bgp_reconfigure(), move ->check_link and ->bfd so that they are part of that area. Also add a note to 'struct bgp_config' to avoid this tripping up people in the future.
Having 'check_link' and 'bfd' outside was intentional, they can be reconfigured without a session flap. -- Elen sila lumenn' omentielvo Ondrej 'Santiago' Zajicek (email: santiago@crfreenet.org) OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net) "To err is human -- to blame it on a computer is even more so."
On Thu, Mar 09, 2017 at 04:28:43PM +0100, Ondrej Zajicek wrote:
Since the area between 'struct proto_config c' and 'char *password' in 'struct bgp_config' is compared using memcmp() in bgp_reconfigure(), move ->check_link and ->bfd so that they are part of that area. Also add a note to 'struct bgp_config' to avoid this tripping up people in the future.
Having 'check_link' and 'bfd' outside was intentional, they can be reconfigured without a session flap.
OK!
If we are using the secondary remote AS mechanism, we don't necessarily need to restart an already established BGP connection if the remote AS for a BGP session changes, as long as the AS number that the peer is currently connected with is still in the list of AS numbers that we accept from the peer. This commit will avoid reconfiguring a BGP protocol if there is a corresponding connection in OpenConfirm or Established state and the only change to the protocol configuration was related to remote AS numbers and the AS number that the peer is currently connected with is still a permitted remote AS number. This allows adding a secondary remote AS number to a BGP protocol without bouncing an already established BGP session for this protocol. --- proto/bgp/bgp.c | 36 +++++++++++++++++++++++++++++------- proto/bgp/bgp.h | 4 +++- 2 files changed, 32 insertions(+), 8 deletions(-) diff --git a/proto/bgp/bgp.c b/proto/bgp/bgp.c index bceae4a..b969b81 100644 --- a/proto/bgp/bgp.c +++ b/proto/bgp/bgp.c @@ -1369,17 +1369,39 @@ bgp_reconfigure(struct proto *P, struct proto_config *C) struct bgp_config *new = (struct bgp_config *) C; struct bgp_proto *p = (struct bgp_proto *) P; struct bgp_config *old = p->cf; + int same = 1; if (proto_get_router_id(C) != p->local_id) return 0; - int same = !memcmp(((byte *) old) + sizeof(struct proto_config), - ((byte *) new) + sizeof(struct proto_config), - // password item is last and must be checked separately - OFFSETOF(struct bgp_config, password) - sizeof(struct proto_config)) - && ((!old->password && !new->password) - || (old->password && new->password && !strcmp(old->password, new->password))) - && (get_igp_table(old) == get_igp_table(new)); + struct bgp_conn *c = p->conn; + if (c != NULL && + (c->state == BS_OPENCONFIRM || c->state == BS_ESTABLISHED) && + (p->remote_as == new->remote_as || p->remote_as == new->remote_as2)) + { + old->remote_as = new->remote_as; + old->remote_as2 = new->remote_as2; + } + else if (old->remote_as != new->remote_as || old->remote_as2 != new->remote_as2) + { + same = 0; + } + + int start = OFFSETOF(struct bgp_config, local_as); + int end = OFFSETOF(struct bgp_config, password); + if (memcmp(((byte *) old) + start, ((byte *) new) + start, end - start)) + same = 0; + + if (old->password && new->password) + { + if (strcmp(old->password, new->password)) + same = 0; + } + else if (old->password || new->password) + same = 0; + + if (get_igp_table(old) != get_igp_table(new)) + same = 0; if (same && (p->start_state > BSS_PREPARE)) bgp_update_bfd(p, new->bfd); diff --git a/proto/bgp/bgp.h b/proto/bgp/bgp.h index 0b69333..29b5051 100644 --- a/proto/bgp/bgp.h +++ b/proto/bgp/bgp.h @@ -20,8 +20,10 @@ struct eattr; struct bgp_config { struct proto_config c; + u32 remote_as, remote_as2; + /* start of config vars compared using memcmp() in bgp_reconfigure() */ - u32 local_as, remote_as, remote_as2; + u32 local_as; ip_addr remote_ip; ip_addr source_addr; /* Source address to use */ struct iface *iface; /* Interface for link-local addresses */ -- 2.9.3
On Thu, Mar 09, 2017 at 04:30:55PM +0200, Lennert Buytenhek wrote:
If we are using the secondary remote AS mechanism, we don't necessarily need to restart an already established BGP connection if the remote AS for a BGP session changes, as long as the AS number that the peer is currently connected with is still in the list of AS numbers that we accept from the peer.
This commit will avoid reconfiguring a BGP protocol if there is a corresponding connection in OpenConfirm or Established state and the only change to the protocol configuration was related to remote AS numbers and the AS number that the peer is currently connected with is still a permitted remote AS number. This allows adding a secondary remote AS number to a BGP protocol without bouncing an already established BGP session for this protocol.
Well, the simple way to avoid reconfiguration in this case is just to move 'remote_as2' field at the end of bgp_config, like 'check_link' or 'bfd'. -- Elen sila lumenn' omentielvo Ondrej 'Santiago' Zajicek (email: santiago@crfreenet.org) OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net) "To err is human -- to blame it on a computer is even more so."
On Thu, Mar 09, 2017 at 04:23:05PM +0100, Ondrej Zajicek wrote:
If we are using the secondary remote AS mechanism, we don't necessarily need to restart an already established BGP connection if the remote AS for a BGP session changes, as long as the AS number that the peer is currently connected with is still in the list of AS numbers that we accept from the peer.
This commit will avoid reconfiguring a BGP protocol if there is a corresponding connection in OpenConfirm or Established state and the only change to the protocol configuration was related to remote AS numbers and the AS number that the peer is currently connected with is still a permitted remote AS number. This allows adding a secondary remote AS number to a BGP protocol without bouncing an already established BGP session for this protocol.
Well, the simple way to avoid reconfiguration in this case is just to move 'remote_as2' field at the end of bgp_config, like 'check_link' or 'bfd'.
Well, e.g. if remote_as is 12345 and the currently established peer AS is 12345, and you change remote_as from 12345 to 23456 and remote_as2 from 0 to 12345, you want to avoid doing the reconfigure. And if you keep remote_as at 12345 and set remote_as2 to 23456 by specifying the ASes the other way around, you would avoid the reconfigure if you leave remote_as2 outside the memcmp() field, but then when the other side completes its maintenance and you reestablish a connection with remote AS 23456, you would then set remote_as from 12345 to 23456 and remote_as2 from 23456 to 0, and that would then unnecessarily trigger a reconfigure as well because remote_as changes.
On Thu, Mar 09, 2017 at 05:36:54PM +0200, Lennert Buytenhek wrote:
Well, the simple way to avoid reconfiguration in this case is just to move 'remote_as2' field at the end of bgp_config, like 'check_link' or 'bfd'.
Well, e.g. if remote_as is 12345 and the currently established peer AS is 12345, and you change remote_as from 12345 to 23456 and remote_as2 from 0 to 12345, you want to avoid doing the reconfigure.
And if you keep remote_as at 12345 and set remote_as2 to 23456 by specifying the ASes the other way around, you would avoid the reconfigure if you leave remote_as2 outside the memcmp() field, but then when the other side completes its maintenance and you reestablish a connection with remote AS 23456, you would then set remote_as from 12345 to 23456 and remote_as2 from 23456 to 0, and that would then unnecessarily trigger a reconfigure as well because remote_as changes.
OK -- Elen sila lumenn' omentielvo Ondrej 'Santiago' Zajicek (email: santiago@crfreenet.org) OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net) "To err is human -- to blame it on a computer is even more so."
participants (3)
-
James Howlett -
Lennert Buytenhek -
Ondrej Zajicek