Hi,

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 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.

Tested locally for the vty, but not in a network:

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

Reply via email to