Perform strict src mac check on ensure end to end

This is required when shared net is used and there are more VMs running than requested in the -scc

Change-Id: I7599169739e6bb9b3e2377473377d5332ef2b68a
Signed-off-by: ahothan <ahothan@cisco.com>

6 files changed, 85 insertions, 52 deletions

diff --git a/nfvbench/traffic_client.py b/nfvbench/traffic_client.py
index 4414710..810f7dd 100755
--- a/nfvbench/traffic_client.py
+++ b/nfvbench/traffic_client.py
@@ -182,9 +182,25 @@ class Device(object):
         return self.generator_config.devices[1 - self.port]
 
     def set_dest_macs(self, dest_macs):
-        """Set the list of dest MACs indexed by the chain id."""
+        """Set the list of dest MACs indexed by the chain id.
+
+        This is only called in 2 cases:
+        - VM macs discovered using openstack API
+        - dest MACs provisioned in config file
+        """
         self.dest_macs = map(str, dest_macs)
 
+    def get_dest_macs(self):
+        """Get the list of dest macs for this device.
+
+        If set_dest_macs was never called, assumes l2-loopback and return
+        a list of peer mac (as many as chains but normally only 1 chain)
+        """
+        if self.dest_macs:
+            return self.dest_macs
+        # assume this is l2-loopback
+        return [self.get_peer_device().mac] * self.chain_count
+
     def set_vlans(self, vlans):
         """Set the list of vlans to use indexed by the chain id."""
         self.vlans = vlans
@@ -211,16 +227,16 @@ class Device(object):
         peer = self.get_peer_device()
         self.ip_block.reset_reservation()
         peer.ip_block.reset_reservation()
+        dest_macs = self.get_dest_macs()
 
         for chain_idx in xrange(self.chain_count):
             src_ip_first, src_ip_last = self.ip_block.reserve_ip_range(cur_chain_flow_count)
             dst_ip_first, dst_ip_last = peer.ip_block.reserve_ip_range(cur_chain_flow_count)
 
