datapath-windows: use the Netlink set API and need new APIs

In this change:
1. we refactor the code that fills up information about the DP into
a seprate function.
2. use the netlink set APIs to fill up the netlink attributes.
3. we define a OVS_DP_STATS to be a typedef of 'struct ovs_dp_stats'
in keeping with the Windows kernel naming conventions.
4. In the absence of netlink set API, I had put in an ASSERT earlier
that the output buffer should be limited to 512 bytes. This is not
true anymore. The netlink set API checks for bounds of the buffer.
Hence removed the ASSERT.

Signed-off-by: Nithin Raju <nithin@vmware.com>
Acked-by: Ankur Sharma <ankursharma@vmware.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>

diff --git a/datapath-windows/include/OvsDpInterfaceExt.h b/datapath-windows/include/OvsDpInterfaceExt.h
index 877b2ad..64fd20e 100644 (file)
--- a/datapath-windows/include/OvsDpInterfaceExt.h
+++ b/datapath-windows/include/OvsDpInterfaceExt.h
@@ -80,4 +80,6 @@ enum ovs_nl_mcast_attr {
     OVS_NL_ATTR_MCAST_JOIN,  /* (UINT8) 1/0 - Join/Unjoin */
 };
 
+typedef struct ovs_dp_stats OVS_DP_STATS;
+
 #endif /* __OVS_DP_INTERFACE_EXT_H_ */

diff --git a/datapath-windows/ovsext/Datapath.c b/datapath-windows/ovsext/Datapath.c
index c145d00..2d1836c 100644 (file)
--- a/datapath-windows/ovsext/Datapath.c
+++ b/datapath-windows/ovsext/Datapath.c
@@ -819,6 +819,59 @@ OvsGetPidCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
     return STATUS_SUCCESS;
 }
 
+/*
+ * --------------------------------------------------------------------------
+ * Utility function to fill up information about the datapath in a reply to
+ * userspace.
+ * Assumes that 'gOvsCtrlLock' lock is acquired.
+ * --------------------------------------------------------------------------
+ */
+static NTSTATUS
+OvsDpFillInfo(POVS_SWITCH_CONTEXT ovsSwitchContext,
+              POVS_MESSAGE msgIn,
+              PNL_BUFFER nlBuf)
+{
+    BOOLEAN writeOk;
+    OVS_MESSAGE msgOutTmp;
+    OVS_DATAPATH *datapath = &ovsSwitchContext->datapath;
+    PNL_MSG_HDR nlMsg;
+
+    /* XXX: Add API for nlBuf->bufRemLen. */
+    ASSERT(NlBufAt(nlBuf, 0, 0) != 0 && nlBuf->bufRemLen >= sizeof msgOutTmp);
+
+    msgOutTmp.nlMsg.nlmsgType = OVS_WIN_NL_DATAPATH_FAMILY_ID;
+    msgOutTmp.nlMsg.nlmsgFlags = 0;  /* XXX: ? */
+    msgOutTmp.nlMsg.nlmsgSeq = msgIn->nlMsg.nlmsgSeq;
+    msgOutTmp.nlMsg.nlmsgPid = msgIn->nlMsg.nlmsgPid;
+
+    msgOutTmp.genlMsg.cmd = OVS_DP_CMD_GET;
+    msgOutTmp.genlMsg.version = nlDatapathFamilyOps.version;
+    msgOutTmp.genlMsg.reserved = 0;
+
+    msgOutTmp.ovsHdr.dp_ifindex = ovsSwitchContext->dpNo;
+
+    writeOk = NlMsgPutHead(nlBuf, (PCHAR)&msgOutTmp, sizeof msgOutTmp);
+    if (writeOk) {
+        writeOk = NlMsgPutTailString(nlBuf, OVS_DP_ATTR_NAME,
+                                     OVS_SYSTEM_DP_NAME);
+    }
+    if (writeOk) {
+        OVS_DP_STATS dpStats;
+
+        dpStats.n_hit = datapath->hits;
+        dpStats.n_missed = datapath->misses;
+        dpStats.n_lost = datapath->lost;
+        dpStats.n_flows = datapath->nFlows;
+        writeOk = NlMsgPutTailUnspec(nlBuf, OVS_DP_ATTR_STATS,
+                                     (PCHAR)&dpStats, sizeof dpStats);
+    }
+    nlMsg = (PNL_MSG_HDR)NlBufAt(nlBuf, 0, 0);
+    nlMsg->nlmsgLen = NlBufSize(nlBuf);
+
+    return writeOk ? STATUS_SUCCESS : STATUS_INVALID_BUFFER_SIZE;
+}
+
+
 /*
  * --------------------------------------------------------------------------
  *  Handler for the get dp command. The function handles the initial call to
@@ -829,7 +882,6 @@ static NTSTATUS
 OvsGetDpCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
                    UINT32 *replyLen)
 {
-    POVS_MESSAGE msgIn = (POVS_MESSAGE)usrParamsCtx->inputBuffer;
     POVS_MESSAGE msgOut = (POVS_MESSAGE)usrParamsCtx->outputBuffer;
     POVS_OPEN_INSTANCE instance =
         (POVS_OPEN_INSTANCE)usrParamsCtx->ovsInstance;
@@ -838,6 +890,10 @@ OvsGetDpCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
         *replyLen = 0;
         OvsSetupDumpStart(usrParamsCtx);
     } else {
+        NL_BUFFER nlBuf;
+        NTSTATUS status;
+        POVS_MESSAGE msgIn = instance->dumpState.ovsMsg;
+
         ASSERT(usrParamsCtx->devOp == OVS_READ_DEV_OP);
 
         if (instance->dumpState.ovsMsg == NULL) {
@@ -850,78 +906,29 @@ OvsGetDpCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
         /* Output buffer has been validated while validating read dev op. */
         ASSERT(msgOut != NULL && usrParamsCtx->outputLength >= sizeof *msgOut);
 
-        /* XXX: Replace this with the netlink set API. */
-        msgIn = instance->dumpState.ovsMsg;
-        msgOut->nlMsg.nlmsgType = OVS_WIN_NL_DATAPATH_FAMILY_ID;
-        msgOut->nlMsg.nlmsgFlags = 0;  /* XXX: ? */
-        msgOut->nlMsg.nlmsgSeq = msgIn->nlMsg.nlmsgSeq;
-        msgOut->nlMsg.nlmsgPid = msgIn->nlMsg.nlmsgPid;
-        msgOut->nlMsg.nlmsgLen = sizeof *msgOut;
-
-        msgOut->genlMsg.cmd = OVS_DP_CMD_GET;
-        msgOut->genlMsg.version = nlDatapathFamilyOps.version;
-        msgOut->genlMsg.reserved = 0;
+        NlBufInit(&nlBuf, usrParamsCtx->outputBuffer,
+                  usrParamsCtx->outputLength);
 
         OvsAcquireCtrlLock();
-        if (gOvsSwitchContext) {
-            msgOut->ovsHdr.dp_ifindex = gOvsSwitchContext->dpNo;
-        } else {
+        if (!gOvsSwitchContext) {
             /* Treat this as a dump done. */
             OvsReleaseCtrlLock();
             *replyLen = 0;
             FreeUserDumpState(instance);
             return STATUS_SUCCESS;
         }
+        status = OvsDpFillInfo(gOvsSwitchContext, msgIn, &nlBuf);
+        OvsReleaseCtrlLock();
 
-        /* XXX: Replace this with the netlink set API. */
-        {
-            /*
-             * Assume that the output buffer is at least 512 bytes. Once the
-             * netlink set API is implemented, the netlink buffer manipulation
-             * API should provide for checking the remaining number of bytes as
-             * we write out the message.
-             */
-            if (usrParamsCtx->outputLength <= 512) {
-                OvsReleaseCtrlLock();
-                return STATUS_NDIS_INVALID_LENGTH;
-            }
-            UINT16 attrTotalSize = 0;
-            UINT16 attrSize = 0;
-            PNL_ATTR nlAttr = (PNL_ATTR) ((PCHAR)msgOut + sizeof (*msgOut));
-            struct ovs_dp_stats *dpStats;
-            OVS_DATAPATH *datapath = &gOvsSwitchContext->datapath;
-
-            nlAttr->nlaType = OVS_DP_ATTR_NAME;
-            /* 'nlaLen' must not include the pad. */
-            nlAttr->nlaLen = sizeof (*nlAttr) + sizeof (OVS_SYSTEM_DP_NAME);
-            attrSize = sizeof (*nlAttr) +
-                       NLA_ALIGN(sizeof (OVS_SYSTEM_DP_NAME));
-            attrTotalSize += attrSize;
-
-            PNL_ATTR nlAttrNext = NlAttrGet(nlAttr);
-            RtlCopyMemory((PCHAR)nlAttrNext, OVS_SYSTEM_DP_NAME,
-                          sizeof (OVS_SYSTEM_DP_NAME));
-
-            nlAttr = (PNL_ATTR)((PCHAR)nlAttr + attrSize);
-            nlAttr->nlaType = OVS_DP_ATTR_STATS;
-            nlAttr->nlaLen = sizeof (*nlAttr) + sizeof (*dpStats);
-            attrSize = sizeof (*nlAttr) + NLA_ALIGN(sizeof (*dpStats));
-            attrTotalSize += attrSize;
-
-            dpStats = NlAttrGet(nlAttr);
-            dpStats->n_hit = datapath->hits;
-            dpStats->n_missed = datapath->misses;
-            dpStats->n_lost = datapath->lost;
-            dpStats->n_flows = datapath->nFlows;
-
-            msgOut->nlMsg.nlmsgLen += attrTotalSize;
+        if (status != STATUS_SUCCESS) {
+            *replyLen = 0;
+            FreeUserDumpState(instance);
+            return status;
         }
-        OvsReleaseCtrlLock();
 
         /* Increment the dump index. */
         instance->dumpState.index[0] = 1;
         *replyLen = msgOut->nlMsg.nlmsgLen;
-        ASSERT(*replyLen <= 512);
 
         /* Free up the dump state, since there's no more data to continue. */
         FreeUserDumpState(instance);