[quagga-dev 10351] Re: [PATCH] ospfd: restore nexthop IP for p2p interfaces

James Li jli at cumulusnetworks.com
Wed Mar 20 18:30:04 GMT 2013


On 3/20/13 10:57 AM, Joakim Tjernlund wrote:
> <equinox at diac24.net> wrote on 2013/03/20 17:28:46:
>> From: Christian Franke <chris at opensourcerouting.org>
>>
>> commit c81ee5c... "ospfd: Optimize and improve SPF nexthop calculation"
>> subtly changed semantics of routes calculated over pointopoint links by
>> removing the nexthop IP address and instead using an ifindex route.
>>
>> This breaks calculation of AS-Ext routes with a forwarding address since
>> in ospf_ase_complete_direct_routes() this will be hit:
>>      if (op->nexthop.s_addr == 0)
>>        op->nexthop.s_addr = nexthop.s_addr;
>> thus turning the route unusable by having an invalid nexthop.
>>
>> Fix by restoring the nexthop IP on routes over PtP links.  This also
>> allows running multi-access (Ethernet) interfaces in PtP mode again.
>>
>> This bug is a regression against 0.99.21 and only present in 0.99.22.
> OOPS, I guess my patch was dropped on the floor, I thought it had
> gone in long time ago. I am very surprised nobody noticed until now.
>
> Anyhow, this version is much better. Using the neighbour list is far
> superior, BIRD already does this.
>
> One question though, will this lookup use nbrs which nbr state is != FULL?
>
> If so, consider adding a test to check for nbr state == FULL

The neighbor state is implied to be FULL if the link is considered for SPF.

James

>
>   Jocke
>
>> Signed-off-by: Christian Franke <chris at opensourcerouting.org>
>> [patch description and code comments rewritten]
>> Cc: Joakim Tjernlund <Joakim.Tjernlund at transmode.se>
>> Cc: Scott Feldman <sfeldma at cumulusnetworks.com>
>> Signed-off-by: David Lamparter <equinox at opensourcerouting.org>
>> ---
>>   ospfd/ospf_spf.c | 18 ++++++++++++++++--
>>   1 file changed, 16 insertions(+), 2 deletions(-)
>>
>> diff --git a/ospfd/ospf_spf.c b/ospfd/ospf_spf.c
>> index a5242b6..bd9564d 100644
>> --- a/ospfd/ospf_spf.c
>> +++ b/ospfd/ospf_spf.c
>> @@ -568,8 +568,22 @@ ospf_nexthop_calculation (struct ospf_area *area,
> struct vertex *v,
>>                 */
>>            if (oi->type == OSPF_IFTYPE_POINTOPOINT)
>>         {
>> -        added = 1;
>> -        nexthop.s_addr = 0; /* Nexthop not required */
>> +        /* Having nexthop = 0 is tempting, but NOT acceptable.
>> +           It breaks AS-External routes with a forwarding address,
>> +           since ospf_ase_complete_direct_routes() will mistakenly
>> +           assume we've reached the last hop and should place the
>> +           forwarding address as nexthop.
>> +           Also, users may configure multi-access links in p2p mode,
>> +           so we need the IP to ARP the nexthop.
>> +        */
>> +        struct ospf_neighbor *nbr_w;
>> +
>> +        nbr_w = ospf_nbr_lookup_by_routerid (oi->nbrs, &l->link_id);
>> +        if (nbr_w != NULL)
>> +          {
>> +            added = 1;
>> +            nexthop = nbr_w->src;
>> +          }
>>         }
>>            else if (oi->type == OSPF_IFTYPE_POINTOMULTIPOINT)
>>         {
>> -- 
>> 1.8.1.5
>>
>
> _______________________________________________
> Quagga-dev mailing list
> Quagga-dev at lists.quagga.net
> http://lists.quagga.net/mailman/listinfo/quagga-dev





More information about the Quagga-dev mailing list