fix(inputs/netprobe): connect TCP probes to IPv6 targets and account for every attempt by leoparente · Pull Request #807 · netboxlabs/pktvisor

leoparente · 2026-10-29T22:50:01Z

Summary

Follow-up to #805: problems in netprobe's TCP probe, all in TcpProbe::_perform_tcp_process() and its interval timer.

IPv6 targets connected to a corrupted address. uvw's connect(ip, port) builds the address with a helper that returns a 16-byte sockaddr. That cuts a 28-byte sockaddr_in6 short, so libuv read the end of the address and the scope id from whatever followed on the stack. This is the same truncation #805 fixed for flow binds. The probe now builds a sockaddr_storage with uv_ip4_addr/uv_ip6_addr and calls connect(const sockaddr &).

Every attempt now gets exactly one result. Before:

connect()'s returned error was ignored, so a connect that failed inside connect() itself, such as a TCP connect to a multicast address, reported neither success nor failure.
A failed attempt never closed its handle, so a refused target leaked a handle and its socket on every interval.
An attempt still connecting at the next interval was never closed. The next attempt replaced it in _client, stop() closed only the newest, and the loop stayed busy until the kernel gave up on the SYNs: 75 s here, 127 s on Linux. When it gave up, the attempt counted as a connect failure, although by then it had long passed timeout_msec.

Now:

A returned error is a connect failure, and a failed attempt closes its handle.
The next interval closes an attempt still connecting, before any DNS lookup, and reports it as a timeout. The stream handler counts a TCP timeout only when a result arrives late, and the next send replaces the pending transaction, so without that report a target that never answers would show only attempts.
An attempt closed by stop() or by the next interval (UV_ECANCELED) is not a connect failure, and neither is an error from the shutdown that follows a successful connect.

A tick that comes before the attempt has run timeout_msec is put off. An attempt can start late behind a loop blocked by another target's synchronous DNS lookup. Also, libuv's loop clock rounds to milliseconds, so when timeout_msec equals interval_msec a tick can come a millisecond early. Dropping the attempt then would count a timeout too early, and skipping the tick would lose 10-30% of attempts in that configuration. So the tick is put off until the attempt has run its timeout, by a steady clock. A deferral shifts that probe's later ticks by the same amount. No attempt is lost, and with the defaults (5000/2000) it takes a block of more than 3 s.

The handler timed results against 2000 ms, whatever timeout_msec said. create_event_proxy passed the stream's _timeout_msec to the handler as xact_ttl_ms. Only start() sets that member, and a policy load creates the proxy before it starts the input (Policies.cpp). So with timeout_msec: 500 a 1.5 s ping reply counted as a success, and with timeout_msec: 5000 a 3 s TCP handshake counted as a timeout. The proxy now reads timeout_msec from the input's config, which is merged in before the proxy is created. This was found while documenting the timeout behaviour, and predates this PR.

Tests

All 25 ctest targets pass on macOS, Release and -O0. The new TCP probe tests drive a TcpProbe directly on its own loop:

IPv6 target: connects to a server on [::1], with every attempt succeeding.
Refused: every attempt is a connect failure and closes its handle.
Immediate failure: a connect to 224.0.0.1 that fails inside connect() is reported.
Still connecting: with a local server that never accepts, an attempt still connecting at the next tick is reported as a timeout.
Late start: with the loop blocked before a tick, no attempt is reported as timed out before 40 ms, every attempt is accounted for, and no tick is lost.
Handler TTL: an event proxy created before the input starts, as a policy load does, carries the input's timeout_msec. This failed on develop with 2000.

Every probe test also checks that no handle is left open and that the loop closes. All of them failed on develop. The IPv6 one fails only unoptimized: at -O2 the truncated address happens to survive on the stack. The Debug code-coverage job runs it unoptimized. The late-start test checks ordering against a timer that ticks with the probe, not wall-clock gaps, so CPU load doesn't affect it. The probe stops 25 ms after its sixth attempt, so the counts are exact.

