Merge pull request #16878 from donaldsharp/increased_test_cover

Add some test cases, and some ability to see what is going on in zebra

lib/table.c

@@ -208,32 +208,6 @@ struct route_node *route_node_match(struct route_table *table,
 	return NULL;
 }
 
-struct route_node *route_node_match_ipv4(struct route_table *table,
-					 const struct in_addr *addr)
-{
-	struct prefix_ipv4 p;
-
-	memset(&p, 0, sizeof(p));
-	p.family = AF_INET;
-	p.prefixlen = IPV4_MAX_BITLEN;
-	p.prefix = *addr;
-
-	return route_node_match(table, (struct prefix *)&p);
-}
-
-struct route_node *route_node_match_ipv6(struct route_table *table,
-					 const struct in6_addr *addr)
-{
-	struct prefix_ipv6 p;
-
-	memset(&p, 0, sizeof(p));
-	p.family = AF_INET6;
-	p.prefixlen = IPV6_MAX_BITLEN;
-	p.prefix = *addr;
-
-	return route_node_match(table, &p);
-}
-
 /* Lookup same prefix node.  Return NULL when we can't find route. */
 struct route_node *route_node_lookup(struct route_table *table,
 				     union prefixconstptr pu)

lib/table.h

@@ -195,10 +195,6 @@ extern struct route_node *route_node_lookup_maynull(struct route_table *table,
 						    union prefixconstptr pu);
 extern struct route_node *route_node_match(struct route_table *table,
 					   union prefixconstptr pu);
-extern struct route_node *route_node_match_ipv4(struct route_table *table,
-						const struct in_addr *addr);
-extern struct route_node *route_node_match_ipv6(struct route_table *table,
-						const struct in6_addr *addr);
 
 extern unsigned long route_table_count(struct route_table *table);
 

ripd/ripd.c

