Thanks Sam for reviewing this code. On the sequence number I will use the nl_sock_send() instead so the sequence number will be 1 rather than zero. But, please not that this is not necessary because our implementation use a single system I/O call for write the NL command and read the response. There is no case where the response and the command sequence numbers will be different. In other words the NL sequence number doesn't have any significant for Windows. But, I will use the nl_sock_send() so user mode code will be consistent. I will add some comments for the attributes in the header file as you suggested. Thank you, Eitan
-----Original Message----- From: Samuel Ghinet [mailto:sghi...@cloudbasesolutions.com] Sent: Wednesday, September 10, 2014 4:10 PM To: dev@openvswitch.org; Eitan Eliahu Subject: RE: [ovs-dev] [PATCH v1] Netlink_socket.c Join/Unjoin an MC group for event subscription Eitan, A few notes: o) regarding the call: > error = nl_sock_send__(sock, &request, 0, true); (parameter 3 is sequence) I found doc comment in nl_sock_allocate_seq, saying: > /* Make it impossible for the next request for sequence numbers to > wrap > * around to 0. Start over with 1 to avoid ever using a sequence > number of > * 0, because the kernel uses sequence number 0 for notifications. */ Is sequence number 0 reserved for notifications only or for notifications and multicasting? Otherwise, you could use nl_sock_send instead. o) ovs_nl_mcast_attr: it could use some doc comments (in OvsDpInterfaceExt.h), such as the data type (UINT32, UINT8) and value (join = 1 or 0) Sam ________________________________________ Date: Wed, 10 Sep 2014 20:43:16 -0700 From: Eitan Eliahu <elia...@vmware.com> To: dev@openvswitch.org Subject: [ovs-dev] [PATCH v1] Netlink_socket.c Join/Unjoin an MC group for event subscription Message-ID: <1410406996-6136-1-git-send-email-elia...@vmware.com> Use a specific out of band device control to subscribe/unsubscribe a socket to the driver event queue for notification. Signed-off-by: Eitan Eliahu <elia...@vmware.com> Acked-by: Nithin Raju <nit...@vmware.com> Acked-by: Saurabh Shah <ssaur...@vmware.com> Acked-by: Ankur Sharma <ankursha...@vmware.com> --- v1 CodingStyle fix alignment --- datapath-windows/include/OvsDpInterfaceExt.h | 7 +++ lib/netlink-socket.c | 82 +++++++++++++--------------- 2 files changed, 46 insertions(+), 43 deletions(-) diff --git a/datapath-windows/include/OvsDpInterfaceExt.h b/datapath-windows/include/OvsDpInterfaceExt.h index 73dfcbe..7e51cd6 100644 --- a/datapath-windows/include/OvsDpInterfaceExt.h +++ b/datapath-windows/include/OvsDpInterfaceExt.h @@ -70,6 +70,13 @@ /* Commands available under the OVS_WIN_CONTROL_FAMILY. */ enum ovs_win_control_cmd { OVS_CTRL_CMD_WIN_GET_PID, + OVS_CTRL_CMD_MC_SUBSCRIBE_REQ, +}; + +/* NL Attributes for joining/unjoining an MC group */ enum +ovs_nl_mcast_attr { + OVS_NL_ATTR_MCAST_GRP, /* Join an MC group */ + OVS_NL_ATTR_MCAST_JOIN, /*1/0 - Join/Unjoin */ }; #endif /* __OVS_DP_INTERFACE_EXT_H_ */ diff --git a/lib/netlink-socket.c b/lib/netlink-socket.c index a6be186..1f1e3f3 100644 --- a/lib/netlink-socket.c +++ b/lib/netlink-socket.c @@ -319,6 +319,33 @@ done: } #endif /* _WIN32 */ +#ifdef _WIN32 +static int __inline +nl_sock_mcgroup(struct nl_sock *sock, unsigned int multicast_group, +bool join) { + struct ofpbuf request; + uint64_t request_stub[128]; + struct ovs_header *ovs_header; + struct nlmsghdr *nlmsg; + int error; + + ofpbuf_use_stub(&request, request_stub, sizeof request_stub); + + nl_msg_put_genlmsghdr(&request, 0, OVS_WIN_NL_CTRL_FAMILY_ID, 0, + OVS_CTRL_CMD_MC_SUBSCRIBE_REQ, + OVS_WIN_CONTROL_VERSION); + + ovs_header = ofpbuf_put_uninit(&request, sizeof *ovs_header); + ovs_header->dp_ifindex = 0; + + nl_msg_put_u32(&request, OVS_NL_ATTR_MCAST_GRP, multicast_group); + nl_msg_put_u8(&request, OVS_NL_ATTR_MCAST_JOIN, join ? 1 : 0); + + error = nl_sock_send__(sock, &request, 0, true); + ofpbuf_uninit(&request); + return error; +} +#endif /* Tries to add 'sock' as a listener for 'multicast_group'. Returns 0 if * successful, otherwise a positive errno value. * @@ -334,31 +361,12 @@ int nl_sock_join_mcgroup(struct nl_sock *sock, unsigned int multicast_group) { #ifdef _WIN32 -#define OVS_VPORT_MCGROUP_FALLBACK_ID 33 - struct ofpbuf msg_buf; - struct message_multicast - { - struct nlmsghdr; - /* if true, join; if else, leave */ - unsigned char join; - unsigned int groupId; - }; - - struct message_multicast msg = { 0 }; - - msg.nlmsg_len = sizeof(struct message_multicast); - msg.nlmsg_type = OVS_VPORT_MCGROUP_FALLBACK_ID; - msg.nlmsg_flags = 0; - msg.nlmsg_seq = 0; - msg.nlmsg_pid = sock->pid; - - msg.join = 1; - msg.groupId = multicast_group; - msg_buf.base_ = &msg; - msg_buf.data_ = &msg; - msg_buf.size_ = msg.nlmsg_len; - - nl_sock_send__(sock, &msg_buf, msg.nlmsg_seq, 0); + int error = nl_sock_mcgroup(sock, multicast_group, true); + if (error) { + VLOG_WARN("could not join multicast group %u (%s)", + multicast_group, ovs_strerror(errno)); + return errno; + } #else if (setsockopt(sock->fd, SOL_NETLINK, NETLINK_ADD_MEMBERSHIP, &multicast_group, sizeof multicast_group) < 0) { @@ -384,24 +392,12 @@ int nl_sock_leave_mcgroup(struct nl_sock *sock, unsigned int multicast_group) { #ifdef _WIN32 - struct ofpbuf msg_buf; - struct message_multicast - { - struct nlmsghdr; - /* if true, join; if else, leave*/ - unsigned char join; - }; - - struct message_multicast msg = { 0 }; - nl_msg_put_nlmsghdr(&msg, sizeof(struct message_multicast), - multicast_group, 0); - msg.join = 0; - - msg_buf.base_ = &msg; - msg_buf.data_ = &msg; - msg_buf.size_ = msg.nlmsg_len; - - nl_sock_send__(sock, &msg_buf, msg.nlmsg_seq, 0); + int error = nl_sock_mcgroup(sock, multicast_group, false); + if (error) { + VLOG_WARN("could not leave multicast group %u (%s)", + multicast_group, ovs_strerror(errno)); + return errno; + } #else if (setsockopt(sock->fd, SOL_NETLINK, NETLINK_DROP_MEMBERSHIP, &multicast_group, sizeof multicast_group) < 0) { -- 1.9.4.msysgit.0 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
