openstack
/
neutron
Code Issues Proposed changes

Use db constraint to ensure mac address uniqueness 

Currently port mac address uniqueness per network is checked before Port
db object create but without locking. It implies 2 port create requests
can allocate the same mac address on a network if each request performs
mac address uniqueness check before the other creates the Port db object.

This change replaces the check by a db unique constraint on Port
(network_id, mac_address).

Change-Id: Iad7460356ceb74d963cddf5ec33268d77792f1fe
Closes-Bug: #1194565
This commit is contained in:
Cedric Brandily 2015-01-10 14:25:04 +00:00
parent 58c624fad3
commit 09fe057cd4
8 changed files with 127 additions and 85 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

114
neutron/db/db_base_plugin_v2.py
View File

@ -13,10 +13,9 @@
# License for the specific language governing permissions and limitations
# under the License.
import random
import netaddr
from oslo.config import cfg
from oslo.db import exception as db_exc
from oslo.utils import excutils
from sqlalchemy import and_
from sqlalchemy import event
@ -27,6 +26,7 @@ from neutron.api.v2 import attributes
from neutron.common import constants
from neutron.common import exceptions as n_exc
from neutron.common import ipv6_utils
from neutron.common import utils
from neutron import context as ctx
from neutron.db import common_db_mixin
from neutron.db import models_v2
@ -127,41 +127,8 @@ class NeutronDbPluginV2(neutron_plugin_base_v2.NeutronPluginBaseV2,
return context.session.query(models_v2.Subnet).all()
@staticmethod
def _generate_mac(context, network_id):
base_mac = cfg.CONF.base_mac.split(':')
max_retries = cfg.CONF.mac_generation_retries
for i in range(max_retries):
mac = [int(base_mac[0], 16), int(base_mac[1], 16),
int(base_mac[2], 16), random.randint(0x00, 0xff),
random.randint(0x00, 0xff), random.randint(0x00, 0xff)]
if base_mac[3] != '00':
mac[3] = int(base_mac[3], 16)
mac_address = ':'.join(map(lambda x: "%02x" % x, mac))
if NeutronDbPluginV2._check_unique_mac(context, network_id,
mac_address):
LOG.debug("Generated mac for network %(network_id)s "
"is %(mac_address)s",
{'network_id': network_id,
'mac_address': mac_address})
return mac_address
else:
LOG.debug("Generated mac %(mac_address)s exists. Remaining "
"attempts %(max_retries)s.",
{'mac_address': mac_address,
'max_retries': max_retries - (i + 1)})
LOG.error(_LE("Unable to generate mac address after %s attempts"),
max_retries)
raise n_exc.MacAddressGenerationFailure(net_id=network_id)
@staticmethod
def _check_unique_mac(context, network_id, mac_address):
mac_qry = context.session.query(models_v2.Port)
try:
mac_qry.filter_by(network_id=network_id,
mac_address=mac_address).one()
except exc.NoResultFound:
return True
return False
def _generate_mac():
return utils.get_random_mac(cfg.CONF.base_mac.split(':'))
@staticmethod
def _delete_ip_allocation(context, network_id, subnet_id, ip_address):
@ -1310,6 +1277,36 @@ class NeutronDbPluginV2(neutron_plugin_base_v2.NeutronPluginBaseV2,
def create_port_bulk(self, context, ports):
return self._create_bulk('port', context, ports)
def _create_port_with_mac(self, context, network_id, port_data,
mac_address, nested=False):
try:
with context.session.begin(subtransactions=True, nested=nested):
db_port = models_v2.Port(mac_address=mac_address, **port_data)
context.session.add(db_port)
return db_port
except db_exc.DBDuplicateEntry:
raise n_exc.MacAddressInUse(net_id=network_id, mac=mac_address)
def _create_port(self, context, network_id, port_data):
max_retries = cfg.CONF.mac_generation_retries
for i in range(max_retries):
mac = self._generate_mac()
try:
# nested = True frames an operation that may potentially fail
# within a transaction, so that it can be rolled back to the
# point before its failure while maintaining the enclosing
# transaction
return self._create_port_with_mac(
context, network_id, port_data, mac, nested=True)
except n_exc.MacAddressInUse:
LOG.debug('Generated mac %(mac_address)s exists on '
'network %(network_id)s',
{'mac_address': mac, 'network_id': network_id})
LOG.error(_LE("Unable to generate mac address after %s attempts"),
max_retries)
raise n_exc.MacAddressGenerationFailure(net_id=network_id)
def create_port(self, context, port):
p = port['port']
port_id = p.get('id') or uuidutils.generate_uuid()
@ -1321,41 +1318,26 @@ class NeutronDbPluginV2(neutron_plugin_base_v2.NeutronPluginBaseV2,
self._enforce_device_owner_not_router_intf_or_device_id(context, p,
tenant_id)
port_data = dict(tenant_id=tenant_id,
name=p['name'],
id=port_id,
network_id=network_id,
admin_state_up=p['admin_state_up'],
status=p.get('status', constants.PORT_STATUS_ACTIVE),
device_id=p['device_id'],
device_owner=p['device_owner'])
with context.session.begin(subtransactions=True):
# Ensure that the network exists.
self._get_network(context, network_id)
# Ensure that a MAC address is defined and it is unique on the
# network
# Create the port
if p['mac_address'] is attributes.ATTR_NOT_SPECIFIED:
#Note(scollins) Add the generated mac_address to the port,
#since _allocate_ips_for_port will need the mac when
#calculating an EUI-64 address for a v6 subnet
p['mac_address'] = NeutronDbPluginV2._generate_mac(context,
network_id)
db_port = self._create_port(context, network_id, port_data)
p['mac_address'] = db_port['mac_address']
else:
# Ensure that the mac on the network is unique
if not NeutronDbPluginV2._check_unique_mac(context,
network_id,
p['mac_address']):
raise n_exc.MacAddressInUse(net_id=network_id,
mac=p['mac_address'])
if 'status' not in p:
status = constants.PORT_STATUS_ACTIVE
else:
status = p['status']
db_port = models_v2.Port(tenant_id=tenant_id,
name=p['name'],
id=port_id,
network_id=network_id,
mac_address=p['mac_address'],
admin_state_up=p['admin_state_up'],
status=status,
device_id=p['device_id'],
device_owner=p['device_owner'])
context.session.add(db_port)
db_port = self._create_port_with_mac(
context, network_id, port_data, p['mac_address'])
# Update the IP's for the port
ips = self._allocate_ips_for_port(context, port)

48
neutron/db/migration/alembic_migrations/versions/2a1ee2fb59e0_add_mac_address_unique_constraint.py Normal file
View File

@ -0,0 +1,48 @@
# Copyright (c) 2015 Thales Services SAS
#
# 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.
#
"""Add mac_address unique constraint
Revision ID: 2a1ee2fb59e0
Revises: 41662e32bce2
Create Date: 2015-01-10 11:44:27.550349
"""
# revision identifiers, used by Alembic.
revision = '2a1ee2fb59e0'
down_revision = '41662e32bce2'
from alembic import op
TABLE_NAME = 'ports'
CONSTRAINT_NAME = 'uniq_ports0network_id0mac_address'
def upgrade():
op.create_unique_constraint(
name=CONSTRAINT_NAME,
source=TABLE_NAME,
local_cols=['network_id', 'mac_address']
)
def downgrade():
op.drop_constraint(
name=CONSTRAINT_NAME,
source=TABLE_NAME,
local_cols=['network_id', 'mac_address']
)

2
neutron/db/migration/alembic_migrations/versions/HEAD
View File

@ -1 +1 @@
41662e32bce2
2a1ee2fb59e0

6
neutron/db/models_v2.py
View File

@ -135,6 +135,12 @@ class Port(model_base.BASEV2, HasId, HasTenant):
status = sa.Column(sa.String(16), nullable=False)
device_id = sa.Column(sa.String(255), nullable=False)
device_owner = sa.Column(sa.String(255), nullable=False)
__table_args__ = (
sa.UniqueConstraint(
network_id, mac_address,
name='uniq_ports0network_id0mac_address'),
model_base.BASEV2.__table_args__
)
def __init__(self, id=None, tenant_id=None, name=None, network_id=None,
mac_address=None, admin_state_up=None, status=None,

3
neutron/tests/unit/metaplugin/test_metaplugin.py
View File

@ -125,8 +125,7 @@ class MetaNeutronPluginV2Test(testlib_api.SqlTestCase,
'admin_state_up': True,
'host_routes': [],
'fixed_ips': [],
'mac_address':
self.plugin._generate_mac(self.context, net_id),
'mac_address': self.plugin._generate_mac(),
'tenant_id': self.fake_tenant_id}}
def _fake_subnet(self, net_id):

12
neutron/tests/unit/ml2/db/test_ml2_db.py
View File

@ -14,6 +14,7 @@
# limitations under the License.
from neutron import context
from neutron.db import db_base_plugin_v2
from neutron.db import models_v2
from neutron.extensions import portbindings
from neutron.openstack.common import uuidutils
@ -34,15 +35,17 @@ class Ml2DBTestCase(testlib_api.SqlTestCase):
self.ctx.session.add(models_v2.Network(id=network_id))
def _setup_neutron_port(self, network_id, port_id):
mac_address = db_base_plugin_v2.NeutronDbPluginV2._generate_mac()
with self.ctx.session.begin(subtransactions=True):
port = models_v2.Port(id=port_id,
network_id=network_id,
mac_address='foo_mac_address',
mac_address=mac_address,
admin_state_up=True,
status='DOWN',
device_id='',
device_owner='')
self.ctx.session.add(port)
return port
def _setup_neutron_portbinding(self, port_id, vif_type, host):
with self.ctx.session.begin(subtransactions=True):
@ -190,12 +193,11 @@ class Ml2DBTestCase(testlib_api.SqlTestCase):
def test_get_port_from_device_mac(self):
network_id = 'foo-network-id'
port_id = 'foo-port-id'
mac_address = 'foo_mac_address'
self._setup_neutron_network(network_id)
self._setup_neutron_port(network_id, port_id)
port = self._setup_neutron_port(network_id, port_id)
port = ml2_db.get_port_from_device_mac(mac_address)
self.assertEqual(port_id, port.id)
observed_port = ml2_db.get_port_from_device_mac(port['mac_address'])
self.assertEqual(port_id, observed_port.id)
def test_get_locked_port_and_binding(self):
network_id = 'foo-network-id'

5
neutron/tests/unit/ml2/db/test_ml2_dvr_db.py
View File

@ -18,6 +18,7 @@ import mock
from sqlalchemy.orm import query
from neutron import context
from neutron.db import db_base_plugin_v2
from neutron.db import l3_db
from neutron.db import models_v2
from neutron.extensions import portbindings
@ -37,9 +38,11 @@ class Ml2DvrDBTestCase(testlib_api.SqlTestCase):
self.ctx.session.add(models_v2.Network(id=network_id))
ports = []
for port_id in port_ids:
mac_address = (db_base_plugin_v2.NeutronDbPluginV2.
_generate_mac())
port = models_v2.Port(id=port_id,
network_id=network_id,
mac_address='foo_mac_address',
mac_address=mac_address,
admin_state_up=True,
status='ACTIVE',
device_id='',

22
neutron/tests/unit/test_db_plugin.py
View File

@ -1298,20 +1298,22 @@ fixed_ips=ip_address%%3D%s&fixed_ips=ip_address%%3D%s&fixed_ips=subnet_id%%3D%s
def test_mac_exhaustion(self):
# rather than actually consuming all MAC (would take a LONG time)
# we just raise the exception that would result.
@staticmethod
def fake_gen_mac(context, net_id):
raise n_exc.MacAddressGenerationFailure(net_id=net_id)
# we try to allocate an already allocated mac address
cfg.CONF.set_override('mac_generation_retries', 3)
with mock.patch.object(neutron.db.db_base_plugin_v2.NeutronDbPluginV2,
'_generate_mac', new=fake_gen_mac):
res = self._create_network(fmt=self.fmt, name='net1',
admin_state_up=True)
network = self.deserialize(self.fmt, res)
net_id = network['network']['id']
res = self._create_network(fmt=self.fmt, name='net1',
admin_state_up=True)
network = self.deserialize(self.fmt, res)
net_id = network['network']['id']
error = n_exc.MacAddressInUse(net_id=net_id, mac='00:11:22:33:44:55')
with mock.patch.object(
neutron.db.db_base_plugin_v2.NeutronDbPluginV2,
'_create_port_with_mac', side_effect=error) as create_mock:
res = self._create_port(self.fmt, net_id=net_id)
self.assertEqual(res.status_int,
webob.exc.HTTPServiceUnavailable.code)
self.assertEqual(3, create_mock.call_count)
def test_requested_duplicate_ip(self):
with self.subnet() as subnet: