diff mbox

[ovs-dev] datapath-windows: Updating an External Adapter causes flow lookup failure

Message ID 1446509119-6220-1-git-send-email-vsairam@vmware.com
State Superseded
Headers show

Commit Message

Sairam Venugopal Nov. 3, 2015, 12:05 a.m. UTC 
This patch fixes an issue with updating the propeties of an external
adapter in Windows. The issue causes flow lookups to fail until the
kernel is reinstalled.

Associated bug - https://github.com/openvswitch/ovs-issues/issues/102

Signed-off-by: Sairam Venugopal <vsairam@vmware.com>
---
 datapath-windows/ovsext/Vport.c | 40 ++++++++++++++++++++++++++++++++++++----
 1 file changed, 36 insertions(+), 4 deletions(-)

Comments

Nithin Raju Nov. 3, 2015, 1:10 a.m. UTC | #1 
Just a minor detail, but ConvertInterfaceLuidToAlias() seems to be
returning something of type NETIO_STATUS, with NO_ERROR indicating
success. We should probably use the appropriate type.

Looks good otherwise. Thanks for doing this.

Acked-by: Nithin Raju <nithin@vmware.com>


-----Original Message-----
From: Sairam Venugopal <vsairam@vmware.com>
Date: Monday, November 2, 2015 at 5:05 PM
To: "dev@openvswitch.org" <dev@openvswitch.org>
Subject: [ovs-dev] [PATCH] datapath-windows: Updating an External
Adapter	causes flow lookup failure

