[ovs-dev] [PATCH v1] datapath-windows Event read handler

Eitan Eliahu eliahue at vmware.com
Thu Sep 25 04:48:25 UTC 2014


Alin, this warning is used to warn about constant in a condition. We shouldn't have these cases at all. There are other parts in the code which uses the while(FALSE) condition.

OVS_USE_NL_INTERFACE is used to exclude code which used for the dpif device. There are many code segments that excluded. I suggest that we will remove all the dpif code in a different change.

OvsRemoveEventEntry is not exactly the opposite function of OvsPostEvent. 
OvsPostEvent insert an element to the queue but also triggers the event through completing the event pending IRP. 

Please let me know your thoughts.
Thanks,
Eitan  

-----Original Message-----
From: Alin Serdean [mailto:aserdean at cloudbasesolutions.com] 
Sent: Wednesday, September 24, 2014 9:13 PM
To: Eitan Eliahu; dev at openvswitch.org
Subject: RE: [ovs-dev] [PATCH v1] datapath-windows Event read handler

1. ovs-dev-v1-datapath-windows-Event-read-handler.patch:12: new blank line at EOF.
+
warning: 1 line adds whitespace errors.

2. OvsPostEvent adds elements to the queue can you please find a similar name for OvsRemoveEventEntry

3. Really don't  like that we have to push in:
+#pragma warning( push )
+#pragma warning( disable:4127 )

Just for
do {
} while (FALSE)
Could you please rewrite that part.

4. At first glance adding:
#if defined OVS_USE_NL_INTERFACE && OVS_USE_NL_INTERFACE == 0 Seems unnecessary just to mark probably a code for deletion.

Alin.

-----Mesaj original-----
De la: dev [mailto:dev-bounces at openvswitch.org] În numele Eitan Eliahu
Trimis: Thursday, September 25, 2014 12:58 PM
Către: dev at openvswitch.org
Subiect: [ovs-dev] [PATCH v1] datapath-windows Event read handler

The Read event handler is executed when user mode issues a socket receive on an MC socket associated with the event queue. A new IOCTL READ command is used to differentiate between transaction based and packet miss sockets.
An entry for the handler will be added once the Vport family implementation checked in.
User mode code for setting the socket type will follow

Signed-off-by: Eitan Eliahu <eliahue at vmware.com>
---
 build-aux/extract-odp-netlink-windows-dp-h   |   4 +
 datapath-windows/include/OvsDpInterfaceExt.h |  10 +-
 datapath-windows/ovsext/Datapath.c           | 162 ++++++++++++++++++++++++++-
 datapath-windows/ovsext/Event.c              |  42 +++++++
 datapath-windows/ovsext/Event.h              |   2 +
 5 files changed, 217 insertions(+), 3 deletions(-)

diff --git a/build-aux/extract-odp-netlink-windows-dp-h b/build-aux/extract-odp-netlink-windows-dp-h
index 70514cb..82d95f1 100755
--- a/build-aux/extract-odp-netlink-windows-dp-h
+++ b/build-aux/extract-odp-netlink-windows-dp-h
@@ -23,3 +23,7 @@ s,#include <linux/if_ether\.h>,\n#ifndef ETH_ADDR_LEN \
 
 # Use OVS's own ETH_ADDR_LEN instead of Linux-specific ETH_ALEN.
 s/ETH_ALEN/ETH_ADDR_LEN/
+
+s/OVS_VPORT_CMD_GET,/OVS_VPORT_CMD_GET,\
+	OVS_VPORT_CMD_NOTIFY,/
+
diff --git a/datapath-windows/include/OvsDpInterfaceExt.h b/datapath-windows/include/OvsDpInterfaceExt.h
index 64fd20e..be1e49f 100644
--- a/datapath-windows/include/OvsDpInterfaceExt.h
+++ b/datapath-windows/include/OvsDpInterfaceExt.h
@@ -34,11 +34,17 @@
 #define OVS_IOCTL_READ \
     CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x0, METHOD_OUT_DIRECT,\
               FILE_READ_ACCESS)
+#define OVS_IOCTL_READ_PACKET \
+    CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x1, METHOD_OUT_DIRECT, \
+              FILE_READ_ACCESS)
+#define OVS_IOCTL_READ_EVENT \
+    CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x2, METHOD_OUT_DIRECT, \
+              FILE_READ_ACCESS)
 #define OVS_IOCTL_WRITE \
-    CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x1, METHOD_IN_DIRECT,\
+    CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x3, 
+ METHOD_IN_DIRECT,\
               FILE_READ_ACCESS)
 #define OVS_IOCTL_TRANSACT \
-    CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x2, METHOD_OUT_DIRECT,\
+    CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x4, 
+ METHOD_OUT_DIRECT,\
               FILE_WRITE_ACCESS)
 
 /*
diff --git a/datapath-windows/ovsext/Datapath.c b/datapath-windows/ovsext/Datapath.c
index 0dfdd57..fa8fa64 100644
--- a/datapath-windows/ovsext/Datapath.c
+++ b/datapath-windows/ovsext/Datapath.c
@@ -22,6 +22,9 @@
  */
 #if defined OVS_USE_NL_INTERFACE && OVS_USE_NL_INTERFACE == 1
 
+#pragma warning( push )
+#pragma warning( disable:4127 )
+
 #include "precomp.h"
 #include "Datapath.h"
 #include "Jhash.h"
@@ -99,7 +102,8 @@ static NetlinkCmdHandler OvsGetPidCmdHandler,
                          OvsGetDpCmdHandler,
                          OvsPendEventCmdHandler,
                          OvsSubscribeEventCmdHandler,
-                         OvsSetDpCmdHandler;
+                         OvsSetDpCmdHandler,
+                         OvsReadEventCmdHandler;
 
 static NTSTATUS HandleGetDpTransaction(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
                                        UINT32 *replyLen); @@ -620,6 +624,30 @@ OvsDeviceControl(PDEVICE_OBJECT deviceObject,
         devOp = OVS_TRANSACTION_DEV_OP;
         break;
 
+    case OVS_IOCTL_READ_EVENT:
+        /* This IOCTL is used to read events */
+        if (outputBufferLen != 0) {
+            status = MapIrpOutputBuffer(irp, outputBufferLen,
+                sizeof *ovsMsg, &outputBuffer);
+            if (status != STATUS_SUCCESS) {
+                goto done;
+            }
+            ASSERT(outputBuffer);
+        }
+        else {
+            status = STATUS_NDIS_INVALID_LENGTH;
+            goto done;
+        }
+        inputBuffer = NULL;
+        inputBufferLen = 0;
+
+        ovsMsg = &ovsMsgReadOp;
+        ovsMsg->nlMsg.nlmsgType = OVS_WIN_NL_VPORT_FAMILY_ID;
+        /* An "artificial" command so we can use NL family function table*/
+        ovsMsg->genlMsg.cmd = OVS_VPORT_CMD_NOTIFY;
+        devOp = OVS_READ_DEV_OP;
+        break;
+
     case OVS_IOCTL_READ:
         /* Output buffer is mandatory. */
         if (outputBufferLen != 0) {
@@ -924,6 +952,7 @@ OvsPendEventCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
     return status;
 }
 
+
 /*
  * --------------------------------------------------------------------------
  *  Handler for the subscription for the event queue @@ -1214,4 +1243,135 @@ MapIrpOutputBuffer(PIRP irp,
     return STATUS_SUCCESS;
 }
 
+
+
+/*
+ *
+-----------------------------------------------------------------------
+---
+ * Utility function to fill up information about the state of a port in 
+a reply
+ * to* userspace.
+ * Assumes that 'gOvsCtrlLock' lock is acquired.
+ *
+-----------------------------------------------------------------------
+---
+ */
+static NTSTATUS
+OvsPortFillInfo(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
+                POVS_EVENT_ENTRY eventEntry,
+                PNL_BUFFER nlBuf)
+{
+    NTSTATUS status;
+    BOOLEAN rc;
+    OVS_MESSAGE msgOutTmp;
+    PNL_MSG_HDR nlMsg;
+    POVS_VPORT_ENTRY vport;
+
+    ASSERT(NlBufAt(nlBuf, 0, 0) != 0 && nlBuf->bufRemLen >= sizeof 
+ msgOutTmp);
+
+    msgOutTmp.nlMsg.nlmsgType = OVS_WIN_NL_VPORT_FAMILY_ID;
+    msgOutTmp.nlMsg.nlmsgFlags = 0;  /* XXX: ? */
+    msgOutTmp.nlMsg.nlmsgSeq = 0; /* driver intiated messages should have */
+    msgOutTmp.nlMsg.nlmsgPid = usrParamsCtx->ovsInstance->pid;
+
+    msgOutTmp.genlMsg.version = nlVportFamilyOps.version;
+    msgOutTmp.genlMsg.reserved = 0;
+
+    /* we don't have netdev yet, treat link up/down a adding/removing a port*/
+    if (eventEntry->status & (OVS_EVENT_LINK_UP | OVS_EVENT_CONNECT)) {
+        msgOutTmp.genlMsg.cmd = OVS_VPORT_CMD_NEW;
+    }
+    else if (eventEntry->status &
+             (OVS_EVENT_LINK_DOWN | OVS_EVENT_DISCONNECT)){
+        msgOutTmp.genlMsg.cmd = OVS_VPORT_CMD_DEL;
+    }
+    else {
+        ASSERT(FALSE);
+        return STATUS_UNSUCCESSFUL;
+    }
+    msgOutTmp.ovsHdr.dp_ifindex = gOvsSwitchContext->dpNo;
+
+    do {
+
+        rc = NlMsgPutHead(nlBuf, (PCHAR)&msgOutTmp, sizeof msgOutTmp);
+        if (!rc) {
+            status = STATUS_INVALID_BUFFER_SIZE;
+            break;
+        }
+
+        vport = OvsFindVportByPortNo(gOvsSwitchContext, eventEntry->portNo);
+        if (!vport) {
+            status = STATUS_DEVICE_DOES_NOT_EXIST;
+            break;
+        }
+
+        rc = NlMsgPutTailU32(nlBuf, OVS_VPORT_ATTR_PORT_NO, eventEntry->portNo) ||
+             NlMsgPutTailU32(nlBuf, OVS_VPORT_ATTR_TYPE, vport->ovsType) ||
+             NlMsgPutTailString(nlBuf, OVS_VPORT_ATTR_NAME, vport->ovsName);
+        if (!rc) {
+            status = STATUS_INVALID_BUFFER_SIZE;
+            break;
+        }
+
+        /* XXXX Should we add the port stats attributes?*/
+        nlMsg = (PNL_MSG_HDR)NlBufAt(nlBuf, 0, 0);
+        nlMsg->nlmsgLen = NlBufSize(nlBuf);
+        rc = STATUS_SUCCESS;
+    } while (FALSE);
+
+    return rc;
+}
+
+
+/*
+ *
+-----------------------------------------------------------------------
+---
+ * Handler for reading events from the driver event queue. This handler 
+is
+ * executed when user modes issues a socket receive on a socket 
+assocaited
+ * with the MC group for events.
+ * XXX user mode should read multiple events in one system call
+ *
+-----------------------------------------------------------------------
+---
+ */
+static NTSTATUS
+OvsReadEventCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
+                       UINT32 *replyLen) { #ifdef DBG
+    POVS_MESSAGE msgOut = (POVS_MESSAGE)usrParamsCtx->outputBuffer;
+    POVS_OPEN_INSTANCE instance =
+        (POVS_OPEN_INSTANCE)usrParamsCtx->ovsInstance;
+#endif
+    NL_BUFFER nlBuf;
+    NTSTATUS status;
+    OVS_EVENT_ENTRY eventEntry;
+
+    ASSERT(usrParamsCtx->devOp == OVS_READ_DEV_OP);
+
+    /* Should never read events with a dump socket */
+    ASSERT(instance->dumpState.ovsMsg == NULL);
+
+    /* Must have an event queue */
+    ASSERT(instance->eventQueue != NULL);
+
+    /* Output buffer has been validated while validating read dev op. */
+    ASSERT(msgOut != NULL && usrParamsCtx->outputLength >= sizeof 
+ *msgOut);
+
+    NlBufInit(&nlBuf, usrParamsCtx->outputBuffer,
+ usrParamsCtx->outputLength);
+
+    OvsAcquireCtrlLock();
+    if (!gOvsSwitchContext) {
+        status = STATUS_SUCCESS;
+        goto cleanup;
+    }
+
+    /* remove an event entry from the event queue */
+    status = OvsRemoveEventEntry(usrParamsCtx->ovsInstance, &eventEntry);
+    if (status != STATUS_SUCCESS) {
+        goto cleanup;
+    }
+
+    status = OvsPortFillInfo(usrParamsCtx, &eventEntry, &nlBuf);
+
+cleanup:
+    OvsReleaseCtrlLock();
+    *replyLen = NlBufSize(&nlBuf);;
+    return status;
+}
+
+#pragma warning( push )
 #endif /* OVS_USE_NL_INTERFACE */
diff --git a/datapath-windows/ovsext/Event.c b/datapath-windows/ovsext/Event.c index fec3485..467771d 100644
--- a/datapath-windows/ovsext/Event.c
+++ b/datapath-windows/ovsext/Event.c
@@ -292,6 +292,7 @@ done_event_subscribe:
     return status;
 }
 
+#if defined OVS_USE_NL_INTERFACE && OVS_USE_NL_INTERFACE == 0
 /*
  * --------------------------------------------------------------------------
  * Poll event queued in the event queue. always synchronous.
@@ -376,6 +377,7 @@ event_poll_done:
     OVS_LOG_TRACE("Exit: numEventPolled: %d", numEntry);
     return STATUS_SUCCESS;
 }
+#endif /* OVS_USE_NL_INTERFACE */
 
 
 /*
@@ -494,3 +496,43 @@ OvsWaitEventIoctl(PIRP irp,
     OVS_LOG_TRACE("Exit: return status: %#x", status);
     return status;
 }
+
+/*
+ 
+*----------------------------------------------------------------------
+----
+ * Poll event queued in the event queue.always synchronous.
+ *
+ * Results:
+ *     STATUS_SUCCESS event was dequeued
+ *     STATUS_UNSUCCESSFUL the queue is empty.
+ *
+-----------------------------------------------------------------------
+---
+ */
+NTSTATUS
+OvsRemoveEventEntry(POVS_OPEN_INSTANCE instance,
+                    POVS_EVENT_ENTRY entry) {
+    NTSTATUS status = STATUS_UNSUCCESSFUL;
+    POVS_EVENT_QUEUE queue;
+    POVS_EVENT_QUEUE_ELEM elem;
+
+    OvsAcquireEventQueueLock();
+
+    queue = (POVS_EVENT_QUEUE)instance->eventQueue;
+
+    if (queue == NULL) {
+        ASSERT(queue);
+        goto remove_event_done;
+    }
+
+    if (queue->numElems) {
+        elem = (POVS_EVENT_QUEUE_ELEM)RemoveHeadList(&queue->elemList);
+        entry->portNo = elem->portNo;
+        entry->status = elem->status;
+        OvsFreeMemory(elem);
+        queue->numElems--;
+        status = STATUS_SUCCESS;
+    }
+
+remove_event_done:
+    OvsReleaseEventQueueLock();
+    return status;
+}
diff --git a/datapath-windows/ovsext/Event.h b/datapath-windows/ovsext/Event.h index f4801b9..a43a0bb 100644
--- a/datapath-windows/ovsext/Event.h
+++ b/datapath-windows/ovsext/Event.h
@@ -47,4 +47,6 @@ NTSTATUS OvsPollEventIoctl(PFILE_OBJECT fileObject, PVOID inputBuffer,
                            UINT32 outputLength, UINT32 *replyLen);  NTSTATUS OvsWaitEventIoctl(PIRP irp, PFILE_OBJECT fileObject,
                            PVOID inputBuffer, UINT32 inputLength);
+NTSTATUS OvsRemoveEventEntry(PVOID instance, POVS_EVENT_ENTRY entry);
+
 #endif /* __EVENT_H_ */
--
1.9.4.msysgit.0

_______________________________________________
dev mailing list
dev at openvswitch.org
https://urldefense.proofpoint.com/v1/url?u=http://openvswitch.org/mailman/listinfo/dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=yTvML8OxA42Jb6ViHe7fUXbvPVOYDPVq87w43doxtlY%3D%0A&m=MU4xsiSuEqxx%2Bv4WfYKm0Cl0rVjWQbNQuqXGJ%2Bz04OE%3D%0A&s=7d8cff33d70e75c82739dbc6a9941f3d97e3d96992f62a4f516ed82eb49c99ba



More information about the dev mailing list