Merge pull request #14333 from fdumontet6WIND/bgp4v2_snmp

bgpd: add support of traps for bgp4-mibv2 

16 files changed, 395 insertions, 23 deletions

diff --git a/bgpd/bgp_snmp.c b/bgpd/bgp_snmp.c
index ce9442c1e0..065ea7672c 100644
--- a/bgpd/bgp_snmp.c
+++ b/bgpd/bgp_snmp.c
@@ -29,10 +29,93 @@
 #include "bgpd/bgp_snmp_bgp4.h"
 #include "bgpd/bgp_snmp_bgp4v2.h"
 #include "bgpd/bgp_mplsvpn_snmp.h"
+#include "bgpd/bgp_snmp_clippy.c"
+
+
+
+static int bgp_cli_snmp_traps_config_write(struct vty *vty);
+
+DEFPY(bgp_snmp_traps_rfc4273, bgp_snmp_traps_rfc4273_cmd,
+      "[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 (no) {
+		UNSET_FLAG(bm->options, BGP_OPT_TRAPS_BGP4MIBV2);
+		return CMD_SUCCESS;
+	}
+	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);
+	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)
+{
+	int write = 0;
+
+	if (!CHECK_FLAG(bm->options, BGP_OPT_TRAPS_RFC4273)) {
+		vty_out(vty, "no bgp snmp traps rfc4273\n");
+		write++;
+	}
+	if (CHECK_FLAG(bm->options, BGP_OPT_TRAPS_BGP4MIBV2)) {
+		vty_out(vty, "bgp snmp traps bgp4-mibv2\n");
+		write++;
+	}
+
+	return write;
+}
+
+int bgpTrapEstablished(struct peer *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)
+{
+	if (CHECK_FLAG(bm->options, BGP_OPT_TRAPS_RFC4273))
+		bgp4TrapBackwardTransition(peer);
+
+	if (CHECK_FLAG(bm->options, BGP_OPT_TRAPS_BGP4MIBV2))
+		bgpv2TrapBackwardTransition(peer);
+
+	return 0;
+}
 
 static int bgp_snmp_init(struct event_loop *tm)
 {
 	smux_init(tm);
+	bgp_snmp_traps_init();
 	bgp_snmp_bgp4_init(tm);
 	bgp_snmp_bgp4v2_init(tm);
 	bgp_mpls_l3vpn_module_init();
@@ -44,6 +127,8 @@ static int bgp_snmp_module_init(void)
 	hook_register(peer_status_changed, bgpTrapEstablished);
 	hook_register(peer_backward_transition, bgpTrapBackwardTransition);
 	hook_register(frr_late_init, bgp_snmp_init);
+	hook_register(bgp_snmp_traps_config_write,
+		      bgp_cli_snmp_traps_config_write);
 	return 0;
 }
 
diff --git a/bgpd/bgp_snmp.h b/bgpd/bgp_snmp.h
index d324782ae3..12ec652f8d 100644
--- a/bgpd/bgp_snmp.h
+++ b/bgpd/bgp_snmp.h
@@ -15,4 +15,7 @@
 #define IPADDRESS ASN_IPADDRESS
 #define GAUGE32 ASN_UNSIGNED
 
+extern int bgpTrapEstablished(struct peer *peer);
+extern int bgpTrapBackwardTransition(struct peer *peer);
+
 #endif /* _FRR_BGP_SNMP_H_ */
diff --git a/bgpd/bgp_snmp_bgp4.c b/bgpd/bgp_snmp_bgp4.c
index 692e232a83..3d04dc2ece 100644
--- a/bgpd/bgp_snmp_bgp4.c
+++ b/bgpd/bgp_snmp_bgp4.c
@@ -757,7 +757,7 @@ static struct variable bgp_variables[] = {
 	 {6, 1, 14}},
 };
 
-int bgpTrapEstablished(struct peer *peer)
+int bgp4TrapEstablished(struct peer *peer)
 {
 	int ret;
 	struct in_addr addr;
@@ -782,7 +782,7 @@ int bgpTrapEstablished(struct peer *peer)
 	return 0;
 }
 
