On Mon, Oct 3, 2016 at 11:26 PM, Thorvald Natvig <thorv...@medallia.com> wrote:
Hello Thorvald, Thanks for the detailed explanation. In fact, confederation feature, once assigned, is used in all transactions ( more info on rfc5065#section-4). For both reasons, I understand the check at announcement should be part of the BGP_SEND_ASPATH_CHECK, since no specific check is planned to be done at announcement. I would ack the fix, then. However, there is still a problem with the testing status ( https://ci1.netdef.org/browse/QUAGGA-QPWORK-357/ ). I would recommend to send the patch once again to quagga-dev, to trigger the running test, and see if the error still arises or not. If it still arises, Martin's help would be great, to identify the culprit ANVL test, or find out the root cause. I don't know if there can be some deprecated test ( rfc5065 is 9 years). Best Regards, Philippe > Hi, > > The #define is, by default, undefined. It's designed to cover > re-announcements of routes learned that contain your own AS. > Unfortunately, that #define only covers half the code; it's missing > for the code that deals with confederations. > > Let's use an example: > > My company has AS 123, and we have two locations. > Location "US" has subnet 1.2.3.0/24 using Router A. > Location "UK" has subnet 4.5.6.0/24 using Router B. > Connectivity between the two sites is over the the internet, passing > through one or more other AS on the way. > > Behind the router B sits another router; let's call this the Problem > Router or PR. Let's assume I've assigned my PR the AS 65000. > > If BGP_SEND_ASPATH_CHECK is defined, then Router B will detect that > the route to 1.2.3.0/24 contains its own AS (since both A and B are on > AS 123), and will not announce 1.2.3.0/24 to PR. Hence, PR can not > reach router A. > > This is clearly a problem, which is why this logic is only > conditionally enabled if BGP_SEND_ASPATH_CHECK is defined. By default > it remains undefined, so B will announce the route to PR. > > Let's reconfigure to a confederation, where B and PR form a > confederation with confederation-id 123. B will have have the internal > AS 65000, and PR has AS 65111. > > There are now two checks for the aspath. The check inside the #define > checks for the confederation-internal AS. B will check: Does the route > to 1.2.3.0/24 contain the AS 65000? It doesn't, so we pass the first > check. > The second check, which is not protected by the #define, checks "does > it contain the confed-id". Clearly, as-path to 1.2.3.0/24 does contain > the confed-id, so this route gets suppressed. > > All this patch does is make sure this second check, which is enabled > only for confederations, is covered by the same #define as the > non-confederation case. > > - Thorvald > > > On Mon, Oct 3, 2016 at 7:45 AM, Philippe Guibert > <philippe.guib...@6wind.com> wrote: >> On Thu, Sep 29, 2016 at 7:25 PM, Thorvald Natvig <thorv...@medallia.com> >> wrote: >> >> Hello Thorvald, >> >>> Extend the check for BGP_SEND_ASPATH_CHECK to also cover confederations. >> .. >>> -#endif /* BGP_SEND_ASPATH_CHECK */ >>> >>> /* If we're a CONFED we need to loop check the CONFED ID too */ >>> if (CHECK_FLAG(bgp->config, BGP_CONFIG_CONFEDERATION)) >>> @@ -916,6 +915,7 @@ bgp_announce_check (struct bgp_info *ri, struct peer >>> *peer, struct prefix *p, >>> return 0; >>> } >>> } >>> +#endif /* BGP_SEND_ASPATH_CHECK */ >>> >> >> Unless unnoticed, the BGP_SEND_ASPATH_CHECK is not enabled in >> compilation process on quagga-dev continuous integration testing. >> So, assuming the error depicted by CI is related to BGPd ( >> interoperating with RIPng?), the problem could be due to the removal >> of code that has been undefined by the patch. >> >> Regarding the patch, confederation feature can be set by BGP VTY >> through "bgp confederation-identifier <id>" command. >> I understand that with modification, no more check is done at >> announcement for condition to send BGP update to a conference-id >> group.No ? >> >> Is it your intention to not systematically test BGP conference id >> updates at announcement ? >> >> Best Regards, >> >> Philippe _______________________________________________ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
