openstack
/
neutron
Code Issues Proposed changes

ovsfw: Raise exception if tag cannot be found in other_config 

Previously, if tag was not present in other_config obtained from ovsdb
for any reason, DEAD VLAN tag was used. This is not smart at all as it
puts all conntrack entries to one point. Also tag is mandatory and if
other_config doesn't contain it, it's a huge mistake that should never
happen.

Change-Id: I91ab75b52b70dbba4c7823550bfdfe0ab9396336
Related-bug: 1564947
This commit is contained in:
Jakub Libosvar 2016-10-17 11:32:17 -04:00
parent b2399d847a
commit a66c271935
7 changed files with 123 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
neutron/agent/linux/openvswitch_firewall/exceptions.py Normal file
View File

@ -0,0 +1,28 @@
# Copyright 2016, Red Hat, Inc.
# All Rights Reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
# a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
from neutron_lib import exceptions
from neutron._i18n import _
class OVSFWPortNotFound(exceptions.NeutronException):
message = _("Port %(port_id)s is not managed by this agent.")
class OVSFWTagNotFound(exceptions.NeutronException):
message = _(
"Cannot get tag for port %(port_name)s from its other_config: "
"%(other_config)s")

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

@ -15,13 +15,13 @@
import netaddr
from neutron_lib import constants as lib_const
from neutron_lib import exceptions
from oslo_log import log as logging
from oslo_utils import netutils
from neutron._i18n import _, _LE, _LW
from neutron._i18n import _LE
from neutron.agent import firewall
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 rules
from neutron.common import constants
from neutron.plugins.ml2.drivers.openvswitch.agent.common import constants \
@ -55,8 +55,21 @@ def create_reg_numbers(flow_params):
_replace_register(flow_params, ovsfw_consts.REG_NET, 'reg_net')
class OVSFWPortNotFound(exceptions.NeutronException):
message = _("Port %(port_id)s is not managed by this agent. ")
def get_tag_from_other_config(bridge, port_name):
"""Return tag stored in OVSDB other_config metadata.
:param bridge: OVSBridge instance where port is.
:param port_name: Name of the port.
:raises OVSFWTagNotFound: In case tag cannot be found in OVSDB.
"""
other_config = None
try:
other_config = bridge.db_get_val(
'Port', port_name, 'other_config')
return int(other_config['tag'])
except (KeyError, TypeError, ValueError):
raise exceptions.OVSFWTagNotFound(
port_name=port_name, other_config=other_config)
class SecurityGroup(object):
@ -237,18 +250,9 @@ class OVSFirewallDriver(firewall.FirewallDriver):
except KeyError:
ovs_port = self.int_br.br.get_vif_port_by_id(port_id)
if not ovs_port:
raise OVSFWPortNotFound(port_id=port_id)
try:
other_config = self.int_br.br.db_get_val(
'Port', ovs_port.port_name, 'other_config')
port_vlan_id = int(other_config['tag'])
except (KeyError, TypeError):
LOG.warning(_LW("Cannot get tag for port %(port_id)s from "
"its other_config: %(other_config)s"),
{'port_id': port_id,
'other_config': other_config})
port_vlan_id = ovs_consts.DEAD_VLAN_TAG
raise exceptions.OVSFWPortNotFound(port_id=port_id)
port_vlan_id = get_tag_from_other_config(
self.int_br.br, ovs_port.port_name)
of_port = OFPort(port, ovs_port, port_vlan_id)
self.sg_port_map.create_port(of_port, port)
else:

19
neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py
View File

@ -800,17 +800,26 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
port_info = self.int_br.get_ports_attributes(
"Port", columns=["name", "tag", "other_config"],
ports=port_names, if_exists=True)
info_by_port = {x['name']: [x['tag'], x['other_config']]
for x in port_info}
info_by_port = {
x['name']: {
'tag': x['tag'],
'other_config': x['other_config'] or {}
}
for x in port_info
}
for port_detail in need_binding_ports:
try:
lvm = self.vlan_manager.get(port_detail['network_id'])
except vlanmanager.MappingNotFound:
continue
port = port_detail['vif_port']
cur_info = info_by_port.get(port.port_name)
if cur_info is not None and cur_info[0] != lvm.vlan:
other_config = cur_info[1] or {}
try:
cur_info = info_by_port[port.port_name]
except KeyError:
continue
other_config = cur_info['other_config']
if (cur_info['tag'] != lvm.vlan or
other_config.get('tag') != lvm.vlan):
other_config['tag'] = str(lvm.vlan)
self.int_br.set_db_attribute(
"Port", port.port_name, "other_config", other_config)

