x/vmware-nsx
Code Issues Proposed changes

Fix dhcp_release lease race condition

Browse Source 
There is a possible race condition when delete or updating fixed_ips
on ports where an instance could renew its ip address again after
dhcp_release has already been executed. To fix this, the order of
reload_allocation and release_lease need to be switched. This way an
instance will not be able to renew it's ip address after it is
removed from the host file.

Fixes bug: 1237028

Change-Id: If05ec2be507378c634f5c1856dab0fbd396f43cc
This commit is contained in:
Aaron Rosen 2013-10-08 12:24:21 -07:00
parent fb65239a0a
commit 838e9bfcc5
2 changed files with 20 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
neutron/agent/dhcp_agent.py
View File

@ -227,20 +227,19 @@ class DhcpAgent(manager.Manager):
else: else:
self.disable_dhcp_helper(network.id) self.disable_dhcp_helper(network.id)
def release_lease_for_removed_ips(self, port, network): def release_lease_for_removed_ips(self, prev_port, updated_port, network):
"""Releases the dhcp lease for ips removed from a port.""" """Releases the dhcp lease for ips removed from a port."""
prev_port = self.cache.get_port_by_id(port.id)
if prev_port: if prev_port:
previous_ips = set(fixed_ip.ip_address previous_ips = set(fixed_ip.ip_address
for fixed_ip in prev_port.fixed_ips) for fixed_ip in prev_port.fixed_ips)
current_ips = set(fixed_ip.ip_address current_ips = set(fixed_ip.ip_address
for fixed_ip in port.fixed_ips) for fixed_ip in updated_port.fixed_ips)
# pass in port with removed ips on it # pass in port with removed ips on it
removed_ips = previous_ips - current_ips removed_ips = previous_ips - current_ips
if removed_ips: if removed_ips:
self.call_driver('release_lease', self.call_driver('release_lease',
network, network,
mac_address=port.mac_address, mac_address=updated_port.mac_address,
removed_ips=removed_ips) removed_ips=removed_ips)
@utils.synchronized('dhcp-agent') @utils.synchronized('dhcp-agent')
@ -283,12 +282,14 @@ class DhcpAgent(manager.Manager):
@utils.synchronized('dhcp-agent') @utils.synchronized('dhcp-agent')
def port_update_end(self, context, payload): def port_update_end(self, context, payload):
"""Handle the port.update.end notification event.""" """Handle the port.update.end notification event."""
port = dhcp.DictModel(payload['port']) updated_port = dhcp.DictModel(payload['port'])
network = self.cache.get_network_by_id(port.network_id) network = self.cache.get_network_by_id(updated_port.network_id)
if network: if network:
self.release_lease_for_removed_ips(port, network) prev_port = self.cache.get_port_by_id(updated_port.id)
self.cache.put_port(port) self.cache.put_port(updated_port)
self.call_driver('reload_allocations', network) self.call_driver('reload_allocations', network)
self.release_lease_for_removed_ips(prev_port, updated_port,
network)
# Use the update handler for the port create event. # Use the update handler for the port create event.
port_create_end = port_update_end port_create_end = port_update_end
@ -300,13 +301,13 @@ class DhcpAgent(manager.Manager):
if port: if port:
network = self.cache.get_network_by_id(port.network_id) network = self.cache.get_network_by_id(port.network_id)
self.cache.remove_port(port) self.cache.remove_port(port)
self.call_driver('reload_allocations', network)
removed_ips = [fixed_ip.ip_address removed_ips = [fixed_ip.ip_address
for fixed_ip in port.fixed_ips] for fixed_ip in port.fixed_ips]
self.call_driver('release_lease', self.call_driver('release_lease',
network, network,
mac_address=port.mac_address, mac_address=port.mac_address,
removed_ips=removed_ips) removed_ips=removed_ips)
self.call_driver('reload_allocations', network)
def enable_isolated_metadata_proxy(self, network): def enable_isolated_metadata_proxy(self, network):

19
neutron/tests/unit/test_dhcp_agent.py
View File

@ -749,12 +749,13 @@ class TestDhcpAgentEventHandler(base.BaseTestCase):
mock.call.get_port_by_id(fake_port1.id), mock.call.get_port_by_id(fake_port1.id),
mock.call.put_port(mock.ANY)]) mock.call.put_port(mock.ANY)])
self.call_driver.assert_has_calls( self.call_driver.assert_has_calls(
[mock.call.call_driver( [mock.call.call_driver('reload_allocations', fake_network),
'release_lease', mock.call.call_driver(
fake_network, 'release_lease',
mac_address=fake_port1.mac_address, fake_network,
removed_ips=set([updated_fake_port1.fixed_ips[0].ip_address])), mac_address=fake_port1.mac_address,
mock.call.call_driver('reload_allocations', fake_network)]) removed_ips=set([updated_fake_port1.fixed_ips[0].ip_address]))
])
def test_port_delete_end(self): def test_port_delete_end(self):
payload = dict(port_id=fake_port2.id) payload = dict(port_id=fake_port2.id)
@ -769,11 +770,11 @@ class TestDhcpAgentEventHandler(base.BaseTestCase):
mock.call.get_network_by_id(fake_network.id), mock.call.get_network_by_id(fake_network.id),
mock.call.remove_port(fake_port2)]) mock.call.remove_port(fake_port2)])
self.call_driver.assert_has_calls( self.call_driver.assert_has_calls(
[mock.call.call_driver('release_lease', [mock.call.call_driver('reload_allocations', fake_network),
mock.call.call_driver('release_lease',
fake_network, fake_network,
mac_address=fake_port2.mac_address, mac_address=fake_port2.mac_address,
removed_ips=removed_ips), removed_ips=removed_ips)])
mock.call.call_driver('reload_allocations', fake_network)])
def test_port_delete_end_unknown_port(self): def test_port_delete_end_unknown_port(self):
payload = dict(port_id='unknown') payload = dict(port_id='unknown')