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

Please modify also the documentation INSTALL.Windows.md to be inline with the modified port names.

Small nit: you should delete also the defines for internal/external (https://github.com/openvswitch/ovs/blob/master/datapath-windows/ovsext/Vport.h#L37-L40)

For this to work in the case for the internal port change you also need to change the following lines:
https://github.com/openvswitch/ovs/blob/master/datapath-windows/ovsext/Vport.c#L2141-L2149

IMO we should not change the internal port name at the moment because in the case of multiple switches/supporting multiple adapters in the extension we will need to remove it.

Alin.

-----Mesaj original-----
De la: dev [mailto:dev-bounces@openvswitch.org] În numele Sorin Vinturis
Trimis: Friday, September 11, 2015 2:10 PM
Către: dev@openvswitch.org
Subiect: [ovs-dev] [PATCH] datapath-windows: Removed hardcoded names for internal/external vports

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(-)

 
 /*
  * --------------------------------------------------------------------------
- * 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);
         }
     }
--
1.9.0.msysgit.0
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

> On Sep 14, 2015, at 7:51 PM, Alin Serdean <aserdean@cloudbasesolutions.com> wrote:

> 

> For this to work in the case for the internal port change you also need to change the following lines:

> https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_openvswitch_ovs_blob_master_datapath-2Dwindows_ovsext_Vport.c-23L2141-2DL2149&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=pNHQcdr7B40b4h6Yb7FIedI1dnBsxdDuTLBYD3JqV80&m=zCHx69GiiUmOAzrRY67SE4QUGabdsfedEcpjTaoZODw&s=OQ7O90tnlLIaxihwmrdXcIPtNCta97dPUYjSX8t0-Rk&e= 


Thanks Alin for pointing this out.

The idea behind the checks above is that we want to be able to easily identify which of the vports represents the “internal adapter”. So, we don’t overwrite the vport->ovsType value to be OVS_VPORT_TYPE_NETDEV and preserve it to be OVS_VPORT_TYPE_INTERNAL.

The second check is that we need to identify ports of type “internal” to be ports that are internal to a bridge and we don’t forward packets to that port.

To implement Sorin’s change, we can probably nuke both the strcmp’s, and start treating hyper-v switch internal port to be a Netdev, but that would mean changes in other places as well.

> IMO we should not change the internal port name at the moment because in the case of multiple switches/supporting multiple adapters in the extension we will need to remove it.


This sounds like a good idea, IMO too.

-- Nithin