Hey Nithin,
I think this could work refactored a bit (such as to better separate commands),
but we should postpone refactor for a later time.
Acked-by: Samuel Ghinet <sghi...@cloudbasesolutions.com>
________________________________________
Date: Tue, 16 Sep 2014 19:06:11 -0700
From: Nithin Raju <nit...@vmware.com>
To: dev@openvswitch.org, sghi...@cloudbasesolutions.com,
elia...@vmware.com, ankursha...@vmware.com
Subject: [ovs-dev] [PATCH 5/5 v1] datapath-windows: add OVS_DP_CMD_SET
and OVS_DP_CMD_GET transaction support
Message-ID: <1410919571-34249-5-git-send-email-nit...@vmware.com>
In this patch, we add support for two commands, both of them are issued
as part of transactions semantics from userspace:
1. OVS_DP_CMD_SET is used to get the properties of a DP as well as set
some properties. The set operations does not seem to make much sense for
the Windows datpath right now.
2. There's already support for OVS_DP_CMD_GET command issued via the
dump semantics from userspace. Turns out that userspace can issue
OVS_DP_CMD_GET as a transaction.
There's lot of common code between these two commands. Hence combining
the implementation and the review.
Also refactories some of the code in the implementation of dump-based
OVS_DP_CMD_GET, and updated some of the comments.
Validation:
- With these series of patches, I was able to run the following command:
> .\utilities\ovs-dpctl.exe show
system@ovs-system:
lookups: hit:0 missed:22 lost:0
flows: 0
- I got so far as to hit the PORT_DUMP command which is currently not
implemented.
Signed-off-by: Nithin Raju <nit...@vmware.com>
Tested-by: Nithin Raju <nit...@vmware.com>
Reported-at: https://github.com/openvswitch/ovs-issues/issues/38
---
datapath-windows/ovsext/Datapath.c | 151 ++++++++++++++++++++++++++++++++++--
1 files changed, 143 insertions(+), 8 deletions(-)
diff --git a/datapath-windows/ovsext/Datapath.c
b/datapath-windows/ovsext/Datapath.c
index 2d1836c..25a1ea0 100644
--- a/datapath-windows/ovsext/Datapath.c
+++ b/datapath-windows/ovsext/Datapath.c
@@ -101,6 +101,16 @@ static NTSTATUS
OvsGetPidCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
static NTSTATUS OvsGetDpCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
UINT32 *replyLen);
+static NTSTATUS OvsSetDpCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
+ UINT32 *replyLen);
+
+static NTSTATUS HandleGetDpTransaction(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
+ UINT32 *replyLen);
+static NTSTATUS HandleGetDpDump(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
+ UINT32 *replyLen);
+static NTSTATUS HandleDpTransaction(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
+ UINT32 *replyLen);
+
/*
* The various netlink families, along with the supported commands. Most of
* these families and commands are part of the openvswitch specification for a
@@ -130,8 +140,15 @@ NETLINK_FAMILY nlControlFamilyOps = {
NETLINK_CMD nlDatapathFamilyCmdOps[] = {
{ .cmd = OVS_DP_CMD_GET,
.handler = OvsGetDpCmdHandler,
- .supportedDevOp = OVS_WRITE_DEV_OP | OVS_READ_DEV_OP,
+ .supportedDevOp = OVS_WRITE_DEV_OP | OVS_READ_DEV_OP |
+ OVS_TRANSACTION_DEV_OP,
.validateDpIndex = FALSE
+ },
+ { .cmd = OVS_DP_CMD_SET,
+ .handler = OvsSetDpCmdHandler,
+ .supportedDevOp = OVS_WRITE_DEV_OP | OVS_READ_DEV_OP |
+ OVS_TRANSACTION_DEV_OP,
+ .validateDpIndex = TRUE
}
};
@@ -705,8 +722,6 @@ done:
* --------------------------------------------------------------------------
* Function to validate a netlink command. Only certain combinations of
* (device operation, netlink family, command) are valid.
- *
- * XXX: Take policy into consideration.
* --------------------------------------------------------------------------
*/
static NTSTATUS
@@ -784,9 +799,10 @@ InvokeNetlinkCmdHandler(POVS_USER_PARAMS_CONTEXT
usrParamsCtx,
return status;
}
-
/*
* --------------------------------------------------------------------------
+ * Command Handler for 'OVS_CTRL_CMD_WIN_GET_PID'.
+ *
* Each handle on the device is assigned a unique PID when the handle is
* created. On platforms that support netlink natively, the PID is available
* to userspace when the netlink socket is created. However, without native
@@ -837,7 +853,7 @@ OvsDpFillInfo(POVS_SWITCH_CONTEXT ovsSwitchContext,
PNL_MSG_HDR nlMsg;
/* XXX: Add API for nlBuf->bufRemLen. */
- ASSERT(NlBufAt(nlBuf, 0, 0) != 0 && nlBuf->bufRemLen >= sizeof msgOutTmp);
+ ASSERT(NlBufAt(nlBuf, 0, 0) != 0 && nlBuf->bufRemLen >= sizeof *msgIn);
msgOutTmp.nlMsg.nlmsgType = OVS_WIN_NL_DATAPATH_FAMILY_ID;
msgOutTmp.nlMsg.nlmsgFlags = 0; /* XXX: ? */
@@ -871,17 +887,49 @@ OvsDpFillInfo(POVS_SWITCH_CONTEXT ovsSwitchContext,
return writeOk ? 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.
+ * Command Handler for 'OVS_DP_CMD_GET'.
+ *
+ * The function handles both the dump based as well as the transaction based
+ * 'OVS_DP_CMD_GET' command. In the dump command, it handles the initial
+ * call to setup dump state, as well as subsequent calls to continue dumping
+ * data.
* --------------------------------------------------------------------------
*/
static NTSTATUS
OvsGetDpCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
UINT32 *replyLen)
{
+ if (usrParamsCtx->devOp == OVS_TRANSACTION_DEV_OP) {
+ return HandleGetDpTransaction(usrParamsCtx, replyLen);
+ } else {
+ return HandleGetDpDump(usrParamsCtx, replyLen);
+ }
+}
+
+/*
+ * --------------------------------------------------------------------------
+ * Function for handling the transaction based 'OVS_DP_CMD_GET' command.
+ * --------------------------------------------------------------------------
+ */
+static NTSTATUS
+HandleGetDpTransaction(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
+ UINT32 *replyLen)
+{
+ return HandleDpTransaction(usrParamsCtx, replyLen);
+}
+
+
+/*
+ * --------------------------------------------------------------------------
+ * Function for handling the dump-based 'OVS_DP_CMD_GET' command.
+ * --------------------------------------------------------------------------
+ */
+static NTSTATUS
+HandleGetDpDump(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
+ UINT32 *replyLen)
+{
POVS_MESSAGE msgOut = (POVS_MESSAGE)usrParamsCtx->outputBuffer;
POVS_OPEN_INSTANCE instance =
(POVS_OPEN_INSTANCE)usrParamsCtx->ovsInstance;
@@ -937,6 +985,93 @@ OvsGetDpCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
return STATUS_SUCCESS;
}
+
+/*
+ * --------------------------------------------------------------------------
+ * Command Handler for 'OVS_DP_CMD_SET'.
+ * --------------------------------------------------------------------------
+ */
+static NTSTATUS
+OvsSetDpCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
+ UINT32 *replyLen)
+{
+ return HandleDpTransaction(usrParamsCtx, replyLen);
+}
+
+/*
+ * --------------------------------------------------------------------------
+ * Function for handling transaction based 'OVS_DP_CMD_GET' and
+ * 'OVS_DP_CMD_SET' commands.
+ * --------------------------------------------------------------------------
+ */
+static NTSTATUS
+HandleDpTransaction(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
+ UINT32 *replyLen)
+{
+ POVS_MESSAGE msgIn = (POVS_MESSAGE)usrParamsCtx->inputBuffer;
+ POVS_MESSAGE msgOut = (POVS_MESSAGE)usrParamsCtx->outputBuffer;
+ NTSTATUS status = STATUS_SUCCESS;
+ NL_BUFFER nlBuf;
+ static const NL_POLICY ovsDatapathSetPolicy[] = {
+ [OVS_DP_ATTR_NAME] = { .type = NL_A_STRING, .maxLen = IFNAMSIZ },
+ [OVS_DP_ATTR_UPCALL_PID] = { .type = NL_A_U32 },
+ [OVS_DP_ATTR_USER_FEATURES] = { .type = NL_A_U32 },
+ };
+ PNL_ATTR dpAttrs[ARRAY_SIZE(ovsDatapathSetPolicy)];
+
+ /* input buffer has been validated while validating write dev op. */
+ ASSERT(msgIn != NULL && usrParamsCtx->inputLength >= sizeof *msgIn);
+
+ /* Parse any attributes in the request. */
+ if (usrParamsCtx->ovsMsg->genlMsg.cmd == OVS_DP_CMD_SET) {
+ if (!NlAttrParse((PNL_MSG_HDR)msgIn,
+ NLMSG_HDRLEN + GENL_HDRLEN + OVS_HDRLEN,
+ ovsDatapathSetPolicy, dpAttrs, ARRAY_SIZE(dpAttrs))) {
+ return STATUS_INVALID_PARAMETER;
+ }
+
+ /*
+ * XXX: Not clear at this stage if there's any role for the
+ * OVS_DP_ATTR_UPCALL_PID and OVS_DP_ATTR_USER_FEATURES attributes
passed
+ * from userspace.
+ */
+
+ } else {
+ RtlZeroMemory(dpAttrs, sizeof dpAttrs);
+ }
+
+ /* Output buffer is optional for OVS_TRANSACTION_DEV_OP. */
+ if (msgOut == NULL || usrParamsCtx->outputLength < sizeof *msgOut) {
+ return STATUS_NDIS_INVALID_LENGTH;
+ }
+ NlBufInit(&nlBuf, usrParamsCtx->outputBuffer, usrParamsCtx->outputLength);
+
+ OvsAcquireCtrlLock();
+ if (dpAttrs[OVS_DP_ATTR_NAME] != NULL) {
+ if (!gOvsSwitchContext &&
+ !OvsCompareString(NlAttrGet(dpAttrs[OVS_DP_ATTR_NAME]),
+ NlAttrGetSize(dpAttrs[OVS_DP_ATTR_NAME]),
+ OVS_SYSTEM_DP_NAME, sizeof OVS_SYSTEM_DP_NAME)) {
+ OvsReleaseCtrlLock();
+ status = STATUS_NOT_FOUND;
+ goto cleanup;
+ }
+ } else if ((UINT32)msgIn->ovsHdr.dp_ifindex != gOvsSwitchContext->dpNo) {
+ OvsReleaseCtrlLock();
+ status = STATUS_NOT_FOUND;
+ goto cleanup;
+ }
+
+ status = OvsDpFillInfo(gOvsSwitchContext, msgIn, &nlBuf);
+ OvsReleaseCtrlLock();
+
+ *replyLen = NlBufSize(&nlBuf);
+
+cleanup:
+ return status;
+}
+
+
static NTSTATUS
OvsSetupDumpStart(POVS_USER_PARAMS_CONTEXT usrParamsCtx)
{
--
1.7.4.1
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
