Skip to content

Commit

Permalink
datapath-windows: Make GET_PID a separate IOCTL
Browse files Browse the repository at this point in the history
Added a new IOCTL in order to retrieve the PID from the kernel datapath.
The new method uses a direct and cleaner way, as opposed to the old way
of using a Netlink transaction, avoiding the unnecessary overhead.

Signed-off-by: Sorin Vinturis <svinturis@cloudbasesolutions.com>
Reported-by: Alin Gabriel Serdean <aserdean@cloudbasesolutions.com>
Reported-at: openvswitch/ovs-issues#31
Acked-by: Nithin Raju <nithin@vmware.com>
Acked-by: Alin Gabriel Serdean <aserdean@cloudbasesolutions.com>
Tested-by: Alin Gabriel Serdean <aserdean@cloudbasesolutions.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
  • Loading branch information
svinturis authored and blp committed Apr 2, 2015
1 parent b3cceba commit 190cf53
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 82 deletions.
18 changes: 11 additions & 7 deletions datapath-windows/include/OvsDpInterfaceExt.h
Expand Up @@ -29,22 +29,27 @@

#define OVS_IOCTL_DEVICE_TYPE 45000

/* We used Direct I/O (zero copy) for the buffers. */
#define OVS_IOCTL_START 0x100
/* We used Direct I/O (zero copy) for the buffers. */
/* Non-Netlink-based IOCTLs. */
#define OVS_IOCTL_GET_PID \
CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x0, METHOD_BUFFERED,\
FILE_WRITE_ACCESS)
/* Netlink-based IOCTLs. */
#define OVS_IOCTL_READ \
CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x0, METHOD_OUT_DIRECT,\
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 + 0x1, METHOD_OUT_DIRECT, \
CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x2, METHOD_OUT_DIRECT, \
FILE_READ_ACCESS)
#define OVS_IOCTL_READ_PACKET \
CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x2, METHOD_OUT_DIRECT, \
CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x3, METHOD_OUT_DIRECT, \
FILE_READ_ACCESS)
#define OVS_IOCTL_WRITE \
CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x3, METHOD_IN_DIRECT,\
CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x4, METHOD_IN_DIRECT,\
FILE_READ_ACCESS)
#define OVS_IOCTL_TRANSACT \
CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x4, METHOD_OUT_DIRECT,\
CTL_CODE (OVS_IOCTL_DEVICE_TYPE, OVS_IOCTL_START + 0x5, METHOD_OUT_DIRECT,\
FILE_WRITE_ACCESS)

/*
Expand Down Expand Up @@ -75,7 +80,6 @@

/* Commands available under the OVS_WIN_CONTROL_FAMILY. */
enum ovs_win_control_cmd {
OVS_CTRL_CMD_WIN_GET_PID,
OVS_CTRL_CMD_WIN_PEND_REQ,
OVS_CTRL_CMD_WIN_PEND_PACKET_REQ,
OVS_CTRL_CMD_MC_SUBSCRIBE_REQ,
Expand Down
63 changes: 35 additions & 28 deletions datapath-windows/ovsext/Datapath.c
Expand Up @@ -87,8 +87,7 @@ typedef struct _NETLINK_FAMILY {
} NETLINK_FAMILY, *PNETLINK_FAMILY;

/* Handlers for the various netlink commands. */
static NetlinkCmdHandler OvsGetPidCmdHandler,
OvsPendEventCmdHandler,
static NetlinkCmdHandler OvsPendEventCmdHandler,
OvsPendPacketCmdHandler,
OvsSubscribeEventCmdHandler,
OvsSubscribePacketCmdHandler,
Expand All @@ -110,6 +109,8 @@ static NTSTATUS HandleGetDpDump(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
UINT32 *replyLen);
static NTSTATUS HandleDpTransactionCommon(
POVS_USER_PARAMS_CONTEXT usrParamsCtx, UINT32 *replyLen);
static NTSTATUS OvsGetPidHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
UINT32 *replyLen);