-int bgpTrapBackwardTransition(struct peer *peer)
+int bgp4TrapBackwardTransition(struct peer *peer)
 {
 	int ret;
 	struct in_addr addr;
diff --git a/bgpd/bgp_snmp_bgp4.h b/bgpd/bgp_snmp_bgp4.h
index ccf00d6b7c..67f7cc640b 100644
--- a/bgpd/bgp_snmp_bgp4.h
+++ b/bgpd/bgp_snmp_bgp4.h
@@ -69,8 +69,8 @@
 #define BGP4PATHATTRBEST 13
 #define BGP4PATHATTRUNKNOWN 14
 
-extern int bgpTrapEstablished(struct peer *peer);
-extern int bgpTrapBackwardTransition(struct peer *peer);
+extern int bgp4TrapEstablished(struct peer *peer);
+extern int bgp4TrapBackwardTransition(struct peer *peer);
 extern int bgp_snmp_bgp4_init(struct event_loop *tm);
 
 #endif /* _FRR_BGP_SNMP_BGP4_H_ */
diff --git a/bgpd/bgp_snmp_bgp4v2.c b/bgpd/bgp_snmp_bgp4v2.c
index fb6f13a6ca..b7a5f94a31 100644
--- a/bgpd/bgp_snmp_bgp4v2.c
+++ b/bgpd/bgp_snmp_bgp4v2.c
@@ -32,6 +32,7 @@
 SNMP_LOCAL_VARIABLES
 
 static oid bgpv2_oid[] = {BGP4V2MIB};
+static oid bgpv2_trap_oid[] = { BGP4V2MIB, 0 };
 static struct in_addr bgp_empty_addr = {};
 
 static struct peer *peer_lookup_all_vrf(struct ipaddr *addr)
@@ -793,6 +794,37 @@ static uint8_t *bgp4v2PathAttrTable(struct variable *v, oid name[],
 	return NULL;
 }
 
+/* BGP V2 Traps. */
+static struct trap_object bgpv2TrapEstListv4[] = {
+	{ 6, { 1, 2, 1, BGP4V2_PEER_STATE, 1, 1 } },
+	{ 6, { 1, 2, 1, BGP4V2_PEER_LOCAL_PORT, 1, 1 } },
+	{ 6, { 1, 2, 1, BGP4V2_PEER_REMOTE_PORT, 1, 1 } }
+};
+
+static struct trap_object bgpv2TrapEstListv6[] = {
+	{ 6, { 1, 2, 1, BGP4V2_PEER_STATE, 1, 2 } },
+	{ 6, { 1, 2, 1, BGP4V2_PEER_LOCAL_PORT, 1, 2 } },
+	{ 6, { 1, 2, 1, BGP4V2_PEER_REMOTE_PORT, 1, 2 } }
+};
+
+static struct trap_object bgpv2TrapBackListv4[] = {
+	{ 6, { 1, 2, 1, BGP4V2_PEER_STATE, 1, 1 } },
+	{ 6, { 1, 2, 1, BGP4V2_PEER_LOCAL_PORT, 1, 1 } },
+	{ 6, { 1, 2, 1, BGP4V2_PEER_REMOTE_PORT, 1, 1 } },
+	{ 6, { 1, 3, 1, BGP4V2_PEER_LAST_ERROR_CODE_RECEIVED, 1, 1 } },
+	{ 6, { 1, 3, 1, BGP4V2_PEER_LAST_ERROR_SUBCODE_RECEIVED, 1, 1 } },
+	{ 6, { 1, 3, 1, BGP4V2_PEER_LAST_ERROR_RECEIVED_TEXT, 1, 1 } }
+};
+
+static struct trap_object bgpv2TrapBackListv6[] = {
+	{ 6, { 1, 2, 1, BGP4V2_PEER_STATE, 1, 2 } },
+	{ 6, { 1, 2, 1, BGP4V2_PEER_LOCAL_PORT, 1, 2 } },
+	{ 6, { 1, 2, 1, BGP4V2_PEER_REMOTE_PORT, 1, 2 } },
+	{ 6, { 1, 3, 1, BGP4V2_PEER_LAST_ERROR_CODE_RECEIVED, 1, 2 } },
+	{ 6, { 1, 3, 1, BGP4V2_PEER_LAST_ERROR_SUBCODE_RECEIVED, 1, 2 } },
+	{ 6, { 1, 3, 1, BGP4V2_PEER_LAST_ERROR_RECEIVED_TEXT, 1, 2 } }
+};
+
 static struct variable bgpv2_variables[] = {
 	/* bgp4V2PeerEntry */
 	{BGP4V2_PEER_INSTANCE,
@@ -1412,6 +1444,79 @@ static struct variable bgpv2_variables[] = {
 	 {1, 9, 1, BGP4V2_NLRI_PATH_ATTR_UNKNOWN, 1, 2}},
 };
 
+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)))
+		return 0;
+
+	switch (sockunion_family(&peer->connection->su)) {
+	case AF_INET:
+		oid_copy_in_addr(index, &peer->connection->su.sin.sin_addr);
+		length = IN_ADDR_SIZE;
+		smux_trap(bgpv2_variables, array_size(bgpv2_variables),
+			  bgpv2_trap_oid, array_size(bgpv2_trap_oid), bgpv2_oid,
+			  sizeof(bgpv2_oid) / sizeof(oid), index, length,
+			  bgpv2TrapEstListv4, array_size(bgpv2TrapEstListv4),
+			  BGP4V2ESTABLISHED);
+		break;
+	case AF_INET6:
+		oid_copy_in6_addr(index, &peer->connection->su.sin6.sin6_addr);
+		length = IN6_ADDR_SIZE;
+		smux_trap(bgpv2_variables, array_size(bgpv2_variables),
+			  bgpv2_trap_oid, array_size(bgpv2_trap_oid), bgpv2_oid,
+			  sizeof(bgpv2_oid) / sizeof(oid), index, length,
+			  bgpv2TrapEstListv6, array_size(bgpv2TrapEstListv6),
+			  BGP4V2ESTABLISHED);
+		break;
+	default:
+		break;
+	}
+
+	return 0;
+}
+
+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);
+		length = IN_ADDR_SIZE;
+		smux_trap(bgpv2_variables, array_size(bgpv2_variables),
+			  bgpv2_trap_oid, array_size(bgpv2_trap_oid), bgpv2_oid,
+			  sizeof(bgpv2_oid) / sizeof(oid), index, length,
+			  bgpv2TrapBackListv4, array_size(bgpv2TrapBackListv4),
+			  BGP4V2BACKWARDTRANSITION);
+		break;
+	case AF_INET6:
+		oid_copy_in6_addr(index, &peer->connection->su.sin6.sin6_addr);
+		length = IN6_ADDR_SIZE;
+		smux_trap(bgpv2_variables, array_size(bgpv2_variables),
+			  bgpv2_trap_oid, array_size(bgpv2_trap_oid), bgpv2_oid,
+			  sizeof(bgpv2_oid) / sizeof(oid), index, length,
+			  bgpv2TrapBackListv6, array_size(bgpv2TrapBackListv6),
+			  BGP4V2BACKWARDTRANSITION);
+		break;
+	default:
+		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/bgp_snmp_bgp4v2.h b/bgpd/bgp_snmp_bgp4v2.h
index 6587a825c5..ca355338a6 100644
--- a/bgpd/bgp_snmp_bgp4v2.h
+++ b/bgpd/bgp_snmp_bgp4v2.h
@@ -16,6 +16,14 @@
  * offset 1.3.6.1.3.5.1.1.2.1.x.(1|2).(4|16) = 13
  * offset 1.3.6.1.4.1.7336.3.2.1.1.2.1.x.1.(1|2) = 16
  */
