Skip to content

Adopt Go generics for typed APIs and decoder selection - #74

Open
dreadl0ck wants to merge 3 commits into
masterfrom
feature/generic-audit-reader
Open

dreadl0ck wants to merge 3 commits into
masterfrom
feature/generic-audit-reader

Conversation

@dreadl0ck

@dreadl0ck dreadl0ck commented Sep 5, 2026

Copy link
Copy Markdown
Owner

Summary

Complete the scoped generics adoption plan without redesigning runtime decoder/writer interfaces or generated protobuf types.

  • Add Go 1.27 AuditRecordReader.NextAs[T] and shared typed traversal for Connection, Service, and DeviceProfile summaries. Preserve header-selected allocation, mismatches, empty arrays, projections, sorting, and existing error policy.
  • Consolidate three slice-based decoder selectors with a generic function. Preserve registry isolation, global validation and wrapped sentinels, leading-empty includes, ordering, duplicate handling, and GoPacket layer names. Leave the map-based stream selector unchanged.
  • Pilot typed packet-handler adapters for ARP and Ethernet with output-equivalence tests and repeatable legacy/generic benchmarks.
  • Add Go 1.27 NetcapClient.GetAs[T] and migrate stable response decoding; retain raw responses and session fallback behavior.
  • Consolidate 30 Maltego transformation aliases and six count aliases into three generic alias families, preserving exported names and function type identity.
  • Align five Docker builders to Go 1.27.0, verify Ubuntu archive checksums, and log the actual CI compiler.

Validation

  • make -f Makefile.test test-unit passed: full default-build unit suite with race detection and coverage, including PCAP capture.
  • Changed packages passed with default and nodpi builds; full changed-package nodpi race run passed.
  • go vet -tags=nodpi ./... passed.
  • make -f Makefile.test test-hyperscan passed.
  • Default and nodpi CLI builds passed.
  • Frontend production build passed for Go embed prerequisites.
  • All five edited Dockerfiles passed docker buildx build --check; Ubuntu archive SHA-256 verified against official metadata. CI YAML/shell checks passed.
  • Go 1.27 formatting and git diff --check passed. Independent code review found no introduced regressions.

Performance Evaluation

Repeated adapter-plus-real-conversion microbenchmarks on Go 1.27.0, darwin/arm64 (Apple M5 Max), at default CPU settings and CPU=1, including a final 10-sample run:

Conversion Legacy B/op Generic B/op allocs/op, both
ARP 320 320 13
Ethernet 144 144 3
Ethernet with entropy 144 144 3

Timing direction varied between runs, with slower generic samples as well as faster ones. These results do not establish a speedup or performance equivalence. Keep the pilot limited to two decoders; no broad packet-path migration. The adapter still performs a runtime type assertion.

Scope Decisions

  • Typed audit-record methods and shared traversal.
  • Generic decoder selection with characterization tests.
  • Two-decoder handler pilot and performance evaluation.
  • Typed MCP responses; no repeated typed POST use justified PostAs.
  • Generic Maltego aliases with compile-time identity/assignability tests.
  • Builder source alignment and validation.
  • Typed cache/pool wrappers deferred: insufficient payoff for extra abstractions.

Limitations And Existing Issues

  • Fresh worktree needed locally generated protobuf/frontend artifacts and a local link to the original checkout PCAP fixture; none are committed. The initial missing-fixture test failure was resolved before the full unit target passed.
  • Existing WebUI skip-on-read-error behavior can loop indefinitely on persistent gzip/transport errors (reader.go:139-149). Preserved rather than changing unrelated error policy; recoverable malformed protobuf records are regression-tested.
  • PATH gofmt is older; used the Go 1.27 toolchain formatter. Installed golangci-lint is incompatible with Go 1.27 and the existing v1 config; CI currently uses go vet. A linter upgrade/config migration remains separate.
  • Production Docker images were not rebuilt or published. Updated builder sources must be rebuilt for downstream consumers.
  • Keep this PR draft as requested.

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown

Thank you for your contribution to NETCAP!

Before we can accept your pull request, you need to sign our Contributor License Agreement (CLA).

To sign the CLA, please read the Individual CLA and comment below with:

I have read and agree to the NETCAP CLA

If you are contributing on behalf of a company or organization, please contact the maintainers to complete the Entity CLA instead.


You can re-trigger this check by commenting recheck on this PR.


I have read and agree to the NETCAP CLA


You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot.

@dreadl0ck dreadl0ck changed the title Adopt Go 1.27 generics for typed audit record reading Adopt Go generics for typed APIs and decoder selection Sep 5, 2026
@dreadl0ck

Copy link
Copy Markdown
Owner Author

Implementation and local evaluation are complete at 975b3b5. Full default unit target (race + coverage), changed-package nodpi race tests, repository-wide nodpi vet, Hyperscan integration tests, and default/nodpi CLI builds passed. The default CLI help smoke test also passed.

Remote status after waiting: all CodeQL analyses passed; Go build/test remains queued with no steps started. CLA Assistant fails because its existing configuration uses the nonexistent main signature branch (Branch main not found), and also reports unsigned contributors. No CLA was signed and no CLA policy/configuration was changed. These are separate from the local generics test results.

The PR remains draft. Detailed implementation scope, benchmark limitations, tooling constraints, and the pre-existing persistent-read-error issue are recorded in the PR description.

@dreadl0ck
dreadl0ck marked this pull request as ready for review September 6, 2026 00:34
@dreadl0ck

Copy link
Copy Markdown
Owner Author

Real-PCAP verification (The Ultimate PCAP v20250325, 15.2 MB, 82 audit record types)

Compared the built binary against an origin/master binary on real traffic. Netcap has substantial inherent run-to-run nondeterminism, so a baseline-vs-baseline control run was used as the noise floor rather than assuming byte equality.

1. Full decode, all decoders

Comparison Types differing
baseline run 1 vs baseline run 2 (control) 17
baseline vs generics 14

The generics differing set is a strict subset of the baseline noise floor. Zero types differ where the baseline is stable, so no regression is attributable to this branch. Generics was actually stable on three types where two baseline runs disagreed (IMAP, TLSCertificate, TLSRecord).

Byte-identical, including the two decoders migrated to typedLayerHandler:
ARP 176, Ethernet 48535, TCP 12892, UDP 23437, IPv4 26648, IPv6 19196, DNS 2892, Connection 3991, HTTP 32, TLSClientHello 58, TLSServerHello 53.

Nondeterministic in both (stream reassembly, random session IDs, header map ordering): DeviceProfile, File, Host, IRC, Kerberos, MQTTSN, Mail, Protobuf, SIP, SMB, SMTP, Secret, Service, Software.

2. Decoder selection — 10/10 scenarios match

Identical audit-record file sets and exit codes for: --include TCP,UDP (3 types), --include ,TCP (82 types — leading-comma disable preserved), --include "TCP,,UDP," (3), --exclude TCP (81), --include TCP,UDP --exclude TCP (2), --exclude OSPF duplicate-layer-name (81), --include TLSRecord,TLSCertificate (3), --include TLSRecord cross-family (2), and both invalid-name cases (exit=1).

The reported sentinel text alternates between invalid abstract decoder and invalid stream decoder. This is pre-existing: the unmodified baseline binary produced both messages across 6 repeat runs. Both sentinels are unchanged in both trees.

3. Typed WebUI readers — byte-identical

Both binaries served the same output directory, isolating reader code from capture nondeterminism:

Endpoint Result
/api/connections identical, 4,033,843 bytes
/api/services identical, 194,837 bytes
/api/devices identical, 64,754 bytes

Edge cases on real audit record files — missing files and wrong-type files (DNS.ncap.gz renamed to Connection.ncap.gz) both return {"connections":[],"totalCount":0} identically on baseline and generics, confirming mismatch-skip and empty-array behavior.

4. Pre-existing bug confirmed triggerable on real data

A truncated Connection.ncap.gz makes /api/connections hang forever on both binaries, spinning CPU and flooding logs (11.3M and 7.4M repetitions of Error reading Connection record: unexpected EOF before being killed). This reproduces identically on origin/master, so it is not a regression from this branch, but it is a real availability bug in the Web UI on master today and should be fixed separately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant