bgpd: 'set comm-list delete' stops as soon as it hits a community-list entry with a deny
'set comm-list delete' stops as soon as it hits a community-list entry with
a deny
Reviewed By: sharpd@cumulusnetworks.com
Testing Done:
'set comm-list FOO delete' stops evaluating the community-list as soon as
we hit
the first "delete" statement. This makes it impossible to use
community-lists
where you deny some subset of communities to delete and then permit all of
the
others.
This patch changes the behavior so that we no longer exit the
community-list at
the first delete statement. Here is our baseline, we are receiving multiple
communities from 10.1.1.2 for the 10.1.3.0/24 prefix.
qct-ly6-04# show ip bgp 10.1.3.0/24
BGP routing table entry for 10.1.3.0/24
Paths: (1 available, best #1, table Default-IP-Routing-Table)
Not advertised to any peer
Local
10.1.1.2 (metric 20) from 10.1.1.2 (10.1.1.2)
Origin IGP, metric 0, localpref 100, valid, internal, best
Community: 1:1 1:2 1:3 20:1 20:2 20:3 99:1
Last update: Wed Mar 4 13:50:36 2015
qct-ly6-04#
We apply the following FOO route-map inbound to this peer and soft clear
the peer
!
ip community-list expanded BAD_COMMS permit 99:.*
ip community-list expanded BAD_COMMS deny 1:.*
ip community-list expanded BAD_COMMS permit 20.*
!
route-map FOO permit 10
set comm-list BAD_COMMS delete
!
router bgp 10
neighbor 10.1.1.2 route-map FOO in
!
qct-ly6-04# clear ip bgp * soft in
qct-ly6-04# show ip bgp 10.1.3.0/24
BGP routing table entry for 10.1.3.0/24
Paths: (1 available, best #1, table Default-IP-Routing-Table)
Not advertised to any peer
Local
10.1.1.2 (metric 20) from 10.1.1.2 (10.1.1.2)
Origin IGP, metric 0, localpref 100, valid, internal, best
Community: 1:1 1:2 1:3
Last update: Wed Mar 4 13:51:12 2015
qct-ly6-04#
qct-ly6-04#
We deleted all communities flagged as "permit" by the BAD_COMMS
community-list
while leaving the ones matched by "deny 1:.*" alone.
#endif /* _QUAGGA_BGP_COMMUNITY_H */
'set comm-list delete' stops as soon as it hits a community-list entry with a deny
Ticket: CM-3513
Reviewed By: sharpd@cumulusnetworks.com
Testing Done:
'set comm-list FOO delete' stops evaluating the community-list as soon as we hit
the first "delete" statement. This makes it impossible to use community-lists
where you deny some subset of communities to delete and then permit all of the
others.
This patch changes the behavior so that we no longer exit the community-list at
the first delete statement. Here is our baseline, we are receiving multiple
communities from 10.1.1.2 for the 10.1.3.0/24 prefix.
qct-ly6-04# show ip bgp 10.1.3.0/24
BGP routing table entry for 10.1.3.0/24
Paths: (1 available, best #1, table Default-IP-Routing-Table)
Not advertised to any peer
Local
10.1.1.2 (metric 20) from 10.1.1.2 (10.1.1.2)
Origin IGP, metric 0, localpref 100, valid, internal, best
Community: 1:1 1:2 1:3 20:1 20:2 20:3 99:1
Last update: Wed Mar 4 13:50:36 2015
qct-ly6-04#
We apply the following FOO route-map inbound to this peer and soft clear the peer
!
ip community-list expanded BAD_COMMS permit 99:.*
ip community-list expanded BAD_COMMS deny 1:.*
ip community-list expanded BAD_COMMS permit 20.*
!
route-map FOO permit 10
set comm-list BAD_COMMS delete
!
router bgp 10
neighbor 10.1.1.2 route-map FOO in
!
qct-ly6-04# clear ip bgp * soft in
qct-ly6-04# show ip bgp 10.1.3.0/24
BGP routing table entry for 10.1.3.0/24
Paths: (1 available, best #1, table Default-IP-Routing-Table)
Not advertised to any peer
Local
10.1.1.2 (metric 20) from 10.1.1.2 (10.1.1.2)
Origin IGP, metric 0, localpref 100, valid, internal, best
Community: 1:1 1:2 1:3
Last update: Wed Mar 4 13:51:12 2015
qct-ly6-04#
qct-ly6-04#
We deleted all communities flagged as "permit" by the BAD_COMMS community-list
while leaving the ones matched by "deny 1:.*" alone.
diff --git a/bgpd/bgp_clist.c b/bgpd/bgp_clist.c
index b91ab81..db2cfa4 100644
--- a/bgpd/bgp_clist.c
+++ b/bgpd/bgp_clist.c
@@ -330,6 +330,94 @@
return NULL;
}
+static char *
+community_str_get (struct community *com, int i)
+{
+ int len;
+ u_int32_t comval;
+ u_int16_t as;
+ u_int16_t val;
+ char *str;
+ char *pnt;
+
+ memcpy (&comval, com_nthval (com, i), sizeof (u_int32_t));
+ comval = ntohl (comval);
+
+ switch (comval)
+ {
+ case COMMUNITY_INTERNET:
+ len = strlen (" internet");
+ break;
+ case COMMUNITY_NO_EXPORT:
+ len = strlen (" no-export");
+ break;
+ case COMMUNITY_NO_ADVERTISE:
+ len = strlen (" no-advertise");
+ break;
+ case COMMUNITY_LOCAL_AS:
+ len = strlen (" local-AS");
+ break;
+ default:
+ len = strlen (" 65536:65535");
+ break;
+ }
+
+ /* Allocate memory. */
+ str = pnt = XMALLOC (MTYPE_COMMUNITY_STR, len);
+
+ switch (comval)
+ {
+ case COMMUNITY_INTERNET:
+ strcpy (pnt, "internet");
+ pnt += strlen ("internet");
+ break;
+ case COMMUNITY_NO_EXPORT:
+ strcpy (pnt, "no-export");
+ pnt += strlen ("no-export");
+ break;
+ case COMMUNITY_NO_ADVERTISE:
+ strcpy (pnt, "no-advertise");
+ pnt += strlen ("no-advertise");
+ break;
+ case COMMUNITY_LOCAL_AS:
+ strcpy (pnt, "local-AS");
+ pnt += strlen ("local-AS");
+ break;
+ default:
+ as = (comval >> 16) & 0xFFFF;
+ val = comval & 0xFFFF;
+ sprintf (pnt, "%u:%d", as, val);
+ pnt += strlen (pnt);
+ break;
+ }
+
+ *pnt = '\0';
+
+ return str;
+}
+
+/* Internal function to perform regular expression match for
+ * * a single community. */
+static int
+community_regexp_include (regex_t * reg, struct community *com, int i)
+{
+ const char *str;
+
+ /* When there is no communities attribute it is treated as empty
+ * string. */
+ if (com == NULL || com->size == 0)
+ str = "";
+ else
+ str = community_str_get (com, i);
+
+ /* Regular expression match. */
+ if (regexec (reg, str, 0, NULL, 0) == 0)
+ return 1;
+
+ /* No match. */
+ return 0;
+}
+
/* Internal function to perform regular expression match for community
attribute. */
static int
@@ -372,54 +460,6 @@
return 0;
}
-/* Delete community attribute using regular expression match. Return
- modified communites attribute. */
-static struct community *
-community_regexp_delete (struct community *com, regex_t * reg)
-{
- int i;
- u_int32_t comval;
- /* Maximum is "65535:65535" + '\0'. */
- char c[12];
- const char *str;
-
- if (!com)
- return NULL;
-
- i = 0;
- while (i < com->size)
- {
- memcpy (&comval, com_nthval (com, i), sizeof (u_int32_t));
- comval = ntohl (comval);
-
- switch (comval)
- {
- case COMMUNITY_INTERNET:
- str = "internet";
- break;
- case COMMUNITY_NO_EXPORT:
- str = "no-export";
- break;
- case COMMUNITY_NO_ADVERTISE:
- str = "no-advertise";
- break;
- case COMMUNITY_LOCAL_AS:
- str = "local-AS";
- break;
- default:
- sprintf (c, "%d:%d", (comval >> 16) & 0xFFFF, comval & 0xFFFF);
- str = c;
- break;
- }
-
- if (regexec (reg, str, 0, NULL, 0) == 0)
- community_del_val (com, com_nthval (com, i));
- else
- i++;
- }
- return com;
-}
-
/* When given community attribute matches to the community-list return
1 else return 0. */
int
@@ -509,41 +549,63 @@
struct community_list *list)
{
struct community_entry *entry;
+ u_int32_t val;
+ u_int32_t com_index_to_delete[com->size];
+ int delete_index = 0;
+ int i;
- for (entry = list->head; entry; entry = entry->next)
+ /* Loop over each community value and evaluate each against the
+ * community-list. If we need to delete a community value add its index to
+ * com_index_to_delete.
+ */
+ for (i = 0; i < com->size; i++)
{
- if (entry->any)
- {
- if (entry->direct == COMMUNITY_PERMIT)
- {
- /* This is a tricky part. Currently only
- * route_set_community_delete() uses this function. In the
- * function com->size is zero, it free the community
- * structure.
- */
- com->size = 0;
- }
- return com;
- }
+ val = community_val_get (com, i);
- if ((entry->style == COMMUNITY_LIST_STANDARD)
- && (community_include (entry->u.com, COMMUNITY_INTERNET)
- || community_match (com, entry->u.com) ))
+ for (entry = list->head; entry; entry = entry->next)
{
+ if (entry->any)
+ {
if (entry->direct == COMMUNITY_PERMIT)
- community_delete (com, entry->u.com);
- else
- break;
- }
- else if ((entry->style == COMMUNITY_LIST_EXPANDED)
- && community_regexp_match (com, entry->reg))
- {
- if (entry->direct == COMMUNITY_PERMIT)
- community_regexp_delete (com, entry->reg);
- else
- break;
- }
+ {
+ com_index_to_delete[delete_index] = i;
+ delete_index++;
+ }
+ break;
+ }
+
+ else if ((entry->style == COMMUNITY_LIST_STANDARD)
+ && (community_include (entry->u.com, COMMUNITY_INTERNET)
+ || community_include (entry->u.com, val) ))
+ {
+ if (entry->direct == COMMUNITY_PERMIT)
+ {
+ com_index_to_delete[delete_index] = i;
+ delete_index++;
+ }
+ break;
+ }
+
+ else if ((entry->style == COMMUNITY_LIST_EXPANDED)
+ && community_regexp_include (entry->reg, com, i))
+ {
+ if (entry->direct == COMMUNITY_PERMIT)
+ {
+ com_index_to_delete[delete_index] = i;
+ delete_index++;
+ }
+ break;
+ }
+ }
+ }
+
+ /* Delete all of the communities we flagged for deletion */
+ for (i = delete_index-1; i >= 0; i--)
+ {
+ val = community_val_get (com, com_index_to_delete[i]);
+ community_del_val (com, &val);
}
+
return com;
}
diff --git a/bgpd/bgp_community.c b/bgpd/bgp_community.c
index 1bd2dd8..f1997bd 100644
--- a/bgpd/bgp_community.c
+++ b/bgpd/bgp_community.c
@@ -144,7 +144,7 @@
return 0;
}
-static u_int32_t
+u_int32_t
community_val_get (struct community *com, int i)
{
u_char *p;
diff --git a/bgpd/bgp_community.h b/bgpd/bgp_community.h
index 9e48377..c73dab3 100644
--- a/bgpd/bgp_community.h
+++ b/bgpd/bgp_community.h
@@ -70,5 +70,6 @@
extern void community_del_val (struct community *, u_int32_t *);
extern unsigned long community_count (void);
extern struct hash *community_hash (void);
+extern u_int32_t community_val_get (struct community *com, int i);
#endif /* _QUAGGA_BGP_COMMUNITY_H */