-            dest_mac = self.dest_macs[chain_idx] if self.dest_macs else peer.mac
             configs.append({
                 'count': cur_chain_flow_count,
                 'mac_src': self.mac,
-                'mac_dst': dest_mac,
+                'mac_dst': dest_macs[chain_idx],
                 'ip_src_addr': src_ip_first,
                 'ip_src_addr_max': src_ip_last,
                 'ip_src_count': cur_chain_flow_count,
@@ -328,6 +344,10 @@ class GeneratorConfig(object):
         self.devices[port_index].set_dest_macs(dest_macs)
         LOG.info('Port %d: dst MAC %s', port_index, [str(mac) for mac in dest_macs])
 
+    def get_dest_macs(self):
+        """Return the list of dest macs indexed by port."""
+        return [dev.get_dest_macs() for dev in self.devices]
+
     def set_vlans(self, port_index, vlans):
         """Set the list of vlans to use indexed by the chain id on given port.
 
@@ -477,16 +497,23 @@ class TrafficClient(object):
         # ensures enough traffic is coming back
         retry_count = (self.config.check_traffic_time_sec +
                        self.config.generic_poll_sec - 1) / self.config.generic_poll_sec
-        mac_addresses = set()
 
         # we expect to see packets coming from 2 unique MAC per chain
-        unique_src_mac_count = self.config.service_chain_count * 2
+        # because there can be flooding in the case of shared net
+        # we must verify that packets from the right VMs are received
+        # and not just count unique src MAC
+        # create a dict of (port, chain) tuples indexed by dest mac
+        mac_map = {}
+        for port, dest_macs in enumerate(self.generator_config.get_dest_macs()):
+            for chain, mac in enumerate(dest_macs):
+                mac_map[mac] = (port, chain)
+        unique_src_mac_count = len(mac_map)
         for it in xrange(retry_count):
             self.gen.clear_stats()
             self.gen.start_traffic()
             self.gen.start_capture()
             LOG.info('Captured unique src mac %d/%d, capturing return packets (retry %d/%d)...',
-                     len(mac_addresses), unique_src_mac_count,
+                     unique_src_mac_count - len(mac_map), unique_src_mac_count,
                      it + 1, retry_count)
             if not self.skip_sleep():
                 time.sleep(self.config.generic_poll_sec)
@@ -496,12 +523,14 @@ class TrafficClient(object):
 
             for packet in self.gen.packet_list:
                 src_mac = packet['binary'][6:12]
-                if src_mac not in mac_addresses:
-                    LOG.info('Received packet from mac: %s',
-                             ':'.join(["%02x" % ord(x) for x in src_mac]))
-                    mac_addresses.add(src_mac)
-
-                if len(mac_addresses) == unique_src_mac_count:
+                src_mac = ':'.join(["%02x" % ord(x) for x in src_mac])
+                if src_mac in mac_map:
+                    port, chain = mac_map[src_mac]
+                    LOG.info('Received packet from mac: %s (chain=%d, port=%d)',
+                             src_mac, chain, port)
+                    mac_map.pop(src_mac, None)
+
+                if not mac_map:
                     LOG.info('End-to-end connectivity established')
                     return
 
@@ -509,7 +538,12 @@ class TrafficClient(object):
 
     def ensure_arp_successful(self):
         """Resolve all IP using ARP and throw an exception in case of failure."""
-        if not self.gen.resolve_arp():
+        dest_macs = self.gen.resolve_arp()
+        if dest_macs:
+            # all dest macs are discovered, saved them into the generator config
+            self.generator_config.set_dest_macs(0, dest_macs[0])
+            self.generator_config.set_dest_macs(1, dest_macs[1])
+        else:
             raise TrafficClientException('ARP cannot be resolved')
 
     def set_traffic(self, frame_size, bidirectional):
diff --git a/nfvbench/traffic_gen/dummy.py b/nfvbench/traffic_gen/dummy.py
index 2a1064f..9beea28 100644
--- a/nfvbench/traffic_gen/dummy.py
+++ b/nfvbench/traffic_gen/dummy.py
@@ -32,13 +32,7 @@ class DummyTG(AbstractTrafficGenerator):
         self.duration_sec = traffic_client.config.duration_sec
         self.intf_speed = traffic_client.generator_config.intf_speed
         self.set_response_curve()
-        # for packet capture, generate 2*scc random packets
-        # normally we should generate packets coming from the right dest macs
-        scc = traffic_client.config.service_chain_count
-        self.packet_list = [self._get_packet_capture(mac_id) for mac_id in range(scc * 2)]
-
-    def _get_packet_capture(self, mac_id):
-        return {'binary': 'SSSSSS01234' + str(mac_id)}
+        self.packet_list = None
 
     def get_version(self):
         return "0.1"
@@ -164,7 +158,7 @@ class DummyTG(AbstractTrafficGenerator):
             latencies[port].avg_usec = 50
 
     def get_macs(self):
-        return ['00.00.00.00.00.01', '00.00.00.00.00.02']
+        return ['00:00:00:00:00:01', '00:00:00:00:00:02']
 
     def get_port_speed_gbps(self):
         """Return the local port speeds.
@@ -180,7 +174,17 @@ class DummyTG(AbstractTrafficGenerator):
         pass
 
     def fetch_capture_packets(self):
-        pass
+        def _get_packet_capture(mac):
+            # convert text to binary
+            src_mac = mac.replace(':', '').decode('hex')
+            return {'binary': 'SSSSSS' + src_mac}
+
+        # for packet capture, generate 2*scc random packets
+        # normally we should generate packets coming from the right dest macs
+        self.packet_list = []
+        for dest_macs in self.traffic_client.generator_config.get_dest_macs():
+            for mac in dest_macs:
+                self.packet_list.append(_get_packet_capture(mac))
 
     def stop_traffic(self):
         pass
@@ -199,5 +203,9 @@ class DummyTG(AbstractTrafficGenerator):
 
     def resolve_arp(self):
         """Resolve ARP sucessfully."""
-        LOG.info('Dummy TG ARP OK')
-        return True
+        def get_macs(port, scc):
+            return ['00:00:00:00:%02x:%02x' % (port, chain) for chain in range(scc)]
+        scc = self.traffic_client.generator_config.service_chain_count
+        res = [get_macs(port, scc) for port in range(2)]
+        LOG.info('Dummy TG ARP: %s', str(res))
+        return res
diff --git a/nfvbench/traffic_gen/traffic_base.py b/nfvbench/traffic_gen/traffic_base.py
index adb2bd0..459af0f 100644
--- a/nfvbench/traffic_gen/traffic_base.py
+++ b/nfvbench/traffic_gen/traffic_base.py
@@ -113,7 +113,9 @@ class AbstractTrafficGenerator(object):
     def resolve_arp(self):
         """Resolve all configured remote IP addresses.
 
-        return: True if ARP resolved successfully
+        return: None if ARP failed to resolve for all IP addresses
+                else a dict of list of dest macs indexed by port#
+                the dest macs in the list are indexed by the chain id
         """
         pass
 
diff --git a/nfvbench/traffic_gen/trex.py b/nfvbench/traffic_gen/trex.py
index 31b0867..71b81c0 100644
--- a/nfvbench/traffic_gen/trex.py
+++ b/nfvbench/traffic_gen/trex.py
@@ -67,9 +67,6 @@ class TRex(AbstractTrafficGenerator):
         self.port_handle = []
         self.chain_count = self.generator_config.service_chain_count
         self.rates = []
-        # A dict of list of dest macs indexed by port#
-        # the dest macs in the list are indexed by the chain id
-        self.arps = {}
         self.capture_id = None
         self.packet_list = []
 
@@ -453,7 +450,9 @@ class TRex(AbstractTrafficGenerator):
     def resolve_arp(self):
         """Resolve all configured remote IP addresses.
 
-        return: True if ARP resolved successfully
+        return: None if ARP failed to resolve for all IP addresses
+                else a dict of list of dest macs indexed by port#
+                the dest macs in the list are indexed by the chain id
         """
         self.client.set_service_mode(ports=self.port_handle)
         LOG.info('Polling ARP until successful...')
@@ -513,9 +512,8 @@ class TRex(AbstractTrafficGenerator):
 
         self.client.set_service_mode(ports=self.port_handle, enabled=False)
         if len(arps) == len(self.port_handle):
-            self.arps = arps
-            return True
-        return False
+            return arps
+        return None
 
     def __is_rate_enough(self, l2frame_size, rates, bidirectional, latency):
         """Check if rate provided by user is above requirements. Applies only if latency is True."""
@@ -567,12 +565,6 @@ class TRex(AbstractTrafficGenerator):
         stream_cfgs = [d.get_stream_configs() for d in self.generator_config.devices]
         self.rates = [utils.to_rate_str(rate) for rate in rates]
         for chain_id, (fwd_stream_cfg, rev_stream_cfg) in enumerate(zip(*stream_cfgs)):
-            if self.arps:
-                # in case of external chain with ARP, fill in the proper dest MAC
-                # based on the 2 ARP replies for each chain
-                fwd_stream_cfg['mac_dst'] = self.arps[self.port_handle[0]][chain_id]
-                rev_stream_cfg['mac_dst'] = self.arps[self.port_handle[1]][chain_id]
-
             streamblock[0].extend(self.generate_streams(self.port_handle[0],
                                                         chain_id,
                                                         fwd_stream_cfg,
@@ -614,19 +606,6 @@ class TRex(AbstractTrafficGenerator):
         """
         return [port['speed'] for port in self.port_info]
 
-    def get_dest_macs(self):
-        """Return the dest MAC for all chains for both ports for the current traffic setup.
-
-        return: a list of MAC addresses indexed by the port# [[m00, m01...], [m10, m11...]]
-
-        If ARP are used, resolve_arp() must be called prior to calling this method.
-        """
-        # if ARP was used, return the dest MACs resolved by ARP
-        if self.arps:
-            return [self.arps[port] for port in self.port_handle]
-        # no ARP, use the dest MACs as configured in the devices
-        return [d.dest_macs for d in self.generator_config.devices]
-
     def clear_stats(self):
         """Clear all stats in the traffic gneerator."""
         if self.port_handle:
diff --git a/test/test_chains.py b/test/test_chains.py
index 14ed0b5..519748b 100644
--- a/test/test_chains.py
+++ b/test/test_chains.py
@@ -21,6 +21,7 @@ from mock import MagicMock
 from mock import patch
 
 from nfvbench.chain_runner import ChainRunner
+from nfvbench.chaining import ChainVnfPort
 from nfvbench.compute import Compute
 import nfvbench.credentials
 from nfvbench.factory import BasicFactory
@@ -159,9 +160,18 @@ def _check_nfvbench_openstack(sc=ChainType.PVP, l2_loopback=False):
             print res
         assert res['status'] == 'OK'
 
+
+mac_seq = 0
+
+def _mock_get_mac(dummy):
+    global mac_seq
+    mac_seq += 1
+    return '01:00:00:00:00:%02x' % mac_seq
+
 @patch.object(Compute, 'get_enabled_az_host_list', _mock_get_enabled_az_host_list)
 @patch.object(Compute, 'find_image', _mock_find_image)
 @patch.object(TrafficClient, 'skip_sleep', lambda x: True)
+@patch.object(ChainVnfPort, 'get_mac', _mock_get_mac)
 @patch('nfvbench.chaining.Client')
 @patch('nfvbench.chaining.neutronclient')
 @patch('nfvbench.chaining.glanceclient')
diff --git a/test/test_nfvbench.py b/test/test_nfvbench.py
index b430436..f532bba 100644
--- a/test/test_nfvbench.py
+++ b/test/test_nfvbench.py
@@ -215,7 +215,7 @@ def test_config():
         expected = fail_pair[1]
         if expected is None:
             expected = fail_pair[0]
-        assert expected in e_info.value.message
+        assert expected in str(e_info)
 
     # whitelist keys
     flavor = {'flavor': {'vcpus': 2, 'ram': 8192, 'disk': 0,