ospfd: Solve crash after removing and adding conf.

Issue number #6291 describes how OSPFd crashes after being deleted and then
added again with configuration when segment routing is used.

The problem occurs in ospf_ri.c because the OspfRI structures retains
the reference to the old area pointer which is mofified when ospfd is
reactivated by configuration. When segment routing is activated, the LSA Router
Information is sent with reference to the old area pointer, instead the new one,
which causes the crash. The same problem is also present in ospf_ext.c with
OspfEXT structure and Extended Link/Prefix structure.

This commit introduces Extended Link/Prefix and Router Information LSAs flusing
when OSPFd is stopped when configuration is removed and adds the correct
initialization to the area pointer in OspfRI and Extended Link/Prefix structure
when OSPFd is re-enabled with the configuration. Area pointer has been removed
from the OspfEXT structure as it is never used with this commit.

Signed-off-by: Olivier Dugeon <olivier.dugeon@orange.com>

diff --git a/ospfd/ospf_ext.c b/ospfd/ospf_ext.c
index 47883d5f398aa56fbfb5d276e60bee5d911dd283..1543e2015dd122f3ff9feb224ddc07d620d2d91b 100644 (file)
--- a/ospfd/ospf_ext.c
+++ b/ospfd/ospf_ext.c
@@ -80,7 +80,6 @@ static struct ospf_ext_lp OspfEXT;
  */
 
 /* Extended Prefix Opaque LSA related callback functions */
-static void ospf_ext_pref_ism_change(struct ospf_interface *oi, int old_status);
 static void ospf_ext_pref_show_info(struct vty *vty, struct ospf_lsa *lsa);
 static int ospf_ext_pref_lsa_originate(void *arg);
 static struct ospf_lsa *ospf_ext_pref_lsa_refresh(struct ospf_lsa *lsa);
@@ -89,7 +88,7 @@ static void ospf_ext_pref_lsa_schedule(struct ext_itf *exti,
 /* Extended Link Opaque LSA related callback functions */
 static int ospf_ext_link_new_if(struct interface *ifp);
 static int ospf_ext_link_del_if(struct interface *ifp);
-static void ospf_ext_link_ism_change(struct ospf_interface *oi, int old_status);
+static void ospf_ext_ism_change(struct ospf_interface *oi, int old_status);
 static void ospf_ext_link_nsm_change(struct ospf_neighbor *nbr, int old_status);
 static void ospf_ext_link_show_info(struct vty *vty, struct ospf_lsa *lsa);
 static int ospf_ext_link_lsa_originate(void *arg);
@@ -125,7 +124,7 @@ int ospf_ext_init(void)
                OSPF_OPAQUE_AREA_LSA, OPAQUE_TYPE_EXTENDED_LINK_LSA,
                ospf_ext_link_new_if,   /* new if */
                ospf_ext_link_del_if,   /* del if */
-               ospf_ext_link_ism_change,    /* ism change */
+               ospf_ext_ism_change,    /* ism change */
                ospf_ext_link_nsm_change,    /* nsm change */
                NULL,                        /* Write router config. */
                NULL,                        /* Write interface conf. */
@@ -148,7 +147,7 @@ int ospf_ext_init(void)
                OspfEXT.scope, OPAQUE_TYPE_EXTENDED_PREFIX_LSA,
                NULL,                        /* new if handle by link */
                NULL,                        /* del if handle by link */
-               ospf_ext_pref_ism_change,    /* ism change */
+               NULL,                        /* ism change */
                NULL,                        /* nsm change */
                ospf_sr_config_write_router, /* Write router config. */
                NULL,                        /* Write interface conf. */
@@ -200,7 +199,15 @@ void ospf_ext_term(void)
  */
 void ospf_ext_finish(void)
 {
-       // list_delete_all_node(OspfEXT.iflist);
+
+       struct listnode *node;
+       struct ext_itf *exti;
+
+       /* Flush Router Info LSA */
+       for (ALL_LIST_ELEMENTS_RO(OspfEXT.iflist, node, exti))
+               if (CHECK_FLAG(exti->flags, EXT_LPFLG_LSA_ENGAGED))
+                       ospf_ext_lsa_schedule(exti, FLUSH_THIS_LSA);
+
        OspfEXT.enabled = false;
 }
 
@@ -471,11 +478,15 @@ uint32_t ospf_ext_schedule_prefix_index(struct interface *ifp, uint32_t index,
                set_prefix_sid(exti, SR_ALGORITHM_SPF, index, SID_INDEX, flags);
 
                /* Try to Schedule LSA */
-               SET_FLAG(exti->flags, EXT_LPFLG_LSA_ACTIVE);
-               if (CHECK_FLAG(exti->flags, EXT_LPFLG_LSA_ENGAGED))
-                       ospf_ext_pref_lsa_schedule(exti, REFRESH_THIS_LSA);
-               else
-                       ospf_ext_pref_lsa_schedule(exti, REORIGINATE_THIS_LSA);
+               // SET_FLAG(exti->flags, EXT_LPFLG_LSA_ACTIVE);
+               if (CHECK_FLAG(exti->flags, EXT_LPFLG_LSA_ACTIVE)) {
+                       if (CHECK_FLAG(exti->flags, EXT_LPFLG_LSA_ENGAGED))
+                               ospf_ext_pref_lsa_schedule(exti,
+                                                          REFRESH_THIS_LSA);
+                       else
+                               ospf_ext_pref_lsa_schedule(
+                                       exti, REORIGINATE_THIS_LSA);
+               }
        } else {
                if (IS_DEBUG_OSPF_SR)
                        zlog_debug("EXT (%s): Remove prefix for interface %s",
@@ -483,8 +494,7 @@ uint32_t ospf_ext_schedule_prefix_index(struct interface *ifp, uint32_t index,
 
                if (CHECK_FLAG(exti->flags, EXT_LPFLG_LSA_ENGAGED)) {
                        ospf_ext_pref_lsa_schedule(exti, FLUSH_THIS_LSA);
-                       UNSET_FLAG(exti->flags, EXT_LPFLG_LSA_ENGAGED);
-                       UNSET_FLAG(exti->flags, EXT_LPFLG_LSA_ACTIVE);
+                       exti->flags = EXT_LPFLG_LSA_INACTIVE;
                }
        }
 
@@ -509,18 +519,26 @@ void ospf_ext_update_sr(bool enable)
 
        if (enable) {
                OspfEXT.enabled = true;
+
                /* Refresh LSAs if already engaged or originate */
-               for (ALL_LIST_ELEMENTS_RO(OspfEXT.iflist, node, exti))
+               for (ALL_LIST_ELEMENTS_RO(OspfEXT.iflist, node, exti)) {
+                       if (!CHECK_FLAG(exti->flags, EXT_LPFLG_LSA_ACTIVE))
+                               continue;
+
                        if (CHECK_FLAG(exti->flags, EXT_LPFLG_LSA_ENGAGED))
                                ospf_ext_lsa_schedule(exti, REFRESH_THIS_LSA);
                        else
                                ospf_ext_lsa_schedule(exti,
                                                      REORIGINATE_THIS_LSA);
+               }
        } else {
                /* Start by Flushing engaged LSAs */
-               for (ALL_LIST_ELEMENTS_RO(OspfEXT.iflist, node, exti))
+               for (ALL_LIST_ELEMENTS_RO(OspfEXT.iflist, node, exti)) {
                        if (CHECK_FLAG(exti->flags, EXT_LPFLG_LSA_ENGAGED))
                                ospf_ext_lsa_schedule(exti, FLUSH_THIS_LSA);
+                       exti->flags = EXT_LPFLG_LSA_INACTIVE;
+               }
+
                /* And then disable Extended Link/Prefix */
                OspfEXT.enabled = false;
        }
@@ -580,39 +598,10 @@ static int ospf_ext_link_del_if(struct interface *ifp)
 }
 
 /*
- * Determine if an Interface belongs to an Extended Link Adjacency or LAN Adj.
- * type and allocate new instance value accordingly
- */
-static void ospf_ext_link_ism_change(struct ospf_interface *oi, int old_status)
-{
-       struct ext_itf *exti;
-
-       /* Get interface information for Segment Routing */
-       exti = lookup_ext_by_ifp(oi->ifp);
-       if (exti == NULL)
-               return;
-
-       /* Determine if interface is related to Adjacency or LAN Adj. SID */
-       if (oi->type != OSPF_IFTYPE_LOOPBACK) {
-               if (oi->state == ISM_DR)
-                       exti->stype = LAN_ADJ_SID;
-               else
-                       exti->stype = ADJ_SID;
-
-               exti->instance = get_ext_link_instance_value();
-               exti->type = OPAQUE_TYPE_EXTENDED_LINK_LSA;
-
-               zlog_debug("EXT (%s): Set %s SID to interface %s ", __func__,
-                          exti->stype == ADJ_SID ? "Adj." : "LAN Adj.",
-                          oi->ifp->name);
-       }
-}
-
-/*
- * Determine if an Interface belongs to an Extended Prefix and
- * allocate new instance value accordingly
+ * Determine if an Interface belongs to an Extended Link Adjacency or
+ * Extended Prefix SID type and allocate new instance value accordingly
  */
-static void ospf_ext_pref_ism_change(struct ospf_interface *oi, int old_status)
+static void ospf_ext_ism_change(struct ospf_interface *oi, int old_status)
 {
        struct ext_itf *exti;
 
@@ -625,18 +614,45 @@ static void ospf_ext_pref_ism_change(struct ospf_interface *oi, int old_status)
                return;
        }
 
-       /* Determine if interface is related to a Node SID */
+       /* Reset Extended information if ospf interface goes Down */
+       if (oi->state == ISM_Down) {
+               exti->area = NULL;
+               exti->flags = EXT_LPFLG_LSA_INACTIVE;
+               return;
+       }
+
+       /* Determine if interface is related to a Prefix or an Adjacency SID */
        if (oi->type == OSPF_IFTYPE_LOOPBACK) {
                exti->stype = PREF_SID;
-               exti->instance = get_ext_pref_instance_value();
                exti->type = OPAQUE_TYPE_EXTENDED_PREFIX_LSA;
+               exti->flags = EXT_LPFLG_LSA_ACTIVE;
+               exti->instance = get_ext_pref_instance_value();
+               exti->area = oi->area;
 
-               zlog_debug("EXT (%s): Set Node SID to interface %s ", __func__,
-                          oi->ifp->name);
+               zlog_debug("EXT (%s): Set Prefix SID to interface %s ",
+                          __func__, oi->ifp->name);
 
                /* Complete SRDB if the interface belongs to a Prefix */
                if (OspfEXT.enabled)
                        ospf_sr_update_prefix(oi->ifp, oi->address);
+       } else {
+               /* Determine if interface is related to Adj. or LAN Adj. SID */
+               if (oi->state == ISM_DR)
+                       exti->stype = LAN_ADJ_SID;
+               else
+                       exti->stype = ADJ_SID;
+
+               exti->type = OPAQUE_TYPE_EXTENDED_LINK_LSA;
+               exti->instance = get_ext_link_instance_value();
+               exti->area = oi->area;
+
+               /*
+                * Note: Adjacency SID information are completed when ospf
+                * adjacency become up see ospf_ext_link_nsm_change()
+                */
+               zlog_debug("EXT (%s): Set %sAdjacency SID for interface %s ",
+                          __func__, exti->stype == ADJ_SID ? "" : "LAN-",
+                          oi->ifp->name);
        }
 }
 
@@ -663,17 +679,6 @@ static void ospf_ext_link_nsm_change(struct ospf_neighbor *nbr, int old_status)
                return;
        }
 
-       if (oi->area == NULL || oi->area->ospf == NULL) {
-               flog_warn(EC_OSPF_EXT_LSA_UNEXPECTED,
-                         "EXT (%s): Cannot refer to OSPF from OI(%s)",
-                         __func__, IF_NAME(oi));
-               return;
-       }
-
-       /* Keep Area information in combination with SR info. */
-       exti->area = oi->area;
-       OspfEXT.area = oi->area;
-
        /* Process only Adjacency/LAN SID */
        if (exti->stype == PREF_SID)
                return;
@@ -731,19 +736,17 @@ static void ospf_ext_link_nsm_change(struct ospf_neighbor *nbr, int old_status)
                break;
 
        default:
-               if (CHECK_FLAG(exti->flags, EXT_LPFLG_LSA_ENGAGED)) {
+               if (CHECK_FLAG(exti->flags, EXT_LPFLG_LSA_ENGAGED))
                        ospf_ext_link_lsa_schedule(exti, FLUSH_THIS_LSA);
-                       UNSET_FLAG(exti->flags, EXT_LPFLG_LSA_ENGAGED);
-                       UNSET_FLAG(exti->flags, EXT_LPFLG_LSA_ACTIVE);
-               }
+               exti->flags = EXT_LPFLG_LSA_INACTIVE;
                return;
        }
 
        if (IS_DEBUG_OSPF_SR)
-               zlog_debug("EXT (%s): Complete %s SID to interface %s ",
-                          __func__,
-                          exti->stype == ADJ_SID ? "Adj." : "LAN Adj.",
-                          oi->ifp->name);
+               zlog_debug(
+                       "EXT (%s): Complete %sAdjacency SID for interface %s ",
+                       __func__, exti->stype == ADJ_SID ? "" : "LAN-",
+                       oi->ifp->name);
 
        /* flood this links params if everything is ok */
        SET_FLAG(exti->flags, EXT_LPFLG_LSA_ACTIVE);
@@ -1244,6 +1247,10 @@ static int ospf_ext_link_lsa_originate(void *arg)
                    || (!IPV4_ADDR_SAME(&exti->area->area_id, &area->area_id)))
                        continue;
 
+               /* Skip Extended Link which are not Active */
+               if (!CHECK_FLAG(exti->flags, EXT_LPFLG_LSA_ACTIVE))
+                       continue;
+
                /* Check if LSA not already engaged */
                if (CHECK_FLAG(exti->flags, EXT_LPFLG_LSA_ENGAGED)) {
                        if (CHECK_FLAG(exti->flags,
@@ -1467,19 +1474,23 @@ static void ospf_ext_pref_lsa_schedule(struct ext_itf *exti,
                   opcode == FLUSH_THIS_LSA ? "Flush" : "",
                   exti->ifp ? exti->ifp->name : "-");
 
-       /* Set LSA header information */
+       /* Verify Area */
        if (exti->area == NULL) {
-               flog_warn(
-                       EC_OSPF_EXT_LSA_UNEXPECTED,
-                       "EXT (%s): Flooding is Area scope but area is not yet set",
-                       __func__);
-               if (OspfEXT.area == NULL) {
-                       top = ospf_lookup_by_vrf_id(VRF_DEFAULT);
-                       OspfEXT.area = ospf_area_lookup_by_area_id(
-                               top, OspfEXT.area_id);
+               if (IS_DEBUG_OSPF(lsa, LSA_GENERATE))
+                       zlog_debug(
+                               "EXT (%s): Area is not yet set. Try to use Backbone Area",
+                               __func__);
+
+               top = ospf_lookup_by_vrf_id(VRF_DEFAULT);
+               struct in_addr backbone = {.s_addr = INADDR_ANY};
+               exti->area = ospf_area_lookup_by_area_id(top, backbone);
+               if (exti->area == NULL) {
+                       flog_warn(EC_OSPF_EXT_LSA_UNEXPECTED,
+                                 "EXT (%s): Unable to set Area", __func__);
+                       return;
                }
-               exti->area = OspfEXT.area;
        }
+       /* Set LSA header information */
        lsa.area = exti->area;
        lsa.data = &lsah;
        lsah.type = OSPF_OPAQUE_AREA_LSA;
@@ -1528,19 +1539,23 @@ static void ospf_ext_link_lsa_schedule(struct ext_itf *exti,
                   opcode == FLUSH_THIS_LSA ? "Flush" : "",
                   exti->ifp ? exti->ifp->name : "-");
 
-       /* Set LSA header information */
+       /* Verify Area */
        if (exti->area == NULL) {
-               flog_warn(
-                       EC_OSPF_EXT_LSA_UNEXPECTED,
-                       "EXT (%s): Flooding is Area scope but area is not yet set",
-                       __func__);
-               if (OspfEXT.area == NULL) {
-                       top = ospf_lookup_by_vrf_id(VRF_DEFAULT);
-                       OspfEXT.area = ospf_area_lookup_by_area_id(
-                               top, OspfEXT.area_id);
+               if (IS_DEBUG_OSPF(lsa, LSA_GENERATE))
+                       zlog_debug(
+                               "EXT (%s): Area is not yet set. Try to use Backbone Area",
+                               __func__);
+
+               top = ospf_lookup_by_vrf_id(VRF_DEFAULT);
+               struct in_addr backbone = {.s_addr = INADDR_ANY};
+               exti->area = ospf_area_lookup_by_area_id(top, backbone);
+               if (exti->area == NULL) {
+                       flog_warn(EC_OSPF_EXT_LSA_UNEXPECTED,
+                                 "EXT (%s): Unable to set Area", __func__);
+                       return;
                }
-               exti->area = OspfEXT.area;
        }
+       /* Set LSA header information */
        lsa.area = exti->area;
        lsa.data = &lsah;
        lsah.type = OSPF_OPAQUE_AREA_LSA;
@@ -1557,7 +1572,6 @@ static void ospf_ext_link_lsa_schedule(struct ext_itf *exti,
                ospf_opaque_lsa_refresh_schedule(&lsa);
                break;
        case FLUSH_THIS_LSA:
-               UNSET_FLAG(exti->flags, EXT_LPFLG_LSA_ENGAGED);
                ospf_opaque_lsa_flush_schedule(&lsa);
                break;
        }

diff --git a/ospfd/ospf_ext.h b/ospfd/ospf_ext.h
index c3f9ae94dcdb54b08aff04ed86a3cd94f34d28c5..0071584e2685b9c637d2fd17a7b707fb907058c3 100644 (file)
--- a/ospfd/ospf_ext.h
+++ b/ospfd/ospf_ext.h
@@ -151,10 +151,6 @@ struct ospf_ext_lp {
         */
        uint8_t scope;
 
-       /* area pointer if flooding is Type 10 Null if flooding is AS scope */
-       struct ospf_area *area;
-       struct in_addr area_id;
-
        /* List of interface with Segment Routing enable */
        struct list *iflist;
 };

diff --git a/ospfd/ospf_ri.c b/ospfd/ospf_ri.c
index c3d53ad5ed7969496b2fc91fb2aa0c8ec21201ae..ececed06430dd6a9595c9e952e263654038aedd7 100644 (file)
--- a/ospfd/ospf_ri.c
+++ b/ospfd/ospf_ri.c
@@ -178,6 +178,14 @@ void ospf_router_info_term(void)
 
 void ospf_router_info_finish(void)
 {
+       struct listnode *node, *nnode;
+       struct ospf_ri_area_info *ai;
+
+       /* Flush Router Info LSA */
+       for (ALL_LIST_ELEMENTS(OspfRI.area_info, node, nnode, ai))
+               if (CHECK_FLAG(ai->flags, RIFLG_LSA_ENGAGED))
+                       ospf_router_info_lsa_schedule(ai, FLUSH_THIS_LSA);
+
        list_delete_all_node(OspfRI.pce_info.pce_domain);
        list_delete_all_node(OspfRI.pce_info.pce_neighbor);
 
@@ -510,6 +518,8 @@ static void initialize_params(struct ospf_router_info *ori)
        /* Try to get available Area's context from ospf at this step.
         * Do it latter if not available */
        if (OspfRI.scope == OSPF_OPAQUE_AREA_LSA) {
+               if (!list_isempty(OspfRI.area_info))
+                       list_delete_all_node(OspfRI.area_info);
                for (ALL_LIST_ELEMENTS(top->areas, node, nnode, area)) {
                        zlog_debug("RI (%s): Add area %s to Router Information",
                                __func__, inet_ntoa(area->area_id));

diff --git a/tests/topotests/ospf-sr-topo1/r2/ospfd.conf b/tests/topotests/ospf-sr-topo1/r2/ospfd.conf
index 8555ea29f1da8ecda10113245460887c4e240741..4d6146aaa7691746490ef955b55fa9017bcc13ea 100644 (file)
--- a/tests/topotests/ospf-sr-topo1/r2/ospfd.conf
+++ b/tests/topotests/ospf-sr-topo1/r2/ospfd.conf
@@ -1,4 +1,6 @@
 !
+debug ospf sr
+!
 interface lo
   ip ospf area 0.0.0.0
 !