Skip to content

Conversation

@lixmal
Copy link
Collaborator

@lixmal lixmal commented Mar 24, 2025

Describe your changes

  • Handle Rst in both directions properly
  • Remove IsEstablished
  • Eliminate mutex
  • Fix first state for inbound conns
  • Update counters before terminating conns
  • Create new conns for packets with Syn flag only
  • Add more tests

Issue ticket number and link

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)
  • Extended the README / documentation, if necessary

@lixmal lixmal changed the title [client] Normalize conntrack TCP Rst flag handling [client] Improve TCP conn tracking Mar 24, 2025
@mlsmaycon mlsmaycon requested a review from Copilot March 27, 2025 11:22
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR improves TCP connection tracking by enhancing state management, handling RST packets bidirectionally, and eliminating the mutex for more efficient atomic operations. Key changes include updating the TCP state transitions with atomic operations, refining the logic for outbound and inbound packet tracking, and adding benchmark tests to assess performance.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
client/firewall/uspfilter/conntrack/tcp_bench_test.go Added benchmark tests for tracking and cleanup operations
client/firewall/uspfilter/conntrack/tcp.go Modified TCP state management, updated state transitions, and removed the established flag and mutex
Comments suppressed due to low confidence (1)

client/firewall/uspfilter/conntrack/tcp.go:308

  • For outbound connections in TCPStateFinWait1, when both TCPFin and TCPAck flags are set, the first case is entered but newState is not assigned due to the '!isOutbound' check. Consider handling outbound transitions explicitly (e.g., setting newState = TCPStateFinWait2) to avoid stalling state transitions.
case flags&TCPFin != 0 && flags&TCPAck != 0:

hakansa
hakansa previously approved these changes Mar 31, 2025
@hakansa hakansa self-requested a review March 31, 2025 09:00
@sonarqubecloud
Copy link

@lixmal lixmal merged commit df9c1b9 into main Apr 5, 2025
31 checks passed
@lixmal lixmal deleted the fix-tcp-rst branch April 5, 2025 09:42
remipcomaite pushed a commit to remipcomaite/netbird that referenced this pull request Apr 9, 2025
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.

3 participants