12
neutron/tests/common/conn_testers.py
View File

@ -372,12 +372,12 @@ class OVSBaseConnectionTester(ConnectionTester):
@staticmethod
def set_tag(port_name, bridge, tag):
bridge.set_db_attribute('Port', port_name, 'tag', tag)
other_config = bridge.db_get_val(
'Port', port_name, 'other_config')
other_config['tag'] = str(tag)
bridge.set_db_attribute(
'Port', port_name, 'other_config', other_config)
ovsdb = bridge.ovsdb
with ovsdb.transaction() as txn:
txn.add(ovsdb.db_set('Port', port_name, ('tag', tag)))
txn.add(
ovsdb.db_add(
'Port', port_name, 'other_config', {'tag': str(tag)}))
class OVSConnectionTester(OVSBaseConnectionTester):

0
neutron/tests/functional/agent/linux/openvswitch_firewall/__init__.py Normal file
View File

53
neutron/tests/functional/agent/linux/openvswitch_firewall/test_firewall.py Normal file
View File

@ -0,0 +1,53 @@
# Copyright 2016, Red Hat, Inc.
# All Rights Reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
# a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
import testtools
from neutron.agent.linux.openvswitch_firewall import exceptions
from neutron.agent.linux.openvswitch_firewall import firewall
from neutron.tests.common import net_helpers
from neutron.tests.functional import base
class TestGetTagFromOtherConfig(base.BaseSudoTestCase):
def setUp(self):
super(TestGetTagFromOtherConfig, self).setUp()
self.bridge = self.useFixture(net_helpers.OVSBridgeFixture()).bridge
def set_port_tag(self, port_name, tag):
self.bridge.set_db_attribute(
'Port', port_name, 'other_config', {'tag': str(tag)})
def test_correct_tag_is_returned(self):
port_number = 42
port = self.useFixture(net_helpers.OVSPortFixture(self.bridge)).port
self.set_port_tag(port.name, port_number)
observed = firewall.get_tag_from_other_config(self.bridge, port.name)
self.assertEqual(port_number, observed)
def test_not_existing_name_raises_exception(self):
with testtools.ExpectedException(exceptions.OVSFWTagNotFound):
firewall.get_tag_from_other_config(self.bridge, 'foo')
def test_bad_tag_value_raises_exception(self):
port = self.useFixture(net_helpers.OVSPortFixture(self.bridge)).port
self.set_port_tag(port.name, 'foo')
with testtools.ExpectedException(exceptions.OVSFWTagNotFound):
firewall.get_tag_from_other_config(self.bridge, port.name)
def test_no_value_set_for_other_config_raises_exception(self):
port = self.useFixture(net_helpers.OVSPortFixture(self.bridge)).port
with testtools.ExpectedException(exceptions.OVSFWTagNotFound):
firewall.get_tag_from_other_config(self.bridge, port.name)

11
neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py
View File

@ -19,6 +19,7 @@ import testtools
from neutron.agent.common import ovs_lib
from neutron.agent import firewall
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.plugins.ml2.drivers.openvswitch.agent.common import constants \
@ -331,17 +332,9 @@ class TestOVSFirewallDriver(base.BaseTestCase):
'device': 'port-id',
'security_groups': [123, 456]}
self.mock_bridge.br.get_vif_port_by_id.return_value = None
with testtools.ExpectedException(ovsfw.OVSFWPortNotFound):
with testtools.ExpectedException(exceptions.OVSFWPortNotFound):
self.firewall.get_or_create_ofport(port_dict)
def test_get_or_create_ofport_not_tagged(self):
port_dict = {
'device': 'port-id',
'security_groups': [123, 456]}
self.mock_bridge.br.db_get_val.return_value = None
port = self.firewall.get_or_create_ofport(port_dict)
self.assertEqual(ovs_consts.DEAD_VLAN_TAG, port.vlan_tag)
def test_is_port_managed_managed_port(self):
port_dict = {'device': 'port-id'}
self.firewall.sg_port_map.ports[port_dict['device']] = object()