openstack/neutron
Code Issues Proposed changes

Merge "ovsfw: Merge multiple conjunction flows"

Browse Source
This commit is contained in:
Zuul 2017-11-21 06:56:58 +00:00 committed by Gerrit Code Review
commit fa9e5ee4d2
5 changed files with 272 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
doc/source/contributor/internals/openvswitch_firewall.rst
View File

@ -114,6 +114,12 @@ the dimension 2 of 2 are generated by the method
``substitute_conjunction_actions``, which represents the portion of
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:
-------------------------------

36
neutron/agent/linux/openvswitch_firewall/firewall.py
View File

@ -984,24 +984,46 @@ class OVSFirewallDriver(firewall.FirewallDriver):
The remaining part is done by ConjIPFlowManager.
"""
port_rules = collections.defaultdict(list)
for sec_group_id, rule in (
self._create_remote_rules_generator_for_port(port)):
direction = rule['direction']
ethertype = rule['ethertype']
protocol = rule.get('protocol')
conj_id = self.conj_ip_manager.add(port.vlan_tag, sec_group_id,
rule['remote_group_id'],
direction, ethertype)
flows = rules.create_flows_from_rule_and_port(rule, port)
for flow in rules.substitute_conjunction_actions(
flows, 2, [conj_id]):
self._add_flow(**flow)
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)
for flow in rules.substitute_conjunction_actions(
flows, 2, conj_ids):
self._add_flow(**flow)
# Install actions=accept flows.
for flow in rules.create_conj_flows(
port, conj_id, direction, ethertype):
self._add_flow(**flow)
for conj_id in all_conj_ids:
for flow in rules.create_conj_flows(
port, conj_id, direction, ethertype):
self._add_flow(**flow)
def add_flows_from_rules(self, port):
self._initialize_tracked_ingress(port)

106
neutron/agent/linux/openvswitch_firewall/rules.py
View File

@ -13,6 +13,8 @@
# License for the specific language governing permissions and limitations
# under the License.
import collections
import netaddr
from neutron_lib import constants as n_consts
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)
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):
ethertype = rule['ethertype']
direction = rule['direction']

51
neutron/tests/fullstack/test_securitygroup.py
View File

@ -106,16 +106,17 @@ class TestSecurityGroupsSameNetwork(BaseSecurityGroupsSameNetworkTest):
4. a security group update takes effect,
5. a remote security group member addition works, and
6. an established connection stops by deleting a SG rule.
7. test other protocol functionality by using SCTP protocol
8. test two vms with same mac on the same host in different
7. multiple overlapping remote rules work,
8. test other protocol functionality by using SCTP protocol
9. test two vms with same mac on the same host in different
networks
"""
index_to_sg = [0, 0, 1]
index_to_sg = [0, 0, 1, 2]
if self.firewall_driver == 'iptables_hybrid':
# The iptables_hybrid driver lacks isolation between agents
index_to_host = [0] * 3
index_to_host = [0] * 4
else:
index_to_host = [0, 1, 1]
index_to_host = [0, 1, 1, 0]
tenant_uuid = uuidutils.generate_uuid()
@ -124,7 +125,7 @@ class TestSecurityGroupsSameNetwork(BaseSecurityGroupsSameNetworkTest):
tenant_uuid, network['id'], '20.0.0.0/24')
sgs = [self.safe_client.create_security_group(tenant_uuid)
for i in range(2)]
for i in range(3)]
ports = [
self.safe_client.create_port(tenant_uuid, network['id'],
self.environment.hosts[host].hostname,
@ -228,22 +229,23 @@ class TestSecurityGroupsSameNetwork(BaseSecurityGroupsSameNetworkTest):
ports.append(
self.safe_client.create_port(tenant_uuid, network['id'],
self.environment.hosts[
index_to_host[3]].hostname,
index_to_host[-1]].hostname,
security_groups=[sgs[1]['id']]))
vms.append(
self.useFixture(
machine.FakeFullstackMachine(
self.environment.hosts[index_to_host[3]],
self.environment.hosts[index_to_host[-1]],
network['id'],
tenant_uuid,
self.safe_client,
neutron_port=ports[3],
neutron_port=ports[-1],
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,
net_helpers.NetcatTester.TCP)
@ -255,7 +257,30 @@ class TestSecurityGroupsSameNetwork(BaseSecurityGroupsSameNetworkTest):
sleep=8)
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(
vms[1].namespace, vms[0].namespace, vms[0].ip, 3366,
net_helpers.NetcatTester.SCTP)
@ -271,7 +296,7 @@ class TestSecurityGroupsSameNetwork(BaseSecurityGroupsSameNetworkTest):
vms[1].namespace, vms[0].namespace, vms[0].ip, 3366,
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()
def _create_vm_on_host(

93
neutron/tests/unit/agent/linux/openvswitch_firewall/test_rules.py
View File

@ -404,3 +404,96 @@ class TestCreateConjFlows(base.BaseTestCase):
del f['ct_state']
self.assertEqual(expected_template, f)
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)