/*
* The various netlink families, along with the supported commands. Most of
Expand All @@ -120,11 +121,6 @@ static NTSTATUS HandleDpTransactionCommon(

/* Netlink control family: this is a Windows specific family. */
NETLINK_CMD nlControlFamilyCmdOps[] = {
{ .cmd = OVS_CTRL_CMD_WIN_GET_PID,
.handler = OvsGetPidCmdHandler,
.supportedDevOp = OVS_TRANSACTION_DEV_OP,
.validateDpIndex = FALSE,
},
{ .cmd = OVS_CTRL_CMD_WIN_PEND_REQ,
.handler = OvsPendEventCmdHandler,
.supportedDevOp = OVS_WRITE_DEV_OP,
Expand Down Expand Up @@ -736,6 +732,24 @@ OvsDeviceControl(PDEVICE_OBJECT deviceObject,
* operation.
*/
switch (code) {
case OVS_IOCTL_GET_PID:
/* Both input buffer and output buffer use the same location. */
outputBuffer = irp->AssociatedIrp.SystemBuffer;
if (outputBufferLen != 0) {
InitUserParamsCtx(irp, instance, 0, NULL,
inputBuffer, inputBufferLen,
outputBuffer, outputBufferLen,
&usrParamsCtx);

ASSERT(outputBuffer);
} else {
status = STATUS_NDIS_INVALID_LENGTH;
goto done;
}

status = OvsGetPidHandler(&usrParamsCtx, &replyLen);
goto done;

case OVS_IOCTL_TRANSACT:
/* Both input buffer and output buffer are mandatory. */
if (outputBufferLen != 0) {
Expand Down Expand Up @@ -947,11 +961,9 @@ ValidateNetlinkCmd(UINT32 devOp,
}

/* Validate the PID. */
if (ovsMsg->genlMsg.cmd != OVS_CTRL_CMD_WIN_GET_PID) {
if (ovsMsg->nlMsg.nlmsgPid != instance->pid) {
status = STATUS_INVALID_PARAMETER;
goto done;
}
if (ovsMsg->nlMsg.nlmsgPid != instance->pid) {
status = STATUS_INVALID_PARAMETER;
goto done;
}

status = STATUS_SUCCESS;
Expand Down Expand Up @@ -992,38 +1004,33 @@ InvokeNetlinkCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,

/*
* --------------------------------------------------------------------------
* Command Handler for 'OVS_CTRL_CMD_WIN_GET_PID'.
* Handler for 'OVS_IOCTL_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
* netlink support on Windows, OVS datapath generates the PID and lets the
* userspace query it.
*
* This function implements the query.
* created. This function passes the PID to userspace using METHOD_BUFFERED
* method.
* --------------------------------------------------------------------------
*/
static NTSTATUS
OvsGetPidCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
UINT32 *replyLen)
OvsGetPidHandler(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;
PUINT32 msgOut = (PUINT32)usrParamsCtx->outputBuffer;

if (usrParamsCtx->outputLength >= sizeof *msgOut) {
POVS_OPEN_INSTANCE instance =
(POVS_OPEN_INSTANCE)usrParamsCtx->ovsInstance;

RtlZeroMemory(msgOut, sizeof *msgOut);
msgOut->nlMsg.nlmsgSeq = msgIn->nlMsg.nlmsgSeq;
msgOut->nlMsg.nlmsgPid = instance->pid;
RtlCopyMemory(msgOut, &instance->pid, sizeof(*msgOut));
*replyLen = sizeof *msgOut;
/* XXX: We might need to return the DP index as well. */
} else {
return STATUS_NDIS_INVALID_LENGTH;
*replyLen = sizeof *msgOut;
status = STATUS_NDIS_INVALID_LENGTH;
}

return STATUS_SUCCESS;
return status;
}

/*
Expand Down
2 changes: 1 addition & 1 deletion datapath-windows/ovsext/Flow.c
Expand Up @@ -319,7 +319,7 @@ OvsFlowNlCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
rc = OvsPutFlowIoctl(&mappedFlow, sizeof (struct OvsFlowPut),
&stats);
if (rc != STATUS_SUCCESS) {
OVS_LOG_ERROR("OvsFlowPut failed.");
OVS_LOG_ERROR("OvsPutFlowIoctl failed.");
goto done;
}

Expand Down
56 changes: 10 additions & 46 deletions lib/netlink-socket.c
Expand Up @@ -273,63 +273,27 @@ nl_sock_destroy(struct nl_sock *sock)

#ifdef _WIN32
/* Reads the pid for 'sock' generated in the kernel datapath. The function
* follows a transaction semantic. Eventually this function should call into
* nl_transact. */
* uses a separate IOCTL instead of a transaction semantic to avoid unnecessary
* message overhead. */
static int
get_sock_pid_from_kernel(struct nl_sock *sock)
{
struct nl_transaction txn;
struct ofpbuf request;
uint64_t request_stub[128];
struct ofpbuf reply;
uint64_t reply_stub[128];
struct ovs_header *ovs_header;
struct nlmsghdr *nlmsg;
uint32_t seq;
int retval;
DWORD bytes;
int ovs_msg_size = sizeof (struct nlmsghdr) + sizeof (struct genlmsghdr) +
sizeof (struct ovs_header);

ofpbuf_use_stub(&request, request_stub, sizeof request_stub);
txn.request = &request;
ofpbuf_use_stub(&reply, reply_stub, sizeof reply_stub);
txn.reply = &reply;

seq = nl_sock_allocate_seq(sock, 1);
nl_msg_put_genlmsghdr(&request, 0, OVS_WIN_NL_CTRL_FAMILY_ID, 0,
OVS_CTRL_CMD_WIN_GET_PID, OVS_WIN_CONTROL_VERSION);
nlmsg = nl_msg_nlmsghdr(txn.request);
nlmsg->nlmsg_seq = seq;

ovs_header = ofpbuf_put_uninit(&request, sizeof *ovs_header);
ovs_header->dp_ifindex = 0;
ovs_header = ofpbuf_put_uninit(&reply, ovs_msg_size);
uint32_t pid = 0;
int retval = 0;
DWORD bytes = 0;

if (!DeviceIoControl(sock->handle, OVS_IOCTL_TRANSACT,
txn.request->data, txn.request->size,
txn.reply->data, txn.reply->size,
if (!DeviceIoControl(sock->handle, OVS_IOCTL_GET_PID,
NULL, 0, &pid, sizeof(pid),
&bytes, NULL)) {
retval = EINVAL;
goto done;
} else {
if (bytes < ovs_msg_size) {
if (bytes < sizeof(pid)) {
retval = EINVAL;
goto done;
}

nlmsg = nl_msg_nlmsghdr(txn.reply);
if (nlmsg->nlmsg_seq != seq) {
retval = EINVAL;
goto done;
} else {
sock->pid = pid;
}
sock->pid = nlmsg->nlmsg_pid;
}
retval = 0;

done:
ofpbuf_uninit(&request);
ofpbuf_uninit(&reply);
return retval;
}
#endif /* _WIN32 */
Expand Down

0 comments on commit 190cf53

Please sign in to comment.