Not changed here

orb-agent's docs/backends/pktvisor/inputs/netprobe.md says the TCP probe does not enforce timeout_msec. After this change it does, at the next tick. The docs are updated in a separate orb-agent PR, to merge after this one.
_is_ipv4 stays unused, as on develop.
The invalid-address branch reports a failure without an attempt. It cannot be reached: the address string always comes from toString() or inet_ntop.

🤖 Generated with Claude Code

…for every attempt uvw's connect(ip, port) builds the address with a helper that returns a 16-byte sockaddr, so an IPv6 target lost the end of its address and the connect went to whatever followed on the stack. The probe now builds a sockaddr_storage and calls connect(const sockaddr &). The same function had three more problems: - connect()'s returned error was ignored, so a connect that failed inside connect() itself was reported neither way. It is now a connect failure. - A failed attempt never closed its handle, leaking it and its socket on every interval. It is now closed. - An attempt still connecting when the next interval started was never closed: stop() closed only the newest, and the loop stayed busy until the kernel gave up. The next interval now closes it, before any DNS lookup, and reports it as a timeout, because the stream handler counts a timeout only when a result arrives late. A tick can come before the attempt has run timeout_msec, when the attempt started late behind a loop blocked by another target's DNS lookup, or by the loop clock's rounding to milliseconds when timeout_msec equals interval_msec. That tick is put off until the attempt has run its timeout. An attempt closed by stop() or by the next interval is not reported as a connect failure, and neither is an error from the shutdown that follows a successful connect. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

leoparente · 2026-10-29T22:50:10Z

@codex review

leoparente · 2026-10-29T22:50:11Z

/ai-review

chatgpt-codex-connector · 2026-10-29T22:50:22Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-30T01:12:40.440354Z	`405fe21`	Manual request

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

nbl-ai-review · 2026-10-29T22:50:30Z

AI Code Review — risk tier: Lite · dimensions run: correctness, security

Advisory only. A human owns the merge decision.

Summary — Net-positive fix: the new attempt-tracking state machine correctly connects TCP probes to IPv6 targets and accounts for every connection attempt, with tests covering the refused/immediate-failure/still-connecting/late-tick cases. No correctness or security issues found.

_{🤖 AI Code Review · run · prompts: code-review/v1 · models: claude-sonnet-5/medium · context: description · 3 comment(s) · 0 prior AI review(s) · usage: $1.03 · 1821k in (95% cached) · 14k out}

nbl-ai-review · 2026-10-29T22:50:38Z

AI Code Review — risk tier: Lite · dimensions run: correctness, security

Advisory only. A human owns the merge decision.

Summary — Net positive change. The TcpProbe attempt-lifecycle rework (IPv6 sockaddr fix, connect()-error handling, timeout deferral, cancel-vs-failure disambiguation) and the create_event_proxy timeout_msec fix are internally consistent, with no correctness or security issues found.

_{🤖 AI Code Review · run · prompts: code-review/v1 · models: claude-sonnet-5/medium · context: description · 7 comment(s) · 2 prior AI review(s) · usage: $1.52 · 2754k in (95% cached) · 14.4k out}

Previous review · 2026-10-29 · c7c2905

AI Code Review — risk tier: Lite · dimensions run: correctness, security

Advisory only. A human owns the merge decision.

Summary — Net-positive fix: corrects IPv6 sockaddr truncation in the TCP probe's connect() path and tightens attempt/timeout accounting so every attempt yields exactly one result. The attempt-lifecycle changes (deferral, cancel-vs-failure disambiguation) are internally consistent, and no security-relevant concerns arise since probe targets come from local configuration/DNS rather than untrusted external input. No findings to report.

_{🤖 AI Code Review · run · prompts: code-review/v1 · models: claude-sonnet-5/medium · context: description · 3 comment(s) · 0 prior AI review(s) · usage: $0.88 · 1632k in (96% cached) · 11.4k out}

chatgpt-codex-connector · 2026-10-29T22:53:02Z

