Hi Nithin,

Changes are good, some minor comments:

a. minor comment: i think we should check for buf_in alloc failure before we 
alloc for buf_out in line 79.
b. i think we should be using the library provided APIs for preparing the 
netlink message (if possible). for eg: nl_msg_put_nlmsghdr

Regards,
Ankur
________________________________________
From: dev <dev-boun...@openvswitch.org> on behalf of Nithin Raju 
<nit...@vmware.com>
Sent: Wednesday, August 13, 2014 7:11 PM
To: dev@openvswitch.org
Subject: [ovs-dev] [PATCH 4/4] netlink-socket.c: implement get pid support      
on Windows

To verify if the netlink support in the kernel works, I updated
the netlink-socket.c code to get the PID for a given device
descriptor.

In the existing code, userspace sets the PID, which will not be
unique across different processes. So, it is better for the
kernel to generate the PID and give it back to userspace.

I had to include odp-netlink.h to get the definition of
struct ovs_header.

dpif-linux.c was ported to Windows (similar to Alin's change in
the cloudbase repo) and was able to exercise the code changes
in netlink-socket.c to read the PID. dpif-linux.c changes are
not being checked in.

Signed-off-by: Nithin Raju <nit...@vmware.com>
---
 lib/netlink-socket.c |   68 +++++++++++++++++++++++++++++++++++++++----------
 1 files changed, 54 insertions(+), 14 deletions(-)

diff --git a/lib/netlink-socket.c b/lib/netlink-socket.c
index 3dbfe59..e7449c2 100644
--- a/lib/netlink-socket.c
+++ b/lib/netlink-socket.c
@@ -28,6 +28,8 @@
 #include "hmap.h"
 #include "netlink.h"
 #include "netlink-protocol.h"
+#include "odp-netlink.h"
+#include "odp-netlink-ext.h"
 #include "ofpbuf.h"
 #include "ovs-thread.h"
 #include "poll-loop.h"
@@ -61,19 +63,56 @@ portid_next(void)
     return g_last_portid;
 }

-static void
-set_sock_pid_in_kernel(HANDLE handle, uint32_t pid)
-    OVS_GUARDED_BY(portid_mutex)
+static int
+get_sock_pid_from_kernel(HANDLE handle, uint32_t *pid)
 {
-    struct nlmsghdr msg = { 0 };
+    int ovs_msg_size = sizeof (struct nlmsghdr) + sizeof (struct genlmsghdr) +
+                       sizeof (struct ovs_header);
+    char *buf_in, *buf_out;
+    struct nlmsghdr *nl_msg_in;
+    struct genlmsghdr *genl_msg_in;
+    struct nlmsghdr *msg_out;
+    DWORD bytes;
+    int retval;
+
+    buf_in = xmalloc(ovs_msg_size);
+    buf_out = xmalloc(ovs_msg_size);
+    if (!buf_in || !buf_out) {
+        free(buf_in);
+        free(buf_out);
+        return -ENOMEM;
+    }

-    msg.nlmsg_len = sizeof(struct nlmsghdr);
-    msg.nlmsg_type = 80; /* target = set file pid */
-    msg.nlmsg_flags = 0;
-    msg.nlmsg_seq = 0;
-    msg.nlmsg_pid = pid;
+    nl_msg_in = (struct nlmsghdr *) buf_in;
+    genl_msg_in = (struct genlmsghdr *)(buf_in + sizeof *nl_msg_in);
+
+    nl_msg_in->nlmsg_len = ovs_msg_size;
+    nl_msg_in->nlmsg_type = OVS_WIN_NL_CTRL_FAMILY_ID;
+    nl_msg_in->nlmsg_flags = 0;
+    nl_msg_in->nlmsg_seq = 0;
+    nl_msg_in->nlmsg_pid = 0;

-    WriteFile(handle, &msg, sizeof(struct nlmsghdr), NULL, NULL);
+    genl_msg_in->cmd = OVS_CTRL_CMD_WIN_GET_PID;
+    genl_msg_in->version = OVS_WIN_CONTROL_VERSION;
+    genl_msg_in->reserved = 0;
+
+    if (!DeviceIoControl(handle, OVS_IOCTL_TRANSACT, buf_in, ovs_msg_size,
+                         buf_out, ovs_msg_size, &bytes, NULL)) {
+        retval = -EINVAL;
+        goto done;
+    } else {
+        if (bytes < ovs_msg_size) {
+            retval = -EINVAL;
+            goto done;
+        }
+        msg_out = (struct nlmsghdr *)buf_out;
+        *pid = msg_out->nlmsg_pid;
+    }
+
+done:
+    free(buf_in);
+    free(buf_out);
+    return retval;
 }
 #endif  /* _WIN32 */

@@ -176,10 +215,11 @@ nl_sock_create(int protocol, struct nl_sock **sockp)
     rcvbuf = 1024 * 1024;
 #ifdef _WIN32
     sock->rcvbuf = rcvbuf;
-    ovs_mutex_lock(&portid_mutex);
-    sock->pid = portid_next();
-    set_sock_pid_in_kernel(sock->handle, sock->pid);
-    ovs_mutex_unlock(&portid_mutex);
+    retval = get_sock_pid_from_kernel(sock->handle, &sock->pid);
+    if (retval < 0) {
+        retval = -retval;
+        goto error;
+    }
 #else
     if (setsockopt(sock->fd, SOL_SOCKET, SO_RCVBUFFORCE,
                    &rcvbuf, sizeof rcvbuf)) {
--
1.7.4.1

_______________________________________________
dev mailing list
dev@openvswitch.org
https://urldefense.proofpoint.com/v1/url?u=http://openvswitch.org/mailman/listinfo/dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=f6EhnZ0ORGZNt5QbYmRaOxfWfx%2Bqd3KEiPf3%2FYaollU%3D%0A&m=S9cgwiOZsCXv9YLETG3Oi9uL7dYala0Wd9aJsPeCX4I%3D%0A&s=27cc5b8752d25ef04e2caa68796cfcdecb1066544f85dd3134597f404f44edcf
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to