>This patch fixes an issue with updating the propeties of an external
>adapter in Windows. The issue causes flow lookups to fail until the
>kernel is reinstalled.
>
>Associated bug - 
>https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_openvswitc
>h_ovs-2Dissues_issues_102&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNt
>Xt-uEs&r=pNHQcdr7B40b4h6Yb7FIedI1dnBsxdDuTLBYD3JqV80&m=uzGKohGZgRODAcZXf_e
>RokVXtUFLDB5d0wjXmf4YDso&s=2qfpFBiaYnXUUwr5rtdDaoA_VofV0qetJrgkiHbXLvQ&e=
>
>Signed-off-by: Sairam Venugopal <vsairam@vmware.com>
>---
> datapath-windows/ovsext/Vport.c | 40
>++++++++++++++++++++++++++++++++++++----
> 1 file changed, 36 insertions(+), 4 deletions(-)
>
>diff --git a/datapath-windows/ovsext/Vport.c
>b/datapath-windows/ovsext/Vport.c
>index 4ade842..812e62b 100644
>--- a/datapath-windows/ovsext/Vport.c
>+++ b/datapath-windows/ovsext/Vport.c
>@@ -322,19 +322,51 @@ HvCreateNic(POVS_SWITCH_CONTEXT switchContext,
>         POVS_VPORT_ENTRY virtExtVport =
>             (POVS_VPORT_ENTRY)switchContext->virtualExternalVport;
> 
>-        vport = (POVS_VPORT_ENTRY)OvsAllocateVport();
>+        vport = OvsFindVportByPortIdAndNicIndex(switchContext,
>+                                                nicParam->PortId,
>+                                                nicParam->NicIndex);
>         if (vport == NULL) {
>-            status = NDIS_STATUS_RESOURCES;
>-            goto add_nic_done;
>+            /* Find by interface name */
>+            WCHAR interfaceName[IF_MAX_STRING_SIZE] = { 0 };
>+            NET_LUID interfaceLuid = { 0 };
>+            size_t len = 0;
>+            status =
>ConvertInterfaceGuidToLuid(&nicParam->NetCfgInstanceId,
>+                                                &interfaceLuid);
>+            if (status == NDIS_STATUS_SUCCESS) {
>+                status = ConvertInterfaceLuidToAlias(&interfaceLuid,
>+                                                     interfaceName,
>+                                                     IF_MAX_STRING_SIZE
>+ 1);
>+                if (status == NDIS_STATUS_SUCCESS) {
>+                    RtlStringCbLengthW(interfaceName,
>+                                       IF_MAX_STRING_SIZE,
>+                                       &len);
>+                    vport = OvsFindVportByHvNameW(switchContext,
>+                                                  interfaceName,
>+                                                  len);
>+                }
>+            }
>+
>+            if (vport == NULL) {
>+                /* XXX: Handle this event appropriately */
>+                vport = (POVS_VPORT_ENTRY)OvsAllocateVport();
>+                if (vport == NULL) {
>+                    status = NDIS_STATUS_RESOURCES;
>+                    goto add_nic_done;
>+                }
>+            }
>         }
>+
>         OvsInitPhysNicVport(vport, virtExtVport, nicParam->NicIndex);
>+        OvsInitVportWithNicParam(switchContext, vport, nicParam);
>         status = InitHvVportCommon(switchContext, vport, TRUE);
>+        vport->isAbsentOnHv = FALSE;
>         if (status != NDIS_STATUS_SUCCESS) {
>             OvsFreeMemoryWithTag(vport, OVS_VPORT_POOL_TAG);
>             goto add_nic_done;
>         }
>+    } else {
>+        OvsInitVportWithNicParam(switchContext, vport, nicParam);
>     }
>-    OvsInitVportWithNicParam(switchContext, vport, nicParam);
>     portNo = vport->portNo;
>     if (vport->ovsState == OVS_STATE_CONNECTED) {
>         event = OVS_EVENT_CONNECT | OVS_EVENT_LINK_UP;
>-- 
>1.9.5.msysgit.0
>
>_______________________________________________
>dev mailing list
>dev@openvswitch.org
>https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailma
>n_listinfo_dev&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=pN
>HQcdr7B40b4h6Yb7FIedI1dnBsxdDuTLBYD3JqV80&m=uzGKohGZgRODAcZXf_eRokVXtUFLDB
>5d0wjXmf4YDso&s=nzNtvc9DhUaN4oQfMh7GTPzwdWw1TMB8xiGsYawULdA&e=
Sairam Venugopal Nov. 3, 2015, 1:18 a.m. UTC | #2 
Hey Nithin,

Thanks for the review. I will send out a V2 with the fix.

Sairam

On 11/2/15, 5:10 PM, "Nithin Raju" <nithin@vmware.com> wrote:

>Just a minor detail, but ConvertInterfaceLuidToAlias() seems to be
>returning something of type NETIO_STATUS, with NO_ERROR indicating
>success. We should probably use the appropriate type.
>
>Looks good otherwise. Thanks for doing this.
>
>Acked-by: Nithin Raju <nithin@vmware.com>
>
>
>-----Original Message-----
>From: Sairam Venugopal <vsairam@vmware.com>
>Date: Monday, November 2, 2015 at 5:05 PM
>To: "dev@openvswitch.org" <dev@openvswitch.org>
>Subject: [ovs-dev] [PATCH] datapath-windows: Updating an External
>Adapter	causes flow lookup failure
>
>>This patch fixes an issue with updating the propeties of an external
>>adapter in Windows. The issue causes flow lookups to fail until the
>>kernel is reinstalled.
>>
>>Associated bug - 
>>https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_openvswit
>>c
>>h_ovs-2Dissues_issues_102&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMN
>>t
>>Xt-uEs&r=pNHQcdr7B40b4h6Yb7FIedI1dnBsxdDuTLBYD3JqV80&m=uzGKohGZgRODAcZXf_
>>e
>>RokVXtUFLDB5d0wjXmf4YDso&s=2qfpFBiaYnXUUwr5rtdDaoA_VofV0qetJrgkiHbXLvQ&e=
>>
>>Signed-off-by: Sairam Venugopal <vsairam@vmware.com>
>>---
>> datapath-windows/ovsext/Vport.c | 40
>>++++++++++++++++++++++++++++++++++++----
>> 1 file changed, 36 insertions(+), 4 deletions(-)
>>
>>diff --git a/datapath-windows/ovsext/Vport.c
>>b/datapath-windows/ovsext/Vport.c
>>index 4ade842..812e62b 100644
>>--- a/datapath-windows/ovsext/Vport.c
>>+++ b/datapath-windows/ovsext/Vport.c
>>@@ -322,19 +322,51 @@ HvCreateNic(POVS_SWITCH_CONTEXT switchContext,
>>         POVS_VPORT_ENTRY virtExtVport =
>>             (POVS_VPORT_ENTRY)switchContext->virtualExternalVport;
>> 
>>-        vport = (POVS_VPORT_ENTRY)OvsAllocateVport();
>>+        vport = OvsFindVportByPortIdAndNicIndex(switchContext,
>>+                                                nicParam->PortId,
>>+                                                nicParam->NicIndex);
>>         if (vport == NULL) {
>>-            status = NDIS_STATUS_RESOURCES;
>>-            goto add_nic_done;
>>+            /* Find by interface name */
>>+            WCHAR interfaceName[IF_MAX_STRING_SIZE] = { 0 };
>>+            NET_LUID interfaceLuid = { 0 };
>>+            size_t len = 0;
>>+            status =
>>ConvertInterfaceGuidToLuid(&nicParam->NetCfgInstanceId,
>>+                                                &interfaceLuid);
>>+            if (status == NDIS_STATUS_SUCCESS) {
>>+                status = ConvertInterfaceLuidToAlias(&interfaceLuid,
>>+                                                     interfaceName,
>>+                                                     IF_MAX_STRING_SIZE
>>+ 1);
>>+                if (status == NDIS_STATUS_SUCCESS) {
>>+                    RtlStringCbLengthW(interfaceName,
>>+                                       IF_MAX_STRING_SIZE,
>>+                                       &len);
>>+                    vport = OvsFindVportByHvNameW(switchContext,
>>+                                                  interfaceName,
>>+                                                  len);
>>+                }
>>+            }
>>+
>>+            if (vport == NULL) {
>>+                /* XXX: Handle this event appropriately */
>>+                vport = (POVS_VPORT_ENTRY)OvsAllocateVport();
>>+                if (vport == NULL) {
>>+                    status = NDIS_STATUS_RESOURCES;
>>+                    goto add_nic_done;
>>+                }
>>+            }
>>         }
>>+
>>         OvsInitPhysNicVport(vport, virtExtVport, nicParam->NicIndex);
>>+        OvsInitVportWithNicParam(switchContext, vport, nicParam);
>>         status = InitHvVportCommon(switchContext, vport, TRUE);
>>+        vport->isAbsentOnHv = FALSE;
>>         if (status != NDIS_STATUS_SUCCESS) {
>>             OvsFreeMemoryWithTag(vport, OVS_VPORT_POOL_TAG);
>>             goto add_nic_done;
>>         }
>>+    } else {
>>+        OvsInitVportWithNicParam(switchContext, vport, nicParam);
>>     }
>>-    OvsInitVportWithNicParam(switchContext, vport, nicParam);
>>     portNo = vport->portNo;
>>     if (vport->ovsState == OVS_STATE_CONNECTED) {
>>         event = OVS_EVENT_CONNECT | OVS_EVENT_LINK_UP;
>>-- 
>>1.9.5.msysgit.0
>>
>>_______________________________________________
>>dev mailing list
>>dev@openvswitch.org
>>https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailm
>>a
>>n_listinfo_dev&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=p
>>N
>>HQcdr7B40b4h6Yb7FIedI1dnBsxdDuTLBYD3JqV80&m=uzGKohGZgRODAcZXf_eRokVXtUFLD
>>B
>>5d0wjXmf4YDso&s=nzNtvc9DhUaN4oQfMh7GTPzwdWw1TMB8xiGsYawULdA&e=
>
diff mbox

Patch

diff --git a/datapath-windows/ovsext/Vport.c b/datapath-windows/ovsext/Vport.c
index 4ade842..812e62b 100644
--- a/datapath-windows/ovsext/Vport.c
+++ b/datapath-windows/ovsext/Vport.c
@@ -322,19 +322,51 @@  HvCreateNic(POVS_SWITCH_CONTEXT switchContext,
         POVS_VPORT_ENTRY virtExtVport =
             (POVS_VPORT_ENTRY)switchContext->virtualExternalVport;
 
-        vport = (POVS_VPORT_ENTRY)OvsAllocateVport();
+        vport = OvsFindVportByPortIdAndNicIndex(switchContext,
+                                                nicParam->PortId,
+                                                nicParam->NicIndex);
         if (vport == NULL) {
-            status = NDIS_STATUS_RESOURCES;
-            goto add_nic_done;
+            /* Find by interface name */
+            WCHAR interfaceName[IF_MAX_STRING_SIZE] = { 0 };
+            NET_LUID interfaceLuid = { 0 };
+            size_t len = 0;
+            status = ConvertInterfaceGuidToLuid(&nicParam->NetCfgInstanceId,
+                                                &interfaceLuid);
+            if (status == NDIS_STATUS_SUCCESS) {
+                status = ConvertInterfaceLuidToAlias(&interfaceLuid,
+                                                     interfaceName,
+                                                     IF_MAX_STRING_SIZE + 1);
+                if (status == NDIS_STATUS_SUCCESS) {
+                    RtlStringCbLengthW(interfaceName,
+                                       IF_MAX_STRING_SIZE,
+                                       &len);
+                    vport = OvsFindVportByHvNameW(switchContext,
+                                                  interfaceName,
+                                                  len);
+                }
+            }
+
+            if (vport == NULL) {
+                /* XXX: Handle this event appropriately */
+                vport = (POVS_VPORT_ENTRY)OvsAllocateVport();
+                if (vport == NULL) {
+                    status = NDIS_STATUS_RESOURCES;
+                    goto add_nic_done;
+                }
+            }
         }
+
         OvsInitPhysNicVport(vport, virtExtVport, nicParam->NicIndex);
+        OvsInitVportWithNicParam(switchContext, vport, nicParam);
         status = InitHvVportCommon(switchContext, vport, TRUE);
+        vport->isAbsentOnHv = FALSE;
         if (status != NDIS_STATUS_SUCCESS) {
             OvsFreeMemoryWithTag(vport, OVS_VPORT_POOL_TAG);
             goto add_nic_done;
         }
+    } else {
+        OvsInitVportWithNicParam(switchContext, vport, nicParam);
     }
-    OvsInitVportWithNicParam(switchContext, vport, nicParam);
     portNo = vport->portNo;
     if (vport->ovsState == OVS_STATE_CONNECTED) {
         event = OVS_EVENT_CONNECT | OVS_EVENT_LINK_UP;