bgpd: fix "bgp as-pah access-list" with "set aspath exclude" set/unset issues

whith the following config

router bgp 65001
 no bgp ebgp-requires-policy
 neighbor 192.168.1.2 remote-as external
 neighbor 192.168.1.2 timers 3 10
 !
 address-family ipv4 unicast
  neighbor 192.168.1.2 route-map r2 in
 exit-address-family
exit
!
bgp as-path access-list FIRST seq 5 permit ^65
bgp as-path access-list SECOND seq 5 permit 2$
!
route-map r2 permit 6
 match ip address prefix-list p2
 set as-path exclude as-path-access-list SECOND
exit
!
route-map r2 permit 10
 match ip address prefix-list p1
 set as-path exclude 65003
exit
!
route-map r2 permit 20
 match ip address prefix-list p3
 set as-path exclude all
exit

making some
no bgp as-path access-list SECOND permit 2$
bgp as-path access-list SECOND permit 3$

clear bgp *

no bgp as-path access-list SECOND permit 3$
bgp as-path access-list SECOND permit 2$

clear bgp *

will induce some crashes

thus  we rework the links between aslists and aspath_exclude

Signed-off-by: Francois Dumontet <francois.dumontet@6wind.com>
(cherry picked from commit 094dcc3cdac19d3da65b38effc45aa88d960909f)

diff --git a/bgpd/bgp_aspath.c b/bgpd/bgp_aspath.c
index bc7e8939b42aaed717f9f0d2d9595d68b728e2b4..c044ad0a1d9a7e1f980145768ba4240259fc4a7f 100644 (file)
--- a/bgpd/bgp_aspath.c
+++ b/bgpd/bgp_aspath.c
@@ -77,6 +77,9 @@ static struct hash *ashash;
 /* Stream for SNMP. See aspath_snmp_pathseg */
 static struct stream *snmp_stream;
 
+/* as-path orphan exclude list */
+static struct as_list_list_head as_exclude_list_orphan;
+
 /* Callers are required to initialize the memory */
 static as_t *assegment_data_new(int num)
 {
@@ -1558,6 +1561,38 @@ struct aspath *aspath_prepend(struct aspath *as1, struct aspath *as2)
        /* Not reached */
 }
 
+/* insert aspath exclude in head of orphan exclude list*/
+void as_exclude_set_orphan(struct aspath_exclude *ase)
+{
+       ase->exclude_aspath_acl = NULL;
+       as_list_list_add_head(&as_exclude_list_orphan, ase);
+}
+
+void as_exclude_remove_orphan(struct aspath_exclude *ase)
+{
+       if (as_list_list_count(&as_exclude_list_orphan))
+               as_list_list_del(&as_exclude_list_orphan, ase);
+}
+
+/* currently provide only one exclude, not a list */
+struct aspath_exclude *as_exclude_lookup_orphan(const char *acl_name)
+{
+       struct aspath_exclude *ase = NULL;
+       char *name = NULL;
+
+       frr_each (as_list_list, &as_exclude_list_orphan, ase) {
+               if (ase->exclude_aspath_acl_name) {
+                       name = ase->exclude_aspath_acl_name;
+                       if (!strcmp(name, acl_name))
+                               break;
+               }
+       }
+       if (ase)
+               as_exclude_remove_orphan(ase);
+
+       return ase;
+}
+
 /* Iterate over AS_PATH segments and wipe all occurrences of the
  * listed AS numbers. Hence some segments may lose some or even
  * all data on the way, the operation is implemented as a smarter
@@ -2236,14 +2271,26 @@ void aspath_init(void)
 {
        ashash = hash_create_size(32768, aspath_key_make, aspath_cmp,
                                  "BGP AS Path");
+
+       as_list_list_init(&as_exclude_list_orphan);
 }
 
 void aspath_finish(void)
 {
+       struct aspath_exclude *ase;
+
        hash_clean_and_free(&ashash, (void (*)(void *))aspath_free);
 
        if (snmp_stream)
                stream_free(snmp_stream);
+
+       while ((ase = as_list_list_pop(&as_exclude_list_orphan))) {
+               aspath_free(ase->aspath);
+               if (ase->exclude_aspath_acl_name)
+                       XFREE(MTYPE_TMP, ase->exclude_aspath_acl_name);
+               XFREE(MTYPE_ROUTE_MAP_COMPILED, ase);
+       }
+       as_list_list_fini(&as_exclude_list_orphan);
 }
 
 /* return and as path value */

