bgpd: add [no] bgp snmp traps bgp4-mibv2 command

There is no command to choose to send or not the bgp4-mibv2 traps. Since the MIB bgp4-mibv2 notification are redundant with MIB RFC4273 we added a command: - bgp snmp traps bgp4-mibv2 <enable|disable> By default, the bgp4-mibv2 traps will be disabled, to prevent from redundancy. Signed-off-by: Francois Dumontet <francois.dumontet@6wind.com>
FRRouting · Oct 18, 2023 · cdf08fd · cdf08fd
1 parent 4c12ccf
commit cdf08fd
Show file tree

Hide file tree

Showing 5 changed files with 50 additions and 31 deletions.
diff --git a/bgpd/bgp_snmp.c b/bgpd/bgp_snmp.c
@@ -31,30 +31,47 @@
 #include "bgpd/bgp_mplsvpn_snmp.h"
 #include "bgpd/bgp_snmp_clippy.c"
 
-uint32_t bgp_snmp_traps_flags;
+
 
 static int bgp_cli_snmp_traps_config_write(struct vty *vty);
 
 DEFPY(bgp_snmp_traps_rfc4273, bgp_snmp_traps_rfc4273_cmd,
-      "bgp snmp traps rfc4273 <enable$on|disable$off>",
-      BGP_STR "Configure BGP SNMP\n"
-	      "Configure SNMP traps for BGP \n"
-	      "Configure use of rfc4273 SNMP traps for BGP \n"
-	      "Enable rfc4273 traps\n"
-	      "Disable rfc4273 traps\n")
+      "[no$no] bgp snmp traps rfc4273",
+      NO_STR BGP_STR
+              "Configure BGP SNMP\n"
+	      "Configure SNMP traps for BGP\n"
+	      "Configure use of rfc4273 SNMP traps for BGP\n")
+{
+	if (no) {
+		UNSET_FLAG(bm->options, BGP_OPT_TRAPS_RFC4273);
+		return CMD_SUCCESS;
+	}
+	SET_FLAG(bm->options, BGP_OPT_TRAPS_RFC4273);
+	return CMD_SUCCESS;
+}
+
+DEFPY(bgp_snmp_traps_bgp4_mibv2, bgp_snmp_traps_bgp4_mibv2_cmd,
+      "[no$no] bgp snmp traps bgp4-mibv2",
+      NO_STR BGP_STR
+              "Configure BGP SNMP\n"
+	      "Configure SNMP traps for BGP\n"
+	      "Configure use of BGP4-MIBv2 SNMP traps for BGP\n")
 {
-	if (on) {
-		SET_FLAG(bgp_snmp_traps_flags, BGP_SNMP_TRAPS_RFC4273_ENABLED);
+	if (no) {
+		UNSET_FLAG(bm->options, BGP_OPT_TRAPS_BGP4MIBV2);
 		return CMD_SUCCESS;
 	}
-	UNSET_FLAG(bgp_snmp_traps_flags, BGP_SNMP_TRAPS_RFC4273_ENABLED);
+	SET_FLAG(bm->options, BGP_OPT_TRAPS_BGP4MIBV2);
 	return CMD_SUCCESS;
 }
 
 static void bgp_snmp_traps_init(void)
 {
 	install_element(CONFIG_NODE, &bgp_snmp_traps_rfc4273_cmd);
-	SET_FLAG(bgp_snmp_traps_flags, BGP_SNMP_TRAPS_RFC4273_ENABLED);
+	install_element(CONFIG_NODE, &bgp_snmp_traps_bgp4_mibv2_cmd);
+
+	SET_FLAG(bm->options, BGP_OPT_TRAPS_RFC4273);
+	/* BGP4MIBv2 traps are disabled by default */
 }
 
 int bgp_cli_snmp_traps_config_write(struct vty *vty)
@@ -75,15 +92,23 @@ int bgp_cli_snmp_traps_config_write(struct vty *vty)
 
 int bgpTrapEstablished(struct peer *peer)
 {
-	bgp4TrapEstablished(peer);
-	bgpv2TrapEstablished(peer);
+	if (CHECK_FLAG(bm->options, BGP_OPT_TRAPS_RFC4273))
+		bgp4TrapEstablished(peer);
+
+	if (CHECK_FLAG(bm->options, BGP_OPT_TRAPS_BGP4MIBV2))
+		bgpv2TrapEstablished(peer);
+
 	return 0;
 }
 
 int bgpTrapBackwardTransition(struct peer *peer)
 {
-	bgp4TrapBackwardTransition(peer);
-	bgpv2TrapBackwardTransition(peer);
+	if (CHECK_FLAG(bm->options, BGP_OPT_TRAPS_RFC4273))
+		bgp4TrapBackwardTransition(peer);
+
+	if (CHECK_FLAG(bm->options, BGP_OPT_TRAPS_BGP4MIBV2))
+		bgpv2TrapBackwardTransition(peer);
+
 	return 0;
 }
 

diff --git a/bgpd/bgp_snmp.h b/bgpd/bgp_snmp.h
@@ -15,10 +15,6 @@
 #define IPADDRESS ASN_IPADDRESS
 #define GAUGE32 ASN_UNSIGNED
 
-extern uint32_t bgp_snmp_traps_flags;
-
-#define BGP_SNMP_TRAPS_RFC4273_ENABLED (1 << 0)
-
 extern int bgpTrapEstablished(struct peer *peer);
 extern int bgpTrapBackwardTransition(struct peer *peer);
 

diff --git a/bgpd/bgp_snmp_bgp4.c b/bgpd/bgp_snmp_bgp4.c
@@ -764,9 +764,6 @@ int bgp4TrapEstablished(struct peer *peer)
 	oid index[sizeof(oid) * IN_ADDR_SIZE];
 	struct peer_connection *connection = peer->connection;
 
-	if (!CHECK_FLAG(bgp_snmp_traps_flags, BGP_SNMP_TRAPS_RFC4273_ENABLED))
-		return 0;
-
 	/* Check if this peer just went to Established */
 	if ((connection->ostatus != OpenConfirm) ||
 	    !(peer_established(connection)))
@@ -791,9 +788,6 @@ int bgp4TrapBackwardTransition(struct peer *peer)
 	struct in_addr addr;
 	oid index[sizeof(oid) * IN_ADDR_SIZE];
 
-	if (!CHECK_FLAG(bgp_snmp_traps_flags, BGP_SNMP_TRAPS_RFC4273_ENABLED))
-		return 0;
-
 	ret = inet_aton(peer->host, &addr);
 	if (ret == 0)
 		return 0;

diff --git a/bgpd/bgp_snmp_bgp4v2.c b/bgpd/bgp_snmp_bgp4v2.c
@@ -825,7 +825,6 @@ static struct trap_object bgpv2TrapBackListv6[] = {
 	{ 6, { 1, 3, 1, BGP4V2_PEER_LAST_ERROR_RECEIVED_TEXT, 1, 2 } }
 };
 
-
 static struct variable bgpv2_variables[] = {
 	/* bgp4V2PeerEntry */
 	{BGP4V2_PEER_INSTANCE,
@@ -1450,6 +1449,9 @@ int bgpv2TrapEstablished(struct peer *peer)
 	oid index[sizeof(oid) * IN6_ADDR_SIZE];
 	size_t length;
 
+	if (!CHECK_FLAG(bm->options, BGP_OPT_TRAPS_BGP4MIBV2))
+		return 0;
+
 	/* Check if this peer just went to Established */
 	if ((peer->connection->ostatus != OpenConfirm) ||
 	    !(peer_established(peer->connection)))
@@ -1475,8 +1477,7 @@ int bgpv2TrapEstablished(struct peer *peer)
 			  BGP4V2ESTABLISHED);
 		break;
 	default:
-		return 0;
-		;
+		break;
 	}
 
 	return 0;
@@ -1487,6 +1488,9 @@ int bgpv2TrapBackwardTransition(struct peer *peer)
 	oid index[sizeof(oid) * IN6_ADDR_SIZE];
 	size_t length;
 
+	if (!CHECK_FLAG(bm->options, BGP_OPT_TRAPS_BGP4MIBV2))
+		return 0;
+
 	switch (sockunion_family(&peer->connection->su)) {
 	case AF_INET:
 		oid_copy_in_addr(index, &peer->connection->su.sin.sin_addr);
@@ -1507,14 +1511,12 @@ int bgpv2TrapBackwardTransition(struct peer *peer)
 			  BGP4V2BACKWARDTRANSITION);
 		break;
 	default:
-		return 0;
-		;
+		break;
 	}
 
 	return 0;
 }
 
-
 int bgp_snmp_bgp4v2_init(struct event_loop *tm)
 {
 	REGISTER_MIB("mibII/bgpv2", bgpv2_variables, variable, bgpv2_oid);

diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h
@@ -121,6 +121,8 @@ struct bgp_master {
 #define BGP_OPT_NO_FIB                   (1 << 0)
 #define BGP_OPT_NO_LISTEN                (1 << 1)
 #define BGP_OPT_NO_ZEBRA                 (1 << 2)
+#define BGP_OPT_TRAPS_RFC4273            (1 << 3)
+#define BGP_OPT_TRAPS_BGP4MIBV2          (1 << 4)
 
 	uint64_t updgrp_idspace;
 	uint64_t subgrp_idspace;