diff options
author | Donald Sharp <sharpd@cumulusnetworks.com> | 2015-05-20 03:04:07 +0200 |
---|---|---|
committer | Donald Sharp <sharpd@cumulusnetworks.com> | 2015-05-20 03:04:07 +0200 |
commit | 2c19a6ec62c6ed4450d237a467734fd24455a983 (patch) | |
tree | ae5661345b3cb23515421323b34aac05a022b95e | |
parent | Show enabled debugs in the running configuration (diff) | |
download | frr-2c19a6ec62c6ed4450d237a467734fd24455a983.tar.xz frr-2c19a6ec62c6ed4450d237a467734fd24455a983.zip |
Changing router-id inline isnt handled correctly in the current implementation.
At the minimum, the OSPF_LSA_SELF logic isnt foolproof, and it may hit assert
in ospf_refresh_unregister_lsa on a router-id change.
Once OSPF has created and flooded LSAs, its not a good idea to change
router-id inline. Tying it to restart has at least two benefits:
- Implementation can remain sane by not having to re-adjust neighbors and LSAs,
based on the new router-id.
- Works as a deterrent for the user to not meddle with the router-id unless
really needed.
-rw-r--r-- | ospfd/ospf_lsa.c | 2 | ||||
-rw-r--r-- | ospfd/ospf_lsa.h | 2 | ||||
-rw-r--r-- | ospfd/ospf_vty.c | 22 | ||||
-rw-r--r-- | ospfd/ospfd.c | 53 |
4 files changed, 70 insertions, 9 deletions
diff --git a/ospfd/ospf_lsa.c b/ospfd/ospf_lsa.c index f988468e3..ebdb390ab 100644 --- a/ospfd/ospf_lsa.c +++ b/ospfd/ospf_lsa.c @@ -3389,7 +3389,7 @@ ospf_lsa_flush_self_originated (struct ospf_neighbor *nbr, self->data->type, inet_ntoa (self->data->id)); } #else /* ORIGINAL_CODING */ -static int +int ospf_lsa_flush_schedule (struct ospf *ospf, struct ospf_lsa *lsa) { if (lsa == NULL || !IS_LSA_SELF (lsa)) diff --git a/ospfd/ospf_lsa.h b/ospfd/ospf_lsa.h index 624efc4c1..ef6bf88c9 100644 --- a/ospfd/ospf_lsa.h +++ b/ospfd/ospf_lsa.h @@ -260,7 +260,7 @@ extern void ospf_lsa_free (struct ospf_lsa *); extern struct ospf_lsa *ospf_lsa_lock (struct ospf_lsa *); extern void ospf_lsa_unlock (struct ospf_lsa **); extern void ospf_lsa_discard (struct ospf_lsa *); - +extern int ospf_lsa_flush_schedule (struct ospf *, struct ospf_lsa *); extern struct lsa_header *ospf_lsa_data_new (size_t); extern struct lsa_header *ospf_lsa_data_dup (struct lsa_header *); extern void ospf_lsa_data_free (struct lsa_header *); diff --git a/ospfd/ospf_vty.c b/ospfd/ospf_vty.c index 109da255a..c3bbccda2 100644 --- a/ospfd/ospf_vty.c +++ b/ospfd/ospf_vty.c @@ -224,6 +224,8 @@ DEFUN (ospf_router_id, "OSPF router-id in IP address format\n") { struct ospf *ospf = vty->index; + struct listnode *node; + struct ospf_area *area; struct in_addr router_id; int ret; @@ -238,7 +240,15 @@ DEFUN (ospf_router_id, } ospf->router_id_static = router_id; - + + for (ALL_LIST_ELEMENTS_RO (ospf->areas, node, area)) + if (area->full_nbrs) + { + vty_out (vty, "For this router-id change to take effect," + " save config and restart ospfd%s", VTY_NEWLINE); + return CMD_SUCCESS; + } + ospf_router_id_update (ospf); return CMD_SUCCESS; @@ -258,12 +268,22 @@ DEFUN (no_ospf_router_id, "router-id for the OSPF process\n") { struct ospf *ospf = vty->index; + struct listnode *node; + struct ospf_area *area; if (!ospf) return CMD_SUCCESS; ospf->router_id_static.s_addr = 0; + for (ALL_LIST_ELEMENTS_RO (ospf->areas, node, area)) + if (area->full_nbrs) + { + vty_out (vty, "For this router-id change to take effect," + " save config and restart ospfd%s", VTY_NEWLINE); + return CMD_SUCCESS; + } + ospf_router_id_update (ospf); return CMD_SUCCESS; diff --git a/ospfd/ospfd.c b/ospfd/ospfd.c index 2feebfc15..73a86c218 100644 --- a/ospfd/ospfd.c +++ b/ospfd/ospfd.c @@ -84,6 +84,7 @@ ospf_router_id_update (struct ospf *ospf) struct ospf_interface *oi; struct interface *ifp; struct listnode *node; + int type; if (!ospf->oi_running) { @@ -111,13 +112,10 @@ ospf_router_id_update (struct ospf *ospf) else router_id = router_id_zebra; - ospf->router_id = router_id; - if (IS_DEBUG_OSPF_EVENT) - zlog_debug ("Router-ID[NEW:%s]: Update", inet_ntoa (ospf->router_id)); - - if (!IPV4_ADDR_SAME (&router_id_old, &router_id)) + if (!IPV4_ADDR_SAME (&router_id_old, &router_id)) { + for (ALL_LIST_ELEMENTS_RO (ospf->oiflist, node, oi)) /* Update self-neighbor's router_id. */ oi->nbr_self->router_id = router_id; @@ -125,7 +123,6 @@ ospf_router_id_update (struct ospf *ospf) /* If AS-external-LSA is queued, then flush those LSAs. */ if (router_id_old.s_addr == 0 && ospf->external_origin) { - int type; /* Originate each redistributed external route. */ for (type = 0; type < ZEBRA_ROUTE_MAX; type++) if (ospf->external_origin & (1 << type)) @@ -138,6 +135,50 @@ ospf_router_id_update (struct ospf *ospf) ospf->external_origin = 0; } + /* Flush (inline) all external LSAs based on the OSPF_LSA_SELF flag */ + if (ospf->lsdb) + { + struct route_node *rn; + struct ospf_lsa *lsa; + + LSDB_LOOP (EXTERNAL_LSDB (ospf), rn, lsa) + if (IS_LSA_SELF(lsa)) + ospf_lsa_flush_schedule(ospf, lsa); + } + + ospf->router_id = router_id; + if (IS_DEBUG_OSPF_EVENT) + zlog_debug ("Router-ID[NEW:%s]: Update", inet_ntoa (ospf->router_id)); + + /* Flush (inline) all external LSAs which now match the new router-id, + need to adjust the OSPF_LSA_SELF flag, so the flush doesnt hit + asserts in ospf_refresher_unregister_lsa(). This step is needed + because the current quagga code does look-up for self-originated LSAs + based on the self router-id alone but expects OSPF_LSA_SELF to be + properly set */ + if (ospf->lsdb) + { + struct route_node *rn; + struct ospf_lsa *lsa; + + LSDB_LOOP (EXTERNAL_LSDB (ospf), rn, lsa) + { + /* AdvRouter and Router ID is the same. */ + if (IPV4_ADDR_SAME (&lsa->data->adv_router, &ospf->router_id)) + { + SET_FLAG (lsa->flags, OSPF_LSA_SELF_CHECKED); + SET_FLAG (lsa->flags, OSPF_LSA_SELF); + ospf_lsa_flush_schedule(ospf, lsa); + } + } + } + + /* Originate each redistributed external route. */ + for (type = 0; type < ZEBRA_ROUTE_MAX; type++) + thread_add_event (master, ospf_external_lsa_originate_timer, + ospf, type); + thread_add_event (master, ospf_default_originate_timer, ospf, 0); + /* update router-lsa's for each area */ ospf_router_lsa_update (ospf); |