At first on reading this code, I was wondering "why?"
Now I think I understand: the function zeroes memory, and must return the ptr
to 'next' item to be filled (which is called "tail"). Am I correct?
Also, I think it would be nice if you could rename NlBufCopyAtTailUninit ->
NlBufZeroAtTailUninit or smth similar, because it does not actually do a copy,
and the name makes it a bit confusing.
But perhaps this would also work as a separate patch.
Thanks,
Sam
________________________________________
Date: Fri, 12 Sep 2014 19:45:07 -0700
From: Nithin Raju <nit...@vmware.com>
To: dev@openvswitch.org, ankursha...@vmware.com
Subject: [ovs-dev] [PATCH 1/2] datapath-windows: fix bug in
NlBufCopyAtTailUninit
Message-ID: <1410576308-392-1-git-send-email-nit...@vmware.com>
We should be returning value of tail before the increment
and not after.
Signed-off-by: Nithin Raju <nit...@vmware.com>
---
datapath-windows/ovsext/Netlink/NetlinkBuf.c | 6 ++----
1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/datapath-windows/ovsext/Netlink/NetlinkBuf.c
b/datapath-windows/ovsext/Netlink/NetlinkBuf.c
index ba10d83..149bfb3 100644
--- a/datapath-windows/ovsext/Netlink/NetlinkBuf.c
+++ b/datapath-windows/ovsext/Netlink/NetlinkBuf.c
@@ -215,13 +215,11 @@ NlBufCopyAtTailUninit(PNL_BUFFER nlBuf, UINT32 len)
{
PCHAR ret = NULL;
+ ret = nlBuf->tail;
if ((NlBufCopyAtTail(nlBuf, NULL, len)) == FALSE) {
- goto done;
+ return NULL;
}
- ret = nlBuf->tail;
-
-done:
return ret;
}
--
1.7.4.1
------------------------------
Message: 4
Date: Fri, 12 Sep 2014 19:45:08 -0700
From: Nithin Raju <nit...@vmware.com>
To: dev@openvswitch.org, ankursha...@vmware.com
Subject: [ovs-dev] [PATCH 2/2] datapath-windows: use the Netlink set
API and need new APIs
Message-ID: <1410576308-392-2-git-send-email-nit...@vmware.com>
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.
Signed-off-by: Nithin Raju <nit...@vmware.com>
---
datapath-windows/ovsext/Datapath.c | 123 +++++++++++++++++++-----------------
1 files changed, 65 insertions(+), 58 deletions(-)
diff --git a/datapath-windows/ovsext/Datapath.c
b/datapath-windows/ovsext/Datapath.c
index c145d00..c0f0a04 100644
--- a/datapath-windows/ovsext/Datapath.c
+++ b/datapath-windows/ovsext/Datapath.c
@@ -821,6 +821,58 @@ OvsGetPidCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
/*
* --------------------------------------------------------------------------
+ * 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 nlWrite;
+ OVS_MESSAGE msgOutTmp;
+ struct ovs_dp_stats dpStats;
+ OVS_DATAPATH *datapath = &ovsSwitchContext->datapath;
+ PNL_MSG_HDR nlMsg;
+
+ /* XXX: Add API for nlBuf->bufRemLen. */
+ ASSERT(NlBufAt(nlBuf, 0, 0) != 0 && nlBuf->bufRemLen >= sizeof *msgIn);
+
+ 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;
+
+ nlWrite = NlMsgPutHead(nlBuf, (PCHAR)&msgOutTmp, sizeof msgOutTmp);
+ if (nlWrite) {
+ nlWrite = NlMsgPutTailString(nlBuf, OVS_DP_ATTR_NAME,
+ OVS_SYSTEM_DP_NAME);
+ }
+ if (nlWrite) {
+ dpStats.n_hit = datapath->hits;
+ dpStats.n_missed = datapath->misses;
+ dpStats.n_lost = datapath->lost;
+ dpStats.n_flows = datapath->nFlows;
+ nlWrite = NlMsgPutTailUnspec(nlBuf, OVS_DP_ATTR_STATS,
+ (PCHAR)&dpStats, sizeof dpStats);
+ }
+ nlMsg = (PNL_MSG_HDR)NlBufAt(nlBuf, 0, 0);
+ nlMsg->nlmsgLen = NlBufSize(nlBuf);
+
+ return nlWrite ? STATUS_SUCCESS : STATUS_INVALID_BUFFER_SIZE;
+}
+
+
+/*
+ * --------------------------------------------------------------------------
* Handler for the get dp command. The function handles the initial call to
* setup the dump state, as well as subsequent calls to continue dumping data.
* --------------------------------------------------------------------------
@@ -829,7 +881,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 +889,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,73 +905,25 @@ 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;
--
1.7.4.1
------------------------------
Subject: Digest Footer
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
------------------------------
End of dev Digest, Vol 62, Issue 189
************************************
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