+
+
+/* bgpTraps */
+#define BGP4V2ESTABLISHED	 1
+#define BGP4V2BACKWARDTRANSITION 2
+
+/* bgpPeerTable */
+
 #define BGP4V2_PEER_ENTRY_OFFSET 13
 #define BGP4V2_PEER_INSTANCE 1
 #define BGP4V2_PEER_LOCAL_ADDR_TYPE 2
@@ -84,5 +92,7 @@
 #define BGP4V2_BACKWARD_TRANSITION_NOTIFICATION 2
 
 extern int bgp_snmp_bgp4v2_init(struct event_loop *tm);
+extern int bgpv2TrapEstablished(struct peer *peer);
+extern int bgpv2TrapBackwardTransition(struct peer *peer);
 
 #endif /* _FRR_BGP_SNMP_BGP4V2_H_ */
diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c
index bce4be9725..5d6ae589fa 100644
--- a/bgpd/bgp_vty.c
+++ b/bgpd/bgp_vty.c
@@ -128,6 +128,7 @@ DEFINE_HOOK(bgp_inst_config_write,
 		(bgp, vty));
 DEFINE_HOOK(bgp_snmp_update_last_changed, (struct bgp *bgp), (bgp));
 DEFINE_HOOK(bgp_snmp_init_stats, (struct bgp *bgp), (bgp));
