Skip to content
Prev Previous commit
Next Next commit
Address review comments.
  • Loading branch information
dvc94ch committed Nov 28, 2020
commit 32f106dc7a084aa309b14a0e2bc2c14639aa1831
35 changes: 26 additions & 9 deletions protocols/mdns/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,10 @@ use async_io::Async;
use dns_parser::{Packet, RData};
use either::Either::{Left, Right};
use futures::{future, prelude::*};
use if_watch::{IfEvent, IfWatcher};
use libp2p_core::{multiaddr::{Multiaddr, Protocol}, PeerId};
use log::warn;
use std::{convert::TryFrom, fmt, io, net::Ipv4Addr, net::{UdpSocket, SocketAddr}, str, time::{Duration, Instant}};
use std::{convert::TryFrom, fmt, io, net::{IpAddr, Ipv4Addr}, net::{UdpSocket, SocketAddr}, str, time::{Duration, Instant}};
use wasm_timer::Interval;
use lazy_static::lazy_static;

Expand Down Expand Up @@ -128,7 +129,7 @@ pub struct MdnsService {
/// Buffers pending to send on the query socket.
query_send_buffers: Vec<Vec<u8>>,
/// Iface watch.
if_watch: if_watch::IfWatcher,
if_watch: IfWatcher,
}

impl MdnsService {
Expand Down Expand Up @@ -206,14 +207,30 @@ impl MdnsService {
pub async fn next(mut self) -> (Self, MdnsPacket) {
loop {
while let Some(event) = self.if_watch.next().now_or_never() {
// errors are non fatal.
if let Ok(if_watch::IfEvent::Up(inet)) = event {
if inet.addr().is_ipv4() && !inet.addr().is_loopback() {
self.socket
.get_ref()
.join_multicast_v4(&From::from([224, 0, 0, 251]), &Ipv4Addr::UNSPECIFIED)
.ok();
let multicast = From::from([224, 0, 0, 251]);
let socket = self.socket.get_ref();
match event {
Ok(IfEvent::Up(inet)) => {
if inet.prefix_len() != inet.max_prefix_len() || inet.addr().is_loopback() {
continue;
}
if let IpAddr::V4(addr) = inet.addr() {
if let Err(err) = socket.join_multicast_v4(&multicast, &addr) {
log::error!("join multicast failed: {}", err);
}
}
}
Ok(IfEvent::Down(inet)) => {
if inet.prefix_len() != inet.max_prefix_len() || inet.addr().is_loopback() {
continue;
}
if let IpAddr::V4(addr) = inet.addr() {
if let Err(err) = socket.leave_multicast_v4(&multicast, &addr) {
Copy link
Contributor

@romanb romanb Nov 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned in an earlier comment, I think it would be good to get clarity on whether doing this on an interface that is down makes any sense. If this is just a no-op in this situation, there is no reason to do it and if it is an error it causes unnecessary noise in the logs. If you're sure that doing this on an interface that is down is useful and only errors when there is an error other than the interface being down, I'm fine with it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well it doesn't cause an error on linux or windows. I'll try to investigate more deeply especially on linux, but we are supporting any platform that rust compiles to I guess.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll try to investigate more deeply especially on linux, [..]

Just curious, did you come to a conclusion on the matter?

log::error!("leave multicast failed: {}", err);
}
}
}
Err(err) => log::error!("if watch returned an error: {}", err),
}
}

Expand Down