[ovs-dev] [PATCH 5/6 v2] datapath-windows: cleanup AssignNicNameSpecial()

Sairam Venugopal vsairam at vmware.com
Wed Nov 18 20:59:57 UTC 2015


Acked-by: Sairam Venugopal <vsairam at vmware.com<mailto:vsairam at vmware.com>>

From: "Nithin Raju" <nithin at vmware.com<mailto:nithin at vmware.com>>
Date: Wed, Nov 18, 2015 at 8:13 AM -0800
Subject: [PATCH 5/6 v2] datapath-windows: cleanup AssignNicNameSpecial()
To: "dev at openvswitch.org<mailto:dev at openvswitch.org>" <dev at openvswitch.org<mailto:dev at openvswitch.org>>
Cc: "Nithin Raju" <nithin at vmware.com<mailto:nithin at vmware.com>>


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 <nithin at vmware.com<mailto:nithin at vmware.com>>
---
 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;
 }


--
1.8.5.6




More information about the dev mailing list