route-map on-match goto prefix-list-seqno
#5719
Closed
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
Signed-off-by: David Lamparter <equinox@diac24.net>
Signed-off-by: David Lamparter <equinox@diac24.net>
Signed-off-by: David Lamparter <equinox@diac24.net>
polychaeta
reviewed
Jan 22, 2020
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/9e336f6ef559abf0753caad2effdf5b3/raw/c331b8877ba1601f955361010fef8bf90c5884a3/cr_5719_1579703467.diff | git apply
diff --git a/bgpd/bgp_routemap.c b/bgpd/bgp_routemap.c
index 83bad8f56..47705c99a 100644
--- a/bgpd/bgp_routemap.c
+++ b/bgpd/bgp_routemap.c
@@ -634,9 +634,8 @@ route_match_address_prefix_list(void *rule, afi_t afi,
return route_match_prefix_list_flowspec(afi, plist,
state->prefix);
- plist_ret = prefix_list_apply_which_prefix(plist, NULL,
- &state->plist_seqno,
- state->prefix);
+ plist_ret = prefix_list_apply_which_prefix(
+ plist, NULL, &state->plist_seqno, state->prefix);
return plist_ret == PREFIX_DENY ? RMAP_NOMATCH : RMAP_MATCH;
}
@@ -657,14 +656,14 @@ static void route_match_ip_address_prefix_list_free(void *rule)
XFREE(MTYPE_ROUTE_MAP_COMPILED, rule);
}
-static const struct route_map_rule_cmd
- route_match_ip_address_prefix_list_cmd = {
- "ip address prefix-list",
- NULL,
- route_match_ip_address_prefix_list_compile,
- route_match_ip_address_prefix_list_free,
- NULL,
- route_match_ip_address_prefix_list_ext,
+static const struct route_map_rule_cmd route_match_ip_address_prefix_list_cmd =
+ {
+ "ip address prefix-list",
+ NULL,
+ route_match_ip_address_prefix_list_compile,
+ route_match_ip_address_prefix_list_free,
+ NULL,
+ route_match_ip_address_prefix_list_ext,
};
/* `match ip next-hop prefix-list PREFIX_LIST' */
@@ -2854,13 +2853,13 @@ static void route_match_ipv6_address_prefix_list_free(void *rule)
}
static const struct route_map_rule_cmd
- route_match_ipv6_address_prefix_list_cmd = {
- "ipv6 address prefix-list",
- NULL,
- route_match_ipv6_address_prefix_list_compile,
- route_match_ipv6_address_prefix_list_free,
- NULL,
- route_match_ipv6_address_prefix_list_ext,
+ route_match_ipv6_address_prefix_list_cmd = {
+ "ipv6 address prefix-list",
+ NULL,
+ route_match_ipv6_address_prefix_list_compile,
+ route_match_ipv6_address_prefix_list_free,
+ NULL,
+ route_match_ipv6_address_prefix_list_ext,
};
/* `match ipv6 next-hop type <TYPE>' */
diff --git a/lib/plist.c b/lib/plist.c
index cf4f2e85a..df3f5a252 100644
--- a/lib/plist.c
+++ b/lib/plist.c
@@ -681,11 +681,10 @@ static int prefix_list_entry_match(struct prefix_list_entry *pentry,
return 1;
}
-enum prefix_list_type prefix_list_apply_which_prefix(
- struct prefix_list *plist,
- const struct prefix **which,
- uint32_t *seq,
- const void *object)
+enum prefix_list_type
+prefix_list_apply_which_prefix(struct prefix_list *plist,
+ const struct prefix **which, uint32_t *seq,
+ const void *object)
{
struct prefix_list_entry *pentry, *pbest = NULL;
diff --git a/lib/plist.h b/lib/plist.h
index 8f5547f1a..60799db0b 100644
--- a/lib/plist.h
+++ b/lib/plist.h
@@ -66,10 +66,10 @@ extern struct prefix_list *prefix_list_lookup(afi_t, const char *);
*/
extern enum prefix_list_type
prefix_list_apply_which_prefix(struct prefix_list *plist,
- const struct prefix **which,
- uint32_t *seq,
+ const struct prefix **which, uint32_t *seq,
const void *object);
-#define prefix_list_apply(A, B) prefix_list_apply_which_prefix((A), NULL, NULL, (B))
+#define prefix_list_apply(A, B) \
+ prefix_list_apply_which_prefix((A), NULL, NULL, (B))
extern struct prefix_list *prefix_bgp_orf_lookup(afi_t, const char *);
extern struct stream *prefix_bgp_orf_entry(struct stream *,
diff --git a/lib/routemap.c b/lib/routemap.c
index aadc2980b..056815988 100644
--- a/lib/routemap.c
+++ b/lib/routemap.c
@@ -1006,7 +1006,8 @@ static void vty_show_route_map_entry(struct vty *vty, struct route_map *map)
if (index->exitpolicy == RMAP_GOTO)
vty_out(vty, " Goto %d\n", index->nextpref);
else if (index->exitpolicy == RMAP_GOTO_PLIST)
- vty_out(vty, " Goto sequence number from prefix-list\n");
+ vty_out(vty,
+ " Goto sequence number from prefix-list\n");
else if (index->exitpolicy == RMAP_NEXT)
vty_out(vty, " Continue to next entry\n");
else if (index->exitpolicy == RMAP_EXIT)
@@ -1628,10 +1629,9 @@ route_map_apply_match(struct route_map_rule_list *match_list,
ret = (*match->cmd->func_apply_ext)(
match->value, state);
else
- ret = (*match->cmd->func_apply)(match->value,
- state->prefix,
- state->type,
- state->object);
+ ret = (*match->cmd->func_apply)(
+ match->value, state->prefix,
+ state->type, state->object);
/*
* If the consolidated result of func_apply is:
@@ -3054,12 +3054,11 @@ DEFUN (rmap_onmatch_goto,
return CMD_SUCCESS;
}
-DEFUN (rmap_onmatch_goto_plist,
- rmap_onmatch_goto_plist_cmd,
- "on-match goto prefix-list-seqno",
- "Exit policy on matches\n"
- "Goto Clause number\n"
- "Use sequence from prefix list entry as goto target\n")
+DEFUN(rmap_onmatch_goto_plist, rmap_onmatch_goto_plist_cmd,
+ "on-match goto prefix-list-seqno",
+ "Exit policy on matches\n"
+ "Goto Clause number\n"
+ "Use sequence from prefix list entry as goto target\n")
{
struct route_map_index *index = VTY_GET_CONTEXT(route_map_index);
@@ -3326,7 +3325,8 @@ static int route_map_config_write(struct vty *vty)
vty_out(vty, " on-match goto %d\n",
index->nextpref);
if (index->exitpolicy == RMAP_GOTO_PLIST)
- vty_out(vty, " on-match goto prefix-list-seqno\n");
+ vty_out(vty,
+ " on-match goto prefix-list-seqno\n");
if (index->exitpolicy == RMAP_NEXT)
vty_out(vty, " on-match next\n");
diff --git a/lib/routemap.h b/lib/routemap.h
index 275761cd6..083ede43c 100644
--- a/lib/routemap.h
+++ b/lib/routemap.h
@@ -142,8 +142,8 @@ struct route_map_rule_cmd {
void *(*func_get_rmap_rule_key)(void *val);
/* Extended "apply" function */
- enum route_map_cmd_result_t (*func_apply_ext)(void *rule,
- struct route_map_state *state);
+ enum route_map_cmd_result_t (*func_apply_ext)(
+ void *rule, struct route_map_state *state);
};
/* Route map apply error. */
diff --git a/pimd/pim_rp.c b/pimd/pim_rp.c
index 38b3dc95b..e2b486e71 100644
--- a/pimd/pim_rp.c
+++ b/pimd/pim_rp.c
@@ -219,7 +219,8 @@ struct rp_info *pim_rp_find_match_group(struct pim_instance *pim,
plist = prefix_list_lookup(AFI_IP, rp_info->plist);
if (prefix_list_apply_which_prefix(plist, &p, NULL,
- group) == PREFIX_DENY)
+ group)
+ == PREFIX_DENY)
continue;
if (!best) {
If you are a new contributor to FRR, please see our contributing guidelines.
|
Instead of tying this to the sequence number, we could also add a new prefix-list parameter (e.g. |
💚 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-10516/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warningsWarnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build: clang_check |
|
Thanks, David.
|
|
My concern here would be -- what if I use the same set of access lists in a lot of different route maps, but want things evaluated in different orders? I don't know if that would work with this solution... |
|
@eqvinox any comments re: the above? |
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 is an alternate approach / "discussion" PR for #5270.
It uses the sequence number from the prefix list as the "goto" target in the route-map. This might be a bit "unconventional", but the change is much less intrusive than #5270 and possibly faster too.
While the concept may be a bit "huh?" to users, I think there is also a benefit in having it be obvious to the user what's happening.
Either way this PR is for discussion / comparing with #5270.
(Downside: due to the way we match prefix lists, all the daemons need a similar "mechanical" change as I've done here for bgpd, but I don't think that's a problem.)