diff --git a/src/source.rs b/src/source.rs index f701751..222dd1b 100644 --- a/src/source.rs +++ b/src/source.rs @@ -132,7 +132,7 @@ struct SacnSourceInternal { } impl SacnSource { - /// Constructs a new `SacnSource` with the given name, binding to an IPv4 address. + /// Constructs a new `SacnSource` with the given name, binding to an IPv4 address on an ephemeral port. /// This generates a new CID automatically using random values. /// /// # Errors @@ -149,12 +149,14 @@ impl SacnSource { pub fn with_cid_v4(name: &str, cid: Uuid) -> Result { let ip = SocketAddr::new( IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)), - ACN_SDT_MULTICAST_PORT, + // Use an ephemeral port. Binding the ACN port would put this socket in the receivers' `SO_REUSEPORT` group, + // and Linux would then hand some of the unicast sACN addressed to this host to this socket, where it is never read. + 0, ); SacnSource::with_cid_ip(name, cid, ip) } - /// Constructs a new `SacnSource` with the given name, binding to an IPv6 address. + /// Constructs a new `SacnSource` with the given name, binding to an IPv6 address on an ephemeral port. /// By default this will only receive IPv6 data but IPv4 can also be enabled by calling `set_ipv6_only(false)`. /// /// # Errors @@ -171,13 +173,19 @@ impl SacnSource { pub fn with_cid_v6(name: &str, cid: Uuid) -> Result { let ip = SocketAddr::new( IpAddr::V6(Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0, 0)), - ACN_SDT_MULTICAST_PORT, + // Use an ephemeral port. Binding the ACN port would put this socket in the receivers' `SO_REUSEPORT` group, + // and Linux would then hand some of the unicast sACN addressed to this host to this socket, where it is never read. + 0, ); SacnSource::with_cid_ip(name, cid, ip) } /// Constructs a new `SacnSource` with the given name and binding to the supplied ip. /// + /// Avoid binding to `ACN_SDT_MULTICAST_PORT` if a `SacnReceiver` may run on the same host. The socket would share the + /// port with the receiver, and on Linux some unicast sACN sent to this host would be delivered to the source's socket + /// and lost. Port 0 (an ephemeral port) is a good choice. + /// /// # Errors /// See (`with_cid_ip`)[`with_cid_ip`] pub fn with_ip(name: &str, ip: SocketAddr) -> Result { diff --git a/tests/src_tests.rs b/tests/src_tests.rs index ee9605f..90c61ff 100644 --- a/tests/src_tests.rs +++ b/tests/src_tests.rs @@ -702,3 +702,53 @@ const TEST_DATA_MULTIPLE_UNIVERSE: [u8; 712] = [ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 100, ]; + +/// A source created with the default constructor must not take a share of the unicast sACN sent to a receiver on the +/// same host. Both bind with `SO_REUSEPORT`, so if the source also bound the ACN port, Linux would spread unicast +/// datagrams between the two sockets by source port and the receiver would miss some of them. +#[test] +#[serial] +fn test_default_source_does_not_steal_unicast() { + use sacn::receive::SacnReceiver; + use std::net::UdpSocket; + use std::time::Duration; + + let rcv_addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::LOCALHOST), ACN_SDT_MULTICAST_PORT); + let mut rcv = SacnReceiver::with_ip(rcv_addr, None).unwrap(); + rcv.listen_universes(&[1]).unwrap(); + + let _src = SacnSource::new_v4("Idle source").unwrap(); + + const PACKETS: usize = 20; + let mut received = 0; + for _ in 0..PACKETS { + let packet = AcnRootLayerProtocol { + pdu: E131RootLayer { + cid: Uuid::new_v4(), + data: E131RootLayerData::DataPacket(DataPacketFramingLayer { + source_name: "Sender".into(), + priority: 100, + synchronization_address: 0, + sequence_number: 0, + preview_data: false, + stream_terminated: false, + force_synchronization: false, + universe: 1, + data: DataPacketDmpLayer { + property_values: vec![0, 1, 2].into(), + }, + }), + }, + }; + // A new socket each time gives a new ephemeral source port, and so a different SO_REUSEPORT hash. + let sender = UdpSocket::bind((Ipv4Addr::LOCALHOST, 0)).unwrap(); + sender + .send_to(&packet.pack_alloc().unwrap(), rcv_addr) + .unwrap(); + if rcv.recv(Some(Duration::from_millis(500))).is_ok() { + received += 1; + } + } + + assert_eq!(received, PACKETS, "receiver missed unicast packets"); +}