diff options
| -rw-r--r-- | ospfd/ChangeLog | 17 | ||||
| -rw-r--r-- | ospfd/ospf_interface.c | 1 | ||||
| -rw-r--r-- | ospfd/ospf_neighbor.c | 74 | ||||
| -rw-r--r-- | ospfd/ospf_nsm.c | 5 |
4 files changed, 71 insertions, 26 deletions
diff --git a/ospfd/ChangeLog b/ospfd/ChangeLog index 265c9c76..ac596705 100644 --- a/ospfd/ChangeLog +++ b/ospfd/ChangeLog @@ -1,3 +1,20 @@ +2006-04-03 Paul Jakma <paul.jakma@sun.com> + + * (general) Fix issues with handling of Vlinks and entries + in the nbrs route-table which were highlighted by the + nsm/nbr_self fixes from bug #234. Many thanks to Juergen + Kammer for his help and efforts in testing out debug patches to + pinpoint the issue. + * ospf_interface.c: (ospf_vl_new) Add nbr_self for Vlink. + * ospf_neighbor.c: (ospf_nbr_key) new static function, helper + to create key in nbrs table for a given nbr. + (ospf_nbr_delete) Use ospf_nbr_key. Add an assert() to + document an expected state. + (ospf_nbr_add_self) Ditto. + (ospf_nbr_lookup_by_addr) Add an assert. + * ospf_nsm.c: (nsm_kill_nbr) Can never kill the nbr_self + psuedo-neighbour. + 2006-03-27 Paul Jakma <paul.jakma@sun.com> * ospf_lsa.c: (ospf_lsa_checksum) Add an explicit cast to avoid diff --git a/ospfd/ospf_interface.c b/ospfd/ospf_interface.c index 8df0280a..52adc420 100644 --- a/ospfd/ospf_interface.c +++ b/ospfd/ospf_interface.c @@ -910,6 +910,7 @@ ospf_vl_new (struct ospf *ospf, struct ospf_vl_data *vl_data) if (IS_DEBUG_OSPF_EVENT) zlog_debug ("ospf_vl_new(): set associated area to the backbone"); + ospf_nbr_add_self (voi); ospf_area_add_if (voi->area, voi); ospf_if_stream_set (voi); diff --git a/ospfd/ospf_neighbor.c b/ospfd/ospf_neighbor.c index 58752366..843e93f6 100644 --- a/ospfd/ospf_neighbor.c +++ b/ospfd/ospf_neighbor.c @@ -43,6 +43,26 @@ #include "ospfd/ospf_flood.h" #include "ospfd/ospf_dump.h" +/* Fill in the the 'key' as appropriate to retrieve the entry for nbr + * from the ospf_interface's nbrs table. Indexed by interface address + * for all cases except Virtual-link interfaces, where neighbours are + * indexed by router-ID instead. + */ +static void +ospf_nbr_key (struct ospf_interface *oi, struct ospf_neighbor *nbr, + struct prefix *key) +{ + key->family = AF_INET; + key->prefixlen = IPV4_MAX_BITLEN; + + /* vlinks are indexed by router-id */ + if (oi->type == OSPF_IFTYPE_VIRTUALLINK) + key->u.prefix4 = nbr->router_id; + else + key->u.prefix4 = nbr->src; + return; +} + struct ospf_neighbor * ospf_nbr_new (struct ospf_interface *oi) { @@ -134,20 +154,22 @@ ospf_nbr_delete (struct ospf_neighbor *nbr) struct prefix p; oi = nbr->oi; - - /* Unlink ospf neighbor from the interface. */ - p.family = AF_INET; - p.prefixlen = IPV4_MAX_BITLEN; - - /* vlinks are indexed by router-id */ - if (oi->type == OSPF_IFTYPE_VIRTUALLINK) - p.u.prefix4 = nbr->router_id; - else - p.u.prefix4 = nbr->src; + + /* get appropriate prefix 'key' */ + ospf_nbr_key (oi, nbr, &p); rn = route_node_lookup (oi->nbrs, &p); if (rn) { + /* If lookup for a NBR succeeds, the leaf route_node could + * only exist because there is (or was) a nbr there. + * If the nbr was deleted, the leaf route_node should have + * lost its last refcount too, and be deleted. + * Therefore a looked-up leaf route_node in nbrs table + * should never have NULL info. + */ + assert (rn->info); + if (rn->info) { rn->info = NULL; @@ -185,24 +207,9 @@ ospf_nbr_bidirectional (struct in_addr *router_id, void ospf_nbr_add_self (struct ospf_interface *oi) { - struct ospf_neighbor *nbr; struct prefix p; struct route_node *rn; - p.family = AF_INET; - p.prefixlen = 32; - p.u.prefix4 = oi->address->u.prefix4; - - rn = route_node_get (oi->nbrs, &p); - if (rn->info) - { - /* There is already pseudo neighbor. */ - nbr = rn->info; - route_unlock_node (rn); - } - else - rn->info = oi->nbr_self; - /* Initial state */ oi->nbr_self->address = *oi->address; oi->nbr_self->priority = OSPF_IF_PARAM (oi, priority); @@ -223,6 +230,19 @@ ospf_nbr_add_self (struct ospf_interface *oi) SET_FLAG (oi->nbr_self->options, OSPF_OPTION_NP); break; } + + /* Add nbr_self to nbrs table */ + ospf_nbr_key (oi, oi->nbr_self, &p); + + rn = route_node_get (oi->nbrs, &p); + if (rn->info) + { + /* There is already pseudo neighbor. */ + assert (oi->nbr_self == rn->info); + route_unlock_node (rn); + } + else + rn->info = oi->nbr_self; } /* Get neighbor count by status. @@ -281,6 +301,9 @@ ospf_nbr_lookup_by_addr (struct route_table *nbrs, rn = route_node_lookup (nbrs, &p); if (! rn) return NULL; + + /* See comment in ospf_nbr_delete */ + assert (rn->info); if (rn->info == NULL) { @@ -439,4 +462,3 @@ ospf_nbr_get (struct ospf_interface *oi, struct ospf_header *ospfh, return nbr; } - diff --git a/ospfd/ospf_nsm.c b/ospfd/ospf_nsm.c index bfd565ef..8a93f0e6 100644 --- a/ospfd/ospf_nsm.c +++ b/ospfd/ospf_nsm.c @@ -440,6 +440,11 @@ nsm_kill_nbr (struct ospf_neighbor *nbr) /* call it here because we cannot call it from ospf_nsm_event */ nsm_change_state (nbr, NSM_Down); + /* killing nbr_self is invalid */ + assert (nbr != nbr->oi->nbr_self); + if (nbr == nbr->oi->nbr_self) + return 1; + /* Reset neighbor. */ nsm_reset_nbr (nbr); |