summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDonald Sharp <sharpd@cumulusnetworks.com>2015-05-20 03:04:07 +0200
committerDonald Sharp <sharpd@cumulusnetworks.com>2015-05-20 03:04:07 +0200
commit2c19a6ec62c6ed4450d237a467734fd24455a983 (patch)
treeae5661345b3cb23515421323b34aac05a022b95e
parentShow enabled debugs in the running configuration (diff)
downloadfrr-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.c2
-rw-r--r--ospfd/ospf_lsa.h2
-rw-r--r--ospfd/ospf_vty.c22
-rw-r--r--ospfd/ospfd.c53
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);