-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
bgpd: Allow using add/subtract for local-preference in route-maps #5855
bgpd: Allow using add/subtract for local-preference in route-maps #5855
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution to FRR!
Click for style suggestions
To apply these suggestions:
curl -s https://gist.githubusercontent.com/polychaeta/0e239cb274c125c92bfd2c17381956b5/raw/f81fddff1b29c02a4a0c135263ec02279494323f/cr_5855_1582488543.diff | git apply
diff --git a/bgpd/bgp_routemap.c b/bgpd/bgp_routemap.c
index 8f498fc3c..5330576ab 100644
--- a/bgpd/bgp_routemap.c
+++ b/bgpd/bgp_routemap.c
@@ -4398,12 +4398,10 @@ DEFUN (no_set_distance,
"distance", NULL);
}
-DEFUN (set_local_pref,
- set_local_pref_cmd,
- "set local-preference WORD",
- SET_STR
- "BGP local preference path attribute\n"
- "Preference value\n")
+DEFUN(set_local_pref, set_local_pref_cmd, "set local-preference WORD",
+ SET_STR
+ "BGP local preference path attribute\n"
+ "Preference value\n")
{
int idx_number = 2;
return generic_set_add(vty, VTY_GET_CONTEXT(route_map_index),
@@ -4411,13 +4409,10 @@ DEFUN (set_local_pref,
}
-DEFUN (no_set_local_pref,
- no_set_local_pref_cmd,
- "no set local-preference WORD",
- NO_STR
- SET_STR
- "BGP local preference path attribute\n"
- "Preference value\n")
+DEFUN(no_set_local_pref, no_set_local_pref_cmd, "no set local-preference WORD",
+ NO_STR SET_STR
+ "BGP local preference path attribute\n"
+ "Preference value\n")
{
int idx_localpref = 3;
if (argc <= idx_localpref)
If you are a new contributor to FRR, please see our contributing guidelines.
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
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-10842/ This is a comment from an automated CI system. CLANG Static Analyzer Summary
2 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-10841/ 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 Static Analyzer Summary
2 Static Analyzer issues remaining.See details at |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Donatas, Is there any other attribute that we treat this way in route maps ? Just an example.
Also, now that this is a string, how to make sure the WORD entered is actually in the range of [0-4294967295], and other numerical validations ?
I will ping you offline for few more questions.
@srimohans validation is done in |
We already have a generic support for add/sub in route-maps. It's already handled in route_value_compile(). Just convert to string (allow passing (-) minus sign) - works like expected. Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
💚 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-10899/ 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:
|
@riw777 Is there anything pending on this ? I will go ahead and merge this otherwise. |
See individual commits.