feat(netprobe): IPv6 (ICMPv6) ping support (#624) (#785) * feat(netprobe): per-target ip_version config (4|6) with literal-conflict validation * fix(netprobe): IPv4-preferred DNS resolution, fix addrinfo iteration and IPv6 family selection * fix(netprobe): per-family send socket and IPPROTO_ICMPV6; resolve before socket create * feat(netprobe): send ICMPv6 echo requests for IPv6 ping targets * feat(netprobe): receive and parse ICMPv6 echo replies (dual-socket PingReceiver) * feat(netprobe): handler ICMPv6 echo metrics + v4/v6 matching unit tests * fix(netprobe): account for the IPv6 receiver socket in ping_sockets * feat(netprobe): support IPv6 targets for TCP probes via ip_version * chore(netprobe): add trailing newline to test_netprobe.cpp * fix(netprobe): correct ICMPv6 receive path + DNS re-resolution (adversarial review) Fixes blockers found reviewing the branch against develop: - Heap corruption: the v6 receive lambda built a freestanding ICMPv6EchoLayer over the PingReceiver::_array6 std::array; pcpp ~Layer() does `if (!isAllocatedToPacket()) delete[] m_Data`, so it delete[]'d non-heap storage on every reply. Replaced with build_icmpv6_reply_carrier(), which synthesizes an IPv6 header so a self-owning pcpp::Packet parses IPv6->ICMPv6. - Every v6 reply silently dropped: the carrier Packet used the default ETHERNET link type, so the fan-out deep-copy re-parsed the 8-byte buffer as Ethernet (PayloadLayer), and getLayerOfType() returned null in the handler -> 100% timeouts. The DLT_RAW1 + synthetic IPv6 header now survives the copy. Added a deterministic test that deep-copies the carrier and asserts the layer + id/seq survive (the prior tests bypassed this by calling the handler with a live layer). - v4 regression: _get_addr set _ip_set on DNS targets, pinning them to their first resolution; develop re-resolved each interval. Removed _ip_set from the DNS branches so round-robin/failover/short-TTL names re-resolve. Minors: zero-init _sa/_sa6 (link-local scope_id), null-check spdlog::get, clarify the host-order id/seq comment. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(netprobe): set IPV6_CHECKSUM on the Windows v6 send socket Windows raw sockets do not auto-fill the mandatory ICMPv6 checksum (Linux/macOS kernels do), so v6 echo requests would egress with checksum 0 and be dropped. Enable IPV6_CHECKSUM (offset 2) on the v6 send socket on Windows so the kernel computes it over the IPv6 pseudo-header. POSIX has this implicitly on for IPPROTO_ICMPV6 and rejects setting it, so the call is #ifdef _WIN32 only. Must be verified end-to-end on Windows (incl. inbound raw ICMPv6 delivery). Co-Authored-By: Claude Opus 4.8 (1M context) * fix(netprobe): drop no-op Windows IPV6_CHECKSUM setsockopt; rely on kernel Re-verification (RFC 3542 sec 3.1) showed the IPPROTO_ICMPV6 raw-socket kernel computes and inserts the mandatory ICMPv6 checksum unconditionally on BOTH POSIX and Windows, and setting IPV6_CHECKSUM on such a socket fails — so the previous setsockopt was a no-op with an inverted-premise comment. Removed it and replaced with an accurate note; _send_icmp_v6 correctly leaves the checksum 0 for the kernel to fill. The Windows v6 send/recv path still requires real-Windows verification (inbound raw ICMPv6 delivery). Also add for memset/memcpy. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(netprobe): make IPv6 ping raw-only and read one datagram per poll Addresses two PR review findings: - IPv6 must not fall back to SOCK_DGRAM. The shared PingReceiver only polls a raw _sock6, and a datagram ICMPv6 socket delivers echo replies to the sending socket (which nothing reads) rather than the receiver — so every v6 reply was silently lost and all probes timed out. Drop the DGRAM fallback on both the receive socket (_setup_receiver) and the send socket (_create_socket, gated on !_is_ipv6); v6 now fails cleanly with a socket error / disabled-IPv6 warning when raw-socket privilege (CAP_NET_RAW/root) is unavailable. ICMP6_FILTER is now unconditional since _sock6 is always RAW. - The v6 poll handler drained the socket in a while-loop; under a reply flood that could starve the v4 poll/timer on the same receiver thread. Read a single datagram per (level-triggered) poll event — uvw re-fires while data remains — matching the v4 path's one-read-per-event behavior. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(netprobe): cleanly disable IPv6 ping when the v6 poll handler can't arm Addresses a PR review finding plus an adjacent send/receive consistency gap of the same class (v6 looks active but silently never works): - PingReceiver::_setup_receiver: if the uvw poll handle for _sock6 cannot be created or armed, the previous code left _sock6 open with no handler, yet v6_active() (which only checks _sock6) still reported IPv6 as working — so replies were never read and every v6 probe silently timed out, leaking the fd for the receiver's lifetime. Now gate readiness on poll_handle::start()'s return (resource<>() already runs init(), so the explicit init() was redundant and is dropped) and, on failure, warn + close + invalidate _sock6 so v6_active() returns false and IPv6 ping is cleanly disabled. The destructor already guards both _poll6 and _sock6, so this is double-close-safe. - PingProbe::_create_socket: the v6 send socket is independent of the receiver's raw ICMPv6 socket, and nothing previously checked the two agreed. If the receiver had no v6 socket but a send socket could still open, echoes were sent that could never be matched. Return SocketError when _is_ipv6 && !receiver_v6_active() so the failure surfaces instead of timing out forever. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(netprobe): close send-socket fd leak and add receiver synchronization Fixes three pre-existing (v4-era) defects in the shared ping receiver that the IPv6 work exposed. Verified by adversarial review (no deadlock, no use-after- close, v4 path preserved end-to-end): 1. Send-socket fd leak. _sock/_sock6 are static thread_local and are opened in _create_socket(), which runs in the interval-timer callback on the io thread. But _close_socket() ran from stop() on the control thread, where those thread-locals are still INVALID_SOCKET — so the real raw descriptors were never closed (a leak per policy start/stop cycle). Move the actual close to a new PingProbe::close_thread_send_sockets() invoked at the end of the io-thread lambda (after the loop stops, before stop() joins), so it runs on the thread that owns the descriptors. _close_socket() now only adjusts the ping_sockets refcount. Socket sharing across probes is unchanged (via the thread_local socket value). TCP probes use libuv-managed handles and are unaffected. 2. recv_packets data race. The process-wide static recv_packets was reassigned every 100ms on the receiver thread while per-probe async callbacks iterated it on other threads — a use-after-free if a publish reallocated mid-iteration. Guard it with recv_packets_mtx: publish under the lock; each consumer snapshots under the lock then iterates its private copy unlocked. 3. _receiver singleton + _callbacks races. Construct the process-wide receiver via std::call_once (no double-construct / pointer race on concurrent starts), and publish it through an atomic _receiver_view so the cross-thread reader receiver_v6_active() (called from info_json on the HTTP thread) cannot observe a torn pointer. Guard _callbacks with _callbacks_mtx in register/remove and the timer fan-out so the list can't be mutated mid-iteration. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(netprobe): preserve DNS order for unforced TCP, tidy v6 poll-fail and ping_sockets Addresses three PR review comments: - DNS resolver order (Codex P2): NetProbe::_resolve_dns forced IPv4-preference even when no ip_version was set, so unpinned dual-stack TCP targets could silently switch away from the resolver/OS-preferred (e.g. AAAA-first) endpoint and report spurious failures. When unforced (first_match) it now returns the first address in the resolver's own order, of either family; family filtering applies only when ip_version pins a family. PingProbe is unaffected (it uses its own _get_addr, which keeps an intentional IPv4-preferred default so unpinned ping doesn't require a v6 raw socket). - v6 poll-handle cleanup (Copilot): when the ICMPv6 poll handle can't be armed, close() AND reset() _poll6 in the failure branch instead of leaving it allocated for the process-wide receiver's lifetime. uvw's poll_handle self-retains from a successful init(), so dropping our reference is safe — the handle lives until the loop processes uv_close. - ping_sockets metric (Copilot): the field was gated on a thread_local sock_count that info_json (an HTTP worker thread) always read as 0, so it was effectively never emitted. Removed sock_count (and the now-empty _close_socket) entirely and derive the count from _probes.size() — a stable per-stream member — gated behind _type == TestType::Ping. The real send-socket close path (close_thread_send_sockets on the io thread) is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(netprobe): restore explicit v6 poll init; document why recv batch is copied Addresses the latest PR review comments: - (Codex P1) Restore the explicit _poll6->init() before _poll6->start(). uvw's resource() already initializes the handle (loop::init dispatches to poll_handle::init), so this is technically redundant, but the v4 receiver does the same explicit init() and the asymmetry was misleading. Re-adding it is harmless (an un-started poll handle can be re-initialized) and keeps the v4/v6 paths symmetric. - (Copilot) std::move the carrier pcpp::Packet into the receive batch instead of copying it. (pcpp::Packet currently has no move ctor, so this still deep-copies today, but it is correct and benefits automatically if pcpp gains move support.) - (Copilot) Keep the per-consumer deep copy of the received batch rather than switching to a shared snapshot. A shared snapshot would be a data race: the v4 reply path calls pcpp::IcmpLayer::getEchoReplyData(), which mutates the layer's internal m_EchoData on access, and the single receiver fans every packet out to probes across all input streams running on different io threads. The copy isolates each consumer; added a comment explaining this so it isn't "optimized" away. Ping reply volume is low, so the cost is negligible. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(netprobe): never block the receiver thread on the ICMPv6 recv socket Copilot review: the v6 poll handler did a single recvfrom() per event but the non-blocking setup (ioctlsocket/fcntl) return value was ignored. If the socket stayed blocking and a READABLE event fired without a datagram ready (spurious wakeup / error condition), that read would block the entire receiver thread, stalling reply processing for every ping probe in the process. Two layers of defense: - Capture whether the socket was actually made non-blocking and fold it into the v6-readiness gate (if (_poll6 && nonblock_ok)). If it can't be set, IPv6 is cleanly disabled via the existing close+invalidate path rather than armed. - On POSIX, pass MSG_DONTWAIT to recvfrom so the read provably never blocks even if the socket somehow ended up blocking. On Windows the socket is verified non-blocking, so a 0 flag is equivalent. Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Claude Opus 4.8 (1M context)

Uh oh!

PcapPlusPlus upgrade version from 22.11 to 23.09 #624

Description

Metadata

Metadata

Assignees

Labels

Type

Fields

Projects

Milestone

Relationships

Development

Issue actions