[quagga-dev 5415] Re: [PATCH] RFC 2328, chap 8.1:

paul at clubi.ie paul at clubi.ie
Mon Jun 2 11:25:29 BST 2008


On Sun, 1 Jun 2008, Joakim Tjernlund wrote:

> Yes, there was one more titled:
> "[PATCH] Make ospf_if_lookup_recv_if() find the right unnumbered i/f"

Ok. I'll have a look asap.

> I am working on modifying ospf_calculate_nexthop() too, not ready 
> yet but if you have some more time I really could use you feedback. 
> Patch last in this mail.

Sure.

> I could try to remember, but on the other hand not doing it will
> speed up the process of switching to a better SCM :)

;)

> Joke aside, did you know the git can emulate a CVS server? If you set
> that up you can have both and let people migrate in their own pace.

I'd heard of that feature of that git, I've not heard how well it 
works though.

> Jocke
>

> +/*
> + * Used for creating a temporary ordered list of pt-to-pt OSPF
> + * interfaces pointing at a common node.
> + * Sorted after increasing output interface cost.
> + */
> +static
> +int ospf_output_cost_cmp(void *v1, void *v2)
> +{
> +  struct ospf_interface *a1 = v1, *a2 = v2;
> +
> +  if (a1->output_cost > a2->output_cost)
> +    return 1;
> +  if (a1->output_cost < a2->output_cost)
> +    return -1;
> +  return 0;
> +}

This would raise concerns.. Why do we need a temporary list to 
determine priority? SPF is supposed to take care of finding the best 
edge between two vertices..

> /* 16.1.1.  Calculate nexthop from root through V (parent) to
>  * vertex W (destination), with given distance from root->W.
> @@ -573,24 +589,50 @@ ospf_nexthop_calculation (struct ospf_area *area, struct vertex *v,
>               else
>                 {
>                   /* l is a regular point-to-point link.
> -                     Look for a link from W to V.
> +                     Look for a links from W to V.
>                    */
> -                  while ((l2 = ospf_get_next_link (w, v, l2)))
> -                    {
> -                      oi = ospf_if_is_configured (area->ospf,
> -                                                  &(l2->link_data));
> -
> -                      if (oi == NULL)
> -                        continue;
> -
> -                      if (!IPV4_ADDR_SAME (&oi->address->u.prefix4,
> -                                           &l->link_data))
> -                        continue;

Note that this, in-part, a sanity check, to ensure that there is 
2-way communication between the parent V and the candidate W*, 
appropriate for V's given link of l (note: 2-way, not symmetric). 
Note that we don't care at all what the best link is, only that one 
exists. You seem to have dropped this part of the check.

The second reason for this code is to find the appropriate IP address 
to use as nexthop for (V,l)->W. This is obviously not possible if the 
/other/ side is advertising an unnumbered link.

> +		  cost = OSPF_OUTPUT_COST_INFINITE;
> +		  /* Only add the cheapest i/f's */
> +		  for (ALL_LIST_ELEMENTS_RO(oiflist, node, tmp_oi))
> +		    {
> +		      if (tmp_oi->output_cost > cost)
> +			break; /* Cost too high, drop out */
> +		      cost = tmp_oi->output_cost;


If I get the sense of this right, you're looking through the links of 
V to find the lowest link to W. This isn't appropriate for nexthop 
calculation - the SPF algorithm has already determined the link to be 
"l" (i.e. the l argument to nexthop_calculation).

What you want is, presuming the other side is unnumbered:

- verify there is a /any/ link l2 for (W,l2)->V

   (because, AFAICT, you can't reconcile the unnumbered link back
    to an exact local link, as you can with addressed links)

- setup a 'struct nexthop' using the oi corresponding to the
   given (V,l), and the router-ID of W.

You shouldn't need to modify the /existing/ Point-to-Point nexthop 
chunk of code at all, other than to make it conditional so as to only 
apply where (W,l2) is numbered.

I **guess** you want something like the following psuedo-ish 
additions to the existing code:

 	           /* l is a regular point-to-point link.
                      Look for a link from W to V.
                    */
                   while ((l2 = ospf_get_next_link (w, v, l2)))
                     {
 		      if (!ospf_link_is_unnumbered (l2))
                         {
                           oi = ospf_if_is_configured (area->ospf,
                                                     &(l2->link_data));

                           if (oi == NULL)
                             continue;

                           if (!IPV4_ADDR_SAME (&oi->address->u.prefix4,
                                                &l->link_data))
                           continue;

                           break;
                        }
                      else
                        {
                          unnumbered_l2 = l2;
                          unnumbered_oi = <fill in>;
                        }
                     }

                     if (oi && l2)
                       ....
                     else if (unnumbered_l2)
                       <nh appropriate for unnumbered, using W's ID I
                        guess>

I guess :)

* This check is slightly redundant, as we have a similar check at a
   higher-layer. The check in nexthop calculation only is done for W's
   that are attached to V though.

regards,
-- 
Paul Jakma	paul at clubi.ie	paul at jakma.org	Key ID: 64A2FF6A
Fortune:
No one can have a higher opinion of him than I have, and I think he's a
dirty little beast.
 		-- W.S. Gilbert



More information about the Quagga-dev mailing list