+DEFINE_HOOK(bgp_snmp_traps_config_write, (struct vty * vty), (vty));
 
 static struct peer_group *listen_range_exists(struct bgp *bgp,
 					      struct prefix *range, int exact);
@@ -18506,6 +18507,8 @@ int bgp_config_write(struct vty *vty)
 	safi_t safi;
 	uint32_t tovpn_sid_index = 0;
 
+	hook_call(bgp_snmp_traps_config_write, vty);
+
 	if (bm->rmap_update_timer != RMAP_DEFAULT_UPDATE_TIMER)
 		vty_out(vty, "bgp route-map delay-timer %u\n",
 			bm->rmap_update_timer);
diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h
index 22082aa52e..42e4c167f6 100644
--- 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;
@@ -832,6 +834,7 @@ DECLARE_HOOK(bgp_inst_delete, (struct bgp *bgp), (bgp));
 DECLARE_HOOK(bgp_inst_config_write,
 		(struct bgp *bgp, struct vty *vty),
 		(bgp, vty));
+DECLARE_HOOK(bgp_snmp_traps_config_write, (struct vty *vty), (vty));
 DECLARE_HOOK(bgp_config_end, (struct bgp *bgp), (bgp));
 
 /* Thread callback information */
diff --git a/bgpd/subdir.am b/bgpd/subdir.am
index c2dd207a49..7de6e7a9f1 100644
--- a/bgpd/subdir.am
+++ b/bgpd/subdir.am
@@ -214,6 +214,7 @@ clippy_scan += \
 	bgpd/bgp_rpki.c \
 	bgpd/bgp_vty.c \
         bgpd/bgp_nexthop.c \
+	bgpd/bgp_snmp.c \
 	# end
 
 nodist_bgpd_bgpd_SOURCES = \
diff --git a/doc/user/snmptrap.rst b/doc/user/snmptrap.rst
index 7e306b743d..df534e28bd 100644
--- a/doc/user/snmptrap.rst
+++ b/doc/user/snmptrap.rst
@@ -4,8 +4,9 @@ Handling SNMP Traps
 To handle snmp traps make sure your snmp setup of frr works correctly as
 described in the frr documentation in :ref:`snmp-support`.
 
-The BGP4 mib will send traps on peer up/down events. These should be visible in
-your snmp logs with a message similar to:
+BGP handles both :rfc:`4273` and [Draft-IETF-idr-bgp4-mibv2-11]_ MIBs.
+The BGP4 MIBs will send traps on peer up/down events. These should be
+visible in your snmp logs with a message similar to:
 
 ::
 
@@ -199,3 +200,18 @@ a siren, have your display flash, etc., be creative ;).
 
    # mail the notification
    echo "$MAIL" | mail -s "$SUBJECT" $EMAILADDR
+
+.. _traps-mib-selection:
+
+Traps Mib Selection in BGP
+--------------------------
+
+Both :rfc:`4273` and [Draft-IETF-idr-bgp4-mibv2-11]_ MIBs define traps for
+dealing with up/down events and state transition. The user has the
+possibility to select the MIB he wants to receive traps from:
+
+.. clicmd:: bgp snmp traps <rfc4273|bgp4-mibv2>
+
+By default, only rfc4273 traps are enabled and sent.
+
+.. [Draft-IETF-idr-bgp4-mibv2-11] <https://tools.ietf.org/id/draft-ietf-idr-bgp4-mibv2-11.txt>
diff --git a/tests/topotests/bgp_snmp_bgp4v2mib/r2/snmpd.conf b/tests/topotests/bgp_snmp_bgp4v2mib/r2/snmpd.conf
index 032b93b676..f0957cca7a 100644
--- a/tests/topotests/bgp_snmp_bgp4v2mib/r2/snmpd.conf
+++ b/tests/topotests/bgp_snmp_bgp4v2mib/r2/snmpd.conf
@@ -6,6 +6,15 @@ access public_group "" any noauth prefix all all none
 
 rocommunity public default
 
+trapsess -v2c -c public 127.0.0.1
+
+notificationEvent  linkUpTrap    linkUp   ifIndex ifAdminStatus ifOperStatus
+notificationEvent  linkDownTrap  linkDown ifIndex ifAdminStatus ifOperStatus
+
+monitor  -r 2 -e linkUpTrap   "Generate linkUp" ifOperStatus != 2
+monitor  -r 2 -e linkDownTrap "Generate linkDown" ifOperStatus == 2
+
+
 view all included .1
 
 iquerySecName frr
diff --git a/tests/topotests/bgp_snmp_bgp4v2mib/r2/snmptrapd.conf b/tests/topotests/bgp_snmp_bgp4v2mib/r2/snmptrapd.conf
new file mode 100644
index 0000000000..f6e4abfef7
--- /dev/null
+++ b/tests/topotests/bgp_snmp_bgp4v2mib/r2/snmptrapd.conf
@@ -0,0 +1,2 @@
+authCommunity net,log public
+disableAuthorization yes
diff --git a/tests/topotests/bgp_snmp_bgp4v2mib/test_bgp_snmp_bgp4v2mib.py b/tests/topotests/bgp_snmp_bgp4v2mib/test_bgp_snmp_bgp4v2mib.py
index 6b6153db46..18a8575793 100755
--- a/tests/topotests/bgp_snmp_bgp4v2mib/test_bgp_snmp_bgp4v2mib.py
+++ b/tests/topotests/bgp_snmp_bgp4v2mib/test_bgp_snmp_bgp4v2mib.py
@@ -24,6 +24,7 @@ sys.path.append(os.path.join(CWD, "../"))
 from lib.topogen import Topogen, TopoRouter, get_topogen
 from lib.snmptest import SnmpTester
 from lib import topotest
+from lib.topolog import logger
 
 pytestmark = [pytest.mark.bgpd, pytest.mark.snmp]
 
@@ -55,11 +56,17 @@ def setup_module(mod):
             os.path.join(CWD, "{}/bgpd.conf".format(rname)),
             "-M snmp",
         )
-        router.load_config(
-            TopoRouter.RD_SNMP,
-            os.path.join(CWD, "{}/snmpd.conf".format(rname)),
-            "-Le -Ivacm_conf,usmConf,iquery -V -DAgentX",
-        )
+
+    tgen.gears["r2"].load_config(
+        TopoRouter.RD_SNMP,
+        os.path.join(CWD, "{}/snmpd.conf".format(rname)),
+        "-Le -Ivacm_conf,usmConf,iquery -V -DAgentX",
+    )
+    tgen.gears["r2"].load_config(
+        TopoRouter.RD_TRAP,
+        os.path.join(CWD, "{}/snmptrapd.conf".format(rname)),
+        " -On -OQ ",
+    )
 
     tgen.start_router()
 
@@ -72,6 +79,7 @@ def teardown_module(mod):
 def test_bgp_snmp_bgp4v2():
     tgen = get_topogen()
 
+    r1 = tgen.gears["r1"]
     r2 = tgen.gears["r2"]
 
     def _bgp_converge_summary():
@@ -197,7 +205,9 @@ def test_bgp_snmp_bgp4v2():
         }
 
         # bgp4V2NlriOrigin
+        # tgen.mininet_cli()
         output, _ = snmp.walk(".1.3.6.1.3.5.1.1.9.1.9")
