Acked-by: Nithin Raju <nit...@vmware.com>

-----Original Message-----
From: Sairam Venugopal <vsai...@vmware.com>
Date: Tuesday, July 12, 2016 at 2:41 PM
To: Nithin Raju <nit...@vmware.com>
Subject: FW: [PATCH v3 1/2] Windows: Add support for handling protocol
(netlink family)

>
>
>On 7/11/16, 2:59 PM, "Sairam Venugopal" <vsai...@vmware.com> wrote:
>
>>Windows datapath currently has no notion of netlink family.
>>It assumes all netlink messages to belong to NETLINK_GENERIC family.
>>This patch adds support for handling other protocols if the userspace
>>sends it down to kernel.
>>
>>This patch introduces a new NETLINK_CMD - OVS_CTRL_CMD_SOCK_PROP to
>>manage
>>all properties associated with a socket. The properties are passed down
>>as
>>netlink message attributes. This makes it easier to introduce other
>>properties in the future.
>>
>>v2: Addressed review comments from Nithin Raju <nit...@vmware.com>
>>
>>Signed-off-by: Sairam Venugopal <vsai...@vmware.com>
>>Acked-by: Nithin Raju <nit...@vmware.com>
>>---
>> datapath-windows/include/OvsDpInterfaceExt.h |  17 ++++-
>> datapath-windows/ovsext/Datapath.c           | 104
>>++++++++++++++++++++++++++-
>> datapath-windows/ovsext/Datapath.h           |   1 +
>> lib/netlink-socket.c                         |  75 +++++++++++++++++++
>> 4 files changed, 194 insertions(+), 3 deletions(-)
>>
>>diff --git a/datapath-windows/include/OvsDpInterfaceExt.h
>>b/datapath-windows/include/OvsDpInterfaceExt.h
>>index 8850535..db91c3e 100644
>>--- a/datapath-windows/include/OvsDpInterfaceExt.h
>>+++ b/datapath-windows/include/OvsDpInterfaceExt.h
>>@@ -92,9 +92,12 @@ enum ovs_win_control_cmd {
>>     OVS_CTRL_CMD_MC_SUBSCRIBE_REQ,
>>     OVS_CTRL_CMD_PACKET_SUBSCRIBE_REQ,
>> 
>>-    /* This command is logically belong to the Vport family */
>>+    /* This command logically belongs to the Vport family */
>>     OVS_CTRL_CMD_EVENT_NOTIFY,
>>-    OVS_CTRL_CMD_READ_NOTIFY
>>+    OVS_CTRL_CMD_READ_NOTIFY,
>>+
>>+    /* Used for Socket property */
>>+    OVS_CTRL_CMD_SOCK_PROP
>> };
>> 
>> /* NL Attributes for joining/unjoining an MC group */
>>@@ -163,4 +166,14 @@ enum ovs_win_netdev_attr {
>> typedef struct ovs_dp_stats OVS_DP_STATS;
>> typedef enum ovs_vport_type OVS_VPORT_TYPE;
>> 
>>+/* NL Attributes for setting socket attributes */
>>+enum ovs_nl_sock_attr {
>>+    /* (UINT32) Netlink Protocol set in Userspace and read in Kernel */
>>+    OVS_NL_ATTR_SOCK_PROTO,
>>+    /* (UINT32) Instance PID set in Kernel and read in Userspace */
>>+    OVS_NL_ATTR_SOCK_PID,
>>+    __OVS_NL_ATTR_SOCK_MAX
>>+};
>>+#define OVS_WIN_SOCK_ATTR_MAX (__OVS_NL_ATTR_SOCK_MAX - 1)
>>+
>> #endif /* __OVS_DP_INTERFACE_EXT_H_ */
>>diff --git a/datapath-windows/ovsext/Datapath.c
>>b/datapath-windows/ovsext/Datapath.c
>>index 16d58ef..4f47be5 100644
>>--- a/datapath-windows/ovsext/Datapath.c
>>+++ b/datapath-windows/ovsext/Datapath.c
>>@@ -87,7 +87,8 @@ static NetlinkCmdHandler OvsPendEventCmdHandler,
>>                          OvsReadEventCmdHandler,
>>                          OvsNewDpCmdHandler,
>>                          OvsGetDpCmdHandler,
>>-                         OvsSetDpCmdHandler;
>>+                         OvsSetDpCmdHandler,
>>+                         OvsSockPropCmdHandler;
>> 
>> NetlinkCmdHandler        OvsGetNetdevCmdHandler,
>>                          OvsGetVportCmdHandler,
>>@@ -147,6 +148,11 @@ NETLINK_CMD nlControlFamilyCmdOps[] = {
>>       .handler = OvsReadPacketCmdHandler,
>>       .supportedDevOp = OVS_READ_DEV_OP,
>>       .validateDpIndex = FALSE,
>>+    },
>>+    { .cmd = OVS_CTRL_CMD_SOCK_PROP,
>>+      .handler = OvsSockPropCmdHandler,
>>+      .supportedDevOp = OVS_TRANSACTION_DEV_OP,
>>+      .validateDpIndex = FALSE,
>>     }
>> };
>> 
>>@@ -1685,3 +1691,99 @@ OvsReadEventCmdHandler(POVS_USER_PARAMS_CONTEXT
>>usrParamsCtx,
>> cleanup:
>>     return status;
>> }
>>+
>>+/*
>>+ * 
>>-------------------------------------------------------------------------
>>-
>>+ *  Command Handler for 'OVS_CTRL_CMD_SOCK_PROP'.
>>+ *
>>+ *  Handler to set and verify socket properties between userspace and
>>kernel.
>>+ *
>>+ *  Protocol is passed down by the userspace. It refers to the NETLINK
>>family
>>+ *  and could be of different types (NETLINK_GENERIC/NETLINK_NETFILTER
>>etc.,)
>>+ *  This function parses out the protocol and adds it to the open
>>instance.
>>+ *
>>+ *  PID is generated by the kernel and is set in userspace after
>>querying the
>>+ *  kernel for it. This function does not modify PID set in the kernel,
>>+ *  instead it verifies if it was sent down correctly.
>>+ *
>>+ *  XXX -This method can be modified to handle all Socket properties
>>thereby
>>+ *  eliminating the use of OVS_IOCTL_GET_PID
>>+ *
>>+ * 
>>-------------------------------------------------------------------------
>>-
>>+ */
>>+static NTSTATUS
>>+OvsSockPropCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
>>+                      UINT32 *replyLen)
>>+{
>>+    static const NL_POLICY ovsSocketPolicy[] = {
>>+        [OVS_NL_ATTR_SOCK_PROTO] = { .type = NL_A_U32, .optional = TRUE
>>},
>>+        [OVS_NL_ATTR_SOCK_PID] = { .type = NL_A_U32, .optional = TRUE }
>>+    };
>>+    PNL_ATTR attrs[ARRAY_SIZE(ovsSocketPolicy)];
>>+
>>+    if (usrParamsCtx->outputLength < sizeof(OVS_MESSAGE)) {
>>+        return STATUS_NDIS_INVALID_LENGTH;
>>+    }
>>+
>>+    NL_BUFFER nlBuf;
>>+    POVS_MESSAGE msgIn = (POVS_MESSAGE)usrParamsCtx->inputBuffer;
>>+    POVS_MESSAGE msgOut = (POVS_MESSAGE)usrParamsCtx->outputBuffer;
>>+    POVS_OPEN_INSTANCE instance =
>>(POVS_OPEN_INSTANCE)usrParamsCtx->ovsInstance;
>>+    UINT32 protocol;
>>+    PNL_MSG_HDR nlMsg;
>>+    UINT32 pid;
>>+
>>+    /* Parse the input */
>>+    if (!NlAttrParse((PNL_MSG_HDR)msgIn,
>>+                     NLMSG_HDRLEN + GENL_HDRLEN + OVS_HDRLEN,
>>+                     NlMsgAttrsLen((PNL_MSG_HDR)msgIn),
>>+                     ovsSocketPolicy,
>>+                     ARRAY_SIZE(ovsSocketPolicy),
>>+                     attrs,
>>+                     ARRAY_SIZE(attrs))) {
>>+        return STATUS_INVALID_PARAMETER;
>>+    }
>>+
>>+    /* Set the Protocol if it was passed down */
>>+    if (attrs[OVS_NL_ATTR_SOCK_PROTO]) {
>>+        protocol = NlAttrGetU32(attrs[OVS_NL_ATTR_SOCK_PROTO]);
>>+        if (protocol) {
>>+            instance->protocol = protocol;
>>+        }
>>+    }
>>+
>>+    /* Verify if the PID sent down matches the kernel */
>>+    if (attrs[OVS_NL_ATTR_SOCK_PID]) {
>>+        pid = NlAttrGetU32(attrs[OVS_NL_ATTR_SOCK_PID]);
>>+        if (pid != instance->pid) {
>>+            OVS_LOG_ERROR("Received invalid pid:%d expected:%d",
>>+                          pid, instance->pid);
>>+            return STATUS_INVALID_PARAMETER;
>>+        }
>>+    }
>>+
>>+    /* Prepare the output */
>>+    NlBufInit(&nlBuf, usrParamsCtx->outputBuffer,
>>usrParamsCtx->outputLength);
>>+    if(!NlFillOvsMsg(&nlBuf, msgIn->nlMsg.nlmsgType, NLM_F_MULTI,
>>+                      msgIn->nlMsg.nlmsgSeq, msgIn->nlMsg.nlmsgPid,
>>+                      msgIn->genlMsg.cmd, msgIn->genlMsg.version,
>>+                      gOvsSwitchContext->dpNo)){
>>+        return STATUS_INVALID_BUFFER_SIZE;
>>+    }
>>+
>>+    if (!NlMsgPutTailU32(&nlBuf, OVS_NL_ATTR_SOCK_PID,
>>+                         instance->pid)) {
>>+        return STATUS_INVALID_BUFFER_SIZE;
>>+    }
>>+
>>+    if (!NlMsgPutTailU32(&nlBuf, OVS_NL_ATTR_SOCK_PROTO,
>>+                         instance->protocol)) {
>>+        return STATUS_INVALID_BUFFER_SIZE;
>>+    }
>>+
>>+    nlMsg = (PNL_MSG_HDR)NlBufAt(&nlBuf, 0, 0);
>>+    nlMsg->nlmsgLen = NlBufSize(&nlBuf);
>>+    *replyLen = msgOut->nlMsg.nlmsgLen;
>>+
>>+    return STATUS_SUCCESS;
>>+}
>>diff --git a/datapath-windows/ovsext/Datapath.h
>>b/datapath-windows/ovsext/Datapath.h
>>index 09e233f..2b41d82 100644
>>--- a/datapath-windows/ovsext/Datapath.h
>>+++ b/datapath-windows/ovsext/Datapath.h
>>@@ -51,6 +51,7 @@ typedef struct _OVS_OPEN_INSTANCE {
>>     PVOID eventQueue;
>>     POVS_USER_PACKET_QUEUE packetQueue;
>>     UINT32 pid;
>>+    UINT32 protocol; /* Refers to NETLINK Family (eg. NETLINK_GENERIC)*/
>> 
>>     struct {
>>         POVS_MESSAGE ovsMsg;    /* OVS message passed during dump start.
>>*/
>>diff --git a/lib/netlink-socket.c b/lib/netlink-socket.c
>>index 32b0cc3..26d909b 100644
>>--- a/lib/netlink-socket.c
>>+++ b/lib/netlink-socket.c
>>@@ -59,6 +59,7 @@ static void log_nlmsg(const char *function, int error,
>>                       const void *message, size_t size, int protocol);
>> #ifdef _WIN32
>> static int get_sock_pid_from_kernel(struct nl_sock *sock);
>>+static int set_sock_property(struct nl_sock *sock);
>> #endif
>> ?
>> /* Netlink sockets. */
>>@@ -165,6 +166,10 @@ nl_sock_create(int protocol, struct nl_sock **sockp)
>>     if (retval != 0) {
>>         goto error;
>>     }
>>+    retval = set_sock_property(sock);
>>+    if (retval != 0) {
>>+        goto error;
>>+    }
>> #else
>>     if (setsockopt(sock->fd, SOL_SOCKET, SO_RCVBUFFORCE,
>>                    &rcvbuf, sizeof rcvbuf)) {
>>@@ -284,6 +289,76 @@ get_sock_pid_from_kernel(struct nl_sock *sock)
>> 
>>     return retval;
>> }
>>+
>>+/* Used for setting and managing socket properties in userspace and
>>kernel.
>>+ * Currently two attributes are tracked - pid and protocol
>>+ * protocol - supplied by userspace based on the netlink family. Windows
>>uses
>>+ *            this property to set the value in kernel datapath.
>>+ *            eg: (NETLINK_GENERIC/ NETLINK_NETFILTER)
>>+ * pid -      generated by windows kernel and set in userspace. The
>>property
>>+ *            is not modified.
>>+ * Also verify if Protocol and PID in Kernel reflects the values in
>>userspace
>>+ * */
>>+static int
>>+set_sock_property(struct nl_sock *sock)
>>+{
>>+    static const struct nl_policy ovs_socket_policy[] = {
>>+        [OVS_NL_ATTR_SOCK_PROTO] = { .type = NL_A_BE32, .optional = true
>>},
>>+        [OVS_NL_ATTR_SOCK_PID] = { .type = NL_A_BE32, .optional = true }
>>+    };
>>+
>>+    struct ofpbuf request, *reply;
>>+    struct ovs_header *ovs_header;
>>+    struct nlattr *attrs[ARRAY_SIZE(ovs_socket_policy)];
>>+    int retval = 0;
>>+    int error;
>>+
>>+    ofpbuf_init(&request, 0);
>>+    nl_msg_put_genlmsghdr(&request, 0, OVS_WIN_NL_CTRL_FAMILY_ID, 0,
>>+                          OVS_CTRL_CMD_SOCK_PROP,
>>OVS_WIN_CONTROL_VERSION);
>>+    ovs_header = ofpbuf_put_uninit(&request, sizeof *ovs_header);
>>+    ovs_header->dp_ifindex = 0;
>>+
>>+    nl_msg_put_be32(&request, OVS_NL_ATTR_SOCK_PROTO, sock->protocol);
>>+    /* pid is already set as part of get_sock_pid_from_kernel()
>>+     * This is added to maintain consistency
>>+     */
>>+    nl_msg_put_be32(&request, OVS_NL_ATTR_SOCK_PID, sock->pid);
>>+
>>+    error = nl_sock_transact(sock, &request, &reply);
>>+    ofpbuf_uninit(&request);
>>+    if (error) {
>>+        retval = EINVAL;
>>+    }
>>+
>>+    if (!nl_policy_parse(reply,
>>+                         NLMSG_HDRLEN + GENL_HDRLEN + sizeof
>>*ovs_header,
>>+                         ovs_socket_policy, attrs,
>>+                         ARRAY_SIZE(ovs_socket_policy))) {
>>+        ofpbuf_delete(reply);
>>+        retval = EINVAL;
>>+    }
>>+    /* Verify if the properties are setup properly */
>>+    if (attrs[OVS_NL_ATTR_SOCK_PROTO]) {
>>+        int protocol = nl_attr_get_be32(attrs[OVS_NL_ATTR_SOCK_PROTO]);
>>+        if (protocol != sock->protocol) {
>>+            VLOG_ERR("Invalid protocol returned:%d expected:%d",
>>+                     protocol, sock->protocol);
>>+            retval = EINVAL;
>>+        }
>>+    }
>>+
>>+    if (attrs[OVS_NL_ATTR_SOCK_PID]) {
>>+        int pid = nl_attr_get_be32(attrs[OVS_NL_ATTR_SOCK_PID]);
>>+        if (pid != sock->pid) {
>>+            VLOG_ERR("Invalid pid returned:%d expected:%d",
>>+                     pid, sock->pid);
>>+            retval = EINVAL;
>>+        }
>>+    }
>>+
>>+    return retval;
>>+}
>> #endif  /* _WIN32 */
>> 
>> #ifdef _WIN32
>>-- 
>>2.9.0.windows.1
>>
>

_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to