lib: Optimizing route-maps #5270
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
💚 Basic BGPD CI results: SUCCESS, 0 tests failedResults table
For details, please contact louberger |
Continuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-9582/ This is a comment from an automated CI system. CLANG Static Analyzer Summary
Fixed warnings:
New warnings:
Static Analysis warning summary compared to base:
3 Static Analyzer issues remaining.See details at |
NaveenThanikachalam
force-pushed
the
rmap
branch
from
2026fc4 to
0ac4503
Compare
💚 Basic BGPD CI results: SUCCESS, 0 tests failedResults table
For details, please contact louberger |
Continuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: FailedTopotest tests on Ubuntu 16.04 i386: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPOI386-9583/test Topology Tests failed for Topotest tests on Ubuntu 16.04 i386: see full log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-9583/artifact/TOPOI386/ErrorLog/log_topotests.txt Topology tests on Ubuntu 16.04 amd64: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPOU1604-9583/test Topology Tests failed for Topology tests on Ubuntu 16.04 amd64: see full log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-9583/artifact/TOPOU1604/ErrorLog/log_topotests.txt Topology tests on Ubuntu 18.04 amd64: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPOU1804-9583/test Topology Tests failed for Topology tests on Ubuntu 18.04 amd64: see full log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-9583/artifact/TOPOU1804/ErrorLog/log_topotests.txt Successful on other platforms
Topology Tests memory analysis: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-9583/artifact/TOPOI386/MemoryLeaks/
|
NaveenThanikachalam
force-pushed
the
rmap
branch
from
0ac4503 to
b784df6
Compare
💚 Basic BGPD CI results: SUCCESS, 0 tests failedResults table
For details, please contact louberger |
Continuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-9584/ This is a comment from an automated CI system. CLANG Static Analyzer Summary
Fixed warnings:
New warnings:
Static Analysis warning summary compared to base:
3 Static Analyzer issues remaining.See details at |
|
NaveenThanikachalam
force-pushed
the
rmap
branch
from
b784df6 to
55c7569
Compare
💚 Basic BGPD CI results: SUCCESS, 0 tests failedResults table
For details, please contact louberger |
Continuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-9619/ This is a comment from an automated CI system. CLANG Static Analyzer Summary
No Changes in Static Analysis warnings compared to base1 Static Analyzer issues remaining.See details at |
NaveenThanikachalam
force-pushed
the
rmap
branch
from
55c7569 to
1efc133
Compare
💚 Basic BGPD CI results: SUCCESS, 0 tests failedResults table
For details, please contact louberger |
Continuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-9639/ This is a comment from an automated CI system. CLANG Static Analyzer Summary
No Changes in Static Analysis warnings compared to base1 Static Analyzer issues remaining.See details at |
riw777
reviewed
Nov 19, 2019
Thanks, Russ. I'll have the data compiled and attach it to this PR. |
|
In order to measure the convergence, the following configuration was used.
For example: The below table captures the convergence time with and without the optimization.
Additional unit test cases have been captured in the "Route-Map_Optimisation_Unit_Tests.pdf" that is attached to this PR. |
thanks -- this is really helpful... Will poke through the code again. |
riw777
approved these changes
Dec 9, 2019
|
I'd like to review this a bit more in depth (mostly with my context of having looked at setups with large filter lists in IXP route server scenarios.) Adding "do not merge" as a marker of this, if I don't finish review until Dec 17th feel free to remove the label. |
NaveenThanikachalam
force-pushed
the
rmap
branch
from
1efc133 to
24215d1
Compare
polychaeta
reviewed
Dec 20, 2019
There was a problem hiding this comment.
Thanks for your contribution to FRR!
Click for style suggestions
To apply these suggestions:
curl -s https://gist.githubusercontent.com/polychaeta/65776b6a75a810955fdcf41c2119e56c/raw/144b15dfcf4c581ea21e3a90d6b5d552bf467644/cr_5270_1576823681.diff | git apply
diff --git a/lib/plist.c b/lib/plist.c
index 0f26d0595..047869d27 100644
--- a/lib/plist.c
+++ b/lib/plist.c
@@ -303,8 +303,7 @@ static void prefix_list_delete(struct prefix_list *plist)
/* If prefix-list contain prefix_list_entry free all of it. */
for (pentry = plist->head; pentry; pentry = next) {
- route_map_notify_pentry_dependencies(plist->name,
- pentry,
+ route_map_notify_pentry_dependencies(plist->name, pentry,
RMAP_EVENT_PLIST_DELETED);
next = pentry->next;
prefix_list_trie_del(plist, pentry);
@@ -522,8 +521,7 @@ static void prefix_list_entry_delete(struct prefix_list *plist,
else
plist->tail = pentry->prev;
- route_map_notify_pentry_dependencies(plist->name,
- pentry,
+ route_map_notify_pentry_dependencies(plist->name, pentry,
RMAP_EVENT_PLIST_DELETED);
prefix_list_entry_free(pentry);
diff --git a/lib/routemap.c b/lib/routemap.c
index eb194c6ac..32ae5a7d7 100644
--- a/lib/routemap.c
+++ b/lib/routemap.c
@@ -50,15 +50,15 @@ DEFINE_QOBJ_TYPE(route_map)
#define IPv4_MATCH_RULE "ip "
#define IPv6_MATCH_RULE "ipv6 "
-#define IS_RULE_IPv4_PREFIX_LIST(S) (strncmp(S, IPv4_PREFIX_LIST, \
- strlen(IPv4_PREFIX_LIST)) == 0)
-#define IS_RULE_IPv6_PREFIX_LIST(S) (strncmp(S, IPv6_PREFIX_LIST, \
- strlen(IPv6_PREFIX_LIST)) == 0)
-
-#define IS_IPv4_RULE(S) (strncmp(S, IPv4_MATCH_RULE, \
- strlen(IPv4_MATCH_RULE)) == 0)
-#define IS_IPv6_RULE(S) (strncmp(S, IPv6_MATCH_RULE, \
- strlen(IPv6_MATCH_RULE)) == 0)
+#define IS_RULE_IPv4_PREFIX_LIST(S) \
+ (strncmp(S, IPv4_PREFIX_LIST, strlen(IPv4_PREFIX_LIST)) == 0)
+#define IS_RULE_IPv6_PREFIX_LIST(S) \
+ (strncmp(S, IPv6_PREFIX_LIST, strlen(IPv6_PREFIX_LIST)) == 0)
+
+#define IS_IPv4_RULE(S) \
+ (strncmp(S, IPv4_MATCH_RULE, strlen(IPv4_MATCH_RULE)) == 0)
+#define IS_IPv6_RULE(S) \
+ (strncmp(S, IPv6_MATCH_RULE, strlen(IPv6_MATCH_RULE)) == 0)
struct route_map_pentry_dep {
struct prefix_list_entry *pentry;
const char *plist_name;
@@ -71,34 +71,25 @@ static vector route_match_vec;
/* Vector for route set rules. */
static vector route_set_vec;
-static void
-route_map_pfx_tbl_update(route_map_event_t event,
- struct route_map_index *index,
- afi_t afi,
- const char *plist_name);
-static void
-route_map_pfx_table_add_default(afi_t afi,
- struct route_map_index *index);
-static void
-route_map_pfx_table_del_default(afi_t afi,
- struct route_map_index *index);
-static void
-route_map_add_plist_entries(afi_t afi,
- struct route_map_index *index,
- const char *plist_name,
- struct prefix_list_entry *entry);
-static void
-route_map_del_plist_entries(afi_t afi,
- struct route_map_index *index,
- const char *plist_name,
- struct prefix_list_entry *entry);
-static bool
-route_map_is_ip_rule_present(struct route_map_index *index);
-static bool
-route_map_is_ipv6_rule_present(struct route_map_index *index);
-
-static struct hash *
-route_map_get_dep_hash(route_map_event_t event);
+static void route_map_pfx_tbl_update(route_map_event_t event,
+ struct route_map_index *index, afi_t afi,
+ const char *plist_name);
+static void route_map_pfx_table_add_default(afi_t afi,
+ struct route_map_index *index);
+static void route_map_pfx_table_del_default(afi_t afi,
+ struct route_map_index *index);
+static void route_map_add_plist_entries(afi_t afi,
+ struct route_map_index *index,
+ const char *plist_name,
+ struct prefix_list_entry *entry);
+static void route_map_del_plist_entries(afi_t afi,
+ struct route_map_index *index,
+ const char *plist_name,
+ struct prefix_list_entry *entry);
+static bool route_map_is_ip_rule_present(struct route_map_index *index);
+static bool route_map_is_ipv6_rule_present(struct route_map_index *index);
+
+static struct hash *route_map_get_dep_hash(route_map_event_t event);
struct route_map_match_set_hooks {
/* match interface */
@@ -1167,15 +1158,12 @@ static void route_map_index_delete(struct route_map_index *index, int notify)
/* Free route match. */
while ((rule = index->match_list.head) != NULL) {
if (IS_RULE_IPv4_PREFIX_LIST(rule->cmd->str))
- route_map_pfx_tbl_update(
- RMAP_EVENT_PLIST_DELETED,
- index, AFI_IP,
- rule->rule_str);
+ route_map_pfx_tbl_update(RMAP_EVENT_PLIST_DELETED,
+ index, AFI_IP, rule->rule_str);
else if (IS_RULE_IPv6_PREFIX_LIST(rule->cmd->str))
- route_map_pfx_tbl_update(
- RMAP_EVENT_PLIST_DELETED,
- index, AFI_IP6,
- rule->rule_str);
+ route_map_pfx_tbl_update(RMAP_EVENT_PLIST_DELETED,
+ index, AFI_IP6,
+ rule->rule_str);
route_map_rule_delete(&index->match_list, rule);
}
@@ -1198,8 +1186,7 @@ static void route_map_index_delete(struct route_map_index *index, int notify)
/* Free 'char *nextrm' if not NULL */
XFREE(MTYPE_ROUTE_MAP_NAME, index->nextrm);
- route_map_pfx_tbl_update(RMAP_EVENT_INDEX_DELETED,
- index, 0, NULL);
+ route_map_pfx_tbl_update(RMAP_EVENT_INDEX_DELETED, index, 0, NULL);
/* Execute event hook. */
if (route_map_master.event_hook && notify) {
@@ -1260,8 +1247,7 @@ route_map_index_add(struct route_map *map, enum route_map_type type, int pref)
point->prev = index;
}
- route_map_pfx_tbl_update(RMAP_EVENT_INDEX_ADDED,
- index, 0, NULL);
+ route_map_pfx_tbl_update(RMAP_EVENT_INDEX_ADDED, index, 0, NULL);
/* Execute event hook. */
if (route_map_master.event_hook) {
@@ -1516,14 +1502,12 @@ enum rmap_compile_rets route_map_add_match(struct route_map_index *index,
*/
if (IS_RULE_IPv4_PREFIX_LIST(match_name))
route_map_pfx_tbl_update(
- RMAP_EVENT_PLIST_DELETED,
- index, AFI_IP,
- rule->rule_str);
+ RMAP_EVENT_PLIST_DELETED, index, AFI_IP,
+ rule->rule_str);
else if (IS_RULE_IPv6_PREFIX_LIST(match_name))
route_map_pfx_tbl_update(
- RMAP_EVENT_PLIST_DELETED,
- index, AFI_IP6,
- rule->rule_str);
+ RMAP_EVENT_PLIST_DELETED, index,
+ AFI_IP6, rule->rule_str);
/* Remove the dependency of the route-map on the rule
* that is being replaced.
@@ -1558,12 +1542,10 @@ enum rmap_compile_rets route_map_add_match(struct route_map_index *index,
* the route-map's prefix table.
*/
if (IS_RULE_IPv4_PREFIX_LIST(match_name)) {
- route_map_pfx_tbl_update(RMAP_EVENT_PLIST_ADDED,
- index, AFI_IP,
+ route_map_pfx_tbl_update(RMAP_EVENT_PLIST_ADDED, index, AFI_IP,
match_arg);
} else if (IS_RULE_IPv6_PREFIX_LIST(match_name)) {
- route_map_pfx_tbl_update(RMAP_EVENT_PLIST_ADDED,
- index, AFI_IP6,
+ route_map_pfx_tbl_update(RMAP_EVENT_PLIST_ADDED, index, AFI_IP6,
match_arg);
} else {
/* If IPv4 match criteria has been added to the route-map
@@ -1574,8 +1556,7 @@ enum rmap_compile_rets route_map_add_match(struct route_map_index *index,
* default route's trie node.
*/
if (IS_IPv4_RULE(match_name))
- route_map_del_plist_entries(AFI_IP6, index,
- NULL, NULL);
+ route_map_del_plist_entries(AFI_IP6, index, NULL, NULL);
/* If IPv6 match criteria has been added to the route-map
* index, check for IPv4 prefix-list match rule presence and
@@ -1585,8 +1566,7 @@ enum rmap_compile_rets route_map_add_match(struct route_map_index *index,
* default route's trie node.
*/
else if (IS_IPv6_RULE(match_name))
- route_map_del_plist_entries(AFI_IP, index,
- NULL, NULL);
+ route_map_del_plist_entries(AFI_IP, index, NULL, NULL);
}
/* Execute event hook. */
@@ -1643,14 +1623,12 @@ enum rmap_compile_rets route_map_delete_match(struct route_map_index *index,
*/
if (IS_RULE_IPv4_PREFIX_LIST(match_name)) {
route_map_pfx_tbl_update(
- RMAP_EVENT_PLIST_DELETED,
- index, AFI_IP,
- match_arg);
+ RMAP_EVENT_PLIST_DELETED, index, AFI_IP,
+ match_arg);
} else if (IS_RULE_IPv6_PREFIX_LIST(match_name)) {
route_map_pfx_tbl_update(
- RMAP_EVENT_PLIST_DELETED,
- index, AFI_IP6,
- match_arg);
+ RMAP_EVENT_PLIST_DELETED, index,
+ AFI_IP6, match_arg);
} else {
/* If no more IPv4 match rules are present in
* this index, check for IPv6 prefix-list match
@@ -1661,9 +1639,8 @@ enum rmap_compile_rets route_map_delete_match(struct route_map_index *index,
* default route's trie node.
*/
if (!route_map_is_ip_rule_present(index))
- route_map_add_plist_entries(AFI_IP6,
- index, NULL,
- NULL);
+ route_map_add_plist_entries(
+ AFI_IP6, index, NULL, NULL);
/* If no more IPv6 match rules are present in
* this index, check for IPv4 prefix-list match
@@ -1674,9 +1651,8 @@ enum rmap_compile_rets route_map_delete_match(struct route_map_index *index,
* default route's trie node.
*/
if (!route_map_is_ipv6_rule_present(index))
- route_map_add_plist_entries(AFI_IP,
- index, NULL,
- NULL);
+ route_map_add_plist_entries(
+ AFI_IP, index, NULL, NULL);
}
return RMAP_COMPILE_SUCCESS;
@@ -1844,10 +1820,9 @@ route_map_apply_match(struct route_map_rule_list *match_list,
return ret;
}
-static struct list *
-route_map_get_index_list(struct route_node **rn,
- const struct prefix *prefix,
- struct route_table *table)
+static struct list *route_map_get_index_list(struct route_node **rn,
+ const struct prefix *prefix,
+ struct route_table *table)
{
struct route_node *tmp_rn = NULL;
@@ -1887,11 +1862,8 @@ route_map_get_index_list(struct route_node **rn,
* This function returns the route-map index that best matches the prefix.
*/
static struct route_map_index *
-route_map_get_index(struct route_map *map,
- const struct prefix *prefix,
- route_map_object_t type,
- void *object,
- uint8_t *match_ret)
+route_map_get_index(struct route_map *map, const struct prefix *prefix,
+ route_map_object_t type, void *object, uint8_t *match_ret)
{
int ret = 0;
struct list *candidate_rmap_list = NULL;
@@ -1911,8 +1883,8 @@ route_map_get_index(struct route_map *map,
return NULL;
do {
- candidate_rmap_list = route_map_get_index_list(&rn, prefix,
- table);
+ candidate_rmap_list =
+ route_map_get_index_list(&rn, prefix, table);
if (!rn)
break;
@@ -1920,10 +1892,10 @@ route_map_get_index(struct route_map *map,
* than that in best_index, ignore the list and get the
* parent node's list.
*/
- head_index = (struct route_map_index *)
- (listgetdata(listhead(candidate_rmap_list)));
- if (best_index && head_index &&
- (best_index->pref < head_index->pref)) {
+ head_index = (struct route_map_index *)(listgetdata(
+ listhead(candidate_rmap_list)));
+ if (best_index && head_index
+ && (best_index->pref < head_index->pref)) {
route_unlock_node(rn);
continue;
}
@@ -1936,9 +1908,8 @@ route_map_get_index(struct route_map *map,
if (best_index && (best_index->pref < index->pref))
break;
- ret = route_map_apply_match(
- &index->match_list,
- prefix, type, object);
+ ret = route_map_apply_match(&index->match_list, prefix,
+ type, object);
if (ret == RMAP_MATCH) {
*match_ret = ret;
@@ -1968,9 +1939,8 @@ route_map_get_index(struct route_map *map,
return best_index;
}
-static int
-route_map_candidate_list_cmp(struct route_map_index *idx1,
- struct route_map_index *idx2)
+static int route_map_candidate_list_cmp(struct route_map_index *idx1,
+ struct route_map_index *idx2)
{
if (!idx1)
return -1;
@@ -1984,9 +1954,8 @@ route_map_candidate_list_cmp(struct route_map_index *idx1,
* This function adds the route-map index into the default route's
* route-node in the route-map's IPv4/IPv6 prefix-table.
*/
-static void
-route_map_pfx_table_add_default(afi_t afi,
- struct route_map_index *index)
+static void route_map_pfx_table_add_default(afi_t afi,
+ struct route_map_index *index)
{
struct route_node *rn = NULL;
struct list *rmap_candidate_list = NULL;
@@ -2037,9 +2006,8 @@ route_map_pfx_table_add_default(afi_t afi,
* This function removes the route-map index from the default route's
* route-node in the route-map's IPv4/IPv6 prefix-table.
*/
-static void
-route_map_pfx_table_del_default(afi_t afi,
- struct route_map_index *index)
+static void route_map_pfx_table_del_default(afi_t afi,
+ struct route_map_index *index)
{
struct route_node *rn = NULL;
struct list *rmap_candidate_list = NULL;
@@ -2076,10 +2044,9 @@ route_map_pfx_table_del_default(afi_t afi,
* This function adds the route-map index to the route-node for
* the prefix-entry in the route-map's IPv4/IPv6 prefix-table.
*/
-static void
-route_map_pfx_table_add(struct route_table *table,
- struct route_map_index *index,
- struct prefix_list_entry *pentry)
+static void route_map_pfx_table_add(struct route_table *table,
+ struct route_map_index *index,
+ struct prefix_list_entry *pentry)
{
struct route_node *rn = NULL;
struct list *rmap_candidate_list = NULL;
@@ -2108,10 +2075,9 @@ route_map_pfx_table_add(struct route_table *table,
* This function removes the route-map index from the route-node for
* the prefix-entry in the route-map's IPv4/IPv6 prefix-table.
*/
-static void
-route_map_pfx_table_del(struct route_table *table,
- struct route_map_index *index,
- struct prefix_list_entry *pentry)
+static void route_map_pfx_table_del(struct route_table *table,
+ struct route_map_index *index,
+ struct prefix_list_entry *pentry)
{
struct route_node *rn = NULL;
struct list *rmap_candidate_list = NULL;
@@ -2135,8 +2101,7 @@ route_map_pfx_table_del(struct route_table *table,
/* This function checks for the presence of an IPv4 match rule
* in the given route-map index.
*/
-static bool
-route_map_is_ip_rule_present(struct route_map_index *index)
+static bool route_map_is_ip_rule_present(struct route_map_index *index)
{
struct route_map_rule_list *match_list = NULL;
struct route_map_rule *rule = NULL;
@@ -2152,8 +2117,7 @@ route_map_is_ip_rule_present(struct route_map_index *index)
/* This function checks for the presence of an IPv6 match rule
* in the given route-map index.
*/
-static bool
-route_map_is_ipv6_rule_present(struct route_map_index *index)
+static bool route_map_is_ipv6_rule_present(struct route_map_index *index)
{
struct route_map_rule_list *match_list = NULL;
struct route_map_rule *rule = NULL;
@@ -2181,11 +2145,10 @@ route_map_is_ipv6_rule_present(struct route_map_index *index)
* prefix-list, create a route-node for this entry and
* add this index to the route-node.
*/
-static void
-route_map_add_plist_entries(afi_t afi,
- struct route_map_index *index,
- const char *plist_name,
- struct prefix_list_entry *entry)
+static void route_map_add_plist_entries(afi_t afi,
+ struct route_map_index *index,
+ const char *plist_name,
+ struct prefix_list_entry *entry)
{
struct route_map_rule_list *match_list = NULL;
struct route_map_rule *match = NULL;
@@ -2200,11 +2163,11 @@ route_map_add_plist_entries(afi_t afi,
if (afi == AFI_IP) {
if (IS_RULE_IPv4_PREFIX_LIST(match->cmd->str))
plist_rule_is_present = true;
- break;
+ break;
} else {
if (IS_RULE_IPv6_PREFIX_LIST(match->cmd->str))
plist_rule_is_present = true;
- break;
+ break;
}
}
@@ -2223,24 +2186,22 @@ route_map_add_plist_entries(afi_t afi,
if (entry) {
if (afi == AFI_IP) {
- route_map_pfx_table_add(
- index->map->ipv4_prefix_table,
- index, entry);
+ route_map_pfx_table_add(index->map->ipv4_prefix_table,
+ index, entry);
} else {
- route_map_pfx_table_add(
- index->map->ipv6_prefix_table,
- index, entry);
+ route_map_pfx_table_add(index->map->ipv6_prefix_table,
+ index, entry);
}
} else {
for (pentry = plist->head; pentry; pentry = pentry->next) {
if (afi == AFI_IP) {
route_map_pfx_table_add(
- index->map->ipv4_prefix_table,
- index, pentry);
+ index->map->ipv4_prefix_table, index,
+ pentry);
} else {
route_map_pfx_table_add(
- index->map->ipv6_prefix_table,
- index, pentry);
+ index->map->ipv6_prefix_table, index,
+ pentry);
}
}
}
@@ -2259,11 +2220,10 @@ route_map_add_plist_entries(afi_t afi,
* prefix-list, remove this index from the route-node
* for the prefix in this prefix-entry.
*/
-static void
-route_map_del_plist_entries(afi_t afi,
- struct route_map_index *index,
- const char *plist_name,
- struct prefix_list_entry *entry)
+static void route_map_del_plist_entries(afi_t afi,
+ struct route_map_index *index,
+ const char *plist_name,
+ struct prefix_list_entry *entry)
{
struct route_map_rule_list *match_list = NULL;
struct route_map_rule *match = NULL;
@@ -2278,11 +2238,11 @@ route_map_del_plist_entries(afi_t afi,
if (afi == AFI_IP) {
if (IS_RULE_IPv4_PREFIX_LIST(match->cmd->str))
plist_rule_is_present = true;
- break;
+ break;
} else {
if (IS_RULE_IPv6_PREFIX_LIST(match->cmd->str))
plist_rule_is_present = true;
- break;
+ break;
}
}
@@ -2299,24 +2259,22 @@ route_map_del_plist_entries(afi_t afi,
if (entry) {
if (afi == AFI_IP) {
- route_map_pfx_table_del(
- index->map->ipv4_prefix_table,
- index, entry);
+ route_map_pfx_table_del(index->map->ipv4_prefix_table,
+ index, entry);
} else {
- route_map_pfx_table_del(
- index->map->ipv6_prefix_table,
- index, entry);
+ route_map_pfx_table_del(index->map->ipv6_prefix_table,
+ index, entry);
}
} else {
for (pentry = plist->head; pentry; pentry = pentry->next) {
if (afi == AFI_IP) {
route_map_pfx_table_del(
- index->map->ipv4_prefix_table,
- index, pentry);
+ index->map->ipv4_prefix_table, index,
+ pentry);
} else {
route_map_pfx_table_del(
- index->map->ipv6_prefix_table,
- index, pentry);
+ index->map->ipv6_prefix_table, index,
+ pentry);
}
}
}
@@ -2327,17 +2285,14 @@ route_map_del_plist_entries(afi_t afi,
* as a match command from a particular route-map index.
* It updates the prefix-table of the route-map accordingly.
*/
-static void
-route_map_trie_update(afi_t afi,
- route_map_event_t event,
- struct route_map_index *index,
- const char *plist_name)
+static void route_map_trie_update(afi_t afi, route_map_event_t event,
+ struct route_map_index *index,
+ const char *plist_name)
{
if (event == RMAP_EVENT_PLIST_ADDED) {
if (afi == AFI_IP) {
if (!route_map_is_ipv6_rule_present(index)) {
- route_map_pfx_table_del_default(AFI_IP6,
- index);
+ route_map_pfx_table_del_default(AFI_IP6, index);
route_map_add_plist_entries(afi, index,
plist_name, NULL);
} else {
@@ -2346,38 +2301,35 @@ route_map_trie_update(afi_t afi,
}
} else {
if (!route_map_is_ip_rule_present(index)) {
- route_map_pfx_table_del_default(AFI_IP,
- index);
+ route_map_pfx_table_del_default(AFI_IP, index);
route_map_add_plist_entries(afi, index,
plist_name, NULL);
} else {
- route_map_del_plist_entries(AFI_IP, index,
- NULL, NULL);
+ route_map_del_plist_entries(AFI_IP, index, NULL,
+ NULL);
}
}
} else if (event == RMAP_EVENT_PLIST_DELETED) {
if (afi == AFI_IP) {
- route_map_del_plist_entries(afi, index,
- plist_name, NULL);
+ route_map_del_plist_entries(afi, index, plist_name,
+ NULL);
if (!route_map_is_ipv6_rule_present(index))
route_map_pfx_table_add_default(afi, index);
if (!route_map_is_ip_rule_present(index))
- route_map_add_plist_entries(AFI_IP6,
- index,
+ route_map_add_plist_entries(AFI_IP6, index,
NULL, NULL);
} else {
- route_map_del_plist_entries(afi, index,
- plist_name, NULL);
+ route_map_del_plist_entries(afi, index, plist_name,
+ NULL);
if (!route_map_is_ip_rule_present(index))
route_map_pfx_table_add_default(afi, index);
if (!route_map_is_ipv6_rule_present(index))
- route_map_add_plist_entries(AFI_IP,
- index,
- NULL, NULL);
+ route_map_add_plist_entries(AFI_IP, index, NULL,
+ NULL);
}
}
}
@@ -2387,11 +2339,9 @@ route_map_trie_update(afi_t afi,
* prefix-list is added/removed.
* It updates the prefix-table of the route-map accordingly.
*/
-static void
-route_map_pfx_tbl_update(route_map_event_t event,
- struct route_map_index *index,
- afi_t afi,
- const char *plist_name)
+static void route_map_pfx_tbl_update(route_map_event_t event,
+ struct route_map_index *index, afi_t afi,
+ const char *plist_name)
{
struct route_map *rmap = NULL;
@@ -2408,8 +2358,7 @@ route_map_pfx_tbl_update(route_map_event_t event,
route_map_pfx_table_del_default(AFI_IP, index);
route_map_pfx_table_del_default(AFI_IP6, index);
- if ((index->map->head == NULL) &&
- (index->map->tail == NULL)) {
+ if ((index->map->head == NULL) && (index->map->tail == NULL)) {
rmap = index->map;
if (rmap->ipv4_prefix_table) {
@@ -2427,8 +2376,7 @@ route_map_pfx_tbl_update(route_map_event_t event,
/* Handle prefix-list match rule addition/deletion.
*/
- route_map_trie_update(afi, event,
- index, plist_name);
+ route_map_trie_update(afi, event, index, plist_name);
}
/*
@@ -2436,11 +2384,10 @@ route_map_pfx_tbl_update(route_map_event_t event,
* a prefix-list or, an existing prefix-entry is removed from the prefix-list.
* It updates the prefix-table of the route-map accordingly.
*/
-static void
-route_map_pentry_update(route_map_event_t event,
- const char *plist_name,
- struct route_map_index *index,
- struct prefix_list_entry *pentry)
+static void route_map_pentry_update(route_map_event_t event,
+ const char *plist_name,
+ struct route_map_index *index,
+ struct prefix_list_entry *pentry)
{
struct prefix_list *plist = NULL;
afi_t afi;
@@ -2458,18 +2405,16 @@ route_map_pentry_update(route_map_event_t event,
if (plist->count == 1) {
if (afi == AFI_IP) {
if (!route_map_is_ipv6_rule_present(index))
- route_map_add_plist_entries(afi, index,
- plist_name,
- pentry);
+ route_map_add_plist_entries(
+ afi, index, plist_name, pentry);
} else {
if (!route_map_is_ip_rule_present(index))
- route_map_add_plist_entries(afi, index,
- plist_name,
- pentry);
+ route_map_add_plist_entries(
+ afi, index, plist_name, pentry);
}
} else {
- route_map_add_plist_entries(afi, index,
- plist_name, pentry);
+ route_map_add_plist_entries(afi, index, plist_name,
+ pentry);
}
} else if (event == RMAP_EVENT_PLIST_DELETED) {
route_map_del_plist_entries(afi, index, plist_name, pentry);
@@ -2488,9 +2433,8 @@ route_map_pentry_update(route_map_event_t event,
}
}
-static void
-route_map_pentry_process_dependency(struct hash_backet *backet,
- void *data)
+static void route_map_pentry_process_dependency(struct hash_backet *backet,
+ void *data)
{
char *rmap_name = NULL;
struct route_map *rmap = NULL;
@@ -2499,7 +2443,7 @@ route_map_pentry_process_dependency(struct hash_backet *backet,
struct route_map_rule *match = NULL;
struct route_map_dep_data *dep_data = NULL;
struct route_map_pentry_dep *pentry_dep =
- (struct route_map_pentry_dep *)data;
+ (struct route_map_pentry_dep *)data;
unsigned char family = pentry_dep->pentry->prefix.family;
dep_data = (struct route_map_dep_data *)backet->data;
@@ -2519,33 +2463,29 @@ route_map_pentry_process_dependency(struct hash_backet *backet,
for (match = match_list->head; match; match = match->next) {
if (strcmp(match->rule_str, pentry_dep->plist_name)
- == 0) {
- if (IS_RULE_IPv4_PREFIX_LIST(
- match->cmd->str) &&
- family == AF_INET) {
+ == 0) {
+ if (IS_RULE_IPv4_PREFIX_LIST(match->cmd->str)
+ && family == AF_INET) {
route_map_pentry_update(
- pentry_dep->event,
- pentry_dep->plist_name,
- index,
- pentry_dep->pentry);
+ pentry_dep->event,
+ pentry_dep->plist_name, index,
+ pentry_dep->pentry);
} else if (IS_RULE_IPv6_PREFIX_LIST(
- match->cmd->str) &&
- family == AF_INET6) {
+ match->cmd->str)
+ && family == AF_INET6) {
route_map_pentry_update(
- pentry_dep->event,
- pentry_dep->plist_name,
- index,
- pentry_dep->pentry);
+ pentry_dep->event,
+ pentry_dep->plist_name, index,
+ pentry_dep->pentry);
}
}
}
}
}
-void
-route_map_notify_pentry_dependencies(const char *affected_name,
- struct prefix_list_entry *pentry,
- route_map_event_t event)
+void route_map_notify_pentry_dependencies(const char *affected_name,
+ struct prefix_list_entry *pentry,
+ route_map_event_t event)
{
struct route_map_dep *dep = NULL;
struct hash *upd8_hash = NULL;
@@ -2649,11 +2589,9 @@ route_map_result_t route_map_apply(struct route_map *map,
map->applied++;
- if ((!map->optimization_disabled) &&
- (map->ipv4_prefix_table ||
- map->ipv6_prefix_table)) {
- index = route_map_get_index(map, prefix,
- type, object,
+ if ((!map->optimization_disabled)
+ && (map->ipv4_prefix_table || map->ipv6_prefix_table)) {
+ index = route_map_get_index(map, prefix, type, object,
(uint8_t *)&match_ret);
if (index) {
if (rmap_debug)
@@ -2664,10 +2602,11 @@ route_map_result_t route_map_apply(struct route_map *map,
route_map_cmd_result_str(match_ret));
} else {
if (rmap_debug)
- zlog_debug("No match for pfx: %s in route-map: %s, result: %s",
- prefix2str(prefix, buf, sizeof(buf)),
- map->name,
- route_map_cmd_result_str(match_ret));
+ zlog_debug(
+ "No match for pfx: %s in route-map: %s, result: %s",
+ prefix2str(prefix, buf, sizeof(buf)),
+ map->name,
+ route_map_cmd_result_str(match_ret));
/*
* No index matches this prefix. Return deny unless,
* match_ret = RMAP_NOOP.
@@ -3148,12 +3087,11 @@ void route_map_notify_dependencies(const char *affected_name,
/* VTY related functions. */
-DEFUN (no_routemap_optimization,
- no_routemap_optimization_cmd,
- "no route-map optimization",
- NO_STR
- "route-map\n"
- "optimization\n")
+DEFUN(no_routemap_optimization, no_routemap_optimization_cmd,
+ "no route-map optimization",
+ NO_STR
+ "route-map\n"
+ "optimization\n")
{
VTY_DECLVAR_CONTEXT(route_map_index, index);
@@ -3161,11 +3099,10 @@ DEFUN (no_routemap_optimization,
return CMD_SUCCESS;
}
-DEFUN (routemap_optimization,
- routemap_optimization_cmd,
- "route-map optimization",
- "route-map\n"
- "optimization\n")
+DEFUN(routemap_optimization, routemap_optimization_cmd,
+ "route-map optimization",
+ "route-map\n"
+ "optimization\n")
{
VTY_DECLVAR_CONTEXT(route_map_index, index);
@@ -4362,13 +4299,12 @@ void route_map_counter_decrement(struct route_map *map)
}
}
-DEFUN_HIDDEN (show_route_map_pfx_tbl,
- show_route_map_pfx_tbl_cmd,
- "show route-map WORD prefix-table",
- SHOW_STR
- "route-map\n"
- "route-map name\n"
- "internal prefix-table\n")
+DEFUN_HIDDEN(show_route_map_pfx_tbl, show_route_map_pfx_tbl_cmd,
+ "show route-map WORD prefix-table",
+ SHOW_STR
+ "route-map\n"
+ "route-map name\n"
+ "internal prefix-table\n")
{
const char *rmap_name = argv[2]->arg;
struct route_map *rmap = NULL;
@@ -4405,18 +4341,16 @@ DEFUN_HIDDEN (show_route_map_pfx_tbl,
if (prn) {
pp = &prn->p;
vty_out(vty, "%s/%d\n",
- inet_ntop(
- pp->family,
- &pp->u.prefix,
- pbuf,
- SU_ADDRSTRLEN),
+ inet_ntop(pp->family,
+ &pp->u.prefix, pbuf,
+ SU_ADDRSTRLEN),
pp->prefixlen);
}
vty_out(vty, "\n");
- rmap_index_list = (struct list *) rn->info;
- if (!rmap_index_list ||
- !listcount(rmap_index_list))
+ rmap_index_list = (struct list *)rn->info;
+ if (!rmap_index_list
+ || !listcount(rmap_index_list))
vty_out(vty, "%*s%s\n", len, "", "-");
else
for (ALL_LIST_ELEMENTS(rmap_index_list,
@@ -4451,18 +4385,16 @@ DEFUN_HIDDEN (show_route_map_pfx_tbl,
if (prn) {
pp = &prn->p;
vty_out(vty, "%s/%d\n",
- inet_ntop(
- pp->family,
- &pp->u.prefix,
- pbuf,
- SU_ADDRSTRLEN),
+ inet_ntop(pp->family,
+ &pp->u.prefix, pbuf,
+ SU_ADDRSTRLEN),
pp->prefixlen);
}
vty_out(vty, "\n");
- rmap_index_list = (struct list *) rn->info;
- if (!rmap_index_list ||
- !listcount(rmap_index_list))
+ rmap_index_list = (struct list *)rn->info;
+ if (!rmap_index_list
+ || !listcount(rmap_index_list))
vty_out(vty, "%*s%s\n", len, "", "-");
else
for (ALL_LIST_ELEMENTS(rmap_index_list,
diff --git a/lib/routemap.h b/lib/routemap.h
index 63168dd44..8c02f1cb6 100644
--- a/lib/routemap.h
+++ b/lib/routemap.h
@@ -297,10 +297,10 @@ extern void route_map_upd8_dependency(route_map_event_t type, const char *arg,
const char *rmap_name);
extern void route_map_notify_dependencies(const char *affected_name,
route_map_event_t event);
-extern void route_map_notify_pentry_dependencies(
- const char *affected_name,
- struct prefix_list_entry *pentry,
- route_map_event_t event);
+extern void
+route_map_notify_pentry_dependencies(const char *affected_name,
+ struct prefix_list_entry *pentry,
+ route_map_event_t event);
extern int generic_match_add(struct vty *vty, struct route_map_index *index,
const char *command, const char *arg,
route_map_event_t type);
If you are a new contributor to FRR, please see our contributing guidelines.
💚 Basic BGPD CI results: SUCCESS, 0 tests failedResults table
For details, please contact louberger |
Continuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: FailedDebian 10 amd64 build: Failed (click for details)Debian 10 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-10176/artifact/DEB10BUILD/config.status/config.statusMake failed for Debian 10 amd64 build: FreeBSD 12 amd64 build: Failed (click for details)FreeBSD 12 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-10176/artifact/FBSD12AMD64/config.status/config.statusMake failed for FreeBSD 12 amd64 build: FreeBSD 11 amd64 build: Failed (click for details)Make failed for FreeBSD 11 amd64 build: FreeBSD 11 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-10176/artifact/CI009BUILD/config.status/config.status Ubuntu 18.04 amd64 build: Failed (click for details)Make failed for Ubuntu 18.04 amd64 build: Ubuntu 18.04 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-10176/artifact/U1804AMD64/config.status/config.status Debian 9 amd64 build: Failed (click for details)Make failed for Debian 9 amd64 build: Debian 9 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-10176/artifact/CI021BUILD/config.status/config.status Ubuntu 18.04 ppc64le build: Failed (click for details)Ubuntu 18.04 ppc64le build: config.log output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-10176/artifact/U1804PPC64LEBUILD/config.log/ Ubuntu 18.04 ppc64le build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-10176/artifact/U1804PPC64LEBUILD/config.status/config.statusMake failed for Ubuntu 18.04 ppc64le build: Fedora 29 amd64 build: Failed (click for details)Fedora 29 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-10176/artifact/F29BUILD/config.status/config.statusMake failed for Fedora 29 amd64 build: Successful on other platforms
|
💚 Basic BGPD CI results: SUCCESS, 0 tests failedResults table
For details, please contact louberger |
💚 Basic BGPD CI results: SUCCESS, 0 tests failedResults table
For details, please contact louberger |
Continuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: FailedTopotest tests on Ubuntu 16.04 i386: Failed (click for details)Topotest tests on Ubuntu 16.04 i386: Unknown Log URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-10179/artifact/TOPOI386/ErrorLog/ Topotest tests on Ubuntu 16.04 i386: No useful log foundSuccessful on other platforms
Topology Tests memory analysis: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-10179/artifact/TOPOI386/MemoryLeaks/CLANG Static Analyzer Summary
1 Static Analyzer issues remaining.See details at |
Continuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-10191/ This is a comment from an automated CI system. CLANG Static Analyzer Summary
1 Static Analyzer issues remaining.See details at |
NaveenThanikachalam
force-pushed
the
rmap
branch
4 times, most recently
from
aed2f26 to
b00f495
Compare
💚 Basic BGPD CI results: SUCCESS, 0 tests failedResults table
For details, please contact louberger |
💚 Basic BGPD CI results: SUCCESS, 0 tests failedResults table
For details, please contact louberger |
|
These changes look good to me, but it would be good to get either @donaldsharp or @eqvinox to take a look before committing. |
|
We'll leave this until Friday to see if David comes back around to it then have another look at it. |
* This commit introduces the building blocks.
A per-route-map prefix tree is introduced.
This tree will consist of the prefixes defined within the prefix-lists
that are added to the match clause of that route-map.
Signed-off-by: NaveenThanikachalam <nthanikachal@vmware.com>
This commit introduces the logic that computes the best-match route-map index for a given prefix. Signed-off-by: NaveenThanikachalam <nthanikachal@vmware.com>
* This commit implements the code style suggestions from Polychaeta. * This commit also introduces a CLI to toggle the optimization and, a hidden CLI to display the contents of the constructed prefix tree. Signed-off-by: NaveenThanikachalam <nthanikachal@vmware.com>
The commit includes the documentation for the newly introduced commands to enable/disable the optimization. Signed-off-by: NaveenThanikachalam <nthanikachal@vmware.com>
NaveenThanikachalam
force-pushed
the
rmap
branch
from
b00f495 to
009d25a
Compare
💚 Basic BGPD CI results: SUCCESS, 0 tests failedResults table
For details, please contact louberger |
Continuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-10762/ This is a comment from an automated CI system. Warnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build: clang_check |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR introduces a new way to fetch the best-match route-map index for a
given prefix.
Instead of sequentially passing through all the route-map indexes until a match is found, the search for the best-match index will be based on a look-up in a prefix-tree.
The details of the concept have been captured in the document below:
Fetching_the_Best-Match_Route-Map_Clause_Through_Longest_Prefix_Match_FRR.pdf
The design approach has been captured in the document below:
Route-map_Optimization_Design_Description.pdf
The below document captures the tests that have been executed:
Route-Map_Optimisation_Unit_Tests.pdf
Signed-off-by: NaveenThanikachalam nthanikachal@vmware.com