Codex Review: Didn't find any major issues. What shall we delve into next?

Reviewed commit: c7c29057f2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

github-actions · 2026-10-29T23:04:42Z

LCOV of commit `405fe21` during Debug Builds #270

  lines......: 86.2% (18508 of 21475 lines)
  functions..: 75.7% (1861 of 2459 functions)
  branches...: no data found

Files changed coverage rate: n/a

Full coverage report

…s transaction TTL create_event_proxy passed the stream's _timeout_msec to the handler as xact_ttl_ms, but only start() sets that member, and a policy load creates the proxy before it starts the input. So the handler of a policy that created its netprobe input always timed ping replies and TCP connections against the 2000 ms default, whatever timeout_msec said. The proxy now reads timeout_msec from the input's config, which is merged in before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

leoparente · 2026-10-30T01:10:50Z

@codex review

leoparente · 2026-10-30T01:10:51Z

/ai-review

nbl-ai-review · 2026-10-30T01:11:21Z

🔁 AI Code Review updated for 405fe218 — see the review · 0 resolved · 0 open (run)

chatgpt-codex-connector · 2026-10-30T01:12:39Z

Codex Review: Didn't find any major issues. 🚀

Reviewed commit: 405fe21886

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

leoparente marked this pull request as ready for review October 30, 2026 00:48

leoparente self-assigned this Oct 30, 2026

samiura approved these changes Oct 30, 2026

View reviewed changes

leoparente merged commit 176778a into develop Oct 30, 2026
23 checks passed

leoparente deleted the fix/netprobe-tcp-ipv6 branch October 30, 2026 14:03

Navigation Menu

Repository navigation

fix(inputs/netprobe): connect TCP probes to IPv6 targets and account for every attempt - #807

fix(inputs/netprobe): connect TCP probes to IPv6 targets and account for every attempt#807
leoparente merged 2 commits into
developfrom
fix/netprobe-tcp-ipv6

leoparente commented Oct 29, 2026 •

edited

Loading

Uh oh!

leoparente commented Oct 29, 2026

Uh oh!

leoparente commented Oct 29, 2026

Uh oh!

chatgpt-codex-connector Bot commented Oct 29, 2026 •

edited

Loading

Uh oh!

nbl-ai-review Bot commented Oct 29, 2026 •

edited

Loading

Uh oh!

nbl-ai-review Bot commented Oct 29, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot commented Oct 29, 2026

Uh oh!

github-actions Bot commented Oct 29, 2026 •

edited

Loading

Uh oh!

leoparente commented Oct 30, 2026

Uh oh!

leoparente commented Oct 30, 2026

Uh oh!

nbl-ai-review Bot commented Oct 30, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot commented Oct 30, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

leoparente commented Oct 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Tests

Not changed here

Uh oh!

leoparente commented Oct 29, 2026

Uh oh!

leoparente commented Oct 29, 2026

Uh oh!

chatgpt-codex-connector Bot commented Oct 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

nbl-ai-review Bot commented Oct 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nbl-ai-review Bot commented Oct 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chatgpt-codex-connector Bot commented Oct 29, 2026

Uh oh!

github-actions Bot commented Oct 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

LCOV of commit 405fe21 during Debug Builds #270

Uh oh!

leoparente commented Oct 30, 2026

Uh oh!

leoparente commented Oct 30, 2026

Uh oh!

nbl-ai-review Bot commented Oct 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chatgpt-codex-connector Bot commented Oct 30, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

leoparente commented Oct 29, 2026 •

edited

Loading

chatgpt-codex-connector Bot commented Oct 29, 2026 •

edited

Loading

nbl-ai-review Bot commented Oct 29, 2026 •

edited

Loading

nbl-ai-review Bot commented Oct 29, 2026 •

edited

Loading

github-actions Bot commented Oct 29, 2026 •

edited

Loading

LCOV of commit `405fe21` during Debug Builds #270

nbl-ai-review Bot commented Oct 30, 2026 •

edited

Loading