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 <svintu...@cloudbasesolutions.com> Reported-by: Alin Gabriel Serdean <aserd...@cloudbasesolutions.com> Reported-at: https://github.com/openvswitch/ovs-issues/issues/31 --- datapath-windows/include/OvsDpInterfaceExt.h | 18 ++++---- datapath-windows/ovsext/Datapath.c | 61 ++++++++++++++++------------ datapath-windows/ovsext/Flow.c | 2 +- lib/netlink-socket.c | 56 +++++-------------------- 4 files changed, 56 insertions(+), 81 deletions(-) diff --git a/datapath-windows/include/OvsDpInterfaceExt.h b/datapath-windows/include/OvsDpInterfaceExt.h index 7e09caf..e235376 100644 --- a/datapath-windows/include/OvsDpInterfaceExt.h +++ b/datapath-windows/include/OvsDpInterfaceExt.h @@ -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) /* @@ -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, diff --git a/datapath-windows/ovsext/Datapath.c b/datapath-windows/ovsext/Datapath.c index 888c6ef..35f26b2 100644 --- a/datapath-windows/ovsext/Datapath.c +++ b/datapath-windows/ovsext/Datapath.c @@ -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, @@ -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 @@ -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, @@ -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) { @@ -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; @@ -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, +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; } /* diff --git a/datapath-windows/ovsext/Flow.c b/datapath-windows/ovsext/Flow.c index 97220b4..f25fe9a 100644 --- a/datapath-windows/ovsext/Flow.c +++ b/datapath-windows/ovsext/Flow.c @@ -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; } diff --git a/lib/netlink-socket.c b/lib/netlink-socket.c index df889d3..fab2a66 100644 --- a/lib/netlink-socket.c +++ b/lib/netlink-socket.c @@ -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 */ -- 1.9.0.msysgit.0 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
