mDNS binds to all interfaces #486
Comments
|
Currently |
|
Well, got Vollibrespot running in a network namespace with macvlan, publishes only what I want and get reliable discovery, only needed to make sure ipv6 and lo interface was properly set up. Nice work on the Vollibrespot btw, using it due to metadata and control functions :) |
Interesting workaround, like you said probably helpful in other situations as well!
I should really clean it up, it started of as a quick PoC for Volumo, but nice to see that it's useful to others as well! :-) |
|
@Johannesd3 I thought about assigning this to you, as you have new mDNS in the works. |
|
I have the same problem and reported upstream. |
I won't use spotifyd or librespot anymore unless this gets resolved Spotifyd/spotifyd#990 librespot-org/librespot#486
|
librespot-org/libmdns#35 released as 0.7 |
|
Hi, since librespot now uses libmdns 0.7 which added support for binding to interface(s), would it be possible to implement this option? I would try a PR myself, but unfortunately have no coding experience so stepping asside here |
Sure, if @roderickvd thinks it's a good idea I think I can do that pretty easily. |
|
It would probably be something like |
|
Sure why not? The "why not" of course is not to have too many command line options, but in this case I think it's in fair demand and a new option is warranted. |
|
I have a proposal for implementing an option to specify to which interfaces the mDNS responder shall bind. I am a new to Rust so my code will be for sure not optimal. I am unsure how to implement parsing IPv4 and IPv6 addresses. I used the libmdns exmaple from here as a reference. Please give feedback :). diff --git a/Cargo.lock b/Cargo.lock
index fe16c16..c97cdbb 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1365,6 +1365,7 @@ dependencies = [
"librespot-playback",
"librespot-protocol",
"log",
+ "regex",
"rpassword",
"sha1",
"thiserror",
diff --git a/Cargo.toml b/Cargo.toml
index 7a423d7..23f41cc 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -60,6 +60,7 @@ sha1 = "0.10"
thiserror = "1.0"
tokio = { version = "1", features = ["rt", "macros", "signal", "sync", "parking_lot", "process"] }
url = "2.2"
+regex = "1.5"
[features]
alsa-backend = ["librespot-playback/alsa-backend"]
diff --git a/discovery/src/lib.rs b/discovery/src/lib.rs
index 3c01003..4732230 100644
--- a/discovery/src/lib.rs
+++ b/discovery/src/lib.rs
@@ -47,6 +47,7 @@ pub struct Discovery {
pub struct Builder {
server_config: server::Config,
port: u16,
+ bind_ips: Vec<std::net::IpAddr>,
}
/// Errors that can occur while setting up a [`Discovery`] instance.
@@ -87,6 +88,7 @@ impl Builder {
client_id: client_id.into(),
},
port: 0,
+ bind_ips: vec![],
}
}
@@ -102,6 +104,12 @@ impl Builder {
self
}
+ /// Set the ip addresses on which the mdns service should bind
+ pub fn bind_ips(mut self, bind_ips: Vec<std::net::IpAddr>) -> Self {
+ self.bind_ips = bind_ips;
+ self
+ }
+
/// Sets the port on which it should listen to incoming connections.
/// The default value `0` means any port.
pub fn port(mut self, port: u16) -> Self {
@@ -117,6 +125,7 @@ impl Builder {
let mut port = self.port;
let name = self.server_config.name.clone().into_owned();
let server = DiscoveryServer::new(self.server_config, &mut port)??;
+ let bind_ips = self.bind_ips;
#[cfg(feature = "with-dns-sd")]
let svc = dns_sd::DNSService::register(
@@ -127,14 +136,23 @@ impl Builder {
port,
&["VERSION=1.0", "CPath=/"],
)?;
-
+
#[cfg(not(feature = "with-dns-sd"))]
- let svc = libmdns::Responder::spawn(&tokio::runtime::Handle::current())?.register(
+ let svc = if !bind_ips.is_empty() {
+ libmdns::Responder::spawn_with_ip_list(&tokio::runtime::Handle::current(), bind_ips)?.register(
"_spotify-connect._tcp".to_owned(),
name,
port,
&["VERSION=1.0", "CPath=/"],
- );
+ )
+ } else {
+ libmdns::Responder::spawn(&tokio::runtime::Handle::current())?.register(
+ "_spotify-connect._tcp".to_owned(),
+ name,
+ port,
+ &["VERSION=1.0", "CPath=/"],
+ )
+ };
Ok(Discovery { server, _svc: svc })
}
diff --git a/src/main.rs b/src/main.rs
index d5c5392..e28f05d 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -14,6 +14,7 @@ use log::{error, info, trace, warn};
use sha1::{Digest, Sha1};
use thiserror::Error;
use url::Url;
+use regex::Regex;
use librespot::{
connect::{config::ConnectConfig, spirc::Spirc},
@@ -185,6 +186,7 @@ struct Setup {
zeroconf_port: u16,
player_event_program: Option<String>,
emit_sink_events: bool,
+ bind_ips: Vec<std::net::IpAddr>,
}
fn get_setup() -> Setup {
@@ -240,6 +242,7 @@ fn get_setup() -> Setup {
const VOLUME_CTRL: &str = "volume-ctrl";
const VOLUME_RANGE: &str = "volume-range";
const ZEROCONF_PORT: &str = "zeroconf-port";
+ const BIND_IP: &str = "bind-ip";
// Mostly arbitrary.
const AP_PORT_SHORT: &str = "a";
@@ -258,6 +261,7 @@ fn get_setup() -> Setup {
const DISABLE_GAPLESS_SHORT: &str = "g";
const DISABLE_CREDENTIAL_CACHE_SHORT: &str = "H";
const HELP_SHORT: &str = "h";
+ const BIND_IP_SHORT: &str = "i";
const CACHE_SIZE_LIMIT_SHORT: &str = "M";
const MIXER_TYPE_SHORT: &str = "m";
const ENABLE_VOLUME_NORMALISATION_SHORT: &str = "N";
@@ -570,6 +574,12 @@ fn get_setup() -> Setup {
AUTOPLAY,
"Explicitly set autoplay {on|off}. Defaults to following the client setting.",
"OVERRIDE",
+ )
+ .optopt(
+ BIND_IP_SHORT,
+ BIND_IP,
+ "Interface IP address wo which mDNS will bind. Defaults to any interface",
+ "IP"
);
#[cfg(feature = "passthrough-decoder")]
@@ -1168,6 +1178,47 @@ fn get_setup() -> Setup {
None => SessionConfig::default().autoplay,
};
+ let bind_ips: Vec<std::net::IpAddr> = if opt_present(BIND_IP) {
+ if let Some(bind_ip) = opt_str(BIND_IP) {
+ let mut ip_addresses: Vec<std::net::IpAddr> = vec![];
+ let re_ipv4 = Regex::new(r"(\d+\.\d+\.\d+\.\d+)").unwrap();
+ for ip_addresse in bind_ip.split(",") {
+ if re_ipv4.is_match(ip_addresse){
+ ip_addresses.push(ip_addresse.parse::<std::net::Ipv4Addr>().unwrap().into());
+ } else {
+ let ip_v6: Vec<&str> = ip_addresse.split(":").collect();
+ if 8 == ip_v6.len(){
+ let new_ip_v6: Vec<u16> = ip_v6.iter().map(|&ip_part|
+ if ip_part.is_empty() {
+ 0
+ } else {
+ u16::from_str_radix(ip_part, 16).unwrap()
+ }).collect();
+ ip_addresses.push( std::net::Ipv6Addr::new(
+ new_ip_v6[0],
+ new_ip_v6[1],
+ new_ip_v6[2],
+ new_ip_v6[3],
+ new_ip_v6[4],
+ new_ip_v6[5],
+ new_ip_v6[6],
+ new_ip_v6[7]
+ ).into());
+ } else {
+ println!("Unable to match string {} to an IPv4 or IPv6 pattern", ip_addresse);
+ }
+ }
+ }
+ ip_addresses
+ } else {
+ println!("Unable to bind-ip option, default to all interfaces.");
+ vec![]
+ }
+ } else {
+ vec![]
+ };
+
+
let connect_config = {
let connect_default_config = ConnectConfig::default();
@@ -1608,6 +1659,7 @@ fn get_setup() -> Setup {
zeroconf_port,
player_event_program,
emit_sink_events,
+ bind_ips,
}
}
@@ -1640,6 +1692,7 @@ async fn main() {
.name(setup.connect_config.name.clone())
.device_type(setup.connect_config.device_type)
.port(setup.zeroconf_port)
+ .bind_ips(setup.bind_ips)
.launch()
{
Ok(d) => discovery = Some(d), |
|
Thanks for the hint. Avoid a regex seems like a good idea. The main issue I had with the propose approach is that I am unable to recover if the string cannot be parsed, e.g. @@ -1168,6 +1177,21 @@ fn get_setup() -> Setup {
None => SessionConfig::default().autoplay,
};
+ let bind_ips: Vec<std::net::IpAddr> = if opt_present(BIND_IP) {
+ if let Some(bind_ip) = opt_str(BIND_IP) {
+ let string_ips: Vec<&str> = bind_ip.split(",").collect();
+ string_ips.iter()
+ .map(|s| s.parse::<std::net::IpAddr>().unwrap())
+ .collect()
+ } else {
+ println!("Unable to bind-ip option, default to all interfaces.");
+ vec![]
+ }
+ } else {
+ vec![]
+ };
+
+
let connect_config = {
let connect_default_config = ConnectConfig::default(); |
|
You should look into |
|
And and |
|
Alright thanks for the hints. I will read up and try again ;) |
|
I was able to write something up with let bind_ips: Vec<std::net::IpAddr> = if opt_present(BIND_IP) {
if let Some(bind_ip) = opt_str(BIND_IP) {
let string_ips: Vec<&str> = bind_ip.split(",").collect();
string_ips.iter()
.map(|s| s.trim().parse::<std::net::IpAddr>().unwrap_or_else(|_| {
invalid_error_msg(
BIND_IP,
BIND_IP_SHORT,
s,
"IPv4 and IPv6 addresses",
""
);
exit(1);
}))
.collect()
} else {
println!("Unable to bind-ip option, default to all interfaces.");
vec![]
}
} else {
vec![]
};Continuing in case of an error: let bind_ips: Vec<std::net::IpAddr> = if opt_present(BIND_IP) {
if let Some(bind_ip) = opt_str(BIND_IP) {
let string_ips: Vec<&str> = bind_ip.split(",").collect();
let mut errors = vec![];
let ip_addresses = string_ips.into_iter()
.map(|s| s.trim().parse::<std::net::IpAddr>())
.filter_map(|r|
r.map_err(|e| errors.push(e.to_string())).ok())
.collect();
if !errors.is_empty(){
println!("issue parsing the address errors {:?}, mapped IP addresses {:?} successfull", errors, ip_addresses);
}
ip_addresses
} else {
println!("Unable to bind-ip option, default to all interfaces.");
vec![]
}
} else {
vec![]
}; |
|
Looks good, please feel free to open a PR! :-) I hope you're enjoying Rust :-) |
|
I have created a PR. Thanks for the help and advice. So far I like Rust. |
|
I updated the wiki :). |
I run librespot on a Raspian which also runs a series of docker containers, which implies a shitload of networking interfaces, not all of them routable on the local network. BUT, the mdns responder in librespot includes all of these in the published record. This results in inconsistent connectivity/detection for Spotify clients.
Just to verify I built a macvlan setup with a separate network namespace including the macvlan interface only which resulted in the expected:
Now, I assume this is an oversight but we should have a config/commandline option to specify which network interface to bind to. As discovery is a core function in the librespot library we probably need to include it here and not push it to "future daemon".
For now I get around this with a network namespace limiting the visibility of interfaces for the daemon but this is not quite a well-known nor documented configuration nor should we expect every implementor such as spotifyd/raspotify/vollibrespot to known and implement.
Best regards
Frafall
Example of local interfaces when running on a host also running docker containers:
Macvlan config:
The text was updated successfully, but these errors were encountered: