diff --git a/docs/adr/0008-multicast-strategy.md b/docs/adr/0008-multicast-strategy.md index 29f0548..2950c66 100644 --- a/docs/adr/0008-multicast-strategy.md +++ b/docs/adr/0008-multicast-strategy.md @@ -60,24 +60,40 @@ single round-trip. ### 3. VXI-11 portmapper broadcast -A minimal ONC RPC encoder (no dependency on `Vxi11Backend`) sends -`PMAPPROC_GETPORT` for the VXI-11 Device Core program -(`0x0607AF`, version 1) over UDP broadcast `255.255.255.255:111`. -Any host that registers the program in its portmapper replies with -the TCP port; the scanner records the sender IP and builds -`TCPIP0::::inst0::INSTR`. - -Wire format follows RFC 1833 exactly: - -- AUTH_NONE credentials + verifier (flavor = 0, length = 0) -- Big-endian XDR throughout -- 72-byte request, 28-byte minimum successful reply +The scanner sends `PMAPPROC_GETPORT` for the VXI-11 Device Core +program (`0x0607AF` / 395183, version 1) over UDP/111. Any host that +registers the program in its portmapper replies with the TCP port; +the scanner records the sender IP and builds +`TCPIP0::::inst0::INSTR`. The GETPORT request/reply codec is +shared with the client backend's unicast portmapper resolution via +`Vxi11Portmapper` (ADR 0029). + +**Per-interface directed broadcast.** The scanner enumerates every +operational, non-loopback IPv4 interface and sends one probe per NIC, +bound to that NIC's local address, addressed to the interface's +**subnet-directed** broadcast (e.g. `192.168.3.255:111`). A limited +broadcast (`255.255.255.255`) egresses only a single interface on a +multi-homed host — typically whichever owns the default route — so +on a machine with the instruments on a secondary lab NIC it never +reaches them. Probing each subnet directly fixes that. Replies are +de-duplicated by sender IP across all interfaces. + +Wire format follows RFC 1833: AUTH_NONE credentials + verifier +(flavor = 0, length = 0), big-endian XDR, 28-byte minimum successful +reply. The scanner does not chase the per-host TCP port (e.g. by issuing `create_link`) — the portmapper response is sufficient evidence that the standard `Vxi11Backend.OpenAsync` path can reach the instrument. +**Inherent limits.** Broadcast/multicast discovery is link-local: it +cannot cross a router into another subnet (limited broadcast is never +forwarded; directed broadcast is dropped by routers by default per +RFC 2644; mDNS is TTL-scoped), and it only finds instruments that +answer a broadcast GETPORT or advertise mDNS. Instruments on another +subnet are reached by `visa add` with the known address, not by scan. + ### 4. `visa scan` UX ```sh @@ -99,6 +115,12 @@ resource shape so repeated invocations are idempotent: Existing alias collisions are surfaced as "skipped (alias taken)" rather than errors. +`visa scan` prints the **unmasked** resource string (real host) in +both human and `--json` output — it is user-requested discovery +output, not a log line, so the `ToLogString()` masking rule (ADR 0017, +scoped to logging) does not apply. This matches the value `--add` +writes to config. + ### 5. Cancellation + timeout semantics - The discovery window per scanner is fixed at 3 s by default; future diff --git a/src/IviCli.Backends.Vxi11/Vxi11BroadcastScanner.cs b/src/IviCli.Backends.Vxi11/Vxi11BroadcastScanner.cs index 24ce47c..18c2c80 100644 --- a/src/IviCli.Backends.Vxi11/Vxi11BroadcastScanner.cs +++ b/src/IviCli.Backends.Vxi11/Vxi11BroadcastScanner.cs @@ -1,6 +1,7 @@ using System.Collections.Concurrent; using System.Collections.Immutable; using System.Net; +using System.Net.NetworkInformation; using System.Net.Sockets; using IviCli.Application.Backends; using IviCli.Domain; @@ -12,20 +13,22 @@ namespace IviCli.Backends.Vxi11; /// -/// VXI-11 portmapper broadcast scanner (ADR 0008, Batch W). +/// VXI-11 portmapper broadcast scanner (ADR 0008). /// -/// Sends an ONC RPC PMAPPROC_GETPORT request asking for the -/// VXI-11 Device Core program (0x0607AF) over UDP broadcast -/// (255.255.255.255:111). Any host with a VXI-11 server registered -/// answers with the TCP port it listens on; the scanner builds a -/// TCPIP::host::inst0::INSTR resource for each responder. +/// Enumerates every operational IPv4 interface and sends an ONC RPC +/// PMAPPROC_GETPORT request asking for the VXI-11 Device Core +/// program to each interface's subnet-directed broadcast +/// address (e.g. 192.168.3.255:111), bound to that interface's +/// local address. Limited broadcast (255.255.255.255) only ever +/// egresses one interface on a multi-homed host, so a dedicated probe +/// per NIC is required to reach instruments on a secondary lab subnet. +/// Any host with a VXI-11 server registered answers with the TCP port +/// it listens on; the scanner builds a TCPIP::host::inst0::INSTR +/// resource for each responder. /// -/// Discovery window is bounded by a configurable timeout (default -/// 3 s). The scanner intentionally does not chase the per-host TCP -/// port (e.g. by issuing a follow-up create_link) — the -/// presence of a portmapper registration is sufficient evidence -/// that ivicli visa add + the standard backend dispatch will -/// reach the instrument. +/// Broadcast/multicast discovery is link-local: it cannot cross a router +/// into another subnet, and it only finds instruments that answer a +/// broadcast GETPORT. Those limits are inherent (see ADR 0008). /// public sealed class Vxi11BroadcastScanner : IBackendScanner { @@ -49,49 +52,66 @@ CancellationToken ct { ct.ThrowIfCancellationRequested(); - var responders = new ConcurrentDictionary(); - - using var udp = new UdpClient(AddressFamily.InterNetwork) { EnableBroadcast = true }; - - // Bind to an ephemeral port so replies have somewhere to land. - udp.Client.Bind(new IPEndPoint(IPAddress.Any, 0)); - - var xid = unchecked((uint)Random.Shared.Next(int.MinValue, int.MaxValue)); - var request = Vxi11Portmapper.BuildGetportRequest(xid); - - try + var targets = EnumerateTargets(); + if (targets.Count == 0) { - await udp.SendAsync(request, new IPEndPoint(IPAddress.Broadcast, PortmapperPort), ct) - .ConfigureAwait(false); - } - catch (SocketException ex) - { - _logger.LogDebug(ex, "VXI-11 broadcast send failed"); return Result.Success, BackendError>( ImmutableArray.Empty ); } + var responders = new ConcurrentDictionary(); + var xid = unchecked((uint)Random.Shared.Next(int.MinValue, int.MaxValue)); + var request = Vxi11Portmapper.BuildGetportRequest(xid); + using var cts = CancellationTokenSource.CreateLinkedTokenSource(ct); cts.CancelAfter(_discoveryWindow); + await Task.WhenAll(targets.Select(t => ProbeAsync(t, request, xid, responders, cts.Token))) + .ConfigureAwait(false); + + ct.ThrowIfCancellationRequested(); + + var resources = responders + .Keys.Select(BuildDiscovered) + .Where(r => r is not null) + .Select(r => r!) + .ToImmutableArray(); + + return Result.Success, BackendError>(resources); + } + + private async Task ProbeAsync( + BroadcastTarget target, + byte[] request, + uint xid, + ConcurrentDictionary responders, + CancellationToken ct + ) + { try { + using var udp = new UdpClient(new IPEndPoint(target.Local, 0)) + { + EnableBroadcast = true, + }; + await udp.SendAsync(request, new IPEndPoint(target.Broadcast, PortmapperPort), ct) + .ConfigureAwait(false); + while (true) { - cts.Token.ThrowIfCancellationRequested(); UdpReceiveResult datagram; try { - datagram = await udp.ReceiveAsync(cts.Token).ConfigureAwait(false); + datagram = await udp.ReceiveAsync(ct).ConfigureAwait(false); } catch (OperationCanceledException) { - break; + break; // discovery window elapsed } catch (SocketException ex) { - _logger.LogDebug(ex, "VXI-11 broadcast receive failed"); + _logger.LogDebug(ex, "VXI-11 probe receive failed on {Local}", target.Local); break; } @@ -104,18 +124,90 @@ CancellationToken ct } } } - finally + catch (OperationCanceledException) + { + // Window elapsed before the send completed — nothing to collect. + } + catch (SocketException ex) { - // UdpClient.Dispose handles socket teardown. + // Binding/sending on this interface failed (e.g. APIPA, tunnel); + // skip it and let the other interfaces report. + _logger.LogDebug(ex, "VXI-11 probe failed on interface {Local}", target.Local); } + } - var resources = responders - .Select(kvp => BuildDiscovered(kvp.Key)) - .Where(r => r is not null) - .Select(r => r!) - .ToImmutableArray(); + /// + /// Computes the subnet-directed broadcast address for + /// under by setting + /// every host bit (e.g. 192.168.3.10 / 255.255.255.0 → + /// 192.168.3.255). + /// + public static IPAddress DirectedBroadcast(IPAddress address, IPAddress mask) + { + var a = address.GetAddressBytes(); + var m = mask.GetAddressBytes(); + var b = new byte[a.Length]; + for (var i = 0; i < b.Length; i++) + { + b[i] = (byte)(a[i] | (byte)~m[i]); + } + return new IPAddress(b); + } - return Result.Success, BackendError>(resources); + /// + /// Decides whether a unicast address belongs on the probe list: it must + /// sit on an operational, non-loopback interface, be IPv4, and carry a + /// usable subnet mask. + /// + public static bool ShouldProbe( + OperationalStatus status, + NetworkInterfaceType type, + AddressFamily family, + IPAddress? mask + ) => + status == OperationalStatus.Up + && type != NetworkInterfaceType.Loopback + && family == AddressFamily.InterNetwork + && mask is not null + && !mask.Equals(IPAddress.Any); + + private static IReadOnlyList EnumerateTargets() + { + var targets = new List(); + foreach (var nic in NetworkInterface.GetAllNetworkInterfaces()) + { + foreach (var addr in nic.GetIPProperties().UnicastAddresses) + { + if (addr.Address.AddressFamily != AddressFamily.InterNetwork) + { + continue; + } + IPAddress? mask = null; + try + { + mask = addr.IPv4Mask; + } + catch (Exception) + { + mask = null; // platform without IPv4 mask info for this address + } + if ( + !ShouldProbe( + nic.OperationalStatus, + nic.NetworkInterfaceType, + addr.Address.AddressFamily, + mask + ) + ) + { + continue; + } + targets.Add( + new BroadcastTarget(addr.Address, DirectedBroadcast(addr.Address, mask!)) + ); + } + } + return targets; } private static DiscoveredResource? BuildDiscovered(IPAddress host) @@ -128,4 +220,6 @@ CancellationToken ct } return new DiscoveredResource(resource, Idn: null); } + + private readonly record struct BroadcastTarget(IPAddress Local, IPAddress Broadcast); } diff --git a/src/IviCli.Cli/Commands/VisaScanCommand.cs b/src/IviCli.Cli/Commands/VisaScanCommand.cs index 9c3d314..6e98377 100644 --- a/src/IviCli.Cli/Commands/VisaScanCommand.cs +++ b/src/IviCli.Cli/Commands/VisaScanCommand.cs @@ -176,7 +176,7 @@ private static int Success(ScanResult scan, bool emitJson) Console.Write(","); } var r = scan.Resources[i]; - var resourceString = r.Resource.ToLogString(); + var resourceString = FormatResource(r.Resource); var idnJson = r.Idn is null ? "null" : $"\"{Escape(r.Idn)}\""; Console.Write( string.Create( @@ -200,7 +200,7 @@ private static int Success(ScanResult scan, bool emitJson) var r = scan.Resources[i]; Console.WriteLine(string.Create(inv, $"[{i + 1}]")); Console.WriteLine( - string.Create(inv, $" Resource: {r.Resource.ToLogString()}") + string.Create(inv, $" Resource: {FormatResource(r.Resource)}") ); if (r.Idn is not null) { diff --git a/tests/IviCli.Backends.Vxi11.Tests/Vxi11BroadcastScannerTests.cs b/tests/IviCli.Backends.Vxi11.Tests/Vxi11BroadcastScannerTests.cs new file mode 100644 index 0000000..b054c77 --- /dev/null +++ b/tests/IviCli.Backends.Vxi11.Tests/Vxi11BroadcastScannerTests.cs @@ -0,0 +1,85 @@ +using System.Net; +using System.Net.NetworkInformation; +using System.Net.Sockets; +using IviCli.Backends.Vxi11; +using Shouldly; + +namespace IviCli.Backends.Vxi11.Tests; + +/// +/// Unit coverage for the pure helpers behind the multi-NIC broadcast +/// scan: per-subnet directed-broadcast computation and the interface +/// filter. The socket round-trip itself is exercised against real +/// hardware (ADR 0008 §Verification), not in unit tests. +/// +public sealed class Vxi11BroadcastScannerTests +{ + [Theory] + [InlineData("192.168.3.10", "255.255.255.0", "192.168.3.255")] + [InlineData("10.0.0.5", "255.0.0.0", "10.255.255.255")] + [InlineData("172.16.5.4", "255.255.0.0", "172.16.255.255")] + [InlineData("192.168.1.130", "255.255.255.128", "192.168.1.255")] + public void DirectedBroadcast_sets_host_bits_from_mask( + string addr, + string mask, + string expected + ) + { + var result = Vxi11BroadcastScanner.DirectedBroadcast( + IPAddress.Parse(addr), + IPAddress.Parse(mask) + ); + + result.ShouldBe(IPAddress.Parse(expected)); + } + + [Fact] + public void ShouldProbe_accepts_an_up_ipv4_ethernet_interface() + { + Vxi11BroadcastScanner + .ShouldProbe( + OperationalStatus.Up, + NetworkInterfaceType.Ethernet, + AddressFamily.InterNetwork, + IPAddress.Parse("255.255.255.0") + ) + .ShouldBeTrue(); + } + + [Theory] + [InlineData(OperationalStatus.Down, NetworkInterfaceType.Ethernet, AddressFamily.InterNetwork)] + [InlineData(OperationalStatus.Up, NetworkInterfaceType.Loopback, AddressFamily.InterNetwork)] + [InlineData(OperationalStatus.Up, NetworkInterfaceType.Ethernet, AddressFamily.InterNetworkV6)] + public void ShouldProbe_rejects_unusable_interfaces( + OperationalStatus status, + NetworkInterfaceType type, + AddressFamily family + ) + { + Vxi11BroadcastScanner + .ShouldProbe(status, type, family, IPAddress.Parse("255.255.255.0")) + .ShouldBeFalse(); + } + + [Fact] + public void ShouldProbe_rejects_a_missing_or_empty_mask() + { + Vxi11BroadcastScanner + .ShouldProbe( + OperationalStatus.Up, + NetworkInterfaceType.Ethernet, + AddressFamily.InterNetwork, + null + ) + .ShouldBeFalse(); + + Vxi11BroadcastScanner + .ShouldProbe( + OperationalStatus.Up, + NetworkInterfaceType.Ethernet, + AddressFamily.InterNetwork, + IPAddress.Any + ) + .ShouldBeFalse(); + } +}