bgpd: Enable rfc8212 by default except datacenter profile #6164
Conversation
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/86d20dab4bf1554a7a3f15665c17c66d/raw/cfca618428cd2f667ea0bacbb0e53d2fbead20e0/cr_6164_1586182845.diff | git apply
diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c
index e817626f9..ff678eeb2 100644
--- a/bgpd/bgp_vty.c
+++ b/bgpd/bgp_vty.c
@@ -101,9 +101,11 @@ FRR_CFG_DEFAULT_ULONG(BGP_KEEPALIVE,
{ .val_ulong = 60 },
)
FRR_CFG_DEFAULT_BOOL(BGP_EBGP_REQUIRES_POLICY,
- { .val_bool = false, .match_profile = "datacenter", },
- { .val_bool = true },
-)
+ {
+ .val_bool = false,
+ .match_profile = "datacenter",
+ },
+ {.val_bool = true}, )
DEFINE_HOOK(bgp_inst_config_write,
(struct bgp *bgp, struct vty *vty),
diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h
index 3fe5eece6..afd4a85d2 100644
--- a/bgpd/bgpd.h
+++ b/bgpd/bgpd.h
@@ -446,7 +446,7 @@ struct bgp {
#define BGP_FLAG_DELETE_IN_PROGRESS (1 << 22)
#define BGP_FLAG_SELECT_DEFER_DISABLE (1 << 23)
#define BGP_FLAG_GR_DISABLE_EOR (1 << 24)
-#define BGP_FLAG_EBGP_REQUIRES_POLICY (1 << 25)
+#define BGP_FLAG_EBGP_REQUIRES_POLICY (1 << 25)
enum global_mode GLOBAL_GR_FSM[BGP_GLOBAL_GR_MODE]
[BGP_GLOBAL_GR_EVENT_CMD];
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 |
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: FailedIPv4 protocols on Ubuntu 14.04: Failed (click for details)RFC Compliance Test ANVL-BGP4-1.3 failing: RFC Compliance Test ANVL-BGP4-1.4 failing: RFC Compliance Test ANVL-BGP4-4.1 failing: RFC Compliance Test ANVL-BGP-AS4-2.3 failing: RFC Compliance Test ANVL-BGP-AS4-2.6 failing: IPv6 protocols on Ubuntu 14.04: Failed (click for details)RFC Compliance Test ANVL-BGPPLUS-AS4-4.1 failing: Successful on other platforms/tests
Warnings Generated during build:Checkout code: Successful with additional warningsIPv4 protocols on Ubuntu 14.04: Failed (click for details)RFC Compliance Test ANVL-BGP4-1.3 failing: RFC Compliance Test ANVL-BGP4-1.4 failing: RFC Compliance Test ANVL-BGP4-4.1 failing: RFC Compliance Test ANVL-BGP-AS4-2.3 failing: RFC Compliance Test ANVL-BGP-AS4-2.6 failing: IPv6 protocols on Ubuntu 14.04: Failed (click for details)RFC Compliance Test ANVL-BGPPLUS-AS4-4.1 failing: Warnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build: CLANG Static Analyzer Summary
No Changes in Static Analysis warnings compared to base1 Static Analyzer issues remaining.See details at |
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: FailedIPv4 protocols on Ubuntu 14.04: Failed (click for details)RFC Compliance Test ANVL-BGP4-1.3 failing: RFC Compliance Test ANVL-BGP4-1.4 failing: RFC Compliance Test ANVL-BGP4-4.1 failing: RFC Compliance Test ANVL-BGP-AS4-2.3 failing: RFC Compliance Test ANVL-BGP-AS4-2.6 failing: IPv6 protocols on Ubuntu 14.04: Failed (click for details)RFC Compliance Test ANVL-BGPPLUS-AS4-4.1 failing: Successful on other platforms/tests
Warnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build: CLANG Static Analyzer Summary
No Changes in Static Analysis warnings compared to base1 Static Analyzer issues remaining.See details at |
|
The ANVL basic test will need to be updated for this, since the RFC changed previous standard behaviour. |
|
Where can I change that? |
You can't, I already poked @mwinter-osr about it :) ... he needs to add |
|
thanks, dude 🥇
|
|
I'll update this branch when ANVL will be fixed 👍 |
|
CI:rerun |
Continuous Integration Result: SUCCESSFULContinuous 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-11781/ 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 Static Analyzer Summary
No Changes in Static Analysis warnings compared to base1 Static Analyzer issues remaining.See details at |
|
CI:rerun |
|
Compliance Tests are adjusted. |
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: FailedCentOS 7 rpm pkg check: Failed (click for details)CentOS 7 rpm pkg check: No useful log foundSuccessful on other platforms/tests
Warnings Generated during build:Checkout code: Successful with additional warningsCentOS 7 rpm pkg check: Failed (click for details)CentOS 7 rpm pkg check: No useful log foundWarnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build: CLANG Static Analyzer Summary
No Changes in Static Analysis warnings compared to base1 Static Analyzer issues remaining.See details at |
|
Thank you @mwinter-osr, I'll rebase and address missing comments. |
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/098e64e702a72d56b5cc7a1e6c3c119e/raw/1e2c7d6968860b5c27a8f525030bfb93df716511/cr_6164_1586764595.diff | git apply
diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c
index 3a97f0a7e..39612eea9 100644
--- a/bgpd/bgp_vty.c
+++ b/bgpd/bgp_vty.c
@@ -101,10 +101,17 @@ FRR_CFG_DEFAULT_ULONG(BGP_KEEPALIVE,
{ .val_ulong = 60 },
)
FRR_CFG_DEFAULT_BOOL(BGP_EBGP_REQUIRES_POLICY,
- { .val_bool = false, .match_profile = "datacenter", },
- { .val_bool = false, .match_version = "<= 7.4", },
- { .val_bool = true, },
-)
+ {
+ .val_bool = false,
+ .match_profile = "datacenter",
+ },
+ {
+ .val_bool = false,
+ .match_version = "<= 7.4",
+ },
+ {
+ .val_bool = true,
+ }, )
DEFINE_HOOK(bgp_inst_config_write,
(struct bgp *bgp, struct vty *vty),
If you are a new contributor to FRR, please see our contributing guidelines.
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: FailedCentOS 7 amd64 build: Failed (click for details)CentOS 7 amd64 build: No useful log foundSuccessful on other platforms/tests
Warnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build: |
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
This comment has been minimized.
This comment has been minimized.
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/66d4e045a4435ab8fef077be97ceb976/raw/4d4b2c30ed98cdec6bb633a6bfdc2784a07427ac/cr_6164_1586805595.diff | git apply
diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c
index 0284f6e4c..2fa2a2ffa 100644
--- a/bgpd/bgp_vty.c
+++ b/bgpd/bgp_vty.c
@@ -101,9 +101,11 @@ FRR_CFG_DEFAULT_ULONG(BGP_KEEPALIVE,
{ .val_ulong = 60 },
)
FRR_CFG_DEFAULT_BOOL(BGP_EBGP_REQUIRES_POLICY,
- { .val_bool = false, .match_profile = "datacenter", },
- { .val_bool = true },
-)
+ {
+ .val_bool = false,
+ .match_profile = "datacenter",
+ },
+ {.val_bool = true}, )
DEFINE_HOOK(bgp_inst_config_write,
(struct bgp *bgp, struct vty *vty),
If you are a new contributor to FRR, please see our contributing guidelines.
This comment has been minimized.
This comment has been minimized.
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/adc0c6836c911c163df1d91b2c7a7b54/raw/4d4b2c30ed98cdec6bb633a6bfdc2784a07427ac/cr_6164_1586807169.diff | git apply
diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c
index 0284f6e4c..2fa2a2ffa 100644
--- a/bgpd/bgp_vty.c
+++ b/bgpd/bgp_vty.c
@@ -101,9 +101,11 @@ FRR_CFG_DEFAULT_ULONG(BGP_KEEPALIVE,
{ .val_ulong = 60 },
)
FRR_CFG_DEFAULT_BOOL(BGP_EBGP_REQUIRES_POLICY,
- { .val_bool = false, .match_profile = "datacenter", },
- { .val_bool = true },
-)
+ {
+ .val_bool = false,
+ .match_profile = "datacenter",
+ },
+ {.val_bool = true}, )
DEFINE_HOOK(bgp_inst_config_write,
(struct bgp *bgp, struct vty *vty),
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: SUCCESSFULContinuous 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-11820/ 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: |
Continuous Integration Result: SUCCESSFULContinuous 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-11821/ 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: |
Some competitive vendors like Cisco, Bird, OpenBGPD, Nokia already have this by default enabled. The list is here: https://github.com/bgp/RFC8212 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>
Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
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/96ea5e3af275962cf726098ca65bccec/raw/105f7a2e77fdf701a9de2b3581d14d3c39e372aa/cr_6164_1586869317.diff | git apply
diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c
index f5a648b50..540be2060 100644
--- a/bgpd/bgp_vty.c
+++ b/bgpd/bgp_vty.c
@@ -101,10 +101,15 @@ FRR_CFG_DEFAULT_ULONG(BGP_KEEPALIVE,
{ .val_ulong = 60 },
)
FRR_CFG_DEFAULT_BOOL(BGP_EBGP_REQUIRES_POLICY,
- { .val_bool = false, .match_profile = "datacenter", },
- { .val_bool = false, .match_version = "< 7.4", },
- { .val_bool = true },
-)
+ {
+ .val_bool = false,
+ .match_profile = "datacenter",
+ },
+ {
+ .val_bool = false,
+ .match_version = "< 7.4",
+ },
+ {.val_bool = true}, )
DEFINE_HOOK(bgp_inst_config_write,
(struct bgp *bgp, struct vty *vty),
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: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-11840/ 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: |
Some competitive vendors like Cisco, Bird, OpenBGPD, Nokia already have this by default enabled.
The list is here: https://github.com/bgp/RFC8212