mikebrady / shairport-sync

AirPlay and AirPlay 2 audio player
Other
7.27k stars 573 forks source link

ipv6: control (and timing) connection from different ip than announced by mdns #254

Closed ejurgensen closed 8 years ago

ejurgensen commented 8 years ago

Hi Mike! As you know I was testing forked-daapd with Shairport sync (ref https://github.com/ejurgensen/forked-daapd/issues/245). I have Shairport sync installed on Ubuntu, and the machine has multiple ipv6 addresses on eth0 (which is apparently normal for ipv6 - but my knowledge of ipv6 is poor):

      inet6 addr: fd60:1a0c:940c:0:20c:29ff:fe25:dfbe/64 Scope:Global
      inet6 addr: fd60:1a0c:940c:0:c87d:3e26:c534:d4ec/64 Scope:Global
      inet6 addr: fd60:1a0c:940c::abe/128 Scope:Global

The mdns announcement from Shairport says to connect to one address (fd60:1a0c:940c::abe), which forked-daapd will then do. Shairport will then connect to the control port of forked-daapd, but using a different source address (in my case: fd60:1a0c:940c:0:c87d:3e26:c534:d4ec). This presents a problem, because when forked-daapd gets a retransmit request on the control port, it doesn't know which AirPlay target requested the retransmit.

To me, forked-daapd using the source address to identify the AirPlay device making the request doesn't sound optimal, but I don't know if there are other id's in the control requests? I did try looking, and it doesn't seem like it. Did I miss something?

Otherwise the only solution I can think of is that Shairport should try and bind with the same source address as in the mdns announcement?

Shairport version is 2.8.1-openssl-Avahi-ALSA-dummy-stdout-pipe-soxr-metadata, so not so recent, sorry! If you have already dealt with this issue please just close this. Regards, Espen

mikebrady commented 8 years ago

Hi Espen, and thanks for the report. I am indeed aware of this problem and made a half-hearted attempt to fix it a few weeks ago by using a Linux extension that would make the system use "public" IPv6 addresses where appropriate. Unfortunately (a) it isn't portable to FreeBSD, (b) OpenWrt doesn't support it and (c) Avahi doesn't know anything about it. I think I'll have to do as you suggest and get the IPv6 number being advertised by Avahi.

How important an issue is it, do you think, to users? (I'm wondering about prioritising it.)

ejurgensen commented 8 years ago

I haven’t seen any users submitting this issue, and there is an easy work-around in forked-daapd that simply consists of disabling ipv6 in the config file. Of course, the user probably wouldn’t know to do that. The error msg in forked-daapd just says a RAOP request was rejected. Anyway, I wouldn’t say the priority is very high.

I wonder how other clients solve this – especially iTunes/iOS. Maybe they just stick to ipv4 if they can.

Right now, what happens with forked-daapd and ipv6 enabled is that Shairport never starts playback, because it doesn’t get a reply from the timer connection. However, for timing, forked-daapd doesn’t actually need to identify the peer, since it just responds on the same connection. So I think I will just remove that check. That will mean playback will start, but the control connection/retransmit will remain broken until you can align the source addresses.

mikebrady commented 8 years ago

Fair enough, thanks. I guess the hard part of this is getting an IPv6 number from Avahi. I'll look into it.

mikebrady commented 8 years ago

I've made a little progress on this – I think I am getting Avahi to tell me what IP numbers it is advertising. Let's see how reliable it is for a few days. Then I'll have to use them when making connections.

mikebrady commented 8 years ago

Hi Espen. I've make an attempt to fix this problem, and I'd be greatly obliged if you'd try it out. The update is in the development branch, at release 2.8.3.3 and the HEAD. The fix is for Shairport Sync to always use the same IP number. It should be okay, but I'm not sure what would happen if the IP number changes due, say, to a new DHCP allocation or a new pseudo-random IPv6 number. I guess it would only make a difference in the very short interval between the start of a session and the start of a play session, but then again, I'm not too sure.

ejurgensen commented 8 years ago

Alright, just tested. As it is, it still makes requests from a different source address than announced (with ipv6), so it didn't immediately work. However, if I add this as line 570 in rtp.c it works: inet_pton(AF_INET6,self_ip_address,&(sa6->sin6_addr)); Since you added a similar line for ipv4 it looks like you may have just forgotten it for ipv6?

Unrelated minor suggestion: Running "./configure" without "--with-ssl=" results in hard-to-understand errors from the linker when building (something like undefined reference to base64_enc). Have you considered letting configure default to openssl when there is no "--with-ssl="?

mikebrady commented 8 years ago

Yes, thank you – somehow it didn't make the pushed version. Many thanks. I'll thanks that default stuff alright.

ejurgensen commented 8 years ago

Tested it again for good measure, and the source address was indeed the same as in the announcement, so that was excellent. Confirmed it with Wireshark as well.

mikebrady commented 8 years ago

Thanks for all your help with this.