+        logger.info(output)
         return output == expected
 
     _, result = topotest.run_and_expect(_snmpwalk_origin, True, count=10, wait=1)
@@ -220,6 +230,78 @@ def test_bgp_snmp_bgp4v2():
     assertmsg = "Can't fetch SNMP for bgp4V2NlriMed"
     assert result, assertmsg
 
+    def _snmptrap_ipv4():
+        expected = [
+            ("1.3.6.1.2.1.15.3.1.7.192.168.12.1", "192.168.12.1"),
+            ("1.3.6.1.2.1.15.3.1.14.192.168.12.1", '"06 04 "'),
+            ("1.3.6.1.2.1.15.3.1.2.192.168.12.1", "7"),
+            ("1.3.6.1.2.1.15.3.1.7.192.168.12.1", "192.168.12.1"),
+            ("1.3.6.1.2.1.15.3.1.14.192.168.12.1", '"06 04 "'),
+            ("1.3.6.1.2.1.15.3.1.2.192.168.12.1", "6"),
+        ]
+
+        # load trapd resulting file
+        # tgen.mininet_cli()
+
+        snmptrapfile = "{}/{}/snmptrapd.log".format(r2.logdir, r2.name)
+        outputfile = open(snmptrapfile).read()
+        output = snmp.trap(outputfile)
+        return output == expected
+
+    # skip tests is SNMP not installed
+    if not os.path.isfile("/usr/sbin/snmptrapd"):
+        error_msg = "SNMP not installed - skipping"
+        pytest.skip(error_msg)
+
+    snmptrapfile = "{}/{}/snmptrapd.log".format(r2.logdir, r2.name)
+    trap_file = open(snmptrapfile, "w")
+    trap_file.truncate(0)
+    trap_file.close()
+    r1.vtysh_cmd("clear bgp *")
+    _, result = topotest.run_and_expect(_snmptrap_ipv4, True, count=2, wait=10)
+    assertmsg = "Can't fetch SNMP trap for ipv4"
+    assert result, assertmsg
+
+    def _snmptrap_ipv6():
+        expected = [
+            ("1.3.6.1.3.5.1.1.2.1.13.1.1.192.168.12.1", "7"),
+            ("1.3.6.1.3.5.1.1.2.1.6.1.1.192.168.12.1", "179"),
+            ("1.3.6.1.3.5.1.1.3.1.1.1.1.192.168.12.1", "6"),
+            ("1.3.6.1.3.5.1.1.3.1.2.1.1.192.168.12.1", "4"),
+            ("1.3.6.1.3.5.1.1.3.1.4.1.1.192.168.12.1", '"00 "'),
+            ("1.3.6.1.3.5.1.1.2.1.13.1.2.32.1.13.184.0.0.0.0.0.0.0.0.0.18.0.1", "7"),
+            ("1.3.6.1.3.5.1.1.2.1.6.1.2.32.1.13.184.0.0.0.0.0.0.0.0.0.18.0.1", "179"),
+            ("1.3.6.1.3.5.1.1.3.1.1.1.2.32.1.13.184.0.0.0.0.0.0.0.0.0.18.0.1", "6"),
+            ("1.3.6.1.3.5.1.1.3.1.2.1.2.32.1.13.184.0.0.0.0.0.0.0.0.0.18.0.1", "4"),
+            (
+                "1.3.6.1.3.5.1.1.3.1.4.1.2.32.1.13.184.0.0.0.0.0.0.0.0.0.18.0.1",
+                '"00 "',
+            ),
+            ("1.3.6.1.3.5.1.1.2.1.13.1.1.192.168.12.1", "6"),
+            ("1.3.6.1.3.5.1.1.2.1.6.1.1.192.168.12.1", "179"),
+            ("1.3.6.1.3.5.1.1.2.1.13.1.2.32.1.13.184.0.0.0.0.0.0.0.0.0.18.0.1", "6"),
+            ("1.3.6.1.3.5.1.1.2.1.6.1.2.32.1.13.184.0.0.0.0.0.0.0.0.0.18.0.1", "179"),
+        ]
+
+        # load trapd resulting file
+        # tgen.mininet_cli()
+
+        snmptrapfile = "{}/{}/snmptrapd.log".format(r2.logdir, r2.name)
+        outputfile = open(snmptrapfile).read()
+        output = snmp.trap(outputfile)
+        return output == expected
+
+    snmptrapfile = "{}/{}/snmptrapd.log".format(r2.logdir, r2.name)
+    trap_file = open(snmptrapfile, "w")
+    trap_file.truncate(0)
+    trap_file.close()
+    r2.vtysh_cmd("conf\nbgp snmp traps bgp4-mibv2")
+    r2.vtysh_cmd("conf\nno bgp snmp traps rfc4273")
+    r1.vtysh_cmd("clear bgp *")
+    _, result = topotest.run_and_expect(_snmptrap_ipv6, True, count=2, wait=10)
+    assertmsg = "Can't fetch SNMP trap for ipv6"
+    assert result, assertmsg
+
 
 def test_memory_leak():
     "Run the memory leak test and report results."
