From patchwork Wed Nov 25 20:00:57 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nithin Raju X-Patchwork-Id: 548742 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from archives.nicira.com (unknown [IPv6:2600:3c00::f03c:91ff:fe6e:bdf7]) by ozlabs.org (Postfix) with ESMTP id 9F5771402DE for ; Thu, 26 Nov 2015 07:01:37 +1100 (AEDT) Received: from archives.nicira.com (localhost [127.0.0.1]) by archives.nicira.com (Postfix) with ESMTP id BDC3522C3A2; Wed, 25 Nov 2015 12:01:07 -0800 (PST) X-Original-To: dev@openvswitch.org Delivered-To: dev@openvswitch.org Received: from mx3v3.cudamail.com (mx3.cudamail.com [64.34.241.5]) by archives.nicira.com (Postfix) with ESMTPS id D602610C5A for ; Wed, 25 Nov 2015 12:01:03 -0800 (PST) Received: from bar3.cudamail.com (localhost [127.0.0.1]) by mx3v3.cudamail.com (Postfix) with ESMTPS id 69EDF1614B1 for ; Wed, 25 Nov 2015 13:01:03 -0700 (MST) X-ASG-Debug-ID: 1448481662-03dd7b0fbda9f70001-byXFYA Received: from mx3-pf1.cudamail.com ([192.168.14.2]) by bar3.cudamail.com with ESMTP id Fiz6yaiMvJXlqKyF (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Wed, 25 Nov 2015 13:01:03 -0700 (MST) X-Barracuda-Envelope-From: nithin@vmware.com X-Barracuda-RBL-Trusted-Forwarder: 192.168.14.2 Received: from unknown (HELO smtp-outbound-1.vmware.com) (208.91.2.12) by mx3-pf1.cudamail.com with ESMTPS (DHE-RSA-AES256-SHA encrypted); 25 Nov 2015 20:01:02 -0000 Received-SPF: error (mx3-pf1.cudamail.com: error in processing during lookup of vmware.com: DNS problem) X-Barracuda-Apparent-Source-IP: 208.91.2.12 X-Barracuda-RBL-IP: 208.91.2.12 Received: from sc9-mailhost3.vmware.com (sc9-mailhost3.vmware.com [10.113.161.73]) by smtp-outbound-1.vmware.com (Postfix) with ESMTP id D7D85293FA for ; Wed, 25 Nov 2015 12:00:59 -0800 (PST) Received: from pa-dbc1118.eng.vmware.com (unknown [10.162.210.18]) by sc9-mailhost3.vmware.com (Postfix) with ESMTP id 95DD1404B7; Wed, 25 Nov 2015 12:01:00 -0800 (PST) X-CudaMail-Envelope-Sender: nithin@vmware.com From: Nithin Raju To: dev@openvswitch.org X-CudaMail-Whitelist-To: dev@openvswitch.org X-CudaMail-MID: CM-V1-1124047114 X-CudaMail-DTE: 112515 X-CudaMail-Originating-IP: 208.91.2.12 Date: Wed, 25 Nov 2015 12:00:57 -0800 X-ASG-Orig-Subj: [##CM-V1-1124047114##][PATCH 5/6 v3] datapath-windows: cleanup AssignNicNameSpecial() Message-Id: <1448481658-28072-6-git-send-email-nithin@vmware.com> X-Mailer: git-send-email 1.8.5.6 In-Reply-To: <1448481658-28072-1-git-send-email-nithin@vmware.com> References: <1448481658-28072-1-git-send-email-nithin@vmware.com> X-Barracuda-Connect: UNKNOWN[192.168.14.2] X-Barracuda-Start-Time: 1448481663 X-Barracuda-Encrypted: DHE-RSA-AES256-SHA X-Barracuda-URL: https://web.cudamail.com:443/cgi-mod/mark.cgi X-ASG-Whitelist: Header =?UTF-8?B?eFwtY3VkYW1haWxcLXdoaXRlbGlzdFwtdG8=?= X-Virus-Scanned: by bsmtpd at cudamail.com X-Barracuda-BRTS-Status: 1 X-ASG-Whitelist: EmailCat (corporate) Subject: [ovs-dev] [PATCH 5/6 v3] datapath-windows: cleanup AssignNicNameSpecial() X-BeenThere: dev@openvswitch.org X-Mailman-Version: 2.1.16 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: dev-bounces@openvswitch.org Sender: "dev" AssignNicNameSpecial() needed to be called outside of a lock and was moved out in a previous change. But, it was accessing vport structure outside of the lock which isn't safe. In this change, we take care of that. I tried to trigger a call to HvUpdateNic() by renaming the interface from the GUI and didn't see any callback. Other changes are tested. Signed-off-by: Nithin Raju Acked-by: Sairam Venugopal Acked-by: Alin Gabriel Serdean --- datapath-windows/ovsext/Vport.c | 62 ++++++++++++++++++++++------------------- 1 file changed, 33 insertions(+), 29 deletions(-) diff --git a/datapath-windows/ovsext/Vport.c b/datapath-windows/ovsext/Vport.c index 388920e..ef21fca 100644 --- a/datapath-windows/ovsext/Vport.c +++ b/datapath-windows/ovsext/Vport.c @@ -94,7 +94,8 @@ static VOID OvsTunnelVportPendingInit(PVOID context, static VOID OvsTunnelVportPendingRemove(PVOID context, NTSTATUS status, UINT32 *replyLen); -static VOID AssignNicNameSpecial(POVS_VPORT_ENTRY vport); +static NTSTATUS GetNICAlias(GUID *netCfgInstanceId, + IF_COUNTED_STRING *portFriendlyName); /* * -------------------------------------------------------------------------- @@ -311,7 +312,7 @@ HvCreateNic(POVS_SWITCH_CONTEXT switchContext, { POVS_VPORT_ENTRY vport; NDIS_STATUS status = NDIS_STATUS_SUCCESS; - + IF_COUNTED_STRING portFriendlyName = {0}; LOCK_STATE_EX lockState; VPORT_NIC_ENTER(nicParam); @@ -326,6 +327,12 @@ HvCreateNic(POVS_SWITCH_CONTEXT switchContext, goto done; } + if (nicParam->NicType == NdisSwitchNicTypeInternal || + (nicParam->NicType == NdisSwitchNicTypeExternal && + nicParam->NicIndex != 0)) { + GetNICAlias(&nicParam->NetCfgInstanceId, &portFriendlyName); + } + NdisAcquireRWLockWrite(switchContext->dispatchLock, &lockState, 0); /* * There can be one or more NICs for the external port. We create a vport @@ -386,17 +393,12 @@ HvCreateNic(POVS_SWITCH_CONTEXT switchContext, if (nicParam->NicType == NdisSwitchNicTypeInternal || (nicParam->NicType == NdisSwitchNicTypeExternal && nicParam->NicIndex != 0)) { - AssignNicNameSpecial(vport); + RtlCopyMemory(&vport->portFriendlyName, &portFriendlyName, + sizeof portFriendlyName); } add_nic_done: NdisReleaseRWLock(switchContext->dispatchLock, &lockState); - if (status == STATUS_SUCCESS && - (vport->portType == NdisSwitchPortTypeInternal || - (vport->portType == NdisSwitchPortTypeExternal && - nicParam->NicIndex != 0))) { - AssignNicNameSpecial(vport); - } done: VPORT_NIC_EXIT(nicParam); @@ -467,6 +469,7 @@ HvUpdateNic(POVS_SWITCH_CONTEXT switchContext, POVS_VPORT_ENTRY vport; LOCK_STATE_EX lockState; UINT32 event = 0; + IF_COUNTED_STRING portFriendlyName = {0}; VPORT_NIC_ENTER(nicParam); @@ -478,6 +481,13 @@ HvUpdateNic(POVS_SWITCH_CONTEXT switchContext, goto update_nic_done; } + /* GetNICAlias() must be called outside of a lock. */ + if (nicParam->NicType == NdisSwitchNicTypeInternal || + (nicParam->NicType == NdisSwitchNicTypeExternal && + nicParam->NicIndex != 0)) { + GetNICAlias(&nicParam->NetCfgInstanceId, &portFriendlyName); + } + NdisAcquireRWLockWrite(switchContext->dispatchLock, &lockState, 0); vport = OvsFindVportByPortIdAndNicIndex(switchContext, nicParam->PortId, @@ -492,7 +502,8 @@ HvUpdateNic(POVS_SWITCH_CONTEXT switchContext, case NdisSwitchNicTypeInternal: RtlCopyMemory(&vport->netCfgInstanceId, &nicParam->NetCfgInstanceId, sizeof (GUID)); - AssignNicNameSpecial(vport); + RtlCopyMemory(&vport->portFriendlyName, &portFriendlyName, + sizeof portFriendlyName); break; case NdisSwitchNicTypeSynthetic: case NdisSwitchNicTypeEmulated: @@ -1033,18 +1044,16 @@ OvsInitBridgeInternalVport(POVS_VPORT_ENTRY vport) * Hyper-V, is overwritten with the interface alias name. * -------------------------------------------------------------------------- */ -static VOID -AssignNicNameSpecial(POVS_VPORT_ENTRY vport) +static NTSTATUS +GetNICAlias(GUID *netCfgInstanceId, + IF_COUNTED_STRING *portFriendlyName) { NTSTATUS status = STATUS_SUCCESS; WCHAR interfaceName[IF_MAX_STRING_SIZE] = { 0 }; NET_LUID interfaceLuid = { 0 }; size_t len = 0; - ASSERT(vport->portType == NdisSwitchPortTypeExternal || - vport->portType == NdisSwitchPortTypeInternal); - - status = ConvertInterfaceGuidToLuid(&vport->netCfgInstanceId, + status = ConvertInterfaceGuidToLuid(netCfgInstanceId, &interfaceLuid); if (status == STATUS_SUCCESS) { /* @@ -1054,26 +1063,21 @@ AssignNicNameSpecial(POVS_VPORT_ENTRY vport) status = ConvertInterfaceLuidToAlias(&interfaceLuid, interfaceName, IF_MAX_STRING_SIZE + 1); if (status == STATUS_SUCCESS) { - if (vport->portType == NdisSwitchPortTypeExternal && - vport->nicIndex == 0) { - RtlStringCbPrintfW(vport->portFriendlyName.String, IF_MAX_STRING_SIZE, - L"%s.virtualAdapter", interfaceName); - } else { - RtlStringCbPrintfW(vport->portFriendlyName.String, - IF_MAX_STRING_SIZE, L"%s", interfaceName); - } - - RtlStringCbLengthW(vport->portFriendlyName.String, IF_MAX_STRING_SIZE, + RtlStringCbPrintfW(portFriendlyName->String, + IF_MAX_STRING_SIZE, L"%s", interfaceName); + RtlStringCbLengthW(portFriendlyName->String, IF_MAX_STRING_SIZE, &len); - vport->portFriendlyName.Length = (USHORT)len; + portFriendlyName->Length = (USHORT)len; } else { - OVS_LOG_INFO("Fail to convert interface LUID to alias, status: %x", + OVS_LOG_ERROR("Fail to convert interface LUID to alias, status: %x", status); } } else { - OVS_LOG_INFO("Fail to convert interface GUID to LUID, status: %x", + OVS_LOG_ERROR("Fail to convert interface GUID to LUID, status: %x", status); } + + return status; }