diff mbox

[ovs-dev] datapath-windows: Removed hardcoded names for internal/external vports

Message ID 1441151827-14894-1-git-send-email-svinturis@cloudbasesolutions.com
State Superseded
Headers show

Commit Message

Sorin Vinturis Sept. 11, 2015, 11:09 a.m. UTC 
The internal/external vports will have the actual OS-based names, which
represent the NIC interface alias that is displayed by running
'Get-NetAdapter' Hyper-V PS command.

Signed-off-by: Sorin Vinturis <svinturis@cloudbasesolutions.com>
---
 datapath-windows/ovsext/Vport.c | 52 +++++++++++++++++++++++++----------------
 1 file changed, 32 insertions(+), 20 deletions(-)

Comments

Nithin Raju Sept. 15, 2015, 5:32 a.m. UTC | #1 
hi Sorin,
Thanks for the change. Looks good mostly. I had a couple of minor comments that are inlined.

> On Sep 11, 2015, at 4:09 AM, Sorin Vinturis <svinturis@cloudbasesolutions.com> wrote:

> 

> The internal/external vports will have the actual OS-based names, which

> represent the NIC interface alias that is displayed by running

> 'Get-NetAdapter' Hyper-V PS command.

> 

> Signed-off-by: Sorin Vinturis <svinturis@cloudbasesolutions.com>


> static VOID

> AssignNicNameSpecial(POVS_VPORT_ENTRY vport)

> {

> -    size_t len;

> +    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,

> +                                        &interfaceLuid);

> +    if (status == STATUS_SUCCESS) {

> +        status = ConvertInterfaceLuidToAlias(&interfaceLuid, interfaceName,

> +                                             IF_MAX_STRING_SIZE + 1);

> +    }


Can you pls. log an error if either of the functions fail along with the ‘status’ value? Also, if the functions do fail, do you want to bail out processing?

> +

> +    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,

> +                           &len);

> +        vport->portFriendlyName.Length = (USHORT)len;

> +    }

> }

> 

> 

> @@ -1399,6 +1409,8 @@ OvsInitConfiguredSwitchNics(POVS_SWITCH_CONTEXT switchContext)

>         }

>         OvsInitVportWithNicParam(switchContext, vport, nicParam);

>         if (nicParam->NicType == NdisSwitchNicTypeInternal) {

> +            /* Overwrite the 'portFriendlyName' of the internal vport. */

> +            AssignNicNameSpecial(vport);

>             OvsInternalAdapterUp(vport->portNo, &nicParam->NetCfgInstanceId);


There’s already a call to AssignNicNameSpecial() from InitHvVportCommon(). Do we need another call?
diff mbox

Patch

diff --git a/datapath-windows/ovsext/Vport.c b/datapath-windows/ovsext/Vport.c
index ea10692..8a7a8b9 100644
--- a/datapath-windows/ovsext/Vport.c
+++ b/datapath-windows/ovsext/Vport.c
@@ -890,6 +890,10 @@  OvsInitPhysNicVport(POVS_VPORT_ENTRY physExtVport,
                   &virtExtVport->portFriendlyName,
                   sizeof(NDIS_SWITCH_PORT_FRIENDLYNAME));
 
+    RtlCopyMemory(&physExtVport->netCfgInstanceId,
+                  &virtExtVport->netCfgInstanceId,
+                  sizeof(physExtVport->netCfgInstanceId));
+
     physExtVport->ovsState = OVS_STATE_PORT_CREATED;
 }
 
@@ -968,36 +972,42 @@  OvsInitBridgeInternalVport(POVS_VPORT_ENTRY vport)
 
 /*
  * --------------------------------------------------------------------------
- * For external vports 'portFriendlyName' provided by Hyper-V is over-written
- * by synthetic names.
+ * For external and internal vports 'portFriendlyName' parameter, provided by
+ * Hyper-V, is overwritten with the interface alias name.
  * --------------------------------------------------------------------------
  */
 static VOID
 AssignNicNameSpecial(POVS_VPORT_ENTRY vport)
 {
-    size_t len;
+    NTSTATUS status = STATUS_SUCCESS;
+    WCHAR interfaceName[IF_MAX_STRING_SIZE] = { 0 };
+    NET_LUID interfaceLuid = { 0 };
+    size_t len = 0;
 
-    if (vport->portType == NdisSwitchPortTypeExternal) {
-        if (vport->nicIndex == 0) {
-            ASSERT(vport->nicIndex == 0);
-            RtlStringCbPrintfW(vport->portFriendlyName.String,
-                               IF_MAX_STRING_SIZE,
-                               L"%s.virtualAdapter", OVS_DPPORT_EXTERNAL_NAME_W);
+    ASSERT(vport->portType == NdisSwitchPortTypeExternal || 
+           vport->portType == NdisSwitchPortTypeInternal);
+
+    status = ConvertInterfaceGuidToLuid(&vport->netCfgInstanceId,
+                                        &interfaceLuid);
+    if (status == STATUS_SUCCESS) {
+        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.%lu", OVS_DPPORT_EXTERNAL_NAME_W,
-                               (UINT32)vport->nicIndex);
+                               IF_MAX_STRING_SIZE, L"%s", interfaceName);
         }
-    } else {
-        RtlStringCbPrintfW(vport->portFriendlyName.String,
-                           IF_MAX_STRING_SIZE,
-                           L"%s", OVS_DPPORT_INTERNAL_NAME_W);
-    }
 
-    RtlStringCbLengthW(vport->portFriendlyName.String, IF_MAX_STRING_SIZE,
-                       &len);
-    vport->portFriendlyName.Length = (USHORT)len;
+        RtlStringCbLengthW(vport->portFriendlyName.String, IF_MAX_STRING_SIZE,
+                           &len);
+        vport->portFriendlyName.Length = (USHORT)len;
+    }
 }
 
 
@@ -1399,6 +1409,8 @@  OvsInitConfiguredSwitchNics(POVS_SWITCH_CONTEXT switchContext)
         }
         OvsInitVportWithNicParam(switchContext, vport, nicParam);
         if (nicParam->NicType == NdisSwitchNicTypeInternal) {
+            /* Overwrite the 'portFriendlyName' of the internal vport. */
+            AssignNicNameSpecial(vport);
             OvsInternalAdapterUp(vport->portNo, &nicParam->NetCfgInstanceId);
         }
     }