[Twisted-Python] RFC: IPv6 multicast join/ticket 6597

Glyph Lefkowitz glyph at twistedmatrix.com
Tue Jul 26 01:57:29 MDT 2016


> On Jul 25, 2016, at 9:43 PM, Jason Litzinger <jlitzingerdev at gmail.com> wrote:
> 
> 
> Hello,
> 
> I'm looking at making the changes to support IPv6 multicast groups as described
> in ticket 6597 but wanted to get some feedback (and get a feel whether this is
> even desirable) before formally submitting any patches.

Thanks for taking this up!

> Specifically:
> 
> 1.  I've read [1] and it alludes to udp hopefully disappearing, is that something in the
>    works?  Is there a new approach to solving this problem I should look at?  A
>    branch in the works where this (conceptual) change belongs?
> 
>    Note:  The addressFamily attribute referenced already exists and is set
>           properly for IPv6.

'twisted.internet.udp', as an importable module; not 'udp' as a feature of Twisted (or of the Internet, for that matter).  

> 2.  The attached change has the side effect that calls to ReactorBase.resolve()
>    with IPv6 literals will now likely succeed where they may have failed in the
>    past.  That means clients counting on resolve raising an exception for an
>    IPv6 literal will break.  Not sure whether this is considered a
>    compatibility issue, but I wanted to raise it.

We have explicitly avoided adding IPv6 name resolution to the reactor because the reactor's API for name resolution is fundamentally the wrong shape for IPv6.  If you want to add the ability to resolve IPv6 names to the reactor itself, please see this ticket: https://twistedmatrix.com/trac/ticket/4362 <https://twistedmatrix.com/trac/ticket/4362>

For the purposes of this ticket alone, you should probably just skip resolution in _joinAddr1 if resolution is 

> 3.  One alternative to the above would be a complete API separation, via
>    something like joinIPv6Group(), and a new resolve.  Is that more appealing
>    in this case?

Given what we've done with connectTCP et. al., it makes sense to leave 'joinGroup' as the API for doing this.  But we probably want to leave '.resolve' alone.

> Caveats:
> 
> 1.  I have not finished all of the documentation related to developers, I will
>    do so prior to formal submission.

I think we can do the narrative docs in a separate PR, as the interface looks like the straightforward expansion of the IPv4 interface.  You should clean up the reference documentation (i.e. docstrings) to ensure they're accurate of course.

> 2.  I know I need tests and docs and will submit them with the final changes.

Testing multicast is ... challenging.  I barely have any idea how to set up a test environment for IPv4, and no idea what to do for IPv6.  If you can speak to this in your tests (and hopefully docs as well) that would be super helpful.

> On to the patches.  With these changes, I can use the joinGroup API to add
> myself to an IPv6 multicast group on Linux (verified via /proc/net/igmp6).
> Additionally, trial reports the same two failures before and after these changes
> (twisted.python.test.test_release.APIBuilderTests.test_build and
> test_buildWithPolicy).  These changes struck me as the obvious approach, but
> given the changes to resolve, not necessarily the best.
> 
> 
> diff --git a/twisted/internet/base.py b/twisted/internet/base.py
> index 4f2c862..e813741 100644
> --- a/twisted/internet/base.py
> +++ b/twisted/internet/base.py
> @@ -567,6 +567,8 @@ class ReactorBase(object):
>             return defer.succeed('0.0.0.0')
>         if abstract.isIPAddress(name):
>             return defer.succeed(name)
> +        elif abstract.isIPv6Address(name):
> +            return defer.succeed(name)
>         return self.resolver.getHostByName(name, timeout)
> 
>     # Installation.
> diff --git a/twisted/internet/udp.py b/twisted/internet/udp.py
> index b5a5322..210b079 100644
> --- a/twisted/internet/udp.py
> +++ b/twisted/internet/udp.py
> @@ -485,7 +485,10 @@ class MulticastMixin:
> 
> 
>     def _joinAddr1(self, addr, interface, join):
> -        return self.reactor.resolve(interface).addCallback(self._joinAddr2, addr, join)
> +        if self.addressFamily == socket.AF_INET:
> +            return self.reactor.resolve(interface).addCallback(self._joinAddr2, addr, join)
> +        else:
> +            return self.reactor.resolve(interface).addCallback(self._joinAddrIPv6, addr, join)
> 
> 
>     def _joinAddr2(self, interface, addr, join):
> @@ -500,6 +503,18 @@ class MulticastMixin:
>         except socket.error as e:
>             return failure.Failure(error.MulticastJoinError(addr, interface, *e.args))
> 
> +    def _joinAddrIPv6(self, interface, addr, join):
> +        addr = socket.inet_pton(socket.AF_INET6, addr)
> +        interface = socket.inet_pton(socket.AF_INET6, interface)
> +        if join:
> +            cmd = socket.IPV6_JOIN_GROUP
> +        else:
> +            cmd = socket.IPV6_LEAVE_GROUP
> +        try:
> +            self.socket.setsockopt(socket.IPPROTO_IPV6, cmd, addr + interface)
> +        except socket.error as e:
> +            return failure.Failure(error.MulticastJoinError(addr, interface, *e.args))
> +
> 
>     def leaveGroup(self, addr, interface=""):
>         """Leave multicast group, return Deferred of success."""
> 
> 
> This does require the client specify the interface argument when calling
> joinGroup, e.g. self.transport.joinGroup("ff02::1", interface="::").
> 
> Thanks in advance for any feedback!
> 
> -Jason Litzinger
> 
> 
> [1] http://twistedmatrix.com/pipermail/twisted-python/2016-March/030188.html
> 
> _______________________________________________
> Twisted-Python mailing list
> Twisted-Python at twistedmatrix.com
> http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python

-------------- next part --------------
An HTML attachment was scrubbed...
URL: </pipermail/twisted-python/attachments/20160726/e52151af/attachment-0002.html>


More information about the Twisted-Python mailing list