@@ -1263,9 +1263,13 @@ static void rip_response_process(struct rip_packet *packet, int size,
 					       rip->vrf->vrf_id)) {
 				struct route_node *rn;
 				struct rip_info *rinfo;
+				struct prefix p = { 0 };
 
-				rn = route_node_match_ipv4(rip->table,
-							   &rte->nexthop);
+				p.family = AF_INET;
+				p.prefixlen = IPV4_MAX_BITLEN;
+				p.u.prefix4 = rte->nexthop;
+
+				rn = route_node_match(rip->table, &p);
 
 				if (rn) {
 					rinfo = rn->info;

tests/topotests/all_protocol_startup/test_all_protocol_startup.py

@@ -20,6 +20,7 @@
 import pytest
 import glob
 from time import sleep
+from lib.topolog import logger
 
 pytestmark = [
     pytest.mark.babeld,
@@ -1715,6 +1716,77 @@ def _show_func():
     net["r1"].cmd('vtysh -c "conf" -c "no nexthop-group resilience"')
 
 
+def test_interface_stuff():
+    global fatal_error
+    net = get_topogen().net
+
+    # Skip if previous fatal error condition is raised
+    if fatal_error != "":
+        pytest.skip(fatal_error)
+
+    print("\n\n** Verifying some interface code")
+    print("************************************\n")
+
+    net["r1"].cmd('vtysh -c "conf" -c "interface r1-eth0" -c "multicast enable"')
+
+    def _test_interface_multicast_on():
+        output = json.loads(net["r1"].cmd('vtysh -c "show int r1-eth0 json"'))
+        expected = {
+            "r1-eth0": {
+                "flags": "<UP,LOWER_UP,BROADCAST,RUNNING,MULTICAST>",
+                "multicastConfig": "Enabled by CLI",
+            }
+        }
+        return topotest.json_cmp(output, expected)
+
+    test_func = functools.partial(_test_interface_multicast_on)
+    _, result = topotest.run_and_expect(test_func, None, count=30, wait=1)
+    assert result is None, "Multicast bit was not set on r1-eth0"
+
+    net["r1"].cmd('vtysh -c "conf" -c "interface r1-eth0" -c "multicast disable"')
+
+    def _test_interface_multicast_off():
+        output = json.loads(
+            net["r1"].cmd('vtysh -c "show int r1-eth0 vrf default json"')
+        )
+        expected = {
+            "r1-eth0": {
+                "flags": "<UP,LOWER_UP,BROADCAST,RUNNING>",
+                "multicastConfig": "Disabled by CLI",
+            }
+        }
+        return topotest.json_cmp(output, expected)
+
+    test_func = functools.partial(_test_interface_multicast_off)
+    _, result = topotest.run_and_expect(test_func, None, count=30, wait=1)
+    assert result is None, "Multicast bit was not turned off on r1-eth0"
+
+    net["r1"].cmd('vtysh -c "conf" -c "interface r1-eth0" -c "no multicast disable"')
+
+    def _test_interface_multicast_disable():
+        output = json.loads(net["r1"].cmd('vtysh -c "show int r1-eth0 json"'))
+        expected = {
+            "r1-eth0": {
+                "flags": "<UP,LOWER_UP,BROADCAST,RUNNING>",
+                "multicastConfig": "Not specified by CLI",
+            }
+        }
+        return topotest.json_cmp(output, expected)
+
+    test_func = functools.partial(_test_interface_multicast_disable)
+    _, result = topotest.run_and_expect(test_func, None, count=30, wait=1)
+    assert result is None, "Multicast bit was set on r1-eth0"
+
+    logger.info("Ensure that these commands are still nominally working")
+    rc, o, e = net["r1"].cmd_status('vtysh -c "show interface description vrf all"')
+    logger.info(o)
+    assert rc == 0
+
+    rc, o, e = net["r1"].cmd_status('vtysh -c "show interface description vrf default"')
+    logger.info(o)
+    assert rc == 0
+
+
 def test_shutdown_check_stderr():
     global fatal_error
     net = get_topogen().net

tests/topotests/lib/snmptest.py

@@ -104,12 +104,16 @@ def get_next(self, oid):
             return None
         return self._get_snmp_value(result)
 
-    def walk(self, oid):
+    def walk(self, oid, raw=False):
         cmd = "snmpwalk {0} {1} 2>&1 | grep -v SNMPv2-PDU".format(
             self._snmp_config(), oid
         )
 
         result = self.router.cmd(cmd)
+
+        if raw:
+            return result
+
         return self._parse_multiline(result)
 
     def parse_notif_ipv4(self, notif):

tests/topotests/simple_snmp_test/r1/isisd.conf

@@ -3,6 +3,8 @@ log stdout debugging
 ! debug isis route-events
 ! debug isis events
 !
+agentx
+!
 interface r1-eth0
   ip router isis ISIS1
   ipv6 router isis ISIS1

tests/topotests/simple_snmp_test/r1/ospf6d.conf

@@ -0,0 +1,12 @@
+agentx
+
+int r1-eth0
+  ipv6 ospf6 area 0.0.0.0
+
+int r1-eth1
+  ipv6 ospf6 area 0.0.0.0
+int r1-eth2
+  ipv6 ospf6 area 0.0.0.0
+
+router ospf6
+  redistribute local

tests/topotests/simple_snmp_test/r1/ospfd.conf

@@ -0,0 +1,11 @@
+agentx
+
+int r1-eth0
+  ip ospf area 0.0.0.0
+int r1-eth1
+  ip ospf area 0.0.0.0
+int r1-eth2
+  ip ospf area 0.0.0.0
+
+router ospf
+  redistribute local

tests/topotests/simple_snmp_test/r1/ripd.conf

@@ -0,0 +1,8 @@
+!
+!
+router rip
+  network 0.0.0.0/0
+  redistribute local
+!
+agentx
+!

tests/topotests/simple_snmp_test/r1/zebra.conf

@@ -1,5 +1,7 @@
 log file zebra.log
 !
+agentx
+!
 interface r1-eth0
  ip address 192.168.12.12/24
  ipv6 address 2000:1:1:12::12/64 
@@ -18,5 +20,4 @@ interface lo
  ipv6 address 2000:1:1:1::1/128
 !
 !
-!
 line vty

tests/topotests/simple_snmp_test/test_simple_snmp.py

@@ -24,7 +24,8 @@
 # Import topogen and topotest helpers
 from lib.topogen import Topogen, TopoRouter, get_topogen
 from lib.snmptest import SnmpTester
-
+from time import sleep
+from lib.topolog import logger
 
 pytestmark = [pytest.mark.bgpd, pytest.mark.isisd, pytest.mark.snmp]
 
@@ -59,16 +60,35 @@ def setup_module(mod):
     # For all registered routers, load the zebra configuration file
     for rname, router in router_list.items():
         router.load_config(
-            TopoRouter.RD_ZEBRA, os.path.join(CWD, "{}/zebra.conf".format(rname))
+            TopoRouter.RD_ZEBRA,
+            os.path.join(CWD, "{}/zebra.conf".format(rname)),
+            "-M snmp",
         )
         router.load_config(
-            TopoRouter.RD_ISIS, os.path.join(CWD, "{}/isisd.conf".format(rname))
+            TopoRouter.RD_ISIS,
+            os.path.join(CWD, "{}/isisd.conf".format(rname)),
+            "-M snmp",
         )
         router.load_config(
             TopoRouter.RD_BGP,
             os.path.join(CWD, "{}/bgpd.conf".format(rname)),
             "-M snmp",
         )
+        router.load_config(
+            TopoRouter.RD_RIP,
+            os.path.join(CWD, "{}/ripd.conf".format(rname)),
+            "-M snmp",
+        )
+        router.load_config(
+            TopoRouter.RD_OSPF,
+            os.path.join(CWD, "{}/ospfd.conf".format(rname)),
+            "-M snmp",
+        )
+        router.load_config(
+            TopoRouter.RD_OSPF6,
+            os.path.join(CWD, "{}/ospf6d.conf".format(rname)),
+            "-M snmp",
+        )
         router.load_config(
             TopoRouter.RD_SNMP,
             os.path.join(CWD, "{}/snmpd.conf".format(rname)),
@@ -77,6 +97,16 @@ def setup_module(mod):
 
     # After loading the configurations, this function loads configured daemons.
     tgen.start_router()
+    # Why this sleep?  If you are using zebra w/ snmp we have a chicken
+    # and egg problem with the snmpd.  snmpd is being started up with
+    # ip addresses, and as such snmpd may not be ready to listen yet
+    # (see startup stuff in topotest.py ) with the 2 second delay
+    # on starting snmpd after zebra.  As such if we want to test
+    # anything in zebra we need to sleep a bit to allow the connection
+    # to happen.  I have no good way to test to see if zebra is up
+    # and running with snmp at this point in time.  So this will have
+    # to do.
+    sleep(17)
 
 
 def teardown_module():
@@ -103,6 +133,22 @@ def test_r1_bgp_version():
     assert r1_snmp.test_oid_walk("bgpVersion", ["10"])
     assert r1_snmp.test_oid_walk("bgpVersion", ["10"], ["0"])
 
+    assert r1_snmp.test_oid(
+        "IP-FORWARD-MIB::ipForwardDest.192.168.12.0", "192.168.12.0"
+    )
+
+    assert r1_snmp.test_oid("ISIS-MIB::isisSysVersion", "one(1)")
+    # rip is not auto-loading agentx from mgmtd
+    # assert r1_snmp.test_oid("RIPv2-MIB::rip2GlobalQueries", "0")
+
+    assert r1_snmp.test_oid("OSPF-MIB::ospfVersionNumber", "version2(2)")
+    assert r1_snmp.test_oid("OSPFV3-MIB::ospfv3VersionNumber", "version3(3)")
+
+    # Let's just dump everything and make sure we get some additional test
+    # coverage
+    logger.info("Let's walk everything")
+    logger.info(r1_snmp.walk(".1", raw=True))
+
 
 def test_memory_leak():
     "Run the memory leak test and report results."

zebra/interface.c

@@ -47,6 +47,20 @@ DEFINE_MTYPE_STATIC(ZEBRA, ZIF_DESC, "Intf desc");
 
 static void if_down_del_nbr_connected(struct interface *ifp);
 
+static const char *if_zebra_data_state(uint8_t state)
+{
+	switch (state) {
+	case IF_ZEBRA_DATA_UNSPEC:
+		return "Not specified by CLI";
+	case IF_ZEBRA_DATA_ON:
+		return "Enabled by CLI";
+	case IF_ZEBRA_DATA_OFF:
+		return "Disabled by CLI";
+	}
+
+	return "STATE IS WRONG DEV ESCAPE";
+}
+
 static void if_zebra_speed_update(struct event *thread)
 {
 	struct interface *ifp = EVENT_ARG(thread);
@@ -366,6 +380,7 @@ int if_subnet_delete(struct interface *ifp, struct connected *ifc)
 	return 0;
 }
 
+#ifndef HAVE_NETLINK
 /* if_flags_mangle: A place for hacks that require mangling
  * or tweaking the interface flags.
  *
@@ -417,6 +432,7 @@ void if_flags_update(struct interface *ifp, uint64_t newflags)
 			if_up(ifp, true);
 	}
 }
+#endif
 
 /* Wake up configured address if it is not in current kernel
    address. */
@@ -2627,8 +2643,8 @@ static void if_dump_vty(struct vty *vty, struct interface *ifp)
 		vty_out(vty, "mtu6 %d ", ifp->mtu6);
 	vty_out(vty, "\n  flags: %s\n", if_flag_dump(ifp->flags));
 
-	if (zebra_if->mpls)
-		vty_out(vty, "  MPLS enabled\n");
+	vty_out(vty, "  MPLS %s %s\n", zebra_if->mpls ? "enabled" : "",
+		if_zebra_data_state(zebra_if->multicast));
 
 	if (zebra_if->linkdown)
 		vty_out(vty, "  Ignore all v4 routes with linkdown\n");
@@ -2640,6 +2656,10 @@ static void if_dump_vty(struct vty *vty, struct interface *ifp)
 	if (zebra_if->v6mcast_on)
 		vty_out(vty, "  v6 Multicast forwarding is on\n");
 
+	vty_out(vty, "  Multicast config is %s\n", if_zebra_data_state(zebra_if->multicast));
+
+	vty_out(vty, "  Shutdown config is %s\n", if_zebra_data_state(zebra_if->shutdown));
+
 	/* Hardware address. */
 	vty_out(vty, "  Type: %s\n", if_link_type_str(ifp->ll_type));
 	if (ifp->hw_addr_len != 0) {
@@ -2988,10 +3008,14 @@ static void if_dump_vty_json(struct vty *vty, struct interface *ifp,
 	json_object_boolean_add(json_if, "mplsEnabled", zebra_if->mpls);
 	json_object_boolean_add(json_if, "linkDown", zebra_if->linkdown);
 	json_object_boolean_add(json_if, "linkDownV6", zebra_if->linkdownv6);
-	json_object_boolean_add(json_if, "mcForwardingV4",
-				zebra_if->v4mcast_on);
-	json_object_boolean_add(json_if, "mcForwardingV6",
-				zebra_if->v6mcast_on);
+	json_object_boolean_add(json_if, "mcForwardingV4", zebra_if->v4mcast_on);
+	json_object_boolean_add(json_if, "mcForwardingV6", zebra_if->v6mcast_on);
+
+	json_object_string_add(json_if, "multicastConfig", if_zebra_data_state(zebra_if->multicast));
+
+	json_object_string_add(json_if, "shutdownConfig", if_zebra_data_state(zebra_if->shutdown));
+
+	json_object_string_add(json_if, "mplsConfig", if_zebra_data_state(zebra_if->mpls_config));
 
 	if (ifp->ifindex == IFINDEX_INTERNAL) {
 		json_object_boolean_add(json_if, "pseudoInterface", true);