Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support IPv6 in snapserver #279

Merged
merged 1 commit into from
Sep 26, 2017
Merged

Support IPv6 in snapserver #279

merged 1 commit into from
Sep 26, 2017

Conversation

miek
Copy link
Contributor

@miek miek commented Sep 26, 2017

This commit allows snapserver to listen on IPv4 and/or IPv6 when available, to go along with #273.

I've followed the example given in a boostcon presentation on IPv6 best practices here: https://raw.githubusercontent.com/boostcon/2011_presentations/master/wed/IPv6.pdf and so it should support systems with dual stack, IPv4 only or IPv6 only.

This works well for me and accepts clients on both IPv4 & IPV6:

$ ./snapserver 
Settings file: /home/mike/.config/snapserver/server.json
2017-09-26 16-00-50 [out] Adding service 'Snapcast'
2017-09-26 16-00-50 [out] PcmStream sampleFormat: 48000:16:2
2017-09-26 16-00-50 [out] PipeStream mode: create
2017-09-26 16-00-50 [out] Stream: {"fragment":"","host":"","path":"/tmp/snapfifo","query":{"buffer_ms":"20","codec":"flac","name":"default","sampleformat":"48000:16:2"},"raw":"pipe:///tmp/snapfifo?name=default","scheme":"pipe"}
2017-09-26 16-00-50 [err] (PipeStream) Exception: end of file
2017-09-26 16-00-51 [out] Service 'Snapcast' successfully established.
2017-09-26 16-00-54 [Notice] ControlServer::NewConnection: 2a04:5d00:70:64f1:bdf0:8419:e02b:4ef5
2017-09-26 16-01-25 [Err] Exception in ControlSession::reader(): read_until: End of file
2017-09-26 16-01-25 [Notice] ControlServer::NewConnection: ::ffff:10.64.10.107

I tried disabling IPv6 with

sudo sh -c 'echo 1 > /proc/sys/net/ipv6/conf/all/disable_ipv6' 

and snapserver still comes up and works correctly on IPv4.

Also it would be easy to add a command-line option to set the bind address now, if you want that?

@badaix
Copy link
Owner

badaix commented Sep 26, 2017

Really nice!

Client with IPv6: ./snapclient -h fe80::be5f:f4ff:feca:cd64%wlan0
Server:

2017-09-26 19-45-42 [Notice] StreamServer::NewConnection: fe80::221:6aff:fe7d:74fc%eth0
2017-09-26 19-45-43 [Info] Hello from d26b04e8811a544a693ac1e3540dd21d, host: T400, v0.11.1, ClientName: Snapclient, OS: Linux Mint 17.3 Rosa, Arch: x86_64, Protocol version: 2

Client with IPv4: ./snapclient -h 192.168.0.2
Server:

2017-09-26 19-46-02 [Notice] StreamServer::NewConnection: ::ffff:192.168.0.54
2017-09-26 19-46-02 [Info] Hello from d26b04e8811a544a693ac1e3540dd21d, host: T400, v0.11.1, ClientName: Snapclient, OS: Linux Mint 17.3 Rosa, Arch: x86_64, Protocol version: 2

@badaix
Copy link
Owner

badaix commented Sep 26, 2017

manually merged into develop branch
Edit: Somehow GitHub didn't recognize the manual merge and didn't mention you as a contributor.
Any idea?

@badaix badaix closed this Sep 26, 2017
@badaix badaix reopened this Sep 26, 2017
@miek
Copy link
Contributor Author

miek commented Sep 26, 2017

Merge looks good to me, I think GitHub only counts contributions once it's in the master branch.

Cheers!

@miek miek closed this Sep 26, 2017
@badaix badaix reopened this Sep 26, 2017
@badaix badaix changed the base branch from master to develop September 26, 2017 18:38
@badaix badaix merged commit 5f06f70 into badaix:develop Sep 26, 2017
@badaix
Copy link
Owner

badaix commented Sep 26, 2017

I did a reset of the first merge and hard pushed the result (bad practice in public repos). Hope no one pulled the latest changes from the develop branch during the last 10min.
Now I managed to merge the PR into develop using the GitHub GUI and it's now properly marked as merged, but still your not listed as contributor. I hope this will change when I merge the develop branch back into master.
Thanks again!

Edit: seems you're right. Looks like it counts just master contributions. According to GitHub I didn't contribute for the last few month, which is true for master, but not for develop

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants