Merge "ovsfw: Use multiple priorities in RULES_*_TABLE"

This commit is contained in:
Zuul 2017-11-24 06:16:24 +00:00 committed by Gerrit Code Review
commit d1277c1630
5 changed files with 209 additions and 51 deletions

View File

@ -90,6 +90,20 @@ section. The third set matches on the conjunction IDs and does accept
actions.
Flow priorities for security group rules
----------------------------------------
The OpenFlow spec says a packet should not match against multiple
flows at the same priority [1]_. The firewall driver uses 8 levels of
priorities to achieve this. The method ``flow_priority_offset``
calculates a priority for a given security group rule. The use of
priorities is essential with conjunction flows, which will be
described later in the conjunction flows examples.
.. [1] Although OVS seems to magically handle overlapping flows under
some cases, we shouldn't rely on that.
Uses of conjunctive flows
-------------------------
@ -101,9 +115,9 @@ m are number of ports in the remote group ID and the port security group,
respectively.
A conj_id is allocated for each (remote_group_id, security_group_id,
direction, ethertype) tuple. The class ``ConjIdMap`` handles the
mapping. The same conj_id is shared between security group rules if
multiple rules belong to the same tuple above.
direction, ethertype, flow_priority_offset) tuple. The class
``ConjIdMap`` handles the mapping. The same conj_id is shared between
security group rules if multiple rules belong to the same tuple above.
Conjunctive flows consist of 2 dimensions. Flows that belong to the
dimension 1 of 2 are generated by the method
@ -141,7 +155,11 @@ the second security group. Ports have following attributes:
- plugged to the port 2 in OVS bridge
- ip address: 192.168.0.2
- mac address: fa:16:3e:24:57:c7
- security group 2: can receive icmp packets from security group 1
- security group 2:
- can receive icmp packets from security group 1
- can receive tcp packets from security group 1
- can receive tcp packets to port 80 from security group 2
- can receive ip packets from security group 3
- allowed address pair: 10.1.0.0/24, fa:16:3e:8c:84:14
``table 0`` contains a low priority rule to continue packets processing in
@ -240,8 +258,8 @@ In case below we allow all icmp egress traffic.
::
table=72, priority=70,ct_state=+est-rel-rpl,icmp,reg5=0x1, actions=resubmit(,73)
table=72, priority=70,ct_state=+new-est,icmp,reg5=0x1, actions=resubmit(,73)
table=72, priority=75,ct_state=+est-rel-rpl,icmp,reg5=0x1 actions=resubmit(,73)
table=72, priority=75,ct_state=+new-est,icmp,reg5=0x1 actions=resubmit(,73)
table=72, priority=50,ct_state=+inv+trk actions=drop
@ -327,22 +345,81 @@ port. Not tracked packets are sent to obtain conntrack information.
Similarly to ``table 72``, ``table 82`` accepts established and related
connections. In this case we allow all icmp traffic coming from
``security group 1`` which is in this case only ``port 1``.
The first two rules match on the ip packets, and the
next two rules match on the icmp protocol.
These four rules define conjunction flows.
The first four flows match on the ip addresses, and the
next two flows match on the icmp protocol.
These six flows define conjunction flows, and the next two define actions for
them.
::
table=82, priority=70,ct_state=+est-rel-rpl,ip,reg6=0x284,nw_src=192.168.0.1 actions=conjunction(2147352552,1/2)
table=82, priority=70,ct_state=+est-rel-rpl,ip,reg6=0x284,nw_src=10.0.0.1 actions=conjunction(2147352552,1/2)
table=82, priority=70,ct_state=+new-est,ip,reg6=0x284,nw_src=192.168.0.1 actions=conjunction(2147352553,1/2)
table=82, priority=70,ct_state=+new-est,ip,reg6=0x284,nw_src=10.0.0.1 actions=conjunction(2147352553,1/2)
table=82, priority=70,ct_state=+est-rel-rpl,icmp,reg5=0x2 actions=conjunction(2147352552,2/2)
table=82, priority=70,ct_state=+new-est,icmp,reg5=0x2 actions=conjunction(2147352553,2/2)
table=82, priority=70,conj_id=2147352552,ct_state=+est-rel-rpl,ip,reg5=0x2 actions=strip_vlan,output:2
table=82, priority=70,conj_id=2147352553,ct_state=+new-est,ip,reg5=0x2 actions=ct(commit,zone=NXM_NX_REG6[0..15]),strip_vlan,output:2
table=82, priority=71,ct_state=+est-rel-rpl,ip,reg6=0x284,nw_src=192.168.0.1 actions=conjunction(18,1/2)
table=82, priority=71,ct_state=+est-rel-rpl,ip,reg6=0x284,nw_src=10.0.0.1 actions=conjunction(18,1/2)
table=82, priority=71,ct_state=+new-est,ip,reg6=0x284,nw_src=192.168.0.1 actions=conjunction(19,1/2)
table=82, priority=71,ct_state=+new-est,ip,reg6=0x284,nw_src=10.0.0.1 actions=conjunction(19,1/2)
table=82, priority=71,ct_state=+est-rel-rpl,icmp,reg5=0x2 actions=conjunction(18,2/2)
table=82, priority=71,ct_state=+new-est,icmp,reg5=0x2 actions=conjunction(19,2/2)
table=82, priority=71,conj_id=18,ct_state=+est-rel-rpl,ip,reg5=0x2 actions=strip_vlan,output:2
table=82, priority=71,conj_id=19,ct_state=+new-est,ip,reg5=0x2 actions=ct(commit,zone=NXM_NX_REG6[0..15]),strip_vlan,output:2
table=82, priority=50,ct_state=+inv+trk actions=drop
There are some more security group rules with remote group IDs. Next
we look at TCP related ones. Excerpt of flows that correspond to those
rules are:
::
table=82, priority=73,ct_state=+est-rel-rpl,tcp,reg5=0x2,tp_dst=0x60/0xffe0 actions=conjunction(22,2/2)
table=82, priority=73,ct_state=+new-est,tcp,reg5=0x2,tp_dst=0x60/0xffe0 actions=conjunction(23,2/2)
table=82, priority=73,ct_state=+est-rel-rpl,tcp,reg5=0x2,tp_dst=0x40/0xfff0 actions=conjunction(22,2/2)
table=82, priority=73,ct_state=+new-est,tcp,reg5=0x2,tp_dst=0x40/0xfff0 actions=conjunction(23,2/2)
table=82, priority=73,ct_state=+est-rel-rpl,tcp,reg5=0x2,tp_dst=0x58/0xfff8 actions=conjunction(22,2/2)
table=82, priority=73,ct_state=+new-est,tcp,reg5=0x2,tp_dst=0x58/0xfff8 actions=conjunction(23,2/2)
table=82, priority=73,ct_state=+est-rel-rpl,tcp,reg5=0x2,tp_dst=0x54/0xfffc actions=conjunction(22,2/2)
table=82, priority=73,ct_state=+new-est,tcp,reg5=0x2,tp_dst=0x54/0xfffc actions=conjunction(23,2/2)
table=82, priority=73,ct_state=+est-rel-rpl,tcp,reg5=0x2,tp_dst=0x52/0xfffe actions=conjunction(22,2/2)
table=82, priority=73,ct_state=+new-est,tcp,reg5=0x2,tp_dst=0x52/0xfffe actions=conjunction(23,2/2)
table=82, priority=73,ct_state=+est-rel-rpl,tcp,reg5=0x2,tp_dst=80 actions=conjunction(22,2/2),conjunction(14,2/2)
table=82, priority=73,ct_state=+est-rel-rpl,tcp,reg5=0x2,tp_dst=81 actions=conjunction(22,2/2)
table=82, priority=73,ct_state=+new-est,tcp,reg5=0x2,tp_dst=80 actions=conjunction(23,2/2),conjunction(15,2/2)
table=82, priority=73,ct_state=+new-est,tcp,reg5=0x2,tp_dst=81 actions=conjunction(23,2/2)
Only dimension 2/2 flows are shown here, as the other are similar to
the previous ICMP example. There are many more flows but only the port
ranges that covers from 64 to 127 are shown for brevity.
The conjunction IDs 14 and 15 correspond to packets from the security
group 1, and the conjunction IDs 22 and 23 correspond to those from
the security group 2. These flows are from the following security group rules,
::
- can receive tcp packets from security group 1
- can receive tcp packets to port 80 from security group 2
and these rules have been processed by ``merge_port_ranges`` into:
::
- can receive tcp packets to port != 80 from security group 1
- can receive tcp packets to port 80 from security group 1 or 2
before translating to flows so that there is only one matching flow
even when the TCP destination port is 80.
The remaining is a L4 protocol agnostic rule.
::
table=82, priority=70,ct_state=+est-rel-rpl,ip,reg5=0x2 actions=conjunction(24,2/2)
table=82, priority=70,ct_state=+new-est,ip,reg5=0x2 actions=conjunction(25,2/2)
Any IP packet that matches the previous TCP flows matches one of these
flows, but the corresponding security group rules have different
remote group IDs. Unlike the above TCP example, there's no convenient
way of expressing ``protocol != TCP`` or ``icmp_code != 1``. So the
OVS firewall uses a different priority than the previous TCP flows so
as not to mix up them.
The mechanism for dropping connections that are not allowed anymore is the
same as in ``table 72``.

View File

@ -218,14 +218,15 @@ class ConjIdMap(object):
# If there is any freed ID, use one.
if self.id_free:
return self.id_free.popleft()
# Allocate new one. It must be an even number.
self.max_id += 2
# Allocate new one. It must be divisible by 8. (See the next function.)
self.max_id += 8
return self.max_id
def get_conj_id(self, sg_id, remote_sg_id, direction, ethertype):
"""Return a conjunction ID specified by the arguments.
Allocate one if necessary. The returned ID is always an even
number, allowing the caller to use 2 IDs for each combination.
Allocate one if necessary. The returned ID is divisible by 8,
as there are 4 priority levels (see rules.flow_priority_offset)
and 2 conjunction IDs are needed per priority.
"""
if direction not in [firewall.EGRESS_DIRECTION,
firewall.INGRESS_DIRECTION]:
@ -317,7 +318,8 @@ class ConjIPFlowManager(object):
addr_to_conj)
self.flow_state[vlan_tag][(direction, ethertype)] = addr_to_conj
def add(self, vlan_tag, sg_id, remote_sg_id, direction, ethertype):
def add(self, vlan_tag, sg_id, remote_sg_id, direction, ethertype,
priority_offset):
"""Get conj_id specified by the arguments
and notify the manager that
(remote_sg_id, direction, ethertype, conj_id) flows need to be
@ -327,7 +329,7 @@ class ConjIPFlowManager(object):
"""
conj_id = self.conj_id_map.get_conj_id(
sg_id, remote_sg_id, direction, ethertype)
sg_id, remote_sg_id, direction, ethertype) + priority_offset * 2
if (direction, ethertype) not in self.conj_ids[vlan_tag]:
self.conj_ids[vlan_tag][(direction, ethertype)] = (
@ -990,10 +992,12 @@ class OVSFirewallDriver(firewall.FirewallDriver):
direction = rule['direction']
ethertype = rule['ethertype']
protocol = rule.get('protocol')
priority_offset = rules.flow_priority_offset(rule)
conj_id = self.conj_ip_manager.add(port.vlan_tag, sec_group_id,
rule['remote_group_id'],
direction, ethertype)
direction, ethertype,
priority_offset)
rule1 = rule.copy()
del rule1['remote_group_id']
@ -1014,7 +1018,8 @@ class OVSFirewallDriver(firewall.FirewallDriver):
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, conjunction=True)
for flow in rules.substitute_conjunction_actions(
flows, 2, conj_ids):
self._add_flow(**flow)
@ -1031,6 +1036,9 @@ class OVSFirewallDriver(firewall.FirewallDriver):
LOG.debug('Creating flow rules for port %s that is port %d in OVS',
port.id, port.ofport)
for rule in self._create_rules_generator_for_port(port):
# NOTE(toshii): A better version of merge_common_rules and
# its friend should be applied here in order to avoid
# overlapping flows.
flows = rules.create_flows_from_rule_and_port(rule, port)
LOG.debug("RULGEN: Rules generated for flow %s are %s",
rule, flows)

View File

@ -152,14 +152,36 @@ def merge_port_ranges(rule_conj_list):
return result
def create_flows_from_rule_and_port(rule, port):
def flow_priority_offset(rule, conjunction=False):
"""Calculate flow priority offset from rule.
Whether the rule belongs to conjunction flows or not is decided
upon existence of rule['remote_group_id'] but can be overridden
to be True using the optional conjunction arg.
"""
conj_offset = 0 if 'remote_group_id' in rule or conjunction else 4
protocol = rule.get('protocol')
if protocol is None:
return conj_offset
if protocol in [n_consts.PROTO_NUM_ICMP, n_consts.PROTO_NUM_IPV6_ICMP]:
if 'port_range_min' not in rule:
return conj_offset + 1
elif 'port_range_max' not in rule:
return conj_offset + 2
return conj_offset + 3
def create_flows_from_rule_and_port(rule, port, conjunction=False):
"""Create flows from given args.
For description of the optional conjunction arg, see flow_priority_offset.
"""
ethertype = rule['ethertype']
direction = rule['direction']
dst_ip_prefix = rule.get('dest_ip_prefix')
src_ip_prefix = rule.get('source_ip_prefix')
flow_template = {
'priority': 70,
'priority': 70 + flow_priority_offset(rule, conjunction),
'dl_type': ovsfw_consts.ethertype_to_dl_type_map[ethertype],
'reg_port': port.ofport,
}
@ -260,16 +282,28 @@ def create_icmp_flows(flow_template, rule):
return [flow]
def _flow_priority_offset_from_conj_id(conj_id):
"Return a flow priority offset encoded in a conj_id."
# A base conj_id, which is returned by ConjIdMap.get_conj_id, is a
# multiple of 8, and we use 2 conj_ids per offset.
return conj_id % 8 // 2
def create_flows_for_ip_address(ip_address, direction, ethertype,
vlan_tag, conj_ids):
"""Create flows from a rule and an ip_address derived from
remote_group_id
"""
# Group conj_ids per priority.
conj_id_lists = [[] for i in range(4)]
for conj_id in conj_ids:
conj_id_lists[
_flow_priority_offset_from_conj_id(conj_id)].append(conj_id)
ip_prefix = str(netaddr.IPNetwork(ip_address).cidr)
flow_template = {
'priority': 70,
'dl_type': ovsfw_consts.ethertype_to_dl_type_map[ethertype],
'reg_net': vlan_tag, # needed for project separation
}
@ -284,7 +318,14 @@ def create_flows_for_ip_address(ip_address, direction, ethertype,
flow_template[FLOW_FIELD_FOR_IPVER_AND_DIRECTION[(
ip_ver, direction)]] = ip_prefix
return substitute_conjunction_actions([flow_template], 1, conj_ids)
result = []
for offset, conj_id_list in enumerate(conj_id_lists):
if not conj_id_list:
continue
flow_template['priority'] = 70 + offset
result.extend(
substitute_conjunction_actions([flow_template], 1, conj_id_list))
return result
def create_accept_flows(flow):
@ -316,7 +357,7 @@ def substitute_conjunction_actions(flows, dimension, conj_ids):
def create_conj_flows(port, conj_id, direction, ethertype):
"""Generate "accept" flows for a given conjunction ID."""
flow_template = {
'priority': 70,
'priority': 70 + _flow_priority_offset_from_conj_id(conj_id),
'conj_id': conj_id,
'dl_type': ovsfw_consts.ethertype_to_dl_type_map[ethertype],
# This reg_port matching is for delete_all_port_flows.

View File

@ -293,7 +293,7 @@ class TestConjIPFlowManager(base.BaseTestCase):
self.driver = mock.Mock()
self.manager = ovsfw.ConjIPFlowManager(self.driver)
self.vlan_tag = 100
self.conj_id = 10
self.conj_id = 16
def test_update_flows_for_vlan(self):
remote_group = self.driver.sg_port_map.get_sg.return_value
@ -303,14 +303,22 @@ class TestConjIPFlowManager(base.BaseTestCase):
'get_conj_id') as get_conj_id_mock:
get_conj_id_mock.return_value = self.conj_id
self.manager.add(self.vlan_tag, 'sg', 'remote_id',
firewall.INGRESS_DIRECTION, constants.IPv4)
firewall.INGRESS_DIRECTION, constants.IPv4, 0)
self.manager.add(self.vlan_tag, 'sg', 'remote_id',
firewall.INGRESS_DIRECTION, constants.IPv4, 3)
self.manager.update_flows_for_vlan(self.vlan_tag)
self.assertEqual(self.driver._add_flow.call_args_list,
[mock.call(actions='conjunction(10,1/2)', ct_state='+est-rel-rpl',
[mock.call(actions='conjunction(16,1/2)', ct_state='+est-rel-rpl',
dl_type=2048, nw_src='10.22.3.4/32', priority=70,
reg_net=self.vlan_tag, table=82),
mock.call(actions='conjunction(11,1/2)', ct_state='+new-est',
mock.call(actions='conjunction(17,1/2)', ct_state='+new-est',
dl_type=2048, nw_src='10.22.3.4/32', priority=70,
reg_net=self.vlan_tag, table=82),
mock.call(actions='conjunction(22,1/2)', ct_state='+est-rel-rpl',
dl_type=2048, nw_src='10.22.3.4/32', priority=73,
reg_net=self.vlan_tag, table=82),
mock.call(actions='conjunction(23,1/2)', ct_state='+new-est',
dl_type=2048, nw_src='10.22.3.4/32', priority=73,
reg_net=self.vlan_tag, table=82)])
def test_sg_removed(self):
@ -321,7 +329,7 @@ class TestConjIPFlowManager(base.BaseTestCase):
get_id_mock.return_value = self.conj_id
delete_sg_mock.return_value = [('remote_id', self.conj_id)]
self.manager.add(self.vlan_tag, 'sg', 'remote_id',
firewall.INGRESS_DIRECTION, constants.IPv4)
firewall.INGRESS_DIRECTION, constants.IPv4, 0)
self.manager.flow_state[self.vlan_tag][(
firewall.INGRESS_DIRECTION, constants.IPv4)] = {
'10.22.3.4': [self.conj_id]}
@ -508,7 +516,7 @@ class TestOVSFirewallDriver(base.BaseTestCase):
'output:{:d}'.format(self.port_ofport),
dl_type="0x{:04x}".format(n_const.ETHERTYPE_IP),
nw_proto=constants.PROTO_NUM_TCP,
priority=70,
priority=77,
reg5=self.port_ofport,
ct_state=ovsfw_consts.OF_STATE_NEW_NOT_ESTABLISHED,
table=ovs_consts.RULES_INGRESS_TABLE,
@ -553,16 +561,16 @@ class TestOVSFirewallDriver(base.BaseTestCase):
ovs_consts.ACCEPT_OR_INGRESS_TABLE),
dl_type="0x{:04x}".format(n_const.ETHERTYPE_IP),
nw_proto=constants.PROTO_NUM_UDP,
priority=70,
priority=77,
ct_state=ovsfw_consts.OF_STATE_NEW_NOT_ESTABLISHED,
reg5=self.port_ofport,
table=ovs_consts.RULES_EGRESS_TABLE),
mock.call(
actions='conjunction({:d},2/2)'.format(conj_id),
actions='conjunction({:d},2/2)'.format(conj_id + 6),
ct_state=ovsfw_consts.OF_STATE_ESTABLISHED_NOT_REPLY,
dl_type=mock.ANY,
nw_proto=6,
priority=70, reg5=self.port_ofport,
priority=73, reg5=self.port_ofport,
table=ovs_consts.RULES_EGRESS_TABLE)]
self.mock_bridge.br.add_flow.assert_has_calls(
filter_rules, any_order=True)

