From c860f510cbb1f0400d28ee6ebb076f27cbcf192d Mon Sep 17 00:00:00 2001 From: Anil Rao Date: Mon, 19 Sep 2016 18:49:19 -0700 Subject: [PATCH] Disable VLAN id checks in create_tap_flow and delete_tap_flow. In order to satisfy the requirement that port MAC addresses are unique only within a Neutron network, the ingress side flow for mirroring traffic needs to match both the VLAN id of the network and the MAC address of the port. However, the current implementation of this logic doesn't work correctly because of the way in which OVS handles VLANs and Neutron's use of the NORMAL action in br-int. As a temporary workaround, the VLAN related check is being disabled. The broad/multi-cast ingress flow is also being disabled because it too relies on a VLAN id check. When a reliable way to implement the VLAN id check is available this logic will be restored. Change-Id: I4a6c518baf568c5638eafbbae7495a66c113e515 Depends-On: Ie6b3811e41a94721679c9178cdd5119bdad8208d Related-Bug: 1529595 --- .../services/taas/drivers/linux/ovs_taas.py | 66 +++++++++++++------ 1 file changed, 46 insertions(+), 20 deletions(-) diff --git a/neutron_taas/services/taas/drivers/linux/ovs_taas.py b/neutron_taas/services/taas/drivers/linux/ovs_taas.py index de771a26..843cdfdf 100755 --- a/neutron_taas/services/taas/drivers/linux/ovs_taas.py +++ b/neutron_taas/services/taas/drivers/linux/ovs_taas.py @@ -293,10 +293,6 @@ class OvsTaasDriver(taas_base.TaasDriverBase): ovs_port = self.int_br.get_vif_port_by_id(port['id']) ovs_port_id = ovs_port.ofport - # Get VLAN id for tap flow port - port_dict = self.int_br.get_port_tag_dict() - port_vlan_id = port_dict[ovs_port.port_name] - # Get patch port ID patch_int_tap_id = self.int_br.get_port_ofport('patch-int-tap') @@ -310,16 +306,39 @@ class OvsTaasDriver(taas_base.TaasDriverBase): if direction == 'IN' or direction == 'BOTH': port_mac = tap_flow['port_mac'] + + # + # Note: The ingress side flow (for unicast traffic) should + # include a check for the 'VLAN id of the Neutron + # network the port belongs to' + 'MAC address of the + # port', to comply with the requirement that port MAC + # addresses are unique only within a Neutron network. + # Unfortunately, at the moment there is no clean way + # to implement such a check, given OVS's handling of + # VLAN tags and Neutron's use of the NORMAL action in + # br-int. + # + # We are therefore temporarily disabling the VLAN id + # check until a mechanism is available to implement + # it correctly. The {broad,multi}cast flow, which is + # also dependent on the VLAN id, has been disabled + # for the same reason. + # + + # Get VLAN id for tap flow port + # port_dict = self.int_br.get_port_tag_dict() + # port_vlan_id = port_dict[ovs_port.port_name] + self.int_br.add_flow(table=0, priority=20, - dl_vlan=port_vlan_id, + # dl_vlan=port_vlan_id, dl_dst=port_mac, actions="normal,mod_vlan_vid:%s,output:%s" % (str(taas_id), str(patch_int_tap_id))) - self._add_update_ingress_bcmc_flow(port_vlan_id, - taas_id, - patch_int_tap_id) + # self._add_update_ingress_bcmc_flow(port_vlan_id, + # taas_id, + # patch_int_tap_id) # Add flow(s) in br-tun for tunnel_type in ovs_consts.TUNNEL_NETWORK_TYPES: @@ -342,7 +361,6 @@ class OvsTaasDriver(taas_base.TaasDriverBase): return def delete_tap_flow(self, tap_flow): - taas_id = tap_flow['taas_id'] port = tap_flow['port'] direction = tap_flow['tap_flow']['direction'] @@ -350,13 +368,6 @@ class OvsTaasDriver(taas_base.TaasDriverBase): ovs_port = self.int_br.get_vif_port_by_id(port['id']) ovs_port_id = ovs_port.ofport - # Get VLAN id for tap flow port - port_dict = self.int_br.get_port_tag_dict() - port_vlan_id = port_dict[ovs_port.port_name] - - # Get patch port ID - patch_int_tap_id = self.int_br.get_port_ofport('patch-int-tap') - # Delete flow(s) from br-int if direction == 'OUT' or direction == 'BOTH': self.int_br.delete_flows(table=0, @@ -364,13 +375,28 @@ class OvsTaasDriver(taas_base.TaasDriverBase): if direction == 'IN' or direction == 'BOTH': port_mac = tap_flow['port_mac'] + + # + # The VLAN id related checks have been temporarily disabled. + # Please see comment in create_tap_flow() for details. + # + + # taas_id = tap_flow['taas_id'] + + # Get VLAN id for tap flow port + # port_dict = self.int_br.get_port_tag_dict() + # port_vlan_id = port_dict[ovs_port.port_name] + + # Get patch port ID + # patch_int_tap_id = self.int_br.get_port_ofport('patch-int-tap') + self.int_br.delete_flows(table=0, - dl_vlan=port_vlan_id, + # dl_vlan=port_vlan_id, dl_dst=port_mac) - self._del_update_ingress_bcmc_flow(port_vlan_id, - taas_id, - patch_int_tap_id) + # self._del_update_ingress_bcmc_flow(port_vlan_id, + # taas_id, + # patch_int_tap_id) return