Revert "Process ingress multicast traffic for 224.0.0.X separately"
This reverts commit b8be1a05fa.
As was reported in bug [1] this patch broke multicast traffic send
from ports with disabled port security. And that broke L3HA routers
as keepalived processes couldn't talk to each other.
During attempt to fix that issue with keepalived we found out another
corner cases which we may break and in fact to fix them, we would
effectively revert this change and allow multicast traffic for all
ports in e.g. networks with ports which have port security and ports
which don't have port security and are on same node.
As we also don't really know what other corner cases we may hit going
further with that, lets revert this patch.
As a follow up patch I will propose new patch which will document
differences in handling multicast traffic between iptables and
openvswitch based firewall drivers.
[1] https://bugs.launchpad.net/neutron/+bug/1899967
Change-Id: I37a8b33cf8e16d5bb5dc1966fc2dca6bb619026c
Closes-Bug: #1899967
This commit is contained in:
Slawek Kaplonski
parent
b8be1a05fa
commit
14a1ad7009
|
|
@ -495,51 +495,6 @@ same as in |table_72|.
|
|||
migrated to a port on a different node, then the new port won't contain
|
||||
conntrack information about previous traffic that happened with VIP.
|
||||
|
||||
Multicast traffic for addresses in 224.0.0.X
|
||||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|
||||
|
||||
By default, as commented in [1]_, "packets with a destination IP (DIP) address
|
||||
in the 224.0.0.X range which are not IGMP must be forwarded on all ports." That
|
||||
means those packets will be forwarded to all ports regardless of any ingress
|
||||
rule. Therefore those packets are processed independently. Any ingress packet
|
||||
incoming from an local VM is sent to the multicast ingress table, |table_101|.
|
||||
This table has one rule that sends the received packets directly to the
|
||||
physical bridges, the tunnel bridges and the multicast rule processing table,
|
||||
|table_102|. Port 1 (output:1) is the integration bridge to physical bridge
|
||||
patch port.
|
||||
|
||||
::
|
||||
|
||||
table=60, priority=50,ip,dl_vlan=1,nw_dst=224.0.0.0/24 actions=load:0x1->NXM_NX_REG6[],strip_vlan,resubmit(,101)
|
||||
table=60, priority=50,ip,dl_vlan=1,nw_dst=224.0.0.0/24 actions=load:0x2->NXM_NX_REG6[],strip_vlan,resubmit(,101)
|
||||
table=101, priority=70 actions=output:1,resubmit(,102)
|
||||
table=101, priority=0 actions=drop
|
||||
|
||||
The goal of this table is to avoid the NORMAl action processing for those
|
||||
packets, not allowing OVS to forward them to all ports. Instead of this, those
|
||||
packets are sent to other hosts via the physical and the tunnel bridges.
|
||||
|
||||
The packets comming from external sources are sent directly to |table_102|.
|
||||
|
||||
::
|
||||
|
||||
table=73, priority=95,ip,nw_dst=224.0.0.0/24 actions=resubmit(,102)
|
||||
|
||||
The next table will process the ingress rules for those multicast packets
|
||||
according to the protocol number defined in each rule, per network (internal
|
||||
VLAN used by Neutron to segment the tenant traffic). The OVS firewall class
|
||||
``OVSFirewallDriver`` instance will keep a list of ports per internal VLAN and
|
||||
rule. When a rule is added or updated, a OpenFlow rule will be added to this
|
||||
|table_102|. This rule matches the rule protocol and outputs the packets to all
|
||||
ports assigned to this rule in a specific VLAN network.
|
||||
|
||||
::
|
||||
|
||||
table=102, priority=70,ip,reg6=0x1,nw_proto=112 actions=output:11
|
||||
table=102, priority=70,ip,reg6=0x2,nw_proto=122 actions=output:12
|
||||
table=102, priority=0 actions=drop
|
||||
|
||||
|
||||
OVS firewall integration points
|
||||
-------------------------------
|
||||
|
||||
|
|
@ -613,5 +568,3 @@ switched to the OVS driver.
|
|||
.. |table_92| replace:: ``table 92`` (ACCEPTED_INGRESS_TRAFFIC)
|
||||
.. |table_93| replace:: ``table 93`` (DROPPED_TRAFFIC)
|
||||
.. |table_94| replace:: ``table 94`` (ACCEPTED_EGRESS_TRAFFIC_NORMAL)
|
||||
.. |table_101| replace:: ``table 101`` (MCAST_INGRESS_TABLE)
|
||||
.. |table_102| replace:: ``table 102`` (MCAST_RULES_INGRESS_TABLE)
|
||||
|
|
|
|||
|
|
@ -160,11 +160,6 @@ class FirewallDriver(object, metaclass=abc.ABCMeta):
|
|||
def remove_trusted_ports(self, port_ids):
|
||||
pass
|
||||
|
||||
def setup_multicast_traffic(self, phy_br_ofports, tun_br_ofports,
|
||||
enable_tunneling):
|
||||
"""Setup filters for multicast traffic"""
|
||||
pass
|
||||
|
||||
|
||||
class NoopFirewallDriver(FirewallDriver):
|
||||
"""Noop Firewall Driver.
|
||||
|
|
|
|||
|
|
@ -36,7 +36,6 @@ from neutron.agent.linux.openvswitch_firewall import constants as ovsfw_consts
|
|||
from neutron.agent.linux.openvswitch_firewall import exceptions
|
||||
from neutron.agent.linux.openvswitch_firewall import iptables
|
||||
from neutron.agent.linux.openvswitch_firewall import rules
|
||||
from neutron.common import _constants as n_const
|
||||
from neutron.plugins.ml2.drivers.openvswitch.agent.common import constants \
|
||||
as ovs_consts
|
||||
|
||||
|
|
@ -480,8 +479,6 @@ class OVSFirewallDriver(firewall.FirewallDriver):
|
|||
self.sg_port_map = SGPortMap()
|
||||
self.conj_ip_manager = ConjIPFlowManager(self)
|
||||
self.sg_to_delete = set()
|
||||
self.vlan_rule_ofports = collections.defaultdict(
|
||||
lambda: collections.defaultdict(set))
|
||||
self._update_cookie = None
|
||||
self._deferred = False
|
||||
self.iptables_helper = iptables.Helper(self.int_br.br)
|
||||
|
|
@ -851,31 +848,6 @@ class OVSFirewallDriver(firewall.FirewallDriver):
|
|||
dl_dst=mac,
|
||||
vlan_tci=ovs_consts.FLAT_VLAN_TCI)
|
||||
|
||||
def setup_multicast_traffic(self, phy_br_ofports, tun_br_ofports,
|
||||
enable_tunneling):
|
||||
ofports = list(phy_br_ofports.values())
|
||||
if enable_tunneling:
|
||||
for network_type in ovs_consts.TUNNEL_NETWORK_TYPES:
|
||||
ofports += list(tun_br_ofports[network_type].values())
|
||||
|
||||
actions = ''
|
||||
for ofport in ofports:
|
||||
actions += 'output:{:d},'.format(ofport)
|
||||
actions += 'resubmit(,{:d})'.format(
|
||||
ovs_consts.MCAST_RULES_INGRESS_TABLE)
|
||||
|
||||
self._add_flow(
|
||||
table=ovs_consts.MCAST_INGRESS_TABLE,
|
||||
priority=70,
|
||||
actions=actions)
|
||||
self._add_flow(
|
||||
table=ovs_consts.ACCEPT_OR_INGRESS_TABLE,
|
||||
priority=95,
|
||||
dl_type=lib_const.ETHERTYPE_IP,
|
||||
nw_dst=n_const.LOCAL_NETWORK_CONTROL_BLOCK,
|
||||
actions='resubmit(,{:d})'.format(
|
||||
ovs_consts.MCAST_RULES_INGRESS_TABLE))
|
||||
|
||||
def initialize_port_flows(self, port):
|
||||
"""Set base flows for port
|
||||
|
||||
|
|
@ -918,19 +890,6 @@ class OVSFirewallDriver(firewall.FirewallDriver):
|
|||
ovs_consts.BASE_INGRESS_TABLE),
|
||||
)
|
||||
|
||||
self._add_flow(
|
||||
table=ovs_consts.TRANSIENT_TABLE,
|
||||
priority=50,
|
||||
dl_vlan='0x%x' % port.vlan_tag,
|
||||
dl_type=lib_const.ETHERTYPE_IP,
|
||||
nw_dst=n_const.LOCAL_NETWORK_CONTROL_BLOCK,
|
||||
actions='set_field:{:d}->reg{:d},'
|
||||
'strip_vlan,resubmit(,{:d})'.format(
|
||||
port.vlan_tag,
|
||||
ovsfw_consts.REG_NET,
|
||||
ovs_consts.MCAST_INGRESS_TABLE),
|
||||
)
|
||||
|
||||
self._initialize_egress(port)
|
||||
self._initialize_ingress(port)
|
||||
|
||||
|
|
@ -1481,21 +1440,6 @@ class OVSFirewallDriver(firewall.FirewallDriver):
|
|||
|
||||
self.conj_ip_manager.update_flows_for_vlan(port.vlan_tag)
|
||||
|
||||
modified_vlan_protocols = set()
|
||||
for rule in self._create_rules_generator_for_port(port):
|
||||
if (rule['ethertype'] != lib_const.IPv4 or
|
||||
rule['direction'] != lib_const.INGRESS_DIRECTION or
|
||||
not rule.get('protocol')):
|
||||
continue
|
||||
self.vlan_rule_ofports[port.vlan_tag][
|
||||
rule['protocol']].add(port.ofport)
|
||||
modified_vlan_protocols.add((port.vlan_tag, rule['protocol']))
|
||||
|
||||
for vlan_tag, protocol in modified_vlan_protocols:
|
||||
flow = rules.create_mcast_flow_from_vlan_protocol(
|
||||
vlan_tag, protocol, self.vlan_rule_ofports[vlan_tag][protocol])
|
||||
self._add_flow(**flow)
|
||||
|
||||
def _create_rules_generator_for_port(self, port):
|
||||
for sec_group in port.sec_groups:
|
||||
for rule in sec_group.raw_rules:
|
||||
|
|
@ -1524,28 +1468,6 @@ class OVSFirewallDriver(firewall.FirewallDriver):
|
|||
in_port=port.ofport)
|
||||
self._delete_flows(reg_port=port.ofport)
|
||||
|
||||
modified_vlan_protocols = set()
|
||||
for protocol, ofports in self.vlan_rule_ofports[port.vlan_tag].items():
|
||||
if port.ofport not in ofports:
|
||||
continue
|
||||
self.vlan_rule_ofports[port.vlan_tag][protocol].discard(
|
||||
port.ofport)
|
||||
modified_vlan_protocols.add((port.vlan_tag, protocol))
|
||||
|
||||
for vlan_tag, protocol in modified_vlan_protocols:
|
||||
ofports = self.vlan_rule_ofports[vlan_tag][protocol]
|
||||
if ofports:
|
||||
flow = rules.create_mcast_flow_from_vlan_protocol(
|
||||
vlan_tag, protocol, ofports)
|
||||
self._add_flow(**flow)
|
||||
else:
|
||||
self._strict_delete_flow(
|
||||
priority=70,
|
||||
table=ovs_consts.MCAST_RULES_INGRESS_TABLE,
|
||||
dl_type=lib_const.ETHERTYPE_IP,
|
||||
nw_proto=protocol,
|
||||
reg_net=vlan_tag)
|
||||
|
||||
def delete_flows_for_flow_state(
|
||||
self, flow_state, addr_to_conj, direction, ethertype, vlan_tag):
|
||||
# Remove rules for deleted IPs and action=conjunction(conj_id, 1/2)
|
||||
|
|
|
|||
|
|
@ -204,22 +204,6 @@ def create_flows_from_rule_and_port(rule, port, conjunction=False):
|
|||
return flows
|
||||
|
||||
|
||||
def create_mcast_flow_from_vlan_protocol(vlan_tag, protocol, ofports):
|
||||
"""Create ingress flows for multicast traffic, destination IP 224.0.0.x"""
|
||||
flow = {
|
||||
'table': ovs_consts.MCAST_RULES_INGRESS_TABLE,
|
||||
'priority': 70,
|
||||
'dl_type': n_consts.ETHERTYPE_IP,
|
||||
'nw_proto': protocol,
|
||||
'reg_net': vlan_tag}
|
||||
|
||||
flow['actions'] = ''
|
||||
for ofport in ofports:
|
||||
flow['actions'] += 'output:{:d},'.format(ofport)
|
||||
|
||||
return flow
|
||||
|
||||
|
||||
def populate_flow_common(direction, flow_template, port):
|
||||
"""Initialize common flow fields."""
|
||||
if direction == n_consts.INGRESS_DIRECTION:
|
||||
|
|
|
|||
|
|
@ -131,12 +131,6 @@ class SecurityGroupAgentRpc(object):
|
|||
def init_ovs_dvr_firewall(self, dvr_agent):
|
||||
dvr_agent.set_firewall(self.firewall)
|
||||
|
||||
@skip_if_noopfirewall_or_firewall_disabled
|
||||
def setup_multicast_traffic(self, phy_br_ofports, tun_br_ofports,
|
||||
enable_tunneling):
|
||||
self.firewall.setup_multicast_traffic(phy_br_ofports, tun_br_ofports,
|
||||
enable_tunneling)
|
||||
|
||||
@skip_if_noopfirewall_or_firewall_disabled
|
||||
def prepare_devices_filter(self, device_ids):
|
||||
if not device_ids:
|
||||
|
|
|
|||
|
|
@ -77,8 +77,3 @@ IDPOOL_SELECT_SIZE = 100
|
|||
# IP allocations being cleaned up by cascade.
|
||||
AUTO_DELETE_PORT_OWNERS = [constants.DEVICE_OWNER_DHCP,
|
||||
constants.DEVICE_OWNER_DISTRIBUTED]
|
||||
|
||||
# NOTE(ralonsoh): move to n-lib
|
||||
# https://www.iana.org/assignments/multicast-addresses/
|
||||
# multicast-addresses.xhtml#multicast-addresses-1
|
||||
LOCAL_NETWORK_CONTROL_BLOCK = '224.0.0.0/24'
|
||||
|
|
|
|||
|
|
@ -67,8 +67,6 @@ RULES_EGRESS_TABLE = 72
|
|||
ACCEPT_OR_INGRESS_TABLE = 73
|
||||
BASE_INGRESS_TABLE = 81
|
||||
RULES_INGRESS_TABLE = 82
|
||||
MCAST_INGRESS_TABLE = 101
|
||||
MCAST_RULES_INGRESS_TABLE = 102
|
||||
|
||||
OVS_FIREWALL_TABLES = (
|
||||
BASE_EGRESS_TABLE,
|
||||
|
|
@ -76,8 +74,6 @@ OVS_FIREWALL_TABLES = (
|
|||
ACCEPT_OR_INGRESS_TABLE,
|
||||
BASE_INGRESS_TABLE,
|
||||
RULES_INGRESS_TABLE,
|
||||
MCAST_INGRESS_TABLE,
|
||||
MCAST_RULES_INGRESS_TABLE,
|
||||
)
|
||||
|
||||
# Tables for parties interacting with ovs firewall
|
||||
|
|
@ -102,10 +98,7 @@ INT_BR_ALL_TABLES = (
|
|||
RULES_INGRESS_TABLE,
|
||||
ACCEPTED_EGRESS_TRAFFIC_TABLE,
|
||||
ACCEPTED_INGRESS_TRAFFIC_TABLE,
|
||||
DROPPED_TRAFFIC_TABLE,
|
||||
MCAST_INGRESS_TABLE,
|
||||
MCAST_RULES_INGRESS_TABLE,
|
||||
)
|
||||
DROPPED_TRAFFIC_TABLE)
|
||||
|
||||
# --- Tunnel bridge (tun_br)
|
||||
|
||||
|
|
|
|||
|
|
@ -306,8 +306,6 @@ class OVSNeutronAgent(l2population_rpc.L2populationRpcCallBackTunnelMixin,
|
|||
self.sg_agent)
|
||||
|
||||
self.sg_agent.init_ovs_dvr_firewall(self.dvr_agent)
|
||||
self.sg_agent.setup_multicast_traffic(
|
||||
self.phys_ofports, self.tun_br_ofports, self.enable_tunneling)
|
||||
|
||||
# we default to False to provide backward compat with out of tree
|
||||
# firewall drivers that expect the logic that existed on the Neutron
|
||||
|
|
|
|||
|
|
@ -27,7 +27,6 @@ from neutron.agent.common import utils
|
|||
from neutron.agent.linux.openvswitch_firewall import constants as ovsfw_consts
|
||||
from neutron.agent.linux.openvswitch_firewall import exceptions
|
||||
from neutron.agent.linux.openvswitch_firewall import firewall as ovsfw
|
||||
from neutron.common import _constants as n_const
|
||||
from neutron.conf.agent import securitygroups_rpc
|
||||
from neutron.plugins.ml2.drivers.openvswitch.agent.common import constants \
|
||||
as ovs_consts
|
||||
|
|
@ -509,12 +508,7 @@ class TestOVSFirewallDriver(base.BaseTestCase):
|
|||
mock.call(actions='drop', priority=0,
|
||||
table=ovs_consts.BASE_INGRESS_TABLE),
|
||||
mock.call(actions='drop', priority=0,
|
||||
table=ovs_consts.RULES_INGRESS_TABLE),
|
||||
mock.call(actions='drop', priority=0,
|
||||
table=ovs_consts.MCAST_INGRESS_TABLE),
|
||||
mock.call(actions='drop', priority=0,
|
||||
table=ovs_consts.MCAST_RULES_INGRESS_TABLE),
|
||||
]
|
||||
table=ovs_consts.RULES_INGRESS_TABLE)]
|
||||
actual_calls = self.firewall.int_br.br.add_flow.call_args_list
|
||||
self.assertEqual(expected_calls, actual_calls)
|
||||
|
||||
|
|
@ -1044,29 +1038,6 @@ class TestOVSFirewallDriver(base.BaseTestCase):
|
|||
addr_to_conj = {'addr1': {8, 16, 24}}
|
||||
self._test_delete_flows_for_flow_state(addr_to_conj, False)
|
||||
|
||||
def test_setup_multicast_traffic(self):
|
||||
phy_br_ofports = {1: 1, 2: 2}
|
||||
tun_br_ofports = {constants.TYPE_GRE: {'ip': 3},
|
||||
constants.TYPE_VXLAN: {'ip': 4},
|
||||
constants.TYPE_GENEVE: {'ip': 5}}
|
||||
|
||||
actions1 = ''
|
||||
for ofport in range(1, 6):
|
||||
actions1 += 'output:%s,' % ofport
|
||||
actions1 += 'resubmit(,%s)' % ovs_consts.MCAST_RULES_INGRESS_TABLE
|
||||
actions2 = 'resubmit(,%s)' % ovs_consts.MCAST_RULES_INGRESS_TABLE
|
||||
|
||||
with mock.patch.object(self.firewall, '_add_flow') as mock_add_flow:
|
||||
self.firewall.setup_multicast_traffic(phy_br_ofports,
|
||||
tun_br_ofports, True)
|
||||
calls = [mock.call(table=ovs_consts.MCAST_INGRESS_TABLE,
|
||||
priority=70, actions=actions1),
|
||||
mock.call(table=ovs_consts.ACCEPT_OR_INGRESS_TABLE,
|
||||
priority=95, dl_type=constants.ETHERTYPE_IP,
|
||||
nw_dst=n_const.LOCAL_NETWORK_CONTROL_BLOCK,
|
||||
actions=actions2)]
|
||||
mock_add_flow.assert_has_calls(calls)
|
||||
|
||||
|
||||
class TestCookieContext(base.BaseTestCase):
|
||||
def setUp(self):
|
||||
|
|
|
|||
Loading…
Reference in New Issue