diff --git a/tests/topotests/lib/snmptest.py b/tests/topotests/lib/snmptest.py
index e7cd657b20..598ad05f58 100644
--- a/tests/topotests/lib/snmptest.py
+++ b/tests/topotests/lib/snmptest.py
@@ -18,6 +18,7 @@ Basic usage instructions:
 """
 
 from lib.topolog import logger
+import re
 
 
 class SnmpTester(object):
@@ -72,15 +73,38 @@ class SnmpTester(object):
         # third token onwards is the value of the object
         return tokens[0].split(".", 1)[1]
 
-    @staticmethod
-    def _get_snmp_oid(snmp_output):
-        tokens = snmp_output.strip().split()
-
-        #        if len(tokens) > 5:
-        #            return None
-
-        # third token is the value of the object
-        return tokens[0].split(".", 1)[1]
+    def _parse_notification_trap(self, snmp_out):
+        # we use the "=" as separator thus we will have
+        # element of list formated "value   oid"
+        # value for index i is corresponding to index i-1
+        results = snmp_out.strip().split("=")
+
+        # remove the notification part date, notification OID
+        del results[0:2]
+
+        index = 0
+        oid_list = []
+        next_oid = ""
+        oid = ""
+        while index < len(results):
+            result = results[index].strip().split()
+            if index < len(results) - 1:
+                raw_oid = result[-1]
+                # remove initial "." of oid
+                next_oid = raw_oid.split(".", 1)[1]
+                # remove oid from result to have only value
+                del result[-1]
+            if index > 0:
+                value = " ".join(result)
+                # ignore remote port oid 1.3.6.1.3.5.1.1.2.1.9 since
+                # it's value is variable
+                local_port = re.search("1.3.6.1.3.5.1.1.2.1.9", oid)
+                if not local_port:
+                    oid_list.append((oid, value))
+
+            oid = next_oid
+            index += 1
+        return oid_list
 
     def _parse_multiline(self, snmp_output):
         results = snmp_output.strip().split("\n")
@@ -93,6 +117,15 @@ class SnmpTester(object):
 
         return out_dict, out_list
 
+    def _parse_multiline_trap(self, results):
+        out_list = []
+        results = [elem for index, elem in enumerate(results) if index % 2 != 0]
+
+        for response in results:
+            oid_list = self._parse_notification_trap(response)
+            out_list += oid_list
+        return out_list
+
     def get(self, oid):
         cmd = "snmpget {0} {1}".format(self._snmp_config(), oid)
 
@@ -116,6 +149,11 @@ class SnmpTester(object):
         result = self.router.cmd(cmd)
         return self._parse_multiline(result)
 
+    def trap(self, outputfile):
+        whitecleanfile = re.sub("\t", " ", outputfile)
+        results = whitecleanfile.strip().split("\n")
+        return self._parse_multiline_trap(results)
+
     def test_oid(self, oid, value):
         print("oid: {}".format(self.get_next(oid)))
         return self.get_next(oid) == value
diff --git a/tests/topotests/lib/topogen.py b/tests/topotests/lib/topogen.py
index 4d935b9538..48caf6f03a 100644
--- a/tests/topotests/lib/topogen.py
+++ b/tests/topotests/lib/topogen.py
@@ -744,6 +744,7 @@ class TopoRouter(TopoGear):
     RD_SNMP = 18
     RD_PIM6 = 19
     RD_MGMTD = 20
+    RD_TRAP = 21
     RD = {
         RD_FRR: "frr",
         RD_ZEBRA: "zebra",
@@ -766,6 +767,7 @@ class TopoRouter(TopoGear):
         RD_PATH: "pathd",
         RD_SNMP: "snmpd",
         RD_MGMTD: "mgmtd",
+        RD_TRAP: "snmptrapd",
     }
 
     def __init__(self, tgen, cls, name, **params):
@@ -842,7 +844,7 @@ class TopoRouter(TopoGear):
         TopoRouter.RD_RIPNG, TopoRouter.RD_OSPF, TopoRouter.RD_OSPF6,
         TopoRouter.RD_ISIS, TopoRouter.RD_BGP, TopoRouter.RD_LDP,
         TopoRouter.RD_PIM, TopoRouter.RD_PIM6, TopoRouter.RD_PBR,
-        TopoRouter.RD_SNMP, TopoRouter.RD_MGMTD.
+        TopoRouter.RD_SNMP, TopoRouter.RD_MGMTD, TopoRouter.RD_TRAP.
 
         Possible `source` values are `None` for an empty config file, a path name which is
         used directly, or a file name with no path components which is first looked for
@@ -880,7 +882,7 @@ class TopoRouter(TopoGear):
         # Enable all daemon command logging, logging files
         # and set them to the start dir.
         for daemon, enabled in nrouter.daemons.items():
-            if enabled and daemon != "snmpd":
+            if enabled and daemon != "snmpd" and daemon != "snmptrapd":
                 self.vtysh_cmd(
                     "\n".join(
                         [
diff --git a/tests/topotests/lib/topotest.py b/tests/topotests/lib/topotest.py
index 093491617d..8491314e16 100644
--- a/tests/topotests/lib/topotest.py
+++ b/tests/topotests/lib/topotest.py
@@ -1423,6 +1423,7 @@ class Router(Node):
             "pathd": 0,
             "snmpd": 0,
             "mgmtd": 0,
+            "snmptrapd": 0,
         }
         self.daemons_options = {"zebra": ""}
         self.reportCores = True
@@ -1887,6 +1888,15 @@ class Router(Node):
                     daemon_opts
                 ) + "{}.pid -x /etc/frr/agentx".format(runbase)
                 # check_daemon_files.append(runbase + ".pid")
+            elif daemon == "snmptrapd":
+                binary = "/usr/sbin/snmptrapd"
+                cmdenv = ""
+                cmdopt = (
+                    "{} ".format(daemon_opts)
+                    + "-C -c /etc/{}/snmptrapd.conf".format(self.routertype)
+                    + " -p {}.pid".format(runbase)
+                    + " -LF 6-7 {}/{}/snmptrapd.log".format(self.logdir, self.name)
+                )
             else:
                 binary = os.path.join(self.daemondir, daemon)
                 check_daemon_files.extend([runbase + ".pid", runbase + ".vty"])
@@ -1926,6 +1936,7 @@ class Router(Node):
                         tail_log_files.append(
                             "{}/{}/{}.log".format(self.logdir, self.name, daemon)
                         )
+
             if extra_opts:
                 cmdopt += " " + extra_opts
 
@@ -2056,7 +2067,7 @@ class Router(Node):
                         "%s: %s %s started with perf", self, self.routertype, daemon
                     )
             else:
-                if daemon != "snmpd":
+                if daemon != "snmpd" and daemon != "snmptrapd":
                     cmdopt += " -d "
                 cmdopt += rediropt
 
@@ -2299,6 +2310,8 @@ class Router(Node):
         for daemon in self.daemons:
             if daemon == "snmpd":
                 continue
+            if daemon == "snmptrapd":
+                continue
             if (self.daemons[daemon] == 1) and not (daemon in daemonsRunning):
                 sys.stderr.write("%s: Daemon %s not running\n" % (self.name, daemon))
                 if daemon == "staticd":