[ovs-dev] [PATCH v2] datapath-windows: Update port property

[Eitan Eliahu]

Yes, I agree. As you said this should be managed in a higher level.

Regarding the alignment, the parameters are aligned but it does not look aligned in the review email.
Thanks for the review!
Eitan

-----Original Message-----
From: Ankur Sharma 
Sent: Friday, November 07, 2014 1:46 PM
To: Eitan Eliahu; dev at openvswitch.org
Subject: RE: [ovs-dev] [PATCH v2] datapath-windows: Update port property

Hi Eitan,

In my opinion we should not allow the friendly name to be changed if corresponding ovs port exists (as this will lead to data inconsistency in userspace and kernel).

But we need not block your changes as this will definitely help in handling update friendly name cases before vsctl add-port is done. And we can always add check to block the case i have mentioned above.

I have a few minor comments regarding code style.

Acked-by: Ankur Sharma <ankursharma at vmware.com>


Regards,
Ankur

________________________________________
From: dev <dev-bounces at openvswitch.org> on behalf of Eitan Eliahu <eliahue at vmware.com>
Sent: Friday, November 7, 2014 6:53 PM
To: dev at openvswitch.org
Subject: [ovs-dev] [PATCH v2] datapath-windows: Update port property

Update Hyper-V port properties on NDIS property port set callback.
Driver update the port friendly name in its internal table.
Since the NIC Create callback won't be called after we need to maintain the NIC and THE OVS states of the Vport.

Setting a new friendly name was tested in the following states:
[1] The port is disconnected
[2] The port is connected but not OVS user mode counter port [3] The port is connected and an OCS port associated with it

Signed-off-by: Eitan Eliahu <eliahue at vmware.com>
---
 datapath-windows/ovsext/Oid.c   |  3 +++
 datapath-windows/ovsext/Vport.c | 40 ++++++++++++++++++++++++++++++++++++++++
 datapath-windows/ovsext/Vport.h |  2 ++
 3 files changed, 45 insertions(+)

diff --git a/datapath-windows/ovsext/Oid.c b/datapath-windows/ovsext/Oid.c index 9659af7..83fa1e3 100644
--- a/datapath-windows/ovsext/Oid.c
+++ b/datapath-windows/ovsext/Oid.c
@@ -171,6 +171,9 @@ OvsProcessSetOidPort(POVS_SWITCH_CONTEXT switchObject,
     case OID_SWITCH_PORT_CREATE:
         status = HvCreatePort(switchObject, portParam);
         break;
+    case OID_SWITCH_PORT_UPDATED:
+        status = HvUpdatePort(switchObject, portParam);
+       break;
     case OID_SWITCH_PORT_TEARDOWN:
         HvTeardownPort(switchObject, portParam);
         break;
diff --git a/datapath-windows/ovsext/Vport.c b/datapath-windows/ovsext/Vport.c index a46d49f..0beb9ef 100644
--- a/datapath-windows/ovsext/Vport.c
+++ b/datapath-windows/ovsext/Vport.c
@@ -102,6 +102,46 @@ create_port_done:
     return status;
 }

+
+/*
+ * Function updating the port properties  */ NDIS_STATUS 
+HvUpdatePort(POVS_SWITCH_CONTEXT switchContext,
+             PNDIS_SWITCH_PORT_PARAMETERS portParam) {
+    POVS_VPORT_ENTRY vport;
+    LOCK_STATE_EX lockState;
+    NDIS_STATUS status = NDIS_STATUS_SUCCESS;
+    OVS_VPORT_STATE ovsState;
+    NDIS_SWITCH_NIC_STATE nicState;
+
+    VPORT_PORT_ENTER(portParam);
+
+    NdisAcquireRWLockWrite(switchContext->dispatchLock, &lockState, 0);
+    vport = OvsFindVportByPortIdAndNicIndex(switchContext,
+                                            portParam->PortId, 0);

[ANKUR]: portParam->PortId is not correctly alligned. Function call should look like vport = OvsFindVportByPortIdAndNicIndex(switchContext,
                                                                        portParam->PortId, 0);

+    /* Update properties only for NETDEV ports*/
+    if (vport == NULL || vport->ovsType != OVS_VPORT_TYPE_NETDEV) {
+        status = STATUS_DATA_NOT_ACCEPTED;
+        goto update_port_done;
+    }
+
+    /* Store the nic and the OVS states as Nic Create won't be called */
+    ovsState = vport->ovsState;
+    nicState = vport->nicState;
+    /* Currently only the port friendly name is being updated */
+    OvsInitVportWithPortParam(vport, portParam);
+    /* Retore the nic and OVS states */
+    vport->nicState = nicState;
+    vport->ovsState = ovsState;

[ANKUR]: May be instead of calling OvsInitVportWithPortParam we can simply copy the new friendly name.

+update_port_done:
+    NdisReleaseRWLock(switchContext->dispatchLock, &lockState);
+    VPORT_PORT_EXIT(portParam);
+    return status;
+}
+
 VOID
 HvTeardownPort(POVS_SWITCH_CONTEXT switchContext,
                PNDIS_SWITCH_PORT_PARAMETERS portParam) diff --git a/datapath-windows/ovsext/Vport.h b/datapath-windows/ovsext/Vport.h index b4e4fca..ec7c21c 100644
--- a/datapath-windows/ovsext/Vport.h
+++ b/datapath-windows/ovsext/Vport.h
@@ -158,6 +158,8 @@ NDIS_STATUS HvCreateNic(POVS_SWITCH_CONTEXT switchContext,
                         PNDIS_SWITCH_NIC_PARAMETERS nicParam);  NDIS_STATUS HvCreatePort(POVS_SWITCH_CONTEXT switchContext,
                          PNDIS_SWITCH_PORT_PARAMETERS portParam);
+NDIS_STATUS HvUpdatePort(POVS_SWITCH_CONTEXT switchContext,
+                         PNDIS_SWITCH_PORT_PARAMETERS portParam);
 VOID HvTeardownPort(POVS_SWITCH_CONTEXT switchContext,
                     PNDIS_SWITCH_PORT_PARAMETERS portParam);  VOID HvDeletePort(POVS_SWITCH_CONTEXT switchContext,
--
1.9.4.msysgit.0

_______________________________________________
dev mailing list
dev at openvswitch.org
https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailman_listinfo_dev&d=AAIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=RDZsrBxhAiOewDSD-jiF-W03FqHevF2o7haW6eQzmtA&m=eBUCs2MIOCqN4sgR3DAvXeLj-AkaQl2zhi6pt76lkzc&s=TSiYWNMTcsAXFNDtMtnhd7TDx5LqGSRfs4nXJSd0zRQ&e=