Skip to content

[client] Fix port reuse in tcp conntrack#5300

Open
lixmal wants to merge 2 commits intomainfrom
fix-reused-ports
Open

[client] Fix port reuse in tcp conntrack#5300
lixmal wants to merge 2 commits intomainfrom
fix-reused-ports

Conversation

@lixmal
Copy link
Collaborator

@lixmal lixmal commented Feb 12, 2026

Describe your changes

Issue ticket number and link

In the userspace filter TCP conntrack, if a port was reused, it extended a connection's lifetime whereas it should treat this as a new connection (for SYN).

Also adds a NB_USPFILTER_LOG_BUFFER env var to increase the uspfilter log buffer

Stack

Checklist

  • Is it a bug fix
  • Is a typo/documentation fix
  • Is a feature enhancement
  • It is a refactor
  • Created tests that fail without the change (if possible)

By submitting this pull request, you confirm that you have read and agree to the terms of the Contributor License Agreement.

Documentation

Select exactly one:

  • I added/updated documentation for this change
  • Documentation is not needed for this change (explain why)

Docs PR URL (required if "docs added" is checked)

Paste the PR link from https://github.com/netbirdio/docs here:

https://github.com/netbirdio/docs/pull/__

Summary by CodeRabbit

Summary by CodeRabbit

  • Bug Fixes

    • Improved connection tracking so closed or TIME-WAITed connections are correctly treated as superseded, enabling reliable port reuse and preventing stale entries from blocking new connections.
  • Tests

    • Added extensive tests covering port reuse, tombstone/TIME-WAIT scenarios, retransmits, and edge-case packet timing.
  • Chores

    • Made log buffer size configurable via an environment variable.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 12, 2026

📝 Walkthrough

Walkthrough

Adds TCP conntrack supersession handling via a new IsSupersededBy(flags uint8) method, updates updateIfExists and inbound validation to treat superseded/tombstoned entries as non-existent, expands tests for port reuse/TIME-WAIT/tombstone scenarios, and makes log channel buffer size configurable via an env var.

Changes

Cohort / File(s) Summary
TCP conntrack logic
client/firewall/uspfilter/conntrack/tcp.go
Added IsSupersededBy(flags uint8) bool; updateIfExists now skips updates when a matching entry is superseded; inbound validation uses IsSupersededBy to treat tombstoned/TIME-WAIT as superseded.
TCP port-reuse tests
client/firewall/uspfilter/conntrack/tcp_test.go
Added extensive tests (TestTCPPortReuseTombstone, TestTCPPortReuseTimeWait) covering outbound/inbound reuse, tombstone replacement, TIME-WAIT behaviors, retransmits, and late-packet scenarios.
Log buffer configuration
client/firewall/uspfilter/log/log.go
Introduced getLogChannelSize() reading NB_USPFILTER_LOG_BUFFER; replaced hard-coded channel size with environment-driven defaultLogChanSize fallback.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

  • pappz

Poem

I’m a rabbit in the kernel stack tonight,
Tombstones hop away to let ports take flight,
Tests scurry through TIME‑WAIT and late ACK light,
Logs stretch their bins to hold messages tight,
Hooray — connections bounce back just right! 🐰✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main fix: TCP conntrack port reuse handling, which is the primary change in the changeset.
Description check ✅ Passed The PR description addresses the core changes, includes issue context, has proper checklist selections (bug fix and tests created), and confirms documentation is not needed with explanation.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-reused-ports

No actionable comments were generated in the recent review. 🎉


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@lixmal lixmal changed the title [client] Treated tombstoned conns as new [client] Fix port reuse in tcp conntrack Feb 12, 2026
@sonarqubecloud
Copy link

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

Comments