ovsfw: Merge multiple conjunction flows
The ovsfw code generated multiple flows with the same or overlapping match fields and different actions=conjunction(nnn,2/2) flows. Merge such flows and generate only one flow with actions=conjunction(mmm,2/2),conjunction(nnn,2/2) so that filtering are correctly performed. Change-Id: I0cd325b02f35e103606595b8b124010fff8dc397 Partial-bug: #1708092
This commit is contained in:
parent
61e00dbda3
commit
237ec30ca9
|
@ -114,6 +114,12 @@ the dimension 2 of 2 are generated by the method
|
||||||
``substitute_conjunction_actions``, which represents the portion of
|
``substitute_conjunction_actions``, which represents the portion of
|
||||||
the rule other than its remote group ID.
|
the rule other than its remote group ID.
|
||||||
|
|
||||||
|
Those dimension 2 of 2 flows are per port and contain no remote group
|
||||||
|
information. When there are multiple security group rules for a port,
|
||||||
|
those flows can overlap. To avoid such a situation, flows are sorted
|
||||||
|
and fed to ``merge_port_ranges`` or ``merge_common_rules`` methods to
|
||||||
|
rearrange them.
|
||||||
|
|
||||||
|
|
||||||
Rules example with explanation:
|
Rules example with explanation:
|
||||||
-------------------------------
|
-------------------------------
|
||||||
|
|
|
@ -984,21 +984,43 @@ class OVSFirewallDriver(firewall.FirewallDriver):
|
||||||
|
|
||||||
The remaining part is done by ConjIPFlowManager.
|
The remaining part is done by ConjIPFlowManager.
|
||||||
"""
|
"""
|
||||||
|
port_rules = collections.defaultdict(list)
|
||||||
for sec_group_id, rule in (
|
for sec_group_id, rule in (
|
||||||
self._create_remote_rules_generator_for_port(port)):
|
self._create_remote_rules_generator_for_port(port)):
|
||||||
direction = rule['direction']
|
direction = rule['direction']
|
||||||
ethertype = rule['ethertype']
|
ethertype = rule['ethertype']
|
||||||
|
protocol = rule.get('protocol')
|
||||||
|
|
||||||
conj_id = self.conj_ip_manager.add(port.vlan_tag, sec_group_id,
|
conj_id = self.conj_ip_manager.add(port.vlan_tag, sec_group_id,
|
||||||
rule['remote_group_id'],
|
rule['remote_group_id'],
|
||||||
direction, ethertype)
|
direction, ethertype)
|
||||||
|
|
||||||
|
rule1 = rule.copy()
|
||||||
|
del rule1['remote_group_id']
|
||||||
|
port_rules_key = (direction, ethertype, protocol)
|
||||||
|
port_rules[port_rules_key].append((rule1, conj_id))
|
||||||
|
|
||||||
|
for (direction, ethertype, protocol), rule_conj_list in (
|
||||||
|
port_rules.items()):
|
||||||
|
all_conj_ids = set()
|
||||||
|
for rule, conj_id in rule_conj_list:
|
||||||
|
all_conj_ids.add(conj_id)
|
||||||
|
|
||||||
|
if protocol in [lib_const.PROTO_NUM_SCTP,
|
||||||
|
lib_const.PROTO_NUM_TCP,
|
||||||
|
lib_const.PROTO_NUM_UDP]:
|
||||||
|
rule_conj_list = rules.merge_port_ranges(rule_conj_list)
|
||||||
|
else:
|
||||||
|
rule_conj_list = rules.merge_common_rules(rule_conj_list)
|
||||||
|
|
||||||
|
for rule, conj_ids in rule_conj_list:
|
||||||
flows = rules.create_flows_from_rule_and_port(rule, port)
|
flows = rules.create_flows_from_rule_and_port(rule, port)
|
||||||
for flow in rules.substitute_conjunction_actions(
|
for flow in rules.substitute_conjunction_actions(
|
||||||
flows, 2, [conj_id]):
|
flows, 2, conj_ids):
|
||||||
self._add_flow(**flow)
|
self._add_flow(**flow)
|
||||||
|
|
||||||
# Install actions=accept flows.
|
# Install actions=accept flows.
|
||||||
|
for conj_id in all_conj_ids:
|
||||||
for flow in rules.create_conj_flows(
|
for flow in rules.create_conj_flows(
|
||||||
port, conj_id, direction, ethertype):
|
port, conj_id, direction, ethertype):
|
||||||
self._add_flow(**flow)
|
self._add_flow(**flow)
|
||||||
|
|
|
@ -13,6 +13,8 @@
|
||||||
# License for the specific language governing permissions and limitations
|
# License for the specific language governing permissions and limitations
|
||||||
# under the License.
|
# under the License.
|
||||||
|
|
||||||
|
import collections
|
||||||
|
|
||||||
import netaddr
|
import netaddr
|
||||||
from neutron_lib import constants as n_consts
|
from neutron_lib import constants as n_consts
|
||||||
from oslo_log import log as logging
|
from oslo_log import log as logging
|
||||||
|
@ -46,6 +48,110 @@ def is_valid_prefix(ip_prefix):
|
||||||
str(netaddr.IPNetwork(ip_prefix)) not in FORBIDDEN_PREFIXES)
|
str(netaddr.IPNetwork(ip_prefix)) not in FORBIDDEN_PREFIXES)
|
||||||
|
|
||||||
|
|
||||||
|
def _assert_mergeable_rules(rule_conj_list):
|
||||||
|
"""Assert a given (rule, conj_ids) list has mergeable rules.
|
||||||
|
|
||||||
|
The given rules must be the same except for port_range_{min,max}
|
||||||
|
differences.
|
||||||
|
"""
|
||||||
|
rule_tmpl = rule_conj_list[0][0].copy()
|
||||||
|
rule_tmpl.pop('port_range_min', None)
|
||||||
|
rule_tmpl.pop('port_range_max', None)
|
||||||
|
for rule, conj_id in rule_conj_list[1:]:
|
||||||
|
rule1 = rule.copy()
|
||||||
|
rule1.pop('port_range_min', None)
|
||||||
|
rule1.pop('port_range_max', None)
|
||||||
|
if rule_tmpl != rule1:
|
||||||
|
raise RuntimeError(
|
||||||
|
"Incompatible SG rules detected: %(rule1)s and %(rule2)s. "
|
||||||
|
"They cannot be merged. This should not happen." %
|
||||||
|
{'rule1': rule_tmpl, 'rule2': rule})
|
||||||
|
|
||||||
|
|
||||||
|
def merge_common_rules(rule_conj_list):
|
||||||
|
"""Take a list of (rule, conj_id) and merge elements with the same rules.
|
||||||
|
Return a list of (rule, conj_id_list).
|
||||||
|
"""
|
||||||
|
if len(rule_conj_list) == 1:
|
||||||
|
rule, conj_id = rule_conj_list[0]
|
||||||
|
return [(rule, [conj_id])]
|
||||||
|
|
||||||
|
_assert_mergeable_rules(rule_conj_list)
|
||||||
|
rule_conj_map = collections.defaultdict(list)
|
||||||
|
for rule, conj_id in rule_conj_list:
|
||||||
|
rule_conj_map[(rule.get('port_range_min'),
|
||||||
|
rule.get('port_range_max'))].append(conj_id)
|
||||||
|
|
||||||
|
result = []
|
||||||
|
rule_tmpl = rule_conj_list[0][0]
|
||||||
|
rule_tmpl.pop('port_range_min', None)
|
||||||
|
rule_tmpl.pop('port_range_max', None)
|
||||||
|
for (port_min, port_max), conj_ids in rule_conj_map.items():
|
||||||
|
rule = rule_tmpl.copy()
|
||||||
|
if port_min is not None:
|
||||||
|
rule['port_range_min'] = port_min
|
||||||
|
if port_max is not None:
|
||||||
|
rule['port_range_max'] = port_max
|
||||||
|
result.append((rule, conj_ids))
|
||||||
|
return result
|
||||||
|
|
||||||
|
|
||||||
|
def _merge_port_ranges_helper(port_range_item):
|
||||||
|
# Sort with 'port' but 'min' things must come first.
|
||||||
|
port, m, dummy = port_range_item
|
||||||
|
return port * 2 + (0 if m == 'min' else 1)
|
||||||
|
|
||||||
|
|
||||||
|
def merge_port_ranges(rule_conj_list):
|
||||||
|
"""Take a list of (rule, conj_id) and transform into a list
|
||||||
|
whose rules don't overlap. Return a list of (rule, conj_id_list).
|
||||||
|
"""
|
||||||
|
if len(rule_conj_list) == 1:
|
||||||
|
rule, conj_id = rule_conj_list[0]
|
||||||
|
return [(rule, [conj_id])]
|
||||||
|
|
||||||
|
_assert_mergeable_rules(rule_conj_list)
|
||||||
|
port_ranges = []
|
||||||
|
for rule, conj_id in rule_conj_list:
|
||||||
|
port_ranges.append((rule.get('port_range_min', 1), 'min', conj_id))
|
||||||
|
port_ranges.append((rule.get('port_range_max', 65535), 'max', conj_id))
|
||||||
|
|
||||||
|
port_ranges.sort(key=_merge_port_ranges_helper)
|
||||||
|
|
||||||
|
# The idea here is to scan the port_ranges list in an ascending order,
|
||||||
|
# keeping active conjunction IDs and range in cur_conj and cur_range_min.
|
||||||
|
# A 'min' port_ranges item means an addition to cur_conj, while a 'max'
|
||||||
|
# item means a removal.
|
||||||
|
result = []
|
||||||
|
rule_tmpl = rule_conj_list[0][0]
|
||||||
|
cur_conj = set()
|
||||||
|
cur_range_min = None
|
||||||
|
for port, m, conj_id in port_ranges:
|
||||||
|
if m == 'min':
|
||||||
|
if cur_conj and cur_range_min != port:
|
||||||
|
rule = rule_tmpl.copy()
|
||||||
|
rule['port_range_min'] = cur_range_min
|
||||||
|
rule['port_range_max'] = port - 1
|
||||||
|
result.append((rule, list(cur_conj)))
|
||||||
|
cur_range_min = port
|
||||||
|
cur_conj.add(conj_id)
|
||||||
|
else:
|
||||||
|
if cur_range_min <= port:
|
||||||
|
rule = rule_tmpl.copy()
|
||||||
|
rule['port_range_min'] = cur_range_min
|
||||||
|
rule['port_range_max'] = port
|
||||||
|
result.append((rule, list(cur_conj)))
|
||||||
|
# The next port range without 'port' starts from (port + 1)
|
||||||
|
cur_range_min = port + 1
|
||||||
|
cur_conj.remove(conj_id)
|
||||||
|
|
||||||
|
if (len(result) == 1 and result[0][0]['port_range_min'] == 1 and
|
||||||
|
result[0][0]['port_range_max'] == 65535):
|
||||||
|
del result[0][0]['port_range_min']
|
||||||
|
del result[0][0]['port_range_max']
|
||||||
|
return result
|
||||||
|
|
||||||
|
|
||||||
def create_flows_from_rule_and_port(rule, port):
|
def create_flows_from_rule_and_port(rule, port):
|
||||||
ethertype = rule['ethertype']
|
ethertype = rule['ethertype']
|
||||||
direction = rule['direction']
|
direction = rule['direction']
|
||||||
|
|
|
@ -106,16 +106,17 @@ class TestSecurityGroupsSameNetwork(BaseSecurityGroupsSameNetworkTest):
|
||||||
4. a security group update takes effect,
|
4. a security group update takes effect,
|
||||||
5. a remote security group member addition works, and
|
5. a remote security group member addition works, and
|
||||||
6. an established connection stops by deleting a SG rule.
|
6. an established connection stops by deleting a SG rule.
|
||||||
7. test other protocol functionality by using SCTP protocol
|
7. multiple overlapping remote rules work,
|
||||||
8. test two vms with same mac on the same host in different
|
8. test other protocol functionality by using SCTP protocol
|
||||||
|
9. test two vms with same mac on the same host in different
|
||||||
networks
|
networks
|
||||||
"""
|
"""
|
||||||
index_to_sg = [0, 0, 1]
|
index_to_sg = [0, 0, 1, 2]
|
||||||
if self.firewall_driver == 'iptables_hybrid':
|
if self.firewall_driver == 'iptables_hybrid':
|
||||||
# The iptables_hybrid driver lacks isolation between agents
|
# The iptables_hybrid driver lacks isolation between agents
|
||||||
index_to_host = [0] * 3
|
index_to_host = [0] * 4
|
||||||
else:
|
else:
|
||||||
index_to_host = [0, 1, 1]
|
index_to_host = [0, 1, 1, 0]
|
||||||
|
|
||||||
tenant_uuid = uuidutils.generate_uuid()
|
tenant_uuid = uuidutils.generate_uuid()
|
||||||
|
|
||||||
|
@ -124,7 +125,7 @@ class TestSecurityGroupsSameNetwork(BaseSecurityGroupsSameNetworkTest):
|
||||||
tenant_uuid, network['id'], '20.0.0.0/24')
|
tenant_uuid, network['id'], '20.0.0.0/24')
|
||||||
|
|
||||||
sgs = [self.safe_client.create_security_group(tenant_uuid)
|
sgs = [self.safe_client.create_security_group(tenant_uuid)
|
||||||
for i in range(2)]
|
for i in range(3)]
|
||||||
ports = [
|
ports = [
|
||||||
self.safe_client.create_port(tenant_uuid, network['id'],
|
self.safe_client.create_port(tenant_uuid, network['id'],
|
||||||
self.environment.hosts[host].hostname,
|
self.environment.hosts[host].hostname,
|
||||||
|
@ -228,22 +229,23 @@ class TestSecurityGroupsSameNetwork(BaseSecurityGroupsSameNetworkTest):
|
||||||
ports.append(
|
ports.append(
|
||||||
self.safe_client.create_port(tenant_uuid, network['id'],
|
self.safe_client.create_port(tenant_uuid, network['id'],
|
||||||
self.environment.hosts[
|
self.environment.hosts[
|
||||||
index_to_host[3]].hostname,
|
index_to_host[-1]].hostname,
|
||||||
security_groups=[sgs[1]['id']]))
|
security_groups=[sgs[1]['id']]))
|
||||||
|
|
||||||
vms.append(
|
vms.append(
|
||||||
self.useFixture(
|
self.useFixture(
|
||||||
machine.FakeFullstackMachine(
|
machine.FakeFullstackMachine(
|
||||||
self.environment.hosts[index_to_host[3]],
|
self.environment.hosts[index_to_host[-1]],
|
||||||
network['id'],
|
network['id'],
|
||||||
tenant_uuid,
|
tenant_uuid,
|
||||||
self.safe_client,
|
self.safe_client,
|
||||||
neutron_port=ports[3],
|
neutron_port=ports[-1],
|
||||||
use_dhcp=True)))
|
use_dhcp=True)))
|
||||||
|
self.assertEqual(5, len(vms))
|
||||||
|
|
||||||
vms[3].block_until_boot()
|
vms[4].block_until_boot()
|
||||||
|
|
||||||
netcat = net_helpers.NetcatTester(vms[3].namespace,
|
netcat = net_helpers.NetcatTester(vms[4].namespace,
|
||||||
vms[0].namespace, vms[0].ip, 3355,
|
vms[0].namespace, vms[0].ip, 3355,
|
||||||
net_helpers.NetcatTester.TCP)
|
net_helpers.NetcatTester.TCP)
|
||||||
|
|
||||||
|
@ -255,7 +257,30 @@ class TestSecurityGroupsSameNetwork(BaseSecurityGroupsSameNetworkTest):
|
||||||
sleep=8)
|
sleep=8)
|
||||||
netcat.stop_processes()
|
netcat.stop_processes()
|
||||||
|
|
||||||
# 7. check SCTP is supported by security group
|
# 7. check if multiple overlapping remote rules work
|
||||||
|
self.safe_client.create_security_group_rule(
|
||||||
|
tenant_uuid, sgs[0]['id'],
|
||||||
|
remote_group_id=sgs[1]['id'], direction='ingress',
|
||||||
|
ethertype=constants.IPv4,
|
||||||
|
protocol=constants.PROTO_NAME_TCP,
|
||||||
|
port_range_min=3333, port_range_max=3333)
|
||||||
|
self.safe_client.create_security_group_rule(
|
||||||
|
tenant_uuid, sgs[0]['id'],
|
||||||
|
remote_group_id=sgs[2]['id'], direction='ingress',
|
||||||
|
ethertype=constants.IPv4)
|
||||||
|
|
||||||
|
for i in range(2):
|
||||||
|
self.assert_connection(
|
||||||
|
vms[0].namespace, vms[1].namespace, vms[1].ip, 3333,
|
||||||
|
net_helpers.NetcatTester.TCP)
|
||||||
|
self.assert_connection(
|
||||||
|
vms[2].namespace, vms[1].namespace, vms[1].ip, 3333,
|
||||||
|
net_helpers.NetcatTester.TCP)
|
||||||
|
self.assert_connection(
|
||||||
|
vms[3].namespace, vms[0].namespace, vms[0].ip, 8080,
|
||||||
|
net_helpers.NetcatTester.TCP)
|
||||||
|
|
||||||
|
# 8. check SCTP is supported by security group
|
||||||
self.assert_no_connection(
|
self.assert_no_connection(
|
||||||
vms[1].namespace, vms[0].namespace, vms[0].ip, 3366,
|
vms[1].namespace, vms[0].namespace, vms[0].ip, 3366,
|
||||||
net_helpers.NetcatTester.SCTP)
|
net_helpers.NetcatTester.SCTP)
|
||||||
|
@ -271,7 +296,7 @@ class TestSecurityGroupsSameNetwork(BaseSecurityGroupsSameNetworkTest):
|
||||||
vms[1].namespace, vms[0].namespace, vms[0].ip, 3366,
|
vms[1].namespace, vms[0].namespace, vms[0].ip, 3366,
|
||||||
net_helpers.NetcatTester.SCTP)
|
net_helpers.NetcatTester.SCTP)
|
||||||
|
|
||||||
# 8. test two vms with same mac on the same host in different networks
|
# 9. test two vms with same mac on the same host in different networks
|
||||||
self._test_overlapping_mac_addresses()
|
self._test_overlapping_mac_addresses()
|
||||||
|
|
||||||
def _create_vm_on_host(
|
def _create_vm_on_host(
|
||||||
|
|
|
@ -404,3 +404,96 @@ class TestCreateConjFlows(base.BaseTestCase):
|
||||||
del f['ct_state']
|
del f['ct_state']
|
||||||
self.assertEqual(expected_template, f)
|
self.assertEqual(expected_template, f)
|
||||||
expected_template['conj_id'] += 1
|
expected_template['conj_id'] += 1
|
||||||
|
|
||||||
|
|
||||||
|
class TestMergeRules(base.BaseTestCase):
|
||||||
|
def setUp(self):
|
||||||
|
super(TestMergeRules, self).setUp()
|
||||||
|
self.rule_tmpl = [('direction', 'ingress'), ('ethertype', 'IPv4'),
|
||||||
|
('protocol', 6)]
|
||||||
|
|
||||||
|
def _test_merge_port_ranges_helper(self, expected, result):
|
||||||
|
"""Take a list of (port_range_min, port_range_max, conj_ids)
|
||||||
|
and an output from rules.merge_port_ranges and check if they
|
||||||
|
are identical, ignoring the other rule fields.
|
||||||
|
"""
|
||||||
|
self.assertEqual(len(expected), len(result))
|
||||||
|
for (range_min, range_max, conj_ids), result1 in zip(
|
||||||
|
expected, result):
|
||||||
|
self.assertEqual(range_min, result1[0]['port_range_min'])
|
||||||
|
self.assertEqual(range_max, result1[0]['port_range_max'])
|
||||||
|
self.assertEqual(conj_ids, set(result1[1]))
|
||||||
|
|
||||||
|
def test__assert_mergeable_rules(self):
|
||||||
|
self.assertRaises(RuntimeError,
|
||||||
|
rules._assert_mergeable_rules,
|
||||||
|
[({'direction': 'ingress', 'ethertype': 'IPv4',
|
||||||
|
'protocol': 1}, 8),
|
||||||
|
({'direction': 'ingress', 'ethertype': 'IPv6'},
|
||||||
|
16)])
|
||||||
|
|
||||||
|
def test_merge_common_rules_single(self):
|
||||||
|
rule_conj_tuple = ({'direction': 'egress', 'ethertype': 'IPv4',
|
||||||
|
'protocol': 1}, 8)
|
||||||
|
result = rules.merge_common_rules([rule_conj_tuple])
|
||||||
|
self.assertEqual([(rule_conj_tuple[0], [rule_conj_tuple[1]])],
|
||||||
|
result)
|
||||||
|
|
||||||
|
def test_merge_common_rules(self):
|
||||||
|
rule_conj_list = [({'direction': 'ingress', 'ethertype': 'IPv4',
|
||||||
|
'protocol': 1}, 8),
|
||||||
|
({'direction': 'ingress', 'ethertype': 'IPv4',
|
||||||
|
'protocol': 1, 'port_range_min': 3}, 16),
|
||||||
|
({'direction': 'ingress', 'ethertype': 'IPv4',
|
||||||
|
'protocol': 1, 'port_range_min': 3,
|
||||||
|
'port_range_max': 0}, 40),
|
||||||
|
({'direction': 'ingress', 'ethertype': 'IPv4',
|
||||||
|
'protocol': 1}, 24)]
|
||||||
|
result = rules.merge_common_rules(rule_conj_list)
|
||||||
|
self.assertItemsEqual(
|
||||||
|
[({'direction': 'ingress', 'ethertype': 'IPv4',
|
||||||
|
'protocol': 1}, [8, 24]),
|
||||||
|
({'direction': 'ingress', 'ethertype': 'IPv4',
|
||||||
|
'protocol': 1, 'port_range_min': 3}, [16]),
|
||||||
|
({'direction': 'ingress', 'ethertype': 'IPv4',
|
||||||
|
'protocol': 1, 'port_range_min': 3, 'port_range_max': 0},
|
||||||
|
[40])],
|
||||||
|
result)
|
||||||
|
|
||||||
|
def test_merge_port_ranges_overlapping(self):
|
||||||
|
result = rules.merge_port_ranges(
|
||||||
|
[(dict([('port_range_min', 20), ('port_range_max', 30)] +
|
||||||
|
self.rule_tmpl), 6),
|
||||||
|
(dict([('port_range_min', 30), ('port_range_max', 40)] +
|
||||||
|
self.rule_tmpl), 14),
|
||||||
|
(dict([('port_range_min', 35), ('port_range_max', 40)] +
|
||||||
|
self.rule_tmpl), 22),
|
||||||
|
(dict([('port_range_min', 20), ('port_range_max', 20)] +
|
||||||
|
self.rule_tmpl), 30)])
|
||||||
|
self._test_merge_port_ranges_helper([
|
||||||
|
# port_range_min, port_range_max, conj_ids
|
||||||
|
(20, 20, {6, 30}),
|
||||||
|
(21, 29, {6}),
|
||||||
|
(30, 30, {6, 14}),
|
||||||
|
(31, 34, {14}),
|
||||||
|
(35, 40, {14, 22})], result)
|
||||||
|
|
||||||
|
def test_merge_port_ranges_no_port_ranges(self):
|
||||||
|
result = rules.merge_port_ranges(
|
||||||
|
[(dict(self.rule_tmpl), 10),
|
||||||
|
(dict(self.rule_tmpl), 12),
|
||||||
|
(dict([('port_range_min', 30), ('port_range_max', 40)] +
|
||||||
|
self.rule_tmpl), 4)])
|
||||||
|
self._test_merge_port_ranges_helper([
|
||||||
|
(1, 29, {10, 12}),
|
||||||
|
(30, 40, {10, 12, 4}),
|
||||||
|
(41, 65535, {10, 12})], result)
|
||||||
|
|
||||||
|
def test_merge_port_ranges_nonoverlapping(self):
|
||||||
|
result = rules.merge_port_ranges(
|
||||||
|
[(dict([('port_range_min', 30), ('port_range_max', 40)] +
|
||||||
|
self.rule_tmpl), 32),
|
||||||
|
(dict([('port_range_min', 100), ('port_range_max', 140)] +
|
||||||
|
self.rule_tmpl), 40)])
|
||||||
|
self._test_merge_port_ranges_helper(
|
||||||
|
[(30, 40, {32}), (100, 140, {40})], result)
|
||||||
|
|
Loading…
Reference in New Issue