View File

@ -77,7 +77,7 @@ class TestCreateFlowsFromRuleAndPort(base.BaseTestCase):
'direction': firewall.INGRESS_DIRECTION,
}
expected_template = {
'priority': 70,
'priority': 74,
'dl_type': n_const.ETHERTYPE_IP,
'reg_port': self.port.ofport,
}
@ -92,7 +92,7 @@ class TestCreateFlowsFromRuleAndPort(base.BaseTestCase):
'dest_ip_prefix': '10.0.0.1/32',
}
expected_template = {
'priority': 70,
'priority': 74,
'dl_type': n_const.ETHERTYPE_IP,
'reg_port': self.port.ofport,
'nw_src': '192.168.0.0/24',
@ -109,7 +109,7 @@ class TestCreateFlowsFromRuleAndPort(base.BaseTestCase):
'dest_ip_prefix': '0.0.0.0/0',
}
expected_template = {
'priority': 70,
'priority': 74,
'dl_type': n_const.ETHERTYPE_IP,
'reg_port': self.port.ofport,
'nw_src': '192.168.0.0/24',
@ -123,7 +123,7 @@ class TestCreateFlowsFromRuleAndPort(base.BaseTestCase):
'direction': firewall.INGRESS_DIRECTION,
}
expected_template = {
'priority': 70,
'priority': 74,
'dl_type': n_const.ETHERTYPE_IPV6,
'reg_port': self.port.ofport,
}
@ -138,7 +138,7 @@ class TestCreateFlowsFromRuleAndPort(base.BaseTestCase):
'dest_ip_prefix': '2001:db8:aaaa::1/64',
}
expected_template = {
'priority': 70,
'priority': 74,
'dl_type': n_const.ETHERTYPE_IPV6,
'reg_port': self.port.ofport,
'ipv6_src': '2001:db8:bbbb::1/64',
@ -155,7 +155,7 @@ class TestCreateFlowsFromRuleAndPort(base.BaseTestCase):
'dest_ip_prefix': '::/0',
}
expected_template = {
'priority': 70,
'priority': 74,
'dl_type': n_const.ETHERTYPE_IPV6,
'reg_port': self.port.ofport,
'ipv6_src': '2001:db8:bbbb::1/64',
@ -343,7 +343,7 @@ class TestCreateFlowsForIpAddress(base.BaseTestCase):
def test_create_flows_for_ip_address_egress(self):
expected_template = {
'table': ovs_consts.RULES_EGRESS_TABLE,
'priority': 70,
'priority': 72,
'dl_type': n_const.ETHERTYPE_IP,
'reg_net': 0x123,
'nw_dst': '192.168.0.1/32'
@ -359,10 +359,9 @@ class TestCreateFlowsForIpAddress(base.BaseTestCase):
flows[0]['ct_state'])
self.assertEqual(ovsfw_consts.OF_STATE_NEW_NOT_ESTABLISHED,
flows[1]['ct_state'])
self.assertEqual(self._generate_conjuncion_actions(conj_ids, 0),
flows[0]['actions'])
self.assertEqual(self._generate_conjuncion_actions(conj_ids, 1),
flows[1]['actions'])
for i in range(2):
self.assertEqual(self._generate_conjuncion_actions(conj_ids, i),
flows[i]['actions'])
for f in flows:
del f['actions']
del f['ct_state']
@ -380,7 +379,7 @@ class TestCreateConjFlows(base.BaseTestCase):
expected_template = {
'table': ovs_consts.RULES_INGRESS_TABLE,
'dl_type': n_const.ETHERTYPE_IPV6,
'priority': 70,
'priority': 71,
'conj_id': conj_id,
'reg_port': port.ofport
}
@ -497,3 +496,28 @@ class TestMergeRules(base.BaseTestCase):
self.rule_tmpl), 40)])
self._test_merge_port_ranges_helper(
[(30, 40, {32}), (100, 140, {40})], result)
class TestFlowPriority(base.BaseTestCase):
def test_flow_priority_offset(self):
self.assertEqual(0,
rules.flow_priority_offset(
{'foo': 'bar',
'remote_group_id': 'hoge'}))
self.assertEqual(4,
rules.flow_priority_offset({'foo': 'bar'}))
self.assertEqual(5,
rules.flow_priority_offset(
{'protocol': constants.PROTO_NUM_ICMP}))
self.assertEqual(7,
rules.flow_priority_offset(
{'protocol': constants.PROTO_NUM_TCP}))
self.assertEqual(6,
rules.flow_priority_offset(
{'protocol': constants.PROTO_NUM_ICMP,
'port_range_min': 0}))
self.assertEqual(7,
rules.flow_priority_offset(
{'protocol': constants.PROTO_NUM_IPV6_ICMP,
'port_range_min': 0, 'port_range_max': 0}))