lib: Changes made to dependencies of a r-map do not take effect.

Say, more than one sequence of a route-map uses the same named entity
in its match clause. After that entity is removed from any one of the
route-map sequences, any further changes made to that entity doesn't
dynamically take effect.
A reference counter, that allows the named entity to keep a count of
the route-maps dependent on it,  has been introduced to address this issue.

Signed-off-by: NaveenThanikachalam <nthanikachal@vmware.com>

diff --git a/bgpd/bgp_routemap.c b/bgpd/bgp_routemap.c
index 85cab0d59fe72d1a1dc6f474bbfbed143a72ffa2..329db187fe1179fd034e7aed9b77a1ec760a8ce8 100644 (file)
--- a/bgpd/bgp_routemap.c
+++ b/bgpd/bgp_routemap.c
@@ -2975,7 +2975,7 @@ static int bgp_route_match_add(struct vty *vty, const char *command,
        int retval = CMD_SUCCESS;
        int ret;
 
-       ret = route_map_add_match(index, command, arg);
+       ret = route_map_add_match(index, command, arg, type);
        switch (ret) {
        case RMAP_RULE_MISSING:
                vty_out(vty, "%% BGP Can't find rule.\n");

diff --git a/bgpd/bgp_rpki.c b/bgpd/bgp_rpki.c
index 27b6347ab6303301037a74e6e1bd5a33d6bfee84..ab93ade5629138cfd10b18f7877f6f832ef221a2 100644 (file)
--- a/bgpd/bgp_rpki.c
+++ b/bgpd/bgp_rpki.c
@@ -1407,7 +1407,8 @@ DEFUN (match_rpki,
        VTY_DECLVAR_CONTEXT(route_map_index, index);
        int ret;
 
-       ret = route_map_add_match(index, "rpki", argv[2]->arg);
+       ret = route_map_add_match(index, "rpki", argv[2]->arg,
+                                 RMAP_EVENT_MATCH_ADDED);
        if (ret) {
                switch (ret) {
                case RMAP_RULE_MISSING:

diff --git a/eigrpd/eigrp_routemap.c b/eigrpd/eigrp_routemap.c
index 60323dcd04db7009a41c1eb9e92a6114c4bb7d1d..961556d432931903111ff1b5b29ee15ec84fe5d9 100644 (file)
--- a/eigrpd/eigrp_routemap.c
+++ b/eigrpd/eigrp_routemap.c
@@ -136,7 +136,7 @@ static int eigrp_route_match_add(struct vty *vty, struct route_map_index *index,
                                 const char *command, const char *arg)
 {
        int ret;
-       ret = route_map_add_match(index, command, arg);
+       ret = route_map_add_match(index, command, arg, type);
        switch (ret) {
        case RMAP_RULE_MISSING:
                vty_out(vty, "%% Can't find rule.\n");

diff --git a/lib/routemap.c b/lib/routemap.c
index e8f4a6c1aa4025b1866c22e2aaf2599c5ba59a1f..807eec9c5951d58ca4ef0db9deac656f2ba39c3c 100644 (file)
--- a/lib/routemap.c
+++ b/lib/routemap.c
@@ -39,6 +39,7 @@ DEFINE_MTYPE(LIB, ROUTE_MAP_RULE, "Route map rule")
 DEFINE_MTYPE_STATIC(LIB, ROUTE_MAP_RULE_STR, "Route map rule str")
 DEFINE_MTYPE(LIB, ROUTE_MAP_COMPILED, "Route map compiled")
 DEFINE_MTYPE_STATIC(LIB, ROUTE_MAP_DEP, "Route map dependency")
+DEFINE_MTYPE_STATIC(LIB, ROUTE_MAP_DEP_DATA, "Route map dependency data")
 
 DEFINE_QOBJ_TYPE(route_map_index)
 DEFINE_QOBJ_TYPE(route_map)
@@ -475,7 +476,7 @@ int generic_match_add(struct vty *vty, struct route_map_index *index,
 {
        int ret;
 
-       ret = route_map_add_match(index, command, arg);
+       ret = route_map_add_match(index, command, arg, type);
        switch (ret) {
        case RMAP_COMPILE_SUCCESS:
                if (type != RMAP_EVENT_MATCH_ADDED) {
@@ -670,6 +671,16 @@ struct route_map_dep {
        struct hash *this_hash; /* ptr to the hash structure this is part of */
 };
 
+struct route_map_dep_data {
+       /* Route-map name.
+        */
+       char *rname;
+       /* Count of number of sequences of this
+        * route-map that depend on the same entity.
+        */
+       uint16_t  refcnt;
+};
+
 /* Hashes maintaining dependency between various sublists used by route maps */
 struct hash *route_map_dep_hash[ROUTE_MAP_DEP_MAX];
 
@@ -1281,14 +1292,58 @@ const char *route_map_get_match_arg(struct route_map_index *index,
        return (NULL);
 }
 
+static route_map_event_t get_route_map_delete_event(route_map_event_t type)
+{
+       switch (type) {
+       case RMAP_EVENT_CALL_ADDED:
+               return RMAP_EVENT_CALL_DELETED;
+       case RMAP_EVENT_PLIST_ADDED:
+               return RMAP_EVENT_PLIST_DELETED;
+       case RMAP_EVENT_CLIST_ADDED:
+               return RMAP_EVENT_CLIST_DELETED;
+       case RMAP_EVENT_ECLIST_ADDED:
+               return RMAP_EVENT_ECLIST_DELETED;
+       case RMAP_EVENT_LLIST_ADDED:
+               return RMAP_EVENT_LLIST_DELETED;
+       case RMAP_EVENT_ASLIST_ADDED:
+               return RMAP_EVENT_ASLIST_DELETED;
+       case RMAP_EVENT_FILTER_ADDED:
+               return RMAP_EVENT_FILTER_DELETED;
+       case RMAP_EVENT_SET_ADDED:
+       case RMAP_EVENT_SET_DELETED:
+       case RMAP_EVENT_SET_REPLACED:
+       case RMAP_EVENT_MATCH_ADDED:
+       case RMAP_EVENT_MATCH_DELETED:
+       case RMAP_EVENT_MATCH_REPLACED:
+       case RMAP_EVENT_INDEX_ADDED:
+       case RMAP_EVENT_INDEX_DELETED:
+       case RMAP_EVENT_CALL_DELETED:
+       case RMAP_EVENT_PLIST_DELETED:
+       case RMAP_EVENT_CLIST_DELETED:
+       case RMAP_EVENT_ECLIST_DELETED:
+       case RMAP_EVENT_LLIST_DELETED:
+       case RMAP_EVENT_ASLIST_DELETED:
+       case RMAP_EVENT_FILTER_DELETED:
+               /* This function returns the appropriate 'deleted' event type
+                * for every 'added' event type passed to this function.
+                * This is done only for named entities used in the
+                * route-map match commands.
+                * This function is not to be invoked for any of the other event
+                * types.
+                */
+               assert(0);
+       }
+}
+
 /* Add match statement to route map. */
 int route_map_add_match(struct route_map_index *index, const char *match_name,
-                       const char *match_arg)
+                       const char *match_arg, route_map_event_t type)
 {
        struct route_map_rule *rule;
        struct route_map_rule *next;
        struct route_map_rule_cmd *cmd;
        void *compile;
+       int8_t delete_rmap_event_type = 0;
 
        /* First lookup rule for add match statement. */
        cmd = route_map_lookup_match(match_name);
@@ -1314,7 +1369,20 @@ int route_map_add_match(struct route_map_index *index, const char *match_name,
                        if (strcmp(match_arg, rule->rule_str) == 0) {
                                if (cmd->func_free)
                                        (*cmd->func_free)(compile);
-                               return RMAP_COMPILE_SUCCESS;
+
+                               return RMAP_DUPLICATE_RULE;
+                       }
+
+                       /* Remove the dependency of the route-map on the rule
+                        * that is being replaced.
+                        */
+                       if (type >= RMAP_EVENT_CALL_ADDED) {
+                               delete_rmap_event_type =
+                                       get_route_map_delete_event(type);
+                               route_map_upd8_dependency(
+                                                       delete_rmap_event_type,
+                                                       rule->rule_str,
+                                                       index->map->name);
                        }
 
                        route_map_rule_delete(&index->match_list, rule);
@@ -1658,7 +1726,9 @@ void route_map_event_hook(void (*func)(const char *name))
 /* Routines for route map dependency lists and dependency processing */
 static bool route_map_rmap_hash_cmp(const void *p1, const void *p2)
 {
-       return (strcmp((const char *)p1, (const char *)p2) == 0);
+       return strcmp(((const struct route_map_dep_data *)p1)->rname,
+                     ((const struct route_map_dep_data *)p2)->rname)
+              == 0;
 }
 
 static bool route_map_dep_hash_cmp(const void *p1, const void *p2)
@@ -1671,14 +1741,17 @@ static bool route_map_dep_hash_cmp(const void *p1, const void *p2)
 
 static void route_map_clear_reference(struct hash_bucket *bucket, void *arg)
 {
-       struct route_map_dep *dep = (struct route_map_dep *)bucket->data;
-       char *rmap_name;
+       struct route_map_dep *dep = bucket->data;
+       struct route_map_dep_data *dep_data = NULL, tmp_dep_data;
 
        if (arg) {
-               rmap_name =
-                       (char *)hash_release(dep->dep_rmap_hash, (void *)arg);
-               if (rmap_name) {
-                       XFREE(MTYPE_ROUTE_MAP_NAME, rmap_name);
+               memset(&tmp_dep_data, 0, sizeof(struct route_map_dep_data));
+               tmp_dep_data.rname = arg;
+               dep_data = hash_release(dep->dep_rmap_hash,
+                                       &tmp_dep_data);
+               if (dep_data) {
+                       XFREE(MTYPE_ROUTE_MAP_NAME, dep_data->rname);
+                       XFREE(MTYPE_ROUTE_MAP_DEP_DATA, dep_data);
                }
                if (!dep->dep_rmap_hash->count) {
                        dep = hash_release(dep->this_hash,
@@ -1700,6 +1773,13 @@ static void route_map_clear_all_references(char *rmap_name)
        }
 }
 
+static unsigned int route_map_dep_data_hash_make_key(const void *p)
+{
+       const struct route_map_dep_data *dep_data = p;
+
+       return string_hash_make(dep_data->rname);
+}
+
 static void *route_map_dep_hash_alloc(void *p)
 {
        char *dep_name = (char *)p;
@@ -1708,16 +1788,22 @@ static void *route_map_dep_hash_alloc(void *p)
        dep_entry = XCALLOC(MTYPE_ROUTE_MAP_DEP, sizeof(struct route_map_dep));
        dep_entry->dep_name = XSTRDUP(MTYPE_ROUTE_MAP_NAME, dep_name);
        dep_entry->dep_rmap_hash =
-               hash_create_size(8, route_map_dep_hash_make_key,
+               hash_create_size(8, route_map_dep_data_hash_make_key,
                                 route_map_rmap_hash_cmp, "Route Map Dep Hash");
        dep_entry->this_hash = NULL;
 
-       return ((void *)dep_entry);
+       return dep_entry;
 }
 
 static void *route_map_name_hash_alloc(void *p)
 {
-       return ((void *)XSTRDUP(MTYPE_ROUTE_MAP_NAME, (const char *)p));
+       struct route_map_dep_data *dep_data = NULL, *tmp_dep_data = NULL;
+
+       dep_data = XCALLOC(MTYPE_ROUTE_MAP_DEP_DATA,
+                          sizeof(struct route_map_dep_data));
+       tmp_dep_data = p;
+       dep_data->rname = XSTRDUP(MTYPE_ROUTE_MAP_NAME, tmp_dep_data->rname);
+       return dep_data;
 }
 
 static unsigned int route_map_dep_hash_make_key(const void *p)
@@ -1727,8 +1813,9 @@ static unsigned int route_map_dep_hash_make_key(const void *p)
 
 static void route_map_print_dependency(struct hash_bucket *bucket, void *data)
 {
-       char *rmap_name = (char *)bucket->data;
-       char *dep_name = (char *)data;
+       struct route_map_dep_data *dep_data = bucket->data;
+       char *rmap_name = dep_data->rname;
+       char *dep_name = data;
 
        zlog_debug("%s: Dependency for %s: %s", __FUNCTION__, dep_name,
                   rmap_name);
@@ -1738,9 +1825,10 @@ static int route_map_dep_update(struct hash *dephash, const char *dep_name,
                                const char *rmap_name, route_map_event_t type)
 {
        struct route_map_dep *dep = NULL;
-       char *ret_map_name;
        char *dname, *rname;
        int ret = 0;
+       struct route_map_dep_data *dep_data = NULL, *ret_dep_data = NULL;
+       struct route_map_dep_data tmp_dep_data;
 
        dname = XSTRDUP(MTYPE_ROUTE_MAP_NAME, dep_name);
        rname = XSTRDUP(MTYPE_ROUTE_MAP_NAME, rmap_name);
@@ -1766,7 +1854,14 @@ static int route_map_dep_update(struct hash *dephash, const char *dep_name,
                if (!dep->this_hash)
                        dep->this_hash = dephash;
 
-               hash_get(dep->dep_rmap_hash, rname, route_map_name_hash_alloc);
+               memset(&tmp_dep_data, 0, sizeof(struct route_map_dep_data));
+               tmp_dep_data.rname = rname;
+               dep_data = hash_lookup(dep->dep_rmap_hash, &tmp_dep_data);
+               if (!dep_data)
+                       dep_data = hash_get(dep->dep_rmap_hash, &tmp_dep_data,
+                                           route_map_name_hash_alloc);
+
+               dep_data->refcnt++;
                break;
        case RMAP_EVENT_PLIST_DELETED:
        case RMAP_EVENT_CLIST_DELETED:
@@ -1783,8 +1878,20 @@ static int route_map_dep_update(struct hash *dephash, const char *dep_name,
                        goto out;
                }
 
-               ret_map_name = (char *)hash_release(dep->dep_rmap_hash, rname);
-               XFREE(MTYPE_ROUTE_MAP_NAME, ret_map_name);
+               memset(&tmp_dep_data, 0, sizeof(struct route_map_dep_data));
+               tmp_dep_data.rname = rname;
+               dep_data = hash_lookup(dep->dep_rmap_hash, &tmp_dep_data);
+               dep_data->refcnt--;
+
+               if (!dep_data->refcnt) {
+                       ret_dep_data = hash_release(dep->dep_rmap_hash,
+                                                   &tmp_dep_data);
+                       if (ret_dep_data) {
+                               XFREE(MTYPE_ROUTE_MAP_NAME,
+                                     ret_dep_data->rname);
+                               XFREE(MTYPE_ROUTE_MAP_DEP_DATA, ret_dep_data);
+                       }
+               }
 
                if (!dep->dep_rmap_hash->count) {
                        dep = hash_release(dephash, dname);
@@ -1872,7 +1979,11 @@ static struct hash *route_map_get_dep_hash(route_map_event_t event)
 
 static void route_map_process_dependency(struct hash_bucket *bucket, void *data)
 {
-       char *rmap_name = (char *)bucket->data;
+       struct route_map_dep_data *dep_data = NULL;
+       char *rmap_name = NULL;
+
+       dep_data = bucket->data;
+       rmap_name = dep_data->rname;
 
        if (rmap_debug)
                zlog_debug("%s: Notifying %s of dependency",

diff --git a/lib/routemap.h b/lib/routemap.h
index 41b8428fa15dc9d0c590f823e3918bfa1a7af454..d7acd7f3f7103657f0ebae21a68c00afd253969d 100644 (file)
--- a/lib/routemap.h
+++ b/lib/routemap.h
@@ -125,13 +125,18 @@ struct route_map_rule_cmd {
 };
 
 /* Route map apply error. */
-enum { RMAP_COMPILE_SUCCESS,
+enum {
+       RMAP_COMPILE_SUCCESS,
 
-       /* Route map rule is missing. */
-       RMAP_RULE_MISSING,
+       /* Route map rule is missing. */
+       RMAP_RULE_MISSING,
 
-       /* Route map rule can't compile */
-       RMAP_COMPILE_ERROR };
+       /* Route map rule can't compile */
+       RMAP_COMPILE_ERROR,
+
+       /* Route map rule is duplicate */
+       RMAP_DUPLICATE_RULE
+};
 
 /* Route map rule list. */
 struct route_map_rule_list {
@@ -213,7 +218,8 @@ extern void route_map_finish(void);
 
 /* Add match statement to route map. */
 extern int route_map_add_match(struct route_map_index *index,
-                              const char *match_name, const char *match_arg);
+                              const char *match_name, const char *match_arg,
+                              route_map_event_t type);
 
 /* Delete specified route match rule. */
 extern int route_map_delete_match(struct route_map_index *index,

diff --git a/zebra/zebra_routemap.c b/zebra/zebra_routemap.c
index 51a644178776d494dae0aadbca8a908d7abb6818..78804635fab0395b5238fc3eab5a461dba139530 100644 (file)
--- a/zebra/zebra_routemap.c
+++ b/zebra/zebra_routemap.c
@@ -67,7 +67,7 @@ static int zebra_route_match_add(struct vty *vty, const char *command,
        int ret;
        int retval = CMD_SUCCESS;
 
-       ret = route_map_add_match(index, command, arg);
+       ret = route_map_add_match(index, command, arg, type);
        switch (ret) {
        case RMAP_RULE_MISSING:
                vty_out(vty, "%% Zebra Can't find rule.\n");