The bug was reported to us by a customer ~4 years ago when they were doing some rfc compliance testing across a mixed vendor environment. We fixed it for them. We have not heard anything since then.
donald On Fri, May 20, 2016 at 10:40 AM, David Lamparter <[email protected]> wrote: > Btw: Donald -- this patch is very early in the Cumulus tree; I assume > it's been part of previous stable releases that are in active use at > customer installations? Have you received any reports of breakage? > > -David > > On Fri, May 20, 2016 at 04:37:37PM +0200, David Lamparter wrote: > > On Fri, May 20, 2016 at 02:12:51PM +0100, Paul Jakma wrote: > > > Could we agree that at a minimum there needs to be a config knob, so > > > admins can at least manage rolling out updates without introducing > > > mixed-environments? > > > > I haven't seen any technical argument that led me to change my opinion > > on migration, i.e. I still strongly believe this needs to be fixed ASAP, > > getting RFC-compliant behaviour without user interaction. That means > > I'm continuing to oppose patches that don't make RFC behaviour the > > default. > > > > The event most likely to change my opinion on this is being presented > > with an actual real-world setup that sees breakage from this. From the > > way this bug works, I guess it'd need to be a Quagga-only production > > network. > > > > If there is an additional default-off "ospf spf-maxcost-quagga" switch, > > I don't see a problem with that. > > > > > I think more transparent migration that avoids exposing Quagga networks > > > (currently fine) to any loop risk, ought to be possible, e.g. with RIs > > > (which we'd have to support at some stage even for H-bit), but that can > > > be another discussion. > > > > Funnily enough, the prefixes for which traffic could loop in a mixed > > setup are the prefixes which would default to a less specific route if > > the router to be maintenance'd is fully removed; i.e. it's the prefixes > > for which service is likely degraded anyway... > > > > > > -David > > > > > > > From 8142989d465d6dd2e41b0bb7a17069b5c0c3a8bb Mon Sep 17 00:00:00 2001 > > > From: Paul Jakma <[email protected]> > > > Date: Fri, 20 May 2016 09:54:34 +0100 > > > Subject: ospfd: Add compatibility setting for max-cost link SPF > behaviour > > > > > > * Quagga OSPF has long treated max-cost links as not suitable for > > > inclusion into the transit SPF tree (doesn't affect reachability of > > > the node itself). Other implementations treat it as just another > > > cost. > > > > > > In mixed environments there is therefore a risk of routing loops. > > > It may be a rare risk, though when hit it could be quite > > > troublesome. > > > > > > Just flipping the default would not address that risk. It need not > > > fix the risk on networks currently exposed, while exposing other > > > networks to the risk that would otherwise not be affected. Which > > > administrators of the latter networks might find less than ideal. > > > > > > TBD: A good transition strategy, that avoids bothering currently > > > unaffected networks with this problem. E.g. RIs could be used to > > > transparently do the right thing for a greater number of networks > > > and minise the risks. > > > > > > Also, the OSPF WG are standardising a method to signal Quagga's > > > current behaviour conformantly. So it might also be good to > > > minimise churn in behaviour, but that depends on when that comes > > > along. > > > > > > Regardless, it would be good to at least give administrators who > > > are exposed a configuration setting. > > > > > > * ospfd.h: Add a OSPF_MAX_METRIC_LINK_FOLLOW flag to (struct ospf). > > > * lib/libospf.h: Add OSPF_OUTPUT_COST_MAX, and a > > > OSPF_OUTPUT_COST_DISCOURAGE - latter metric works universally to > > > signal 'discourage but still allow transit'. > > > > > > * ospf_lsa.c: (ospf_link_cost) Use the DISCOURAGE cost to signal > > > stub-router, if SPF is set to 'follow'. These are somewhat > > > separate, and with ways to signal the transit/no-transit intent of > > > the local router this might be done differently, but for this patch > > > probably best done together. > > > > > > * ospf_spf.c: (ospf_spf_next) make the behaviour on max-cost links > > > configurable. > > > > > > * ospf_vty.c: (show_ip_ospf) Print the status of the SPF max-cost > > > behaviour in 'show ip ospf'. > > > > > > (ospf_compatible_max_metric) Add "compatible max-metric > > > (infinite|follow)" to control the SPF behaviour. > > > > > > (no_ospf_compatible_max_metric) clear back to default, which will > > > still be written out explicitly, as below. > > > > > > (config_write_stub_router) Write out the config for the "compatible > > > max-metric (infinite|follow)" flag. In this version, the flag > > > state is always written out explicitly - the default state is not > > > suppressed. There's pros and cons to this of course. > > > > > > (ospf_vty_init) add prev. > > > > > > * docs/ospfd.texi: docs on the "compatible max-metric > > > (infinite|follow)" command. > > > > > > diff --git a/doc/ospfd.texi b/doc/ospfd.texi > > > index 86cabe4..da6608f 100644 > > > --- a/doc/ospfd.texi > > > +++ b/doc/ospfd.texi > > > @@ -172,6 +172,8 @@ releases. > > > @deffn {OSPF Command} {max-metric router-lsa > [on-startup|on-shutdown] <5-86400>} {} > > > @deffnx {OSPF Command} {max-metric router-lsa administrative} {} > > > @deffnx {OSPF Command} {no max-metric router-lsa > [on-startup|on-shutdown|administrative]} {} > > > +@anchor{OSPF stub-router} > > > +@anchor{max-metric router-lsa} > > > This enables @cite{RFC3137, OSPF Stub Router Advertisement} support, > > > where the OSPF process describes its transit links in its router-LSA > as > > > having infinite distance so that other routers will avoid calculating > > > @@ -202,6 +204,51 @@ number of second remaining till on-startup or > on-shutdown ends, can be > > > viewed with the @ref{show ip ospf} command. > > > @end deffn > > > > > > + > > > +@deffn {OSPF Command} {compatible max-metric (infinite|follow)} {} > > > +@deffnx {OSPF Command} {no compatible max-metric} {} > > > + > > > +This flag controls the behaviour of SPF, and how it treats links in > > > +Router-LSAs with the maximum possible metric value. It toggles > between the > > > +@cite{RFC2328} specified behaviour, and a Quagga variation. > > > + > > > +When this flag is set to @var{follow}, then the standard RFC2328 > > > +behaviour applies. The SPF calculation will still take links with > > > +the maximum value metric of 0xffff into consideration when > > > +calculating transit paths out of remote routers. > > > + > > > +When this flag is set to @var{infinite}, then Quagga deviates from > > > +RFC2328. The SPF transit tree calculation will treat the maximum > > > +link metric of 0xffff as an infinite cost. Such links will not be > > > +considered for the transit tree. This only affects whether links > > > +@emph{out} of a remote router will be used for transit - it does not > > > +affect the reachability to the router itself, or to its stub > > > +addresses and networks. > > > + > > > +The @var{infinite} behaviour exists in Quagga as it allows routers to > > > +advertise links as @emph{strictly} non-transit or merely as > > > +discouraged, but still available for transit. The @var{infinite} > > > +behaviour is the default, and is the only possible behaviour on most > > > +older releases of Quagga. > > > + > > > +In networks with routers with mixed behaviour, if links are > > > +advertised with the 0xffff maximum metric (e.g., through the > > > +@ref{OSPF stub-router} functionality) then routing loops may > > > +potentially occur depending on the topology. > > > + > > > +The appropriate behaviour to configure depends on whether the > > > +administrator prefers stub-routers to be strictly not available for > > > +transit or not, on whether the network is mixed, and the consequences > > > +of routing loops. > > > + > > > +The default behaviour is @var{infinite} for compatibility with > > > +previous Quagga releases, for now. In older Quagga releases this > > > +behaviour is not configurable. The default is likely to change from > > > +@var{infinite} to @var{follow} in a future release. > > > + > > > +@end deffn > > > + > > > + > > > @deffn {OSPF Command} {auto-cost reference-bandwidth <1-4294967>} {} > > > @deffnx {OSPF Command} {no auto-cost reference-bandwidth} {} > > > @anchor{OSPF auto-cost reference-bandwidth}This sets the reference > > > diff --git a/lib/libospf.h b/lib/libospf.h > > > index 9265ca5..8d9e40f 100644 > > > --- a/lib/libospf.h > > > +++ b/lib/libospf.h > > > @@ -63,6 +63,8 @@ > > > /* OSPF interface default values. */ > > > #define OSPF_OUTPUT_COST_DEFAULT 10 > > > #define OSPF_OUTPUT_COST_INFINITE UINT16_MAX > > > +#define OSPF_OUTPUT_COST_MAX OSPF_OUTPUT_COST_INFINITE > > > +#define OSPF_OUTPUT_COST_DISCOURAGE (OSPF_OUTPUT_COST_MAX - 1) > > > #define OSPF_ROUTER_DEAD_INTERVAL_DEFAULT 40 > > > #define OSPF_ROUTER_DEAD_INTERVAL_MINIMAL 1 > > > #define OSPF_HELLO_INTERVAL_DEFAULT 10 > > > diff --git a/ospfd/ospf_lsa.c b/ospfd/ospf_lsa.c > > > index 634bc43..84dc5de 100644 > > > --- a/ospfd/ospf_lsa.c > > > +++ b/ospfd/ospf_lsa.c > > > @@ -488,7 +488,8 @@ ospf_link_cost (struct ospf_interface *oi) > > > if (!CHECK_FLAG (oi->area->stub_router_state, > OSPF_AREA_IS_STUB_ROUTED)) > > > return oi->output_cost; > > > else > > > - return OSPF_OUTPUT_COST_INFINITE; > > > + return CHECK_FLAG(oi->area->ospf->config, > OSPF_MAX_METRIC_LINK_FOLLOW) > > > + ? OSPF_OUTPUT_COST_DISCOURAGE : OSPF_OUTPUT_COST_INFINITE; > > > } > > > > > > /* Set a link information. */ > > > diff --git a/ospfd/ospf_spf.c b/ospfd/ospf_spf.c > > > index 58a3992..43a6619 100644 > > > --- a/ospfd/ospf_spf.c > > > +++ b/ospfd/ospf_spf.c > > > @@ -837,11 +837,55 @@ ospf_spf_next (struct vertex *v, struct > ospf_area *area, > > > if ((type = l->m[0].type) == LSA_LINK_TYPE_STUB) > > > continue; > > > > > > - /* Infinite distance links shouldn't be followed, except > > > - * for local links (a stub-routed router still wants to > > > - * calculate tree, so must follow its own links). > > > + /* RFC3137 has wording in places that suggests non-transit > > > + * links / links out of a stub-routed node shouldn't be > > > + * followed, RFC6987 even uses "should not". Unfortunately > > > + * however, it seems these RFCs were never intended to > > > + * change the behaviour of OSPF. It seems they weren't > > > + * meant amount to anything more than notes to say > > > + * "increasing link costs may discourage traffic through a > > > + * router" and there wasn't actually intended to be > > > + * anything special about 0xffff. > > > + * > > > + * Quagga however interpreted 0xffff as special, as meaning > > > + * infinite cost and hence untraversable / strictly "no > > > + * transit", as it appeared to be useful (and the > > > + * implementor specifically wanted that behaviour on his > > > + * network). As did original OSPF, prior to 1583. > > > + * > > > + * Other implementations treat 0xffff costs as just a very > > > + * high cost, OSPF_OUTPUT_COST_MAX, but will still include > > > + * them in the transit tree. > > > + * > > > + * Mixed environments of the two behaviours can therefore > > > + * lead to routing loops. > > > + * > > > + * In some use-cases, the administrator may even prefer > > > + * "no transit stub-router" to really mean that. > > > + * > > > + * This behaviour is configurable, via the OSPF instance > > > + * OSPF_MAX_METRIC_LINK_FOLLOW config flag, defaulting to > > > + * the long-standing Quagga behaviour to not follow 0xfff > > > + * links out for now, to avoid causing issues on networks > > > + * that currently are not exposed to routing-loop risks > > > + * from this, but may be if the default is flipped. > > > + * > > > + * TBD: A transition strategy to OSPF conformance that > > > + * minimises exposing /further/ networks to risks of loops. > > > + * > > > + * Note: this has no effect on stub networks attached to > > > + * the router, such as local loopback and PtP addresses, > > > + * and broadcast networks without any established OSPF > > > + * adjacencies. These should always be reachable, > > > + * regardless. > > > + * > > > + * Note: a cost of 0xfffe or lower works universally to > > > + * signal "use of this link is discouraged, but it is still > > > + * available to transit through". > > > */ > > > - if ((v != area->spf) && l->m[0].metric >= > OSPF_OUTPUT_COST_INFINITE) > > > + if ((v != area->spf) > > > + && l->m[0].metric >= OSPF_OUTPUT_COST_MAX > > > + && !CHECK_FLAG(area->ospf->config, > OSPF_MAX_METRIC_LINK_FOLLOW)) > > > continue; > > > > > > /* (b) Otherwise, W is a transit vertex (router or transit > > > diff --git a/ospfd/ospf_vty.c b/ospfd/ospf_vty.c > > > index fb17dff..b34aa4c 100644 > > > --- a/ospfd/ospf_vty.c > > > +++ b/ospfd/ospf_vty.c > > > @@ -2836,6 +2836,12 @@ DEFUN (show_ip_ospf, > > > CHECK_FLAG (ospf->config, OSPF_OPAQUE_CAPABLE) ? > > > "enabled" : "disabled", > > > VTY_NEWLINE); > > > + vty_out (vty, " SPF treats max-metric links as:%s %s%s", > > > + VTY_NEWLINE, > > > + CHECK_FLAG(ospf->config, OSPF_MAX_METRIC_LINK_FOLLOW) > > > + ? "low preference but transit capable (RFC standard)" > > > + : "infinite cost and not transit capable (RFC > deviation)", > > > + VTY_NEWLINE); > > > > > > /* Show stub-router configuration */ > > > if (ospf->stub_router_startup_time != OSPF_STUB_ROUTER_UNCONFIGURED > > > @@ -6417,10 +6423,51 @@ ALIAS (no_ip_ospf_mtu_ignore, > > > "OSPF interface commands\n" > > > "Disable mtu mismatch detection\n") > > > > > > +#define OSPF_CMD_MAX_METRIC_STR \ > > > + "OSPF maximum / infinite-distance metric links\n" > > > + > > > +DEFUN (ospf_compatible_max_metric, > > > + ospf_compatible_max_metric_cmd, > > > + "compatible max-metric (infinite|follow)", > > > + "OSPF compatibility list\n" > > > + "Behaviour for " OSPF_CMD_MAX_METRIC_STR > > > + "SPF treats max-cost links as infinite and not considered when" > > > + " examining LSAs for transit out of a router (RFC > deviation)\n" > > > + "SPF treats max-cost links as lowest-preference links, " > > > + "but still available for transit\n") > > > +{ > > > + struct ospf *ospf = vty->index; > > > + switch (argv[0][0]) > > > + { > > > + case 'i': > > > + UNSET_FLAG (ospf->config, OSPF_MAX_METRIC_LINK_FOLLOW); > > > + break; > > > + case 'f': > > > + SET_FLAG (ospf->config, OSPF_MAX_METRIC_LINK_FOLLOW); > > > + break; > > > + default: > > > + vty_out (vty, "%% Unknown argument %s%s", argv[0], > VTY_NEWLINE); > > > + return CMD_WARNING; > > > + } > > > + return CMD_SUCCESS; > > > +} > > > + > > > +DEFUN (no_ospf_compatible_max_metric, > > > + no_ospf_compatible_max_metric_cmd, > > > + "no compatible max-metric", > > > + NO_STR > > > + "OSPF compatibility list\n" > > > + "Behaviour for " OSPF_CMD_MAX_METRIC_STR) > > > +{ > > > + struct ospf *ospf = vty->index; > > > + UNSET_FLAG (ospf->config, OSPF_MAX_METRIC_LINK_FOLLOW); > > > + return CMD_SUCCESS; > > > +} > > > + > > > DEFUN (ospf_max_metric_router_lsa_admin, > > > ospf_max_metric_router_lsa_admin_cmd, > > > "max-metric router-lsa administrative", > > > - "OSPF maximum / infinite-distance metric\n" > > > + OSPF_CMD_MAX_METRIC_STR > > > "Advertise own Router-LSA with infinite distance (stub > router)\n" > > > "Administratively applied, for an indefinite period\n") > > > { > > > @@ -6446,7 +6493,7 @@ DEFUN (no_ospf_max_metric_router_lsa_admin, > > > no_ospf_max_metric_router_lsa_admin_cmd, > > > "no max-metric router-lsa administrative", > > > NO_STR > > > - "OSPF maximum / infinite-distance metric\n" > > > + OSPF_CMD_MAX_METRIC_STR > > > "Advertise own Router-LSA with infinite distance (stub > router)\n" > > > "Administratively applied, for an indefinite period\n") > > > { > > > @@ -6473,7 +6520,7 @@ DEFUN (no_ospf_max_metric_router_lsa_admin, > > > DEFUN (ospf_max_metric_router_lsa_startup, > > > ospf_max_metric_router_lsa_startup_cmd, > > > "max-metric router-lsa on-startup <5-86400>", > > > - "OSPF maximum / infinite-distance metric\n" > > > + OSPF_CMD_MAX_METRIC_STR > > > "Advertise own Router-LSA with infinite distance (stub > router)\n" > > > "Automatically advertise stub Router-LSA on startup of OSPF\n" > > > "Time (seconds) to advertise self as stub-router\n") > > > @@ -6498,7 +6545,7 @@ DEFUN (no_ospf_max_metric_router_lsa_startup, > > > no_ospf_max_metric_router_lsa_startup_cmd, > > > "no max-metric router-lsa on-startup", > > > NO_STR > > > - "OSPF maximum / infinite-distance metric\n" > > > + OSPF_CMD_MAX_METRIC_STR > > > "Advertise own Router-LSA with infinite distance (stub > router)\n" > > > "Automatically advertise stub Router-LSA on startup of > OSPF\n") > > > { > > > @@ -6526,7 +6573,7 @@ DEFUN (no_ospf_max_metric_router_lsa_startup, > > > DEFUN (ospf_max_metric_router_lsa_shutdown, > > > ospf_max_metric_router_lsa_shutdown_cmd, > > > "max-metric router-lsa on-shutdown <5-86400>", > > > - "OSPF maximum / infinite-distance metric\n" > > > + OSPF_CMD_MAX_METRIC_STR > > > "Advertise own Router-LSA with infinite distance (stub > router)\n" > > > "Advertise stub-router prior to full shutdown of OSPF\n" > > > "Time (seconds) to wait till full shutdown\n") > > > @@ -6551,7 +6598,7 @@ DEFUN (no_ospf_max_metric_router_lsa_shutdown, > > > no_ospf_max_metric_router_lsa_shutdown_cmd, > > > "no max-metric router-lsa on-shutdown", > > > NO_STR > > > - "OSPF maximum / infinite-distance metric\n" > > > + OSPF_CMD_MAX_METRIC_STR > > > "Advertise own Router-LSA with infinite distance (stub > router)\n" > > > "Advertise stub-router prior to full shutdown of OSPF\n") > > > { > > > @@ -6568,6 +6615,10 @@ config_write_stub_router (struct vty *vty, > struct ospf *ospf) > > > struct listnode *ln; > > > struct ospf_area *area; > > > > > > + vty_out (vty, " compatible max-metric %s%s", > > > + CHECK_FLAG (ospf->config, OSPF_MAX_METRIC_LINK_FOLLOW) > > > + ? "follow" : "infinite", > > > + VTY_NEWLINE); > > > if (ospf->stub_router_startup_time != > OSPF_STUB_ROUTER_UNCONFIGURED) > > > vty_out (vty, " max-metric router-lsa on-startup %u%s", > > > ospf->stub_router_startup_time, VTY_NEWLINE); > > > @@ -7877,6 +7928,8 @@ ospf_vty_init (void) > > > install_element (OSPF_NODE, &no_ospf_refresh_timer_cmd); > > > > > > /* max-metric commands */ > > > + install_element (OSPF_NODE, &ospf_compatible_max_metric_cmd); > > > + install_element (OSPF_NODE, &no_ospf_compatible_max_metric_cmd); > > > install_element (OSPF_NODE, &ospf_max_metric_router_lsa_admin_cmd); > > > install_element (OSPF_NODE, > &no_ospf_max_metric_router_lsa_admin_cmd); > > > install_element (OSPF_NODE, > &ospf_max_metric_router_lsa_startup_cmd); > > > diff --git a/ospfd/ospfd.h b/ospfd/ospfd.h > > > index 098fc5f..1c4d2c4 100644 > > > --- a/ospfd/ospfd.h > > > +++ b/ospfd/ospfd.h > > > @@ -130,6 +130,7 @@ struct ospf > > > #define OSPF_OPAQUE_CAPABLE (1 << 2) > > > #define OSPF_LOG_ADJACENCY_CHANGES (1 << 3) > > > #define OSPF_LOG_ADJACENCY_DETAIL (1 << 4) > > > +#define OSPF_MAX_METRIC_LINK_FOLLOW (1 << 5) > > > > > > /* Opaque-LSA administrative flags. */ > > > u_char opaque; > > > > > > regards, > > > -- > > > Paul Jakma [email protected] @pjakma Key ID: 64A2FF6A > > > Fortune: > > > You are not dead yet. But watch for further reports. >
_______________________________________________ Quagga-dev mailing list [email protected] https://lists.quagga.net/mailman/listinfo/quagga-dev
