fix and improve test brb/brb2

   o disable ipv6 for newly-created net devices to avoid unaccountable pkt counting
   o explicitly enable ip_forward in router namespace
   o proper cleanup in case of validation failure
   o in test_brb, use different tc_index to differentiate the packet source (pem or router_ns)
   o change bpf program return value to 1 so that the packet will be dropped instead of being
     passed up to the stack
Signed-off-by: Yonghong Song <yhs@plumgrid.com>

tests/cc/test_brb.c

@@ -101,13 +101,12 @@ int pem(struct __sk_buff *skb) {
         }
     }
 
-    return 0;
+    return 1;
 }
 
 static int br_common(struct __sk_buff *skb, int which_br) __attribute__((always_inline));
 static int br_common(struct __sk_buff *skb, int which_br) {
     u8 *cursor = 0;
-    bpf_metadata_t meta = {};
     u16 proto;
     u16 arpop;
     eth_addr_t dmac;
@@ -118,20 +117,16 @@ static int br_common(struct __sk_buff *skb, int which_br) {
     bpf_dest_t *dest_p;
     u32 index, *rtrif_p;
 
-    if (skb->tc_index == 0) {
-        skb->tc_index = 1;
-        skb->cb[0] = skb->cb[1] = 0;
-        meta.prog_id = meta.rx_port_id = 0;
-    } else {
-        meta.prog_id = skb->cb[0];
-        meta.rx_port_id = skb->cb[1];
-    }
-
     struct ethernet_t *ethernet = cursor_advance(cursor, sizeof(*ethernet));
+    /* handle ethernet packet header */
     {
         dmac.addr = ethernet->dst;
-        if (meta.prog_id != 0) {
-            /* send to the router */
+        /* skb->tc_index may be preserved accross router namespace if router simply rewrite packet
+         * and send it back.
+         */
+        if (skb->tc_index == 1) {
+            /* packet from pem, send to the router, set tc_index to 2 */
+            skb->tc_index = 2;
             if (dmac.addr == 0xffffffffffffULL) {
                  index = 0;
                  if (which_br == 1)
@@ -149,9 +144,11 @@ static int br_common(struct __sk_buff *skb, int which_br) {
                  if (rtrif_p)
                      bpf_clone_redirect(skb, *rtrif_p, 0);
              }
-             return 0;
+             return 1;
         }
 
+        /* set the tc_index to 1 so pem knows it is from internal */
+        skb->tc_index = 1;
         switch (ethernet->type) {
             case ETH_P_IP: goto ip;
             case ETH_P_ARP: goto arp;
@@ -217,7 +214,7 @@ xmit:
     }
 
 EOP:
-    return 0;
+    return 1;
 }
 
 int br1(struct __sk_buff *skb) {

tests/cc/test_brb.py

@@ -65,7 +65,6 @@ from ctypes import c_ubyte, c_ushort, c_uint, c_ulonglong, Structure
 from netaddr import IPAddress, EUI
 from bpf import BPF
 from pyroute2 import IPRoute
-from socket import socket, AF_INET, SOCK_DGRAM
 import sys
 from time import sleep
 from unittest import main, TestCase
@@ -80,10 +79,12 @@ class TestBPFSocket(TestCase):
         subprocess.call(["ip", "netns", "add", ns])
         subprocess.call(["ip", "link", "set", veth_in, "netns", ns])
         subprocess.call(["ip", "netns", "exec", ns, "ip", "link", "set", veth_in, "name", "eth0"])
+        subprocess.call(["sysctl", "-q", "-w", "net.ipv6.conf." + veth_out+ ".disable_ipv6=1"])
         subprocess.call(["ip", "link", "set", veth_out, "up"])
 
     def config_vm_ns(self, ns, ip_addr, net_mask, ip_gw):
         subprocess.call(["ip", "netns", "exec", ns, "ip", "addr", "add", ip_addr + "/24", "dev", "eth0"])
+        subprocess.call(["ip", "netns", "exec", ns, "sysctl", "-q", "-w", "net.ipv6.conf.eth0.disable_ipv6=1"])
         subprocess.call(["ip", "netns", "exec", ns, "ip", "link", "set", "eth0", "up"])
         subprocess.call(["ip", "netns", "exec", ns, "ip", "route", "add", net_mask + "/24", "via", ip_gw])
 
@@ -95,14 +96,19 @@ class TestBPFSocket(TestCase):
         subprocess.call(["ip", "link", "add", veth2_in, "type", "veth", "peer", "name", veth2_out])
         subprocess.call(["ip", "link", "set", veth2_in, "netns", ns])
         subprocess.call(["ip", "netns", "exec", ns, "ip", "link", "set", veth2_in, "name", "eth1"])
+        subprocess.call(["sysctl", "-q", "-w", "net.ipv6.conf." + veth1_out+ ".disable_ipv6=1"])
+        subprocess.call(["sysctl", "-q", "-w", "net.ipv6.conf." + veth2_out+ ".disable_ipv6=1"])
         subprocess.call(["ip", "link", "set", veth1_out, "up"])
         subprocess.call(["ip", "link", "set", veth2_out, "up"])
 
     def config_router_ns(self, ns, ip_eth0, ip_eth1):
         subprocess.call(["ip", "netns", "exec", ns, "ip", "addr", "add", ip_eth0 + "/24", "dev", "eth0"])
+        subprocess.call(["ip", "netns", "exec", ns, "sysctl", "-q", "-w", "net.ipv6.conf.eth0.disable_ipv6=1"])
         subprocess.call(["ip", "netns", "exec", ns, "ip", "link", "set", "eth0", "up"])
         subprocess.call(["ip", "netns", "exec", ns, "ip", "addr", "add", ip_eth1 + "/24", "dev", "eth1"])
+        subprocess.call(["ip", "netns", "exec", ns, "sysctl", "-q", "-w", "net.ipv6.conf.eth1.disable_ipv6=1"])
         subprocess.call(["ip", "netns", "exec", ns, "ip", "link", "set", "eth1", "up"])
+        subprocess.call(["ip", "netns", "exec", ns, "sysctl", "-w", "net.ipv4.ip_forward=1"])
 
     def set_default_const(self):
         self.ns1            = "ns1"
@@ -206,8 +212,8 @@ class TestBPFSocket(TestCase):
         self.attach_filter(ip, self.nsrtr_eth0_out, br1_fn.fd, br1_fn.name)
         self.attach_filter(ip, self.nsrtr_eth1_out, br2_fn.fd, br2_fn.name)
 
-    def setUp(self):
-
+    def test_brb(self):
+        try:
             # set up the environment
             self.set_default_const()
             self.setup_vm_ns(self.ns1, self.ns1_eth_in, self.ns1_eth_out)
@@ -227,7 +233,6 @@ class TestBPFSocket(TestCase):
             # load the program and configure maps
             self.config_maps()
 
-    def test_brb(self):
             # our bridge is not smart enough, so send arping for router learning to prevent router
             # from sending out arp request
             subprocess.call(["ip", "netns", "exec", self.ns1, "arping", "-w", "1", "-c", "1", "-I", "eth0",
@@ -236,8 +241,9 @@ class TestBPFSocket(TestCase):
                              self.vm2_rtr_ip])
             # ping
             subprocess.call(["ip", "netns", "exec", self.ns1, "ping", self.vm2_ip, "-c", "2"])
-        # minimum one arp reply, 5 icmp reply
-        self.assertGreater(self.pem_stats[c_uint(0)].value, 5)
+            # pem_stats only counts pem->bridge traffic, each VM has 4: arping/arp request/2 icmp request
+            # total 8 packets should be counted
+            self.assertEqual(self.pem_stats[c_uint(0)].value, 8)
 
             # iperf, run server on the background
             subprocess.Popen(["ip", "netns", "exec", self.ns2, "iperf", "-s", "-xSCD"])
@@ -252,10 +258,12 @@ class TestBPFSocket(TestCase):
             subprocess.call(["ip", "netns", "exec", self.ns1, "netperf", "-l", "1", "-H", self.vm2_ip, "-t", "TCP_RR"])
             subprocess.call(["ip", "netns", "exec", self.ns2, "killall", "netserver"])
 
+        finally:
             # cleanup, tear down the veths and namespaces
-        subprocess.call(["ip", "netns", "del", self.ns1])
-        subprocess.call(["ip", "netns", "del", self.ns2])
-        subprocess.call(["ip", "netns", "del", self.ns_router])
+            ns_list = subprocess.check_output(["ip", "netns", "list"]).split()
+            if self.ns1 in ns_list: subprocess.call(["ip", "netns", "del", self.ns1])
+            if self.ns2 in ns_list: subprocess.call(["ip", "netns", "del", self.ns2])
+            if self.ns_router in ns_list: subprocess.call(["ip", "netns", "del", self.ns_router])
 
 
 if __name__ == "__main__":

tests/cc/test_brb2.c

@@ -24,5 +24,5 @@ int pem(struct __sk_buff *skb) {
         bpf_clone_redirect(skb, *ifindex_p, 0);
     }
 
-    return 0;
+    return 1;
 }

tests/cc/test_brb2.py

@@ -58,7 +58,6 @@ from ctypes import c_ubyte, c_ushort, c_uint, c_ulonglong, Structure
 from netaddr import IPAddress
 from bpf import BPF
 from pyroute2 import IPRoute
-from socket import socket, AF_INET, SOCK_DGRAM
 import sys
 from time import sleep
 from unittest import main, TestCase
@@ -72,10 +71,12 @@ class TestBPFSocket(TestCase):
         subprocess.call(["ip", "netns", "add", ns])
         subprocess.call(["ip", "link", "set", veth_in, "netns", ns])
         subprocess.call(["ip", "netns", "exec", ns, "ip", "link", "set", veth_in, "name", "eth0"])
+        subprocess.call(["sysctl", "-q", "-w", "net.ipv6.conf." + veth_out+ ".disable_ipv6=1"])
         subprocess.call(["ip", "link", "set", veth_out, "up"])
 
     def config_vm_ns(self, ns, ip_addr, net_mask, ip_gw):
         subprocess.call(["ip", "netns", "exec", ns, "ip", "addr", "add", ip_addr + "/24", "dev", "eth0"])
+        subprocess.call(["ip", "netns", "exec", ns, "sysctl", "-q", "-w", "net.ipv6.conf.eth0.disable_ipv6=1"])
         subprocess.call(["ip", "netns", "exec", ns, "ip", "link", "set", "eth0", "up"])
         subprocess.call(["ip", "netns", "exec", ns, "route", "add", "-net", net_mask + "/24", "gw", ip_gw])
 
@@ -87,25 +88,33 @@ class TestBPFSocket(TestCase):
         subprocess.call(["ip", "link", "add", veth2_in, "type", "veth", "peer", "name", veth2_out])
         subprocess.call(["ip", "link", "set", veth2_in, "netns", ns])
         subprocess.call(["ip", "netns", "exec", ns, "ip", "link", "set", veth2_in, "name", "eth1"])
+        subprocess.call(["sysctl", "-q", "-w", "net.ipv6.conf." + veth1_out+ ".disable_ipv6=1"])
+        subprocess.call(["sysctl", "-q", "-w", "net.ipv6.conf." + veth2_out+ ".disable_ipv6=1"])
         subprocess.call(["ip", "link", "set", veth1_out, "up"])
         subprocess.call(["ip", "link", "set", veth2_out, "up"])
 
     def config_router_ns(self, ns, ip_eth0, ip_eth1):
         subprocess.call(["ip", "netns", "exec", ns, "ip", "addr", "add", ip_eth0 + "/24", "dev", "eth0"])
+        subprocess.call(["ip", "netns", "exec", ns, "sysctl", "-q", "-w", "net.ipv6.conf.eth0.disable_ipv6=1"])
         subprocess.call(["ip", "netns", "exec", ns, "ip", "link", "set", "eth0", "up"])
         subprocess.call(["ip", "netns", "exec", ns, "ip", "addr", "add", ip_eth1 + "/24", "dev", "eth1"])
+        subprocess.call(["ip", "netns", "exec", ns, "sysctl", "-q", "-w", "net.ipv6.conf.eth1.disable_ipv6=1"])
         subprocess.call(["ip", "netns", "exec", ns, "ip", "link", "set", "eth1", "up"])
+        subprocess.call(["ip", "netns", "exec", ns, "sysctl", "-w", "net.ipv4.ip_forward=1"])
 
     def setup_br(self, br, veth_rt_2_br):
         # set up the bridge and add router interface as one of its slaves
         subprocess.call(["ip", "link", "add", "name", br, "type", "bridge"])
         subprocess.call(["ip", "link", "set", "dev", veth_rt_2_br, "master", br])
+        subprocess.call(["sysctl", "-q", "-w", "net.ipv6.conf." + br + ".disable_ipv6=1"])
         subprocess.call(["ip", "link", "set", br, "up"])
 
     def br_add_pem_link(self, br, veth_pem_2_br, veth_br_2_pem):
         subprocess.call(["ip", "link", "add", veth_pem_2_br, "type", "veth", "peer", "name", veth_br_2_pem])
         subprocess.call(["ip", "link", "set", "dev", veth_pem_2_br, "master", br])
+        subprocess.call(["sysctl", "-q", "-w", "net.ipv6.conf." + veth_pem_2_br + ".disable_ipv6=1"])
         subprocess.call(["ip", "link", "set", veth_pem_2_br, "up"])
+        subprocess.call(["sysctl", "-q", "-w", "net.ipv6.conf." + veth_br_2_pem + ".disable_ipv6=1"])
         subprocess.call(["ip", "link", "set", veth_br_2_pem, "up"])
 
     def set_default_const(self):
@@ -164,8 +173,8 @@ class TestBPFSocket(TestCase):
         self.attach_filter(ip, self.veth_br1_2_pem, pem_fn.fd, pem_fn.name)
         self.attach_filter(ip, self.veth_br2_2_pem, pem_fn.fd, pem_fn.name)
 
-    def setUp(self):
-
+    def test_brb2(self):
+        try:
             # set up the environment
             self.set_default_const()
             self.setup_vm_ns(self.ns1, self.ns1_eth_in, self.ns1_eth_out)
@@ -186,11 +195,10 @@ class TestBPFSocket(TestCase):
             # load the program and configure maps
             self.config_maps()
 
-    def test_brb2(self):
             # ping
             subprocess.call(["ip", "netns", "exec", self.ns1, "ping", self.vm2_ip, "-c", "2"])
-        # minimum one arp request/reply, 5 icmp request/reply
-        self.assertGreater(self.pem_stats[c_uint(0)].value, 11)
+            # one arp request/reply, 2 icmp request/reply per VM, total 6 packets per VM, 12 packets total
+            self.assertEqual(self.pem_stats[c_uint(0)].value, 12)
 
             # iperf, run server on the background
             subprocess.Popen(["ip", "netns", "exec", self.ns2, "iperf", "-s", "-xSCD"])
@@ -205,14 +213,17 @@ class TestBPFSocket(TestCase):
             subprocess.call(["ip", "netns", "exec", self.ns1, "netperf", "-l", "1", "-H", self.vm2_ip, "-t", "TCP_RR"])
             subprocess.call(["ip", "netns", "exec", self.ns2, "killall", "netserver"])
 
+        finally:
             # cleanup, tear down the veths and namespaces
-        subprocess.call(["ip", "link", "del", self.veth_br1_2_pem])
-        subprocess.call(["ip", "link", "del", self.veth_br2_2_pem])
-        subprocess.call(["ip", "link", "del", self.br1])
-        subprocess.call(["ip", "link", "del", self.br2])
-        subprocess.call(["ip", "netns", "del", self.ns1])
-        subprocess.call(["ip", "netns", "del", self.ns2])
-        subprocess.call(["ip", "netns", "del", self.ns_router])
+            net_list = subprocess.check_output(["ls", "/sys/class/net"]).split()
+            ns_list = subprocess.check_output(["ip", "netns", "list"]).split()
+            if self.veth_br1_2_pem in net_list: subprocess.call(["ip", "link", "del", self.veth_br1_2_pem])
+            if self.veth_br2_2_pem in net_list: subprocess.call(["ip", "link", "del", self.veth_br2_2_pem])
+            if self.br1 in net_list: subprocess.call(["ip", "link", "del", self.br1])
+            if self.br2 in net_list: subprocess.call(["ip", "link", "del", self.br2])
+            if self.ns1 in ns_list: subprocess.call(["ip", "netns", "del", self.ns1])
+            if self.ns2 in ns_list: subprocess.call(["ip", "netns", "del", self.ns2])
+            if self.ns_router in ns_list: subprocess.call(["ip", "netns", "del", self.ns_router])
 
 
 if __name__ == "__main__":