From d37c84f28b9f13a38da4a3a0074b9bf73669b90d Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Tue, 15 Feb 2022 15:54:53 -0500 Subject: [PATCH 1/5] bgpd: Remove impossible invalid state confederations are checking to see that the bgp pointer is non-null. But it's impossible to have a null pointer in the cli and in all paths we have already deref'ed the bgp pointer. Let's remove that error code as that it is impossible to happen. Signed-off-by: Donald Sharp (cherry picked from commit 8b4a0b6631c9484a8925a7279b7a111c3e340de1) --- bgpd/bgpd.c | 3 --- bgpd/bgpd.h | 1 - 2 files changed, 4 deletions(-) diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index e6d4000ad8..6abd6ef8c4 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -651,9 +651,6 @@ int bgp_confederation_peers_add(struct bgp *bgp, as_t as) struct peer *peer; struct listnode *node, *nnode; - if (!bgp) - return BGP_ERR_INVALID_BGP; - if (bgp->as == as) return BGP_ERR_INVALID_AS; diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h index ae6427b356..d102a17dc5 100644 --- a/bgpd/bgpd.h +++ b/bgpd/bgpd.h @@ -1921,7 +1921,6 @@ enum bgp_clear_type { #define BGP_ERR_INVALID_VALUE -1 #define BGP_ERR_INVALID_FLAG -2 #define BGP_ERR_INVALID_AS -3 -#define BGP_ERR_INVALID_BGP -4 #define BGP_ERR_PEER_GROUP_MEMBER -5 #define BGP_ERR_PEER_GROUP_NO_REMOTE_AS -7 #define BGP_ERR_PEER_GROUP_CANT_CHANGE -8 From b462a0a7f62bc688e4bf772f31f7014201033fdc Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Tue, 15 Feb 2022 16:04:50 -0500 Subject: [PATCH 2/5] bgpd: Move some error codes to bgp_vty_return handling BGP_ERR_PEER_GROUP_MEMBER and BGP_ERR_PEER_GROUP_PEER_TYPE_DIFFERENT both are not handled by bgp_vty_return, but both can be handled by this function as that there is nothing special going on here. Signed-off-by: Donald Sharp (cherry picked from commit 6dcea6fe05f38d8542edd7b06d301d237e0c028a) --- bgpd/bgp_vty.c | 24 ++++++------------------ 1 file changed, 6 insertions(+), 18 deletions(-) diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index df134687d5..05b49c5689 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -879,6 +879,12 @@ int bgp_vty_return(struct vty *vty, int ret) case BGP_ERR_GR_OPERATION_FAILED: str = "The Graceful Restart Operation failed due to an err."; break; + case BGP_ERR_PEER_GROUP_MEMBER: + str = "Peer-group member cannot override remote-as of peer-group."; + break; + case BGP_ERR_PEER_GROUP_PEER_TYPE_DIFFERENT: + str = "Peer-group members must be all internal or all external."; + break; } if (str) { vty_out(vty, "%% %s\n", str); @@ -4213,17 +4219,6 @@ static int peer_remote_as_vty(struct vty *vty, const char *peer_str, ret = peer_remote_as(bgp, &su, NULL, &as, as_type); } - /* This peer belongs to peer group. */ - switch (ret) { - case BGP_ERR_PEER_GROUP_MEMBER: - vty_out(vty, - "%% Peer-group member cannot override remote-as of peer-group\n"); - return CMD_WARNING_CONFIG_FAILED; - case BGP_ERR_PEER_GROUP_PEER_TYPE_DIFFERENT: - vty_out(vty, - "%% Peer-group members must be all internal or all external\n"); - return CMD_WARNING_CONFIG_FAILED; - } return bgp_vty_return(vty, ret); } @@ -4960,13 +4955,6 @@ DEFUN (neighbor_set_peer_group, ret = peer_group_bind(bgp, &su, peer, group, &as); - if (ret == BGP_ERR_PEER_GROUP_PEER_TYPE_DIFFERENT) { - vty_out(vty, - "%% Peer with AS %u cannot be in this peer-group, members must be all internal or all external\n", - as); - return CMD_WARNING_CONFIG_FAILED; - } - return bgp_vty_return(vty, ret); } From beaab6f79f00a89592e01d6046e9f3609e283a04 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Tue, 15 Feb 2022 16:12:02 -0500 Subject: [PATCH 3/5] bgpd: Remove unused BGP_ERR_MAX #define Signed-off-by: Donald Sharp (cherry picked from commit b5d6f2d068346e532d132421c52277206368c8af) --- bgpd/bgpd.h | 1 - 1 file changed, 1 deletion(-) diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h index d102a17dc5..b2fc3ed549 100644 --- a/bgpd/bgpd.h +++ b/bgpd/bgpd.h @@ -1947,7 +1947,6 @@ enum bgp_clear_type { #define BGP_ERR_DYNAMIC_NEIGHBORS_RANGE_EXISTS -30 #define BGP_ERR_DYNAMIC_NEIGHBORS_RANGE_NOT_FOUND -31 #define BGP_ERR_INVALID_FOR_DYNAMIC_PEER -32 -#define BGP_ERR_MAX -33 #define BGP_ERR_INVALID_FOR_DIRECT_PEER -34 #define BGP_ERR_PEER_SAFI_CONFLICT -35 From 152487b3e5b44e4e75898b0d80a2c8e4577ec76d Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Tue, 15 Feb 2022 15:53:30 -0500 Subject: [PATCH 4/5] bgpd: Convert bgp error codes for cli input to an enum Conversion of bgp error codes returned for cli input into an enum and then properly handling all the error cases in bgp_vty_return. Because not all error codes returned were properly handled in this function there existed configuration examples that were accepted on the cli without an error message but not saved. Fixes: #10589 Signed-off-by: Donald Sharp (cherry picked from commit 4b7e23e9f2822def568cc9dd1a942d9d36cbb3a5) --- bgpd/bgp_vty.c | 30 +++++++++++++++++++- bgpd/bgpd.h | 76 ++++++++++++++++++++++++++------------------------ 2 files changed, 68 insertions(+), 38 deletions(-) diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index 05b49c5689..95c06bd21e 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -805,11 +805,15 @@ struct peer *peer_and_group_lookup_vty(struct vty *vty, const char *peer_str) return NULL; } -int bgp_vty_return(struct vty *vty, int ret) +int bgp_vty_return(struct vty *vty, enum bgp_create_error_code ret) { const char *str = NULL; switch (ret) { + case BGP_SUCCESS: + case BGP_CREATED: + case BGP_GR_NO_OPERATION: + break; case BGP_ERR_INVALID_VALUE: str = "Invalid value"; break; @@ -885,6 +889,30 @@ int bgp_vty_return(struct vty *vty, int ret) case BGP_ERR_PEER_GROUP_PEER_TYPE_DIFFERENT: str = "Peer-group members must be all internal or all external."; break; + case BGP_ERR_DYNAMIC_NEIGHBORS_RANGE_NOT_FOUND: + str = "Range specified cannot be deleted because it is not part of current config."; + break; + case BGP_ERR_INSTANCE_MISMATCH: + str = "Instance specified does not match the current instance."; + break; + case BGP_ERR_NO_INTERFACE_CONFIG: + str = "Interface specified is not being used for interface based peer."; + break; + case BGP_ERR_SOFT_RECONFIG_UNCONFIGURED: + str = "No configuration already specified for soft reconfiguration."; + break; + case BGP_ERR_AS_MISMATCH: + str = "BGP is already running."; + break; + case BGP_ERR_AF_UNCONFIGURED: + str = "AFI/SAFI specified is not currently configured."; + break; + case BGP_ERR_CANNOT_HAVE_LOCAL_AS_SAME_AS_REMOTE_AS: + str = "AS specified for local as is the same as the remote as and this is not allowed."; + break; + case BGP_ERR_INVALID_AS: + str = "Confederation AS specified is the same AS as our AS."; + break; } if (str) { vty_out(vty, "%% %s\n", str); diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h index b2fc3ed549..f3023507f7 100644 --- a/bgpd/bgpd.h +++ b/bgpd/bgpd.h @@ -1916,44 +1916,46 @@ enum bgp_clear_type { (((S) == OpenSent) || ((S) == OpenConfirm) || ((S) == Established)) /* BGP error codes. */ -#define BGP_SUCCESS 0 -#define BGP_CREATED 1 -#define BGP_ERR_INVALID_VALUE -1 -#define BGP_ERR_INVALID_FLAG -2 -#define BGP_ERR_INVALID_AS -3 -#define BGP_ERR_PEER_GROUP_MEMBER -5 -#define BGP_ERR_PEER_GROUP_NO_REMOTE_AS -7 -#define BGP_ERR_PEER_GROUP_CANT_CHANGE -8 -#define BGP_ERR_PEER_GROUP_MISMATCH -9 -#define BGP_ERR_PEER_GROUP_PEER_TYPE_DIFFERENT -10 -#define BGP_ERR_AS_MISMATCH -12 -#define BGP_ERR_PEER_FLAG_CONFLICT -13 -#define BGP_ERR_PEER_GROUP_SHUTDOWN -14 -#define BGP_ERR_PEER_FILTER_CONFLICT -15 -#define BGP_ERR_NOT_INTERNAL_PEER -16 -#define BGP_ERR_REMOVE_PRIVATE_AS -17 -#define BGP_ERR_AF_UNCONFIGURED -18 -#define BGP_ERR_SOFT_RECONFIG_UNCONFIGURED -19 -#define BGP_ERR_INSTANCE_MISMATCH -20 -#define BGP_ERR_LOCAL_AS_ALLOWED_ONLY_FOR_EBGP -21 -#define BGP_ERR_CANNOT_HAVE_LOCAL_AS_SAME_AS -22 -#define BGP_ERR_TCPSIG_FAILED -23 -#define BGP_ERR_NO_EBGP_MULTIHOP_WITH_TTLHACK -24 -#define BGP_ERR_NO_IBGP_WITH_TTLHACK -25 -#define BGP_ERR_NO_INTERFACE_CONFIG -26 -#define BGP_ERR_CANNOT_HAVE_LOCAL_AS_SAME_AS_REMOTE_AS -27 -#define BGP_ERR_AS_OVERRIDE -28 -#define BGP_ERR_INVALID_DYNAMIC_NEIGHBORS_LIMIT -29 -#define BGP_ERR_DYNAMIC_NEIGHBORS_RANGE_EXISTS -30 -#define BGP_ERR_DYNAMIC_NEIGHBORS_RANGE_NOT_FOUND -31 -#define BGP_ERR_INVALID_FOR_DYNAMIC_PEER -32 -#define BGP_ERR_INVALID_FOR_DIRECT_PEER -34 -#define BGP_ERR_PEER_SAFI_CONFLICT -35 +enum bgp_create_error_code { + BGP_SUCCESS = 0, + BGP_CREATED = 1, + BGP_ERR_INVALID_VALUE = -1, + BGP_ERR_INVALID_FLAG = -2, + BGP_ERR_INVALID_AS = -3, + BGP_ERR_PEER_GROUP_MEMBER = -5, + BGP_ERR_PEER_GROUP_NO_REMOTE_AS = -7, + BGP_ERR_PEER_GROUP_CANT_CHANGE = -8, + BGP_ERR_PEER_GROUP_MISMATCH = -9, + BGP_ERR_PEER_GROUP_PEER_TYPE_DIFFERENT = -10, + BGP_ERR_AS_MISMATCH = -12, + BGP_ERR_PEER_FLAG_CONFLICT = -13, + BGP_ERR_PEER_GROUP_SHUTDOWN = -14, + BGP_ERR_PEER_FILTER_CONFLICT = -15, + BGP_ERR_NOT_INTERNAL_PEER = -16, + BGP_ERR_REMOVE_PRIVATE_AS = -17, + BGP_ERR_AF_UNCONFIGURED = -18, + BGP_ERR_SOFT_RECONFIG_UNCONFIGURED = -19, + BGP_ERR_INSTANCE_MISMATCH = -20, + BGP_ERR_LOCAL_AS_ALLOWED_ONLY_FOR_EBGP = -21, + BGP_ERR_CANNOT_HAVE_LOCAL_AS_SAME_AS = -22, + BGP_ERR_TCPSIG_FAILED = -23, + BGP_ERR_NO_EBGP_MULTIHOP_WITH_TTLHACK = -24, + BGP_ERR_NO_IBGP_WITH_TTLHACK = -25, + BGP_ERR_NO_INTERFACE_CONFIG = -26, + BGP_ERR_CANNOT_HAVE_LOCAL_AS_SAME_AS_REMOTE_AS = -27, + BGP_ERR_AS_OVERRIDE = -28, + BGP_ERR_INVALID_DYNAMIC_NEIGHBORS_LIMIT = -29, + BGP_ERR_DYNAMIC_NEIGHBORS_RANGE_EXISTS = -30, + BGP_ERR_DYNAMIC_NEIGHBORS_RANGE_NOT_FOUND = -31, + BGP_ERR_INVALID_FOR_DYNAMIC_PEER = -32, + BGP_ERR_INVALID_FOR_DIRECT_PEER = -34, + BGP_ERR_PEER_SAFI_CONFLICT = -35, -/* BGP GR ERRORS */ -#define BGP_ERR_GR_INVALID_CMD -36 -#define BGP_ERR_GR_OPERATION_FAILED -37 -#define BGP_GR_NO_OPERATION -38 + /* BGP GR ERRORS */ + BGP_ERR_GR_INVALID_CMD = -36, + BGP_ERR_GR_OPERATION_FAILED = -37, + BGP_GR_NO_OPERATION = -38, +}; /* * Enumeration of different policy kinds a peer can be configured with. From bd971829f3cfe35fef0bbdaad0825ff851988ea8 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Tue, 15 Feb 2022 16:36:30 -0500 Subject: [PATCH 5/5] bgpd: Renumber bgp_create_error_code enum values Signed-off-by: Donald Sharp (cherry picked from commit e4aa4745f2e6e53e4cadb103551b2044ca0e8b59) --- bgpd/bgpd.h | 62 ++++++++++++++++++++++++++--------------------------- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h index f3023507f7..669912b382 100644 --- a/bgpd/bgpd.h +++ b/bgpd/bgpd.h @@ -1922,39 +1922,39 @@ enum bgp_create_error_code { BGP_ERR_INVALID_VALUE = -1, BGP_ERR_INVALID_FLAG = -2, BGP_ERR_INVALID_AS = -3, - BGP_ERR_PEER_GROUP_MEMBER = -5, - BGP_ERR_PEER_GROUP_NO_REMOTE_AS = -7, - BGP_ERR_PEER_GROUP_CANT_CHANGE = -8, - BGP_ERR_PEER_GROUP_MISMATCH = -9, - BGP_ERR_PEER_GROUP_PEER_TYPE_DIFFERENT = -10, - BGP_ERR_AS_MISMATCH = -12, - BGP_ERR_PEER_FLAG_CONFLICT = -13, - BGP_ERR_PEER_GROUP_SHUTDOWN = -14, - BGP_ERR_PEER_FILTER_CONFLICT = -15, - BGP_ERR_NOT_INTERNAL_PEER = -16, - BGP_ERR_REMOVE_PRIVATE_AS = -17, - BGP_ERR_AF_UNCONFIGURED = -18, - BGP_ERR_SOFT_RECONFIG_UNCONFIGURED = -19, - BGP_ERR_INSTANCE_MISMATCH = -20, - BGP_ERR_LOCAL_AS_ALLOWED_ONLY_FOR_EBGP = -21, - BGP_ERR_CANNOT_HAVE_LOCAL_AS_SAME_AS = -22, - BGP_ERR_TCPSIG_FAILED = -23, - BGP_ERR_NO_EBGP_MULTIHOP_WITH_TTLHACK = -24, - BGP_ERR_NO_IBGP_WITH_TTLHACK = -25, - BGP_ERR_NO_INTERFACE_CONFIG = -26, - BGP_ERR_CANNOT_HAVE_LOCAL_AS_SAME_AS_REMOTE_AS = -27, - BGP_ERR_AS_OVERRIDE = -28, - BGP_ERR_INVALID_DYNAMIC_NEIGHBORS_LIMIT = -29, - BGP_ERR_DYNAMIC_NEIGHBORS_RANGE_EXISTS = -30, - BGP_ERR_DYNAMIC_NEIGHBORS_RANGE_NOT_FOUND = -31, - BGP_ERR_INVALID_FOR_DYNAMIC_PEER = -32, - BGP_ERR_INVALID_FOR_DIRECT_PEER = -34, - BGP_ERR_PEER_SAFI_CONFLICT = -35, + BGP_ERR_PEER_GROUP_MEMBER = -4, + BGP_ERR_PEER_GROUP_NO_REMOTE_AS = -5, + BGP_ERR_PEER_GROUP_CANT_CHANGE = -6, + BGP_ERR_PEER_GROUP_MISMATCH = -7, + BGP_ERR_PEER_GROUP_PEER_TYPE_DIFFERENT = -8, + BGP_ERR_AS_MISMATCH = -9, + BGP_ERR_PEER_FLAG_CONFLICT = -10, + BGP_ERR_PEER_GROUP_SHUTDOWN = -11, + BGP_ERR_PEER_FILTER_CONFLICT = -12, + BGP_ERR_NOT_INTERNAL_PEER = -13, + BGP_ERR_REMOVE_PRIVATE_AS = -14, + BGP_ERR_AF_UNCONFIGURED = -15, + BGP_ERR_SOFT_RECONFIG_UNCONFIGURED = -16, + BGP_ERR_INSTANCE_MISMATCH = -17, + BGP_ERR_LOCAL_AS_ALLOWED_ONLY_FOR_EBGP = -18, + BGP_ERR_CANNOT_HAVE_LOCAL_AS_SAME_AS = -19, + BGP_ERR_TCPSIG_FAILED = -20, + BGP_ERR_NO_EBGP_MULTIHOP_WITH_TTLHACK = -21, + BGP_ERR_NO_IBGP_WITH_TTLHACK = -22, + BGP_ERR_NO_INTERFACE_CONFIG = -23, + BGP_ERR_CANNOT_HAVE_LOCAL_AS_SAME_AS_REMOTE_AS = -24, + BGP_ERR_AS_OVERRIDE = -25, + BGP_ERR_INVALID_DYNAMIC_NEIGHBORS_LIMIT = -26, + BGP_ERR_DYNAMIC_NEIGHBORS_RANGE_EXISTS = -27, + BGP_ERR_DYNAMIC_NEIGHBORS_RANGE_NOT_FOUND = -28, + BGP_ERR_INVALID_FOR_DYNAMIC_PEER = -29, + BGP_ERR_INVALID_FOR_DIRECT_PEER = -30, + BGP_ERR_PEER_SAFI_CONFLICT = -31, /* BGP GR ERRORS */ - BGP_ERR_GR_INVALID_CMD = -36, - BGP_ERR_GR_OPERATION_FAILED = -37, - BGP_GR_NO_OPERATION = -38, + BGP_ERR_GR_INVALID_CMD = -32, + BGP_ERR_GR_OPERATION_FAILED = -33, + BGP_GR_NO_OPERATION = -34, }; /*