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
