GNOME Bugzilla – Bug 626589
gio is missing functions to properly use multicasting
Last modified: 2012-01-16 17:44:13 UTC
Created attachment 167555 [details] [review] Multicasting functions patch While you can use gio in its current form to send datagrams to a multicast address, there is no way to receive (participate) in a multicast group, as it requires the client to explicitely join a multicast group first. The attached patch adds this functionality to gio, and has been tested on linux / ipv4. More work would likely be required to make it work on windows, as well as with ipv6.
Bug 623187 talks about trying to make setsockopt/getsockopt generically more accessible/usable with GSocket, but this stuff would probably be good to have as real API anyway. (In reply to comment #0) > More work would likely be required to make it work on windows, as well as with > ipv6. It looks like there are two ways to handle IPv6; with IPV6_ADD_MEMBERSHIP, etc, just like the IPv4 versions, or with MCAST_JOIN_GROUP, etc, which is a new API that works with either IPv4 or v6. (I can't figure out what Linux man page describes the new API, although it is supported, and it's specified in RFC 3678.) WinSock seems to support both APIs as well (http://msdn.microsoft.com/en-us/library/ms738558(v=VS.85).aspx) although the MCAST_* one is only supported as of Vista, so maybe GSocket should just stick to the older API.
Comment on attachment 167555 [details] [review] Multicasting functions patch the patch looks mostly good, but we'll want to have IPv6 support too, and we should make sure it at least doesn't break the compile on Windows.
Dan, What's the default build environment for windows? windows/mingw, or linux/mingw cross-compile? If I recall correctly, last time I tried to build glib on windows/mingw I had trouble with gtkdoc. If can get clear instructions regarding windows builds, I'll look into making sure the code compiles (and work) on windows.
I just realized that this patch clearly isn't the latest version I've been working on (it's missing function comments, and is plain buggy). I'll re-submit a new one once I have ipv6 in.
mingw-cross-compile is easiest (and in theory, you can test that things work by trying to run the binaries under wine, but last time I tried, wine was missing IPv6 support)
Created attachment 167652 [details] [review] Updated patch with ipv4 and ipv6 support
Created attachment 204323 [details] [review] GSocket: add multicast-related functions Add APIs for sending and receiving multicast datagrams with GSocket. Based on an earlier patch from Olivier Chalouhi. ==== This fixes a bunch of style issues with the earlier patch and adds properties for multicast-loopback and multicast-ttl. Olivier, if you're still interested in this patch, it would be great to get some testing of it to make sure it all still works...
*** Bug 667857 has been marked as a duplicate of this bug. ***
From bug #667857: Something for SO_BROADCAST and IP_ADD_SOURCE_MEMBERSHIP would be useful to have too.
The patch in this bug looks good to me but I don't know if something else is needed for Windows portability here. I'll extend it with SO_BROADCAST and IP_ADD_SOURCE_MEMBERSHIP functions later and then we can worry about Windows support ;)
Review of attachment 204323 [details] [review]: Actually some notes for this patch, I'll attach a new patch that does this ::: gio/gsocket.c @@ +1318,3 @@ + */ +guint +g_socket_get_multicast_ttl (GSocket *socket) Maybe add a second function here for unicast TTL and use IP_TTL @@ +1706,3 @@ + if (socket->priv->family == G_SOCKET_FAMILY_IPV4) + { + struct ip_mreq mc_req; With struct ip_mreqn it's possible to select an interface to. So maybe add interface selection here if it is available? @@ +1720,3 @@ + + memcpy (&mc_req_ipv6.ipv6mr_multiaddr, native_addr, sizeof (struct in6_addr)); + mc_req_ipv6.ipv6mr_interface = 0; Same here, add interface selection.
I've added all this to my branch at: http://cgit.collabora.com/git/user/slomo/glib.git/log/?h=socket-multicast I've also fixed a minor bug in the patch by Dan (wrong enum value for property installation)
Comments on the socket-multicast branch: e0b6325b GSocket: add possibility to join a multicast group only on a specific interface move the <net/if.h> include to gnetworkingprivate.h, in the !G_OS_WIN32 section What's the argument for using imr_ifindex rather than the more portable imr_address/imr_interface, which seems more-or-less equivalent? > mc_req_ipv6.ipv6mr_interface = 0; >+ if (interface) >+ mc_req_ipv6.ipv6mr_interface = if_nametoindex (interface); should be if/else rather than setting the value twice 28198c78 GSocket: Add support for source specific multicast (RFC 4604) nitpicky, but "source-specific" should be hyphenated everywhere 050fe880 GSocket: Add function for setting unicast TTL > /** >+ * GSocket:ttl: >+ * >+ * Time-to-live out outgoing unicast packets s/out/for/ >+ P_("Ttl"), "TTL" >+ * Sets the time-to-live for outgoing unicast packets on @socket. >+ * By default, this is 1, meaning that unicast packets will not leave >+ * the local network. um, no, that wouldn't be a very useful default :-) I'd use "0" as the default in the paramspec I guess, and note in both the property docs and the method docs that the actual default value is platform-specific. >+g_socket_set_ttl (GSocket *socket, >+ guint ttl) indentation 546ec5f2 GSocket: Add function to set/get the broadcast setting on a socket which is bug 623187, so note that in the commit message >+ * g_socket_get_broadcast: >+ * @socket: a #GSocket. >+ * >+ * Gets the broadcast setting on @socket; if %TRUE (the >+ * default), it is possible to send packets to broadcast The default is FALSE. >+ result = getsockopt (socket->priv->fd, SOL_SOCKET, SO_BROADCAST, >+ &value, &optlen); >+ result = setsockopt (socket->priv->fd, SOL_SOCKET, SO_BROADCAST, >+ &value, sizeof (value)); the indentation is wrong on both of those 50fb0db1 Bug 668009 – GSocket: Add function to get the currently available bytes for reading I'm more a fan of putting the bugzilla URL at the end of the commit body rather than putting the bug number in the summary. >+ * g_socket_peek_bytes_pending: >+ * @socket: a #GSocket >+ * >+ * Peek amount of data pending in the OS input buffer. Hm... "Check the amount of data pending..." >+ * Since: 2.24 >+ */ 2.32 >+ gulong pending = 0; It appears that long is right on Windows, but on unix it's an int. The return value should probably be gsize (which is probably the same as gulong anyway, but is more consistent with the other APIs). bdf150cd Bug 667989 – Reset the timeout in the GSocket GSource after it was triggered same comment as above on the commit message >- return (*func) (socket_source->socket, >+ ret = (*func) (socket_source->socket, > socket_source->pollfd.revents & socket_source->condition, > user_data); you can use "socket" instead of "socket_source->socket" now. and you need to fix the indent on the 2nd and 3rd line.
(In reply to comment #13) > What's the argument for using imr_ifindex rather than the more portable > imr_address/imr_interface, which seems more-or-less equivalent? I don't know, we've used this code since forever in GStreamer and it works. I've changed everything else according to your review, it's in the same branch now (just rebased).
(In reply to comment #13) > Comments on the socket-multicast branch: > > 50fb0db1 Bug 668009 – GSocket: Add function to get the currently available > bytes for reading > > I'm more a fan of putting the bugzilla URL at the end of the commit body rather > than putting the bug number in the summary. > > >+ * g_socket_peek_bytes_pending: > >+ * @socket: a #GSocket > >+ * > >+ * Peek amount of data pending in the OS input buffer. > > Hm... "Check the amount of data pending..." I don't like the name of that function btw, if you prefer something else feel free to change it when merging my branch. Something like g_socket_get_available_bytes() maybe.
commit ffb5f8b10191ddf51ccd021c1e4dbba4eafbc370 Author: Sebastian Dröge <sebastian.droege@collabora.co.uk> Date: Fri Jan 13 13:01:35 2012 +0100 GSocket: Add function to set/get the broadcast setting on a socket https://bugzilla.gnome.org/show_bug.cgi?id=623187 commit 5560d9b8800de1144d77ed924286759286b7d27e Author: Sebastian Dröge <sebastian.droege@collabora.co.uk> Date: Fri Jan 13 12:48:02 2012 +0100 GSocket: Add function for setting unicast TTL commit 03b40522df4fb247f76e5948eddd2d7a02c097a5 Author: Sebastian Dröge <sebastian.droege@collabora.co.uk> Date: Fri Jan 13 12:53:50 2012 +0100 GSocket: Add support for source-specific multicast (RFC 4604) commit 97f25892ea38e227fe802d0cc50fd88be12cdb17 Author: Sebastian Dröge <sebastian.droege@collabora.co.uk> Date: Fri Jan 13 12:37:31 2012 +0100 GSocket: Add possibility to join a multicast group only on a specific interface commit a62d1bb74728aa80af2410cee57950a545f6fa04 Author: Dan Winship <danw@gnome.org> Date: Thu Dec 29 11:01:23 2011 -0500 GSocket: Add multicast-related functions Add APIs for sending and receiving multicast datagrams with GSocket. Based on an earlier patch from Olivier Chalouhi. https://bugzilla.gnome.org/show_bug.cgi?id=626589