From 6f2b2976121246bb6413d2e789087dc4daf0f29d Mon Sep 17 00:00:00 2001 From: Gary Kotton Date: Sun, 22 Nov 2015 00:22:39 -0800 Subject: [PATCH] NSX|V: add locking for interface management There are some cases where this was not locked. This patch ensures that it is done in a more unified manner. In addition to this the patch adds additional locking when the conflicting shared routers are check. This is to ensure that the router interface is taken into account. Co-Authored-By: Bo Lin Change-Id: Ib760b0bf5fd7341e04c2116d69f7c39c17331413 Closes-bug: #1519667 --- .../nsx_v/drivers/shared_router_driver.py | 120 ++++++++++-------- .../plugins/nsx_v/vshield/edge_utils.py | 46 +++++++ 2 files changed, 113 insertions(+), 53 deletions(-) diff --git a/vmware_nsx/plugins/nsx_v/drivers/shared_router_driver.py b/vmware_nsx/plugins/nsx_v/drivers/shared_router_driver.py index 9ebadf0c53..eca18c8c4b 100644 --- a/vmware_nsx/plugins/nsx_v/drivers/shared_router_driver.py +++ b/vmware_nsx/plugins/nsx_v/drivers/shared_router_driver.py @@ -621,68 +621,82 @@ class RouterSharedDriver(router_driver.RouterBaseDriver): self._add_router_services_on_available_edge(context, router_id) + def _base_add_router_interface(self, context, router_id, interface_info): + with locking.LockManager.get_lock("router", lock_file_prefix="bind-", + external=True): + return super(nsx_v.NsxVPluginV2, self.plugin).add_router_interface( + context, router_id, interface_info) + def add_router_interface(self, context, router_id, interface_info): self.plugin._check_intf_number_of_router(context, router_id) edge_id = edge_utils.get_router_edge_id(context, router_id) router_db = self.plugin._get_router(context, router_id) if edge_id: is_migrated = False - with locking.LockManager.get_lock( - str(edge_id), - lock_file_prefix=NSXV_ROUTER_RECONFIG): - router_ids = self.edge_manager.get_routers_on_same_edge( - context, router_id) - info = super(nsx_v.NsxVPluginV2, - self.plugin).add_router_interface( - context, router_id, interface_info) - subnet = self.plugin.get_subnet(context, info['subnet_id']) - network_id = subnet['network_id'] - # Collect all conflict networks whose cidr are overlapped - # with networks attached to the router and conflct routers - # which has same network with the router's. - conflict_network_ids, conflict_router_ids, _ = ( - self._get_conflict_network_and_router_ids_by_intf( - context, router_id)) + with locking.LockManager.get_lock("router", + lock_file_prefix="bind-", + external=True): + with locking.LockManager.get_lock( + str(edge_id), + lock_file_prefix=NSXV_ROUTER_RECONFIG, + external=True): + router_ids = self.edge_manager.get_routers_on_same_edge( + context, router_id) + info = super(nsx_v.NsxVPluginV2, + self.plugin).add_router_interface( + context, router_id, interface_info) + subnet = self.plugin.get_subnet(context, info['subnet_id']) + network_id = subnet['network_id'] + # Collect all conflict networks whose cidr are overlapped + # with networks attached to the router and conflct routers + # which has same network with the router's. + conflict_network_ids, conflict_router_ids, _ = ( + self._get_conflict_network_and_router_ids_by_intf( + context, router_id)) - _, new_conflict_router_ids = ( - self._get_available_and_conflicting_ids(context, - router_id)) - conflict_router_ids.extend(new_conflict_router_ids) - conflict_router_ids = list(set(conflict_router_ids)) + _, new_conflict_router_ids = ( + self._get_available_and_conflicting_ids(context, + router_id)) + conflict_router_ids.extend(new_conflict_router_ids) + conflict_router_ids = list(set(conflict_router_ids)) - interface_ports = ( - self.plugin._get_router_interface_ports_by_network( - context, router_id, network_id)) - # Consider whether another subnet of the same network - # has been attached to the router. - if len(interface_ports) > 1: - is_conflict = self.edge_manager.is_router_conflict_on_edge( - context, router_id, conflict_router_ids, - conflict_network_ids, 0) - else: - is_conflict = self.edge_manager.is_router_conflict_on_edge( - context, router_id, conflict_router_ids, - conflict_network_ids, 1) - if is_conflict: + interface_ports = ( + self.plugin._get_router_interface_ports_by_network( + context, router_id, network_id)) + # Consider whether another subnet of the same network + # has been attached to the router. if len(interface_ports) > 1: - self._remove_router_services_on_edge( - context, router_id) + is_conflict = ( + self.edge_manager.is_router_conflict_on_edge( + context, router_id, conflict_router_ids, + conflict_network_ids, 0)) else: - self._remove_router_services_on_edge( + is_conflict = ( + self.edge_manager.is_router_conflict_on_edge( + context, router_id, conflict_router_ids, + conflict_network_ids, 1)) + if is_conflict: + if len(interface_ports) > 1: + self._remove_router_services_on_edge( + context, router_id) + else: + self._remove_router_services_on_edge( + context, router_id, network_id) + self._unbind_router_on_edge(context, router_id) + is_migrated = True + else: + address_groups = self.plugin._get_address_groups( context, router_id, network_id) - self._unbind_router_on_edge(context, router_id) - is_migrated = True - else: - address_groups = self.plugin._get_address_groups( - context, router_id, network_id) - edge_utils.update_internal_interface( - self.nsx_v, context, router_id, - network_id, address_groups, router_db.admin_state_up) - if router_db.gw_port and router_db.enable_snat: - self._update_nat_rules_on_routers( - context, router_id, router_ids) - self._update_subnets_and_dnat_firewall_on_routers( - context, router_id, router_ids, allow_external=True) + edge_utils.update_internal_interface( + self.nsx_v, context, router_id, + network_id, address_groups, + router_db.admin_state_up) + if router_db.gw_port and router_db.enable_snat: + self._update_nat_rules_on_routers( + context, router_id, router_ids) + self._update_subnets_and_dnat_firewall_on_routers( + context, router_id, router_ids, + allow_external=True) if is_migrated: self._bind_router_on_available_edge( context, router_id, router_db.admin_state_up) @@ -693,8 +707,8 @@ class RouterSharedDriver(router_driver.RouterBaseDriver): self._add_router_services_on_available_edge(context, router_id) else: - info = super(nsx_v.NsxVPluginV2, self.plugin).add_router_interface( - context, router_id, interface_info) + info = self._base_add_router_interface(context, router_id, + interface_info) # bind and configure routing servie on an availab edge self._bind_router_on_available_edge( context, router_id, router_db.admin_state_up) diff --git a/vmware_nsx/plugins/nsx_v/vshield/edge_utils.py b/vmware_nsx/plugins/nsx_v/vshield/edge_utils.py index ba7c7fe301..67c4c5fa82 100644 --- a/vmware_nsx/plugins/nsx_v/vshield/edge_utils.py +++ b/vmware_nsx/plugins/nsx_v/vshield/edge_utils.py @@ -1518,6 +1518,16 @@ def clear_gateway(nsxv_manager, context, router_id): def update_external_interface( + nsxv_manager, context, router_id, ext_net_id, + ipaddr, netmask, secondary=None): + with locking.LockManager.get_lock( + str(router_id), lock_file_prefix='nsx-edge-interface-', external=True): + _update_external_interface(nsxv_manager, context, router_id, + ext_net_id, ipaddr, netmask, + secondary=secondary) + + +def _update_external_interface( nsxv_manager, context, router_id, ext_net_id, ipaddr, netmask, secondary=None): secondary = secondary or [] @@ -1538,6 +1548,15 @@ def update_external_interface( def update_internal_interface(nsxv_manager, context, router_id, int_net_id, address_groups, is_connected=True): + with locking.LockManager.get_lock( + str(router_id), lock_file_prefix='nsx-edge-interface-', external=True): + _update_internal_interface(nsxv_manager, context, router_id, + int_net_id, address_groups, + is_connected=is_connected) + + +def _update_internal_interface(nsxv_manager, context, router_id, int_net_id, + address_groups, is_connected=True): # Get the pg/wire id of the network id mappings = nsx_db.get_nsx_switch_ids(context.session, int_net_id) if mappings: @@ -1566,6 +1585,15 @@ def update_internal_interface(nsxv_manager, context, router_id, int_net_id, def add_vdr_internal_interface(nsxv_manager, context, router_id, int_net_id, address_groups, is_connected=True): + with locking.LockManager.get_lock( + str(router_id), lock_file_prefix='nsx-edge-interface-', external=True): + _add_vdr_internal_interface(nsxv_manager, context, router_id, + int_net_id, address_groups, + is_connected=is_connected) + + +def _add_vdr_internal_interface(nsxv_manager, context, router_id, + int_net_id, address_groups, is_connected=True): # Get the pg/wire id of the network id mappings = nsx_db.get_nsx_switch_ids(context.session, int_net_id) if mappings: @@ -1592,6 +1620,16 @@ def add_vdr_internal_interface(nsxv_manager, context, router_id, def update_vdr_internal_interface(nsxv_manager, context, router_id, int_net_id, address_groups, is_connected=True): + with locking.LockManager.get_lock( + str(router_id), lock_file_prefix='nsx-edge-interface-', external=True): + _update_vdr_internal_interface(nsxv_manager, context, router_id, + int_net_id, address_groups, + is_connected=is_connected) + + +def _update_vdr_internal_interface(nsxv_manager, context, router_id, + int_net_id, address_groups, + is_connected=True): # Get the pg/wire id of the network id mappings = nsx_db.get_nsx_switch_ids(context.session, int_net_id) if mappings: @@ -1612,6 +1650,14 @@ def update_vdr_internal_interface(nsxv_manager, context, router_id, int_net_id, def delete_interface(nsxv_manager, context, router_id, network_id, dist=False, is_wait=True): + with locking.LockManager.get_lock( + str(router_id), lock_file_prefix='nsx-edge-interface-', external=True): + _delete_interface(nsxv_manager, context, router_id, network_id, + dist=dist, is_wait=is_wait) + + +def _delete_interface(nsxv_manager, context, router_id, network_id, + dist=False, is_wait=True): # Get the pg/wire id of the network id mappings = nsx_db.get_nsx_switch_ids(context.session, network_id) if mappings: