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 <paul.ja...@hpe.com>
> > 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 p...@jakma.org @pjakma Key ID: 64A2FF6A
> > Fortune:
> > You are not dead yet. But watch for further reports.
_______________________________________________
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev
