[quagga-dev 1383] Re: ospfd dies -- bug in ospf_spf_consider_nexthop

Paul Jakma paul at clubi.ie
Wed Aug 4 17:01:57 BST 2004


On Wed, 4 Aug 2004, Kir Kostuchenko wrote:

>   I don't see this in the CVS...
>
>   Please.

ah, you're too quick. I'm proposing the following, with a nice big 
comment cause I had to go reacquint myself with it all. essentially:

- compare only the list head, because all the interfaces on the 
nexthop list should already have same cost

- delete the whole list if we have new < hop, but in a tidier list 
loop (backward jumps are a bit evil imho, shouldnt be used unless 
absolutely neccesary).

Hmm, i accidently deleted the hop->oi assert..

Index: ospf_spf.c
===================================================================
RCS file: /var/cvsroot/quagga/ospfd/ospf_spf.c,v
retrieving revision 1.9
diff -u -r1.9 ospf_spf.c
--- ospf_spf.c	8 Apr 2004 07:43:45 -0000	1.9
+++ ospf_spf.c	4 Aug 2004 15:53:17 -0000
@@ -341,31 +341,49 @@
    return NULL;
  }

-/* Consider supplied next-hop for inclusion to the supplied list
- * of next-hops, adjust list as neccessary
+/* 
+ * Consider supplied next-hop for inclusion to the supplied list of
+ * equal-cost next-hops, adjust list as neccessary. 
+ *
+ * (Discussed on GNU Zebra list 27 May 2003, [zebra 19184])
+ *
+ * Note that below is a bit of a hack, and limits ECMP to paths that go to
+ * same nexthop. Where as paths via inequal output_cost interfaces could
+ * still quite easily be ECMP due to remote cost differences.
+ *
+ * It really should be done by way of recording currently valid backlinks
+ * and determining the appropriate nexthops from the list of backlinks.
   */
  void
  ospf_spf_consider_nexthop (struct list *nexthops,
                             struct vertex_nexthop *newhop)
  {
-  struct listnode *nnode;
    struct vertex_nexthop *hop;
+  struct listnode *ln, *nn;

-  LIST_LOOP (nexthops, hop, nnode)
-  {
-    assert (hop->oi);
-    /* weed out hops with higher cost than the newhop */
-    if (hop->oi->output_cost > newhop->oi->output_cost)
-      {
-        /* delete the existing nexthop */
-        listnode_delete (nexthops, hop);
-        vertex_nexthop_free (hop);
-      }
-    else if (hop->oi->output_cost < newhop->oi->output_cost)
-      {
+  /* nexthop list should contain only the set of nexthops that have the lowest
+   * equal cost
+   */
+  if (nexthops->head != NULL)
+    {
+      hop = getdata (nexthops->head);
+ 
+      /* weed out hops with higher cost than the newhop */
+      if (hop->oi->output_cost > newhop->oi->output_cost)
+        {
+          /* delete the existing nexthops */
+          for (ln = nexthops->head; ln; ln = nn)
+            {
+              nn = ln->next;
+              hop = getdata (ln);
+ 
+              listnode_delete (nexthops, hop);
+              vertex_nexthop_free (hop);
+            }
+        }
+      else if (hop->oi->output_cost < newhop->oi->output_cost)
          return;
-      }
-  }
+    }

    /* new hop is <= existing hops, add it */
    listnode_add (nexthops, newhop);
@@ -455,7 +473,7 @@
                    nh = vertex_nexthop_new (v);
                    nh->oi = oi;
                    nh->router.s_addr = 0;
-                  listnode_add (w->nexthop, nh);
+                  ospf_spf_consider_nexthop (w->nexthop, nh);
                  }
              }
          }
@@ -474,7 +492,7 @@
                    nh = vertex_nexthop_new (v);
                    nh->oi = x->oi;
                    nh->router = l->link_data;
-                  listnode_add (w->nexthop, nh);
+                  ospf_spf_consider_nexthop (w->nexthop, nh);
                  }
                return;
              }

regards,
-- 
Paul Jakma	paul at clubi.ie	paul at jakma.org	Key ID: 64A2FF6A
Fortune:
In the stairway of life, you'd best take the elevator.



More information about the Quagga-dev mailing list