diff --git a/bgpd/bgp_aspath.h b/bgpd/bgp_aspath.h
index 2a831c3a551088c372aff687b44fc44f87271219..f7e57fd66ddafbc04612919aaca5cc07dde4d5b6 100644 (file)
--- a/bgpd/bgp_aspath.h
+++ b/bgpd/bgp_aspath.h
@@ -9,6 +9,7 @@
 #include "lib/json.h"
 #include "bgpd/bgp_route.h"
 #include "bgpd/bgp_filter.h"
+#include <typesafe.h>
 
 /* AS path segment type.  */
 #define AS_SET                       1
@@ -67,11 +68,14 @@ struct aspath {
 
 /* `set as-path exclude ASn' */
 struct aspath_exclude {
+       struct as_list_list_item exclude_list;
        struct aspath *aspath;
        bool exclude_all;
        char *exclude_aspath_acl_name;
        struct as_list *exclude_aspath_acl;
 };
+DECLARE_DLIST(as_list_list, struct aspath_exclude, exclude_list);
+
 
 /* Prototypes. */
 extern void aspath_init(void);
@@ -83,6 +87,9 @@ extern struct aspath *aspath_parse(struct stream *s, size_t length,
 extern struct aspath *aspath_dup(struct aspath *aspath);
 extern struct aspath *aspath_aggregate(struct aspath *as1, struct aspath *as2);
 extern struct aspath *aspath_prepend(struct aspath *as1, struct aspath *as2);
+extern void as_exclude_set_orphan(struct aspath_exclude *ase);
+extern void as_exclude_remove_orphan(struct aspath_exclude *ase);
+extern struct aspath_exclude *as_exclude_lookup_orphan(const char *acl_name);
 extern struct aspath *aspath_filter_exclude(struct aspath *source,
                                            struct aspath *exclude_list);
 extern struct aspath *aspath_filter_exclude_all(struct aspath *source);

diff --git a/bgpd/bgp_filter.c b/bgpd/bgp_filter.c
index a85117965aaab2f2f93301a464b114f75292412a..002f054f5e412190dcd0f66a68ed07cc5cc3b21d 100644 (file)
--- a/bgpd/bgp_filter.c
+++ b/bgpd/bgp_filter.c
@@ -16,7 +16,7 @@
 #include "bgpd/bgp_aspath.h"
 #include "bgpd/bgp_regex.h"
 
-/* List of AS filter list. */
+/* List of AS list. */
 struct as_list_list {
        struct as_list *head;
        struct as_list *tail;
@@ -205,14 +205,6 @@ static struct as_list *as_list_new(void)
 
 static void as_list_free(struct as_list *aslist)
 {
-       struct aspath_exclude_list *cur_bp = aslist->exclude_list;
-       struct aspath_exclude_list *next_bp = NULL;
-
-       while (cur_bp) {
-               next_bp = cur_bp->next;
-               XFREE(MTYPE_ROUTE_MAP_COMPILED, cur_bp);
-               cur_bp = next_bp;
-       }
 
        XFREE (MTYPE_AS_STR, aslist->name);
        XFREE (MTYPE_AS_LIST, aslist);
@@ -299,7 +291,6 @@ static void as_list_delete(struct as_list *aslist)
 {
        struct as_list_list *list;
        struct as_filter *filter, *next;
-       struct aspath_exclude_list *cur_bp;
 
        for (filter = aslist->head; filter; filter = next) {
                next = filter->next;
@@ -318,12 +309,6 @@ static void as_list_delete(struct as_list *aslist)
        else
                list->head = aslist->next;
 
-       cur_bp = aslist->exclude_list;
-       while (cur_bp) {
-               cur_bp->bp_as_excl->exclude_aspath_acl = NULL;
-               cur_bp = cur_bp->next;
-       }
-
        as_list_free(aslist);
 }
 
@@ -431,6 +416,7 @@ DEFUN(as_path, bgp_as_path_cmd,
        enum as_filter_type type;
        struct as_filter *asfilter;
        struct as_list *aslist;
+       struct aspath_exclude *ase;
        regex_t *regex;
        char *regstr;
        int64_t seqnum = ASPATH_SEQ_NUMBER_AUTO;
@@ -482,6 +468,22 @@ DEFUN(as_path, bgp_as_path_cmd,
        else
                as_list_filter_add(aslist, asfilter);
 
+       /* init the exclude rule list*/
+       as_list_list_init(&aslist->exclude_rule);
+
+       /* get aspath orphan exclude that are using this acl */
+       ase = as_exclude_lookup_orphan(alname);
+       if (ase) {
+               as_list_list_add_head(&aslist->exclude_rule, ase);
+               /* set reverse pointer */
+               ase->exclude_aspath_acl = aslist;
+               /* set list of aspath excludes using that acl */
+               while ((ase = as_exclude_lookup_orphan(alname))) {
+                       as_list_list_add_head(&aslist->exclude_rule, ase);
+                       ase->exclude_aspath_acl = aslist;
+               }
+       }
+
        return CMD_SUCCESS;
 }
 
@@ -502,6 +504,7 @@ DEFUN(no_as_path, no_bgp_as_path_cmd,
        enum as_filter_type type;
        struct as_filter *asfilter;
        struct as_list *aslist;
+       struct aspath_exclude *ase;
        char *regstr;
        regex_t *regex;
 
@@ -556,6 +559,12 @@ DEFUN(no_as_path, no_bgp_as_path_cmd,
 
        XFREE(MTYPE_TMP, regstr);
 
+       /* put aspath exclude list into orphan */
+       if (as_list_list_count(&aslist->exclude_rule))
+               while ((ase = as_list_list_pop(&aslist->exclude_rule)))
+                       as_exclude_set_orphan(ase);
+
+       as_list_list_fini(&aslist->exclude_rule);
        as_list_filter_delete(aslist, asfilter);
 
        return CMD_SUCCESS;

diff --git a/bgpd/bgp_filter.h b/bgpd/bgp_filter.h
index 2d9f07ce84a3e0084ec65abe7ce7d00229b07d77..77a3f3c2f727c2339dd9c42cc96473edd7935f5a 100644 (file)
--- a/bgpd/bgp_filter.h
+++ b/bgpd/bgp_filter.h
@@ -6,11 +6,12 @@
 #ifndef _QUAGGA_BGP_FILTER_H
 #define _QUAGGA_BGP_FILTER_H
 
+#include <typesafe.h>
+
 #define ASPATH_SEQ_NUMBER_AUTO -1
 
 enum as_filter_type { AS_FILTER_DENY, AS_FILTER_PERMIT };
 
-
 /* Element of AS path filter. */
 struct as_filter {
        struct as_filter *next;
@@ -25,11 +26,7 @@ struct as_filter {
        int64_t seq;
 };
 
-struct aspath_exclude_list {
-       struct aspath_exclude_list *next;
-       struct aspath_exclude *bp_as_excl;
-};
-
+PREDECL_DLIST(as_list_list);
 /* AS path filter list. */
 struct as_list {
        char *name;
@@ -39,7 +36,9 @@ struct as_list {
 
        struct as_filter *head;
        struct as_filter *tail;
-       struct aspath_exclude_list *exclude_list;
+
+       /* Changes in AS path */
+       struct as_list_list_head exclude_rule;
 };
 
 

diff --git a/bgpd/bgp_routemap.c b/bgpd/bgp_routemap.c
index dbc3d6445dc640ea06fad1a7be4f134728f6371a..e059357f0900ce876068a2cf5051ca2648dc6919 100644 (file)
--- a/bgpd/bgp_routemap.c
+++ b/bgpd/bgp_routemap.c
@@ -2326,7 +2326,7 @@ static const struct route_map_rule_cmd route_set_aspath_prepend_cmd = {
 static void *route_aspath_exclude_compile(const char *arg)
 {
        struct aspath_exclude *ase;
-       struct aspath_exclude_list *ael;
+       struct as_list *aux_aslist;
        const char *str = arg;
        static const char asp_acl[] = "as-path-access-list";
 
@@ -2338,44 +2338,37 @@ static void *route_aspath_exclude_compile(const char *arg)
                while (*str == ' ')
                        str++;
                ase->exclude_aspath_acl_name = XSTRDUP(MTYPE_TMP, str);
-               ase->exclude_aspath_acl = as_list_lookup(str);
+               aux_aslist = as_list_lookup(str);
+               if (!aux_aslist)
+                       /* new orphan filter */
+                       as_exclude_set_orphan(ase);
+               else
+                       as_list_list_add_head(&aux_aslist->exclude_rule, ase);
+
+               ase->exclude_aspath_acl = aux_aslist;
        } else
                ase->aspath = aspath_str2aspath(str, bgp_get_asnotation(NULL));
 
-       if (ase->exclude_aspath_acl) {
-               ael = XCALLOC(MTYPE_ROUTE_MAP_COMPILED,
-                               sizeof(struct aspath_exclude_list));
-               ael->bp_as_excl = ase;
-               ael->next = ase->exclude_aspath_acl->exclude_list;
-               ase->exclude_aspath_acl->exclude_list = ael;
-       }
-
        return ase;
 }
 
 static void route_aspath_exclude_free(void *rule)
 {
        struct aspath_exclude *ase = rule;
-       struct aspath_exclude_list *cur_ael = NULL;
-       struct aspath_exclude_list *prev_ael = NULL;
+       struct as_list *acl;
+
+       /* manage references to that rule*/
+       if (ase->exclude_aspath_acl) {
+               acl = ase->exclude_aspath_acl;
+               as_list_list_del(&acl->exclude_rule, ase);
+       } else {
+               /* no ref to acl, this aspath exclude is orphan */
+               as_exclude_remove_orphan(ase);
+       }
 
        aspath_free(ase->aspath);
        if (ase->exclude_aspath_acl_name)
                XFREE(MTYPE_TMP, ase->exclude_aspath_acl_name);
-       if (ase->exclude_aspath_acl)
-               cur_ael = ase->exclude_aspath_acl->exclude_list;
-       while (cur_ael) {
-               if (cur_ael->bp_as_excl == ase) {
-                       if (prev_ael)
-                               prev_ael->next = cur_ael->next;
-                       else
-                               ase->exclude_aspath_acl->exclude_list = NULL;
-                       XFREE(MTYPE_ROUTE_MAP_COMPILED, cur_ael);
-                       break;
-               }
-               prev_ael = cur_ael;
-               cur_ael = cur_ael->next;
-       }
        XFREE(MTYPE_ROUTE_MAP_COMPILED, ase);
 }
 
@@ -2410,16 +2403,10 @@ route_set_aspath_exclude(void *rule, const struct prefix *dummy, void *object)
        else if (ase->exclude_all)
                path->attr->aspath = aspath_filter_exclude_all(new_path);
 
-       else if (ase->exclude_aspath_acl_name) {
-               if (!ase->exclude_aspath_acl)
-                       ase->exclude_aspath_acl =
-                               as_list_lookup(ase->exclude_aspath_acl_name);
-               if (ase->exclude_aspath_acl)
-                       path->attr->aspath =
-                               aspath_filter_exclude_acl(new_path,
-                                                         ase->exclude_aspath_acl);
-       }
-
+       else if (ase->exclude_aspath_acl)
+               path->attr->aspath =
+                       aspath_filter_exclude_acl(new_path,
+                                                 ase->exclude_aspath_acl);
        return RMAP_OKAY;
 }