Skip to content

[cllient] Don't track ebpf traffic in conntrack#5166

Merged
lixmal merged 3 commits intomainfrom
no-track-ebpf-traffic
Jan 27, 2026
Merged

[cllient] Don't track ebpf traffic in conntrack#5166
lixmal merged 3 commits intomainfrom
no-track-ebpf-traffic

Conversation

@lixmal
Copy link
Copy Markdown
Collaborator

@lixmal lixmal commented Jan 23, 2026

Describe your changes

In certain cases, NAT rules match our lo <-> lo traffic for wg <-> ebpf proxy (relay). This breaks connectivity.
This PR sets up notrack rules to exclude this traffic that is supposed to be rewritten by ebpf only.

The rules looks like this.

iptables:

*raw
:PREROUTING ACCEPT [0:0]
:OUTPUT ACCEPT [0:0]
:NETBIRD-RAW - [0:0]
-A PREROUTING -j NETBIRD-RAW
-A OUTPUT -j NETBIRD-RAW
-A NETBIRD-RAW -s 127.0.0.1/32 -d 127.0.0.1/32 -o lo -p udp -m udp --sport 40427 -j NOTRACK
-A NETBIRD-RAW -s 127.0.0.1/32 -d 127.0.0.1/32 -o lo -p udp -m udp --dport 40427 -j NOTRACK
-A NETBIRD-RAW -s 127.0.0.1/32 -d 127.0.0.1/32 -i lo -p udp -m udp --dport 40427 -j NOTRACK
-A NETBIRD-RAW -s 127.0.0.1/32 -d 127.0.0.1/32 -i lo -p udp -m udp --dport 3128 -j NOTRACK

nftables:

        chain netbird-raw-out {
                type filter hook output priority raw; policy accept;
                oifname "lo" ip saddr 127.0.0.1 ip daddr 127.0.0.1 udp sport 54255 counter packets 0 bytes 0 notrack
                oifname "lo" ip saddr 127.0.0.1 ip daddr 127.0.0.1 udp dport 54255 counter packets 0 bytes 0 notrack
        }

        chain netbird-raw-pre {
                type filter hook prerouting priority raw; policy accept;
                iifname "lo" ip saddr 127.0.0.1 ip daddr 127.0.0.1 udp dport 54255 counter packets 0 bytes 0 notrack
                iifname "lo" ip saddr 127.0.0.1 ip daddr 127.0.0.1 udp dport 3128 counter packets 0 bytes 0 notrack
        }

Issue ticket number and link

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

  • New Features
    • Added eBPF proxy NOTRACK support to prevent conntrack interference with WireGuard proxy communication
    • Added proxy port retrieval capability for firewall management across different implementations
    • Integrated automatic NOTRACK rule setup during engine initialization

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jan 23, 2026

📝 Walkthrough

Walkthrough

This PR introduces eBPF proxy notrack functionality by adding NOTRACK chain management to firewall implementations (iptables and nftables), exposing proxy port discovery across the proxy stack, and integrating notrack setup into the engine's initialization flow.

Changes

Cohort / File(s) Summary
Firewall Interface Definition
client/firewall/manager/firewall.go
Adds SetupEBPFProxyNoTrack method signature to Manager interface for configuring notrack rules on specified ports.
Iptables Implementation
client/firewall/iptables/manager_linux.go
Introduces NOTRACK chain creation/cleanup helpers and SetupEBPFProxyNoTrack method; integrates chain initialization into Init and cleanup into Close with error handling.
Nftables Implementation
client/firewall/nftables/manager_linux.go
Adds notrack output/prerouting chain management, SetupEBPFProxyNoTrack method, and notrack rule setup across four traffic scenarios; integrates chain initialization into Init and refresh into Flush.
USPFilter Implementation
client/firewall/uspfilter/filter.go
Adds SetupEBPFProxyNoTrack method that delegates to native firewall when available.
Proxy Port Interface & Implementation
client/iface/iface.go, client/iface/wgproxy/ebpf/proxy.go, client/iface/wgproxy/factory_kernel.go, client/iface/wgproxy/factory_usp.go
Introduces GetProxyPort accessor across proxy factory chain: wgProxyFactory interface, WGIface delegation, eBPF proxy storage/retrieval, KernelFactory delegation, and USPFactory stub (returns 0).
Common Interface
client/internal/iface_common.go
Adds GetProxyPort method to wgIfaceBase interface.
Engine Integration
client/internal/engine.go
Adds setupWGProxyNoTrack method that configures notrack rules via firewall when proxy port is available; invoked during Start after WG interface is up.
Engine Testing
client/internal/engine_test.go
Adds GetProxyPortFunc field and GetProxyPort method to MockWGIface for test support.

Sequence Diagram

sequenceDiagram
    participant Engine
    participant WGIface
    participant ProxyFactory
    participant Firewall
    participant KernelIPStack

    Engine->>WGIface: Start()
    WGIface->>ProxyFactory: GetProxyPort()
    ProxyFactory-->>WGIface: proxyPort
    WGIface-->>Engine: interface ready

    Engine->>Engine: setupWGProxyNoTrack()
    Engine->>WGIface: GetProxyPort()
    WGIface-->>Engine: proxyPort (non-zero)
    
    Engine->>Firewall: SetupEBPFProxyNoTrack(proxyPort, wgPort)
    
    alt Iptables Backend
        Firewall->>KernelIPStack: Create NETBIRD-RAW chain
        Firewall->>KernelIPStack: Insert NOTRACK rules for proxy/wg traffic
        KernelIPStack-->>Firewall: rules configured
    else Nftables Backend
        Firewall->>KernelIPStack: Create notrack output chain
        Firewall->>KernelIPStack: Create notrack prerouting chain
        Firewall->>KernelIPStack: Apply NOTRACK on 4 traffic scenarios
        KernelIPStack-->>Firewall: chains configured
    end
    
    Firewall-->>Engine: success/error
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • pappz

Poem

🐰 With ports exposed and chains now set,
The notrack rules prevent regret,
No conntrack noise on proxy's line,
eBPF traffic flows so fine!
Interface stacks align at last,

🚥 Pre-merge checks | ✅ 2 | ❌ 3
❌ Failed checks (2 warnings, 1 inconclusive)
Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR description does not include an issue ticket number or link, making it impossible to verify traceability to related work or requirements. Add the issue ticket number and link in the 'Issue ticket number and link' section of the PR description.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title is partially related but contains a typo ('cllient' instead of 'client') and is somewhat vague about the specific purpose—preventing NAT interference with eBPF proxy traffic via notrack rules is the core intent, but the title doesn't clearly convey this. Fix the typo to '[client]' and consider clarifying the purpose, e.g., 'Add notrack rules for eBPF proxy loopback traffic' or 'Prevent NAT rules from matching eBPF proxy traffic'.
✅ Passed checks (2 passed)
Check name Status Explanation
Description check ✅ Passed The description is mostly complete with clear explanation of the problem, proposed solution, example rules for both iptables and nftables, and properly filled checklist. The Issue ticket number field and Stack section are left empty, but these are non-critical for understanding the change.
Out of Scope Changes check ✅ Passed The changes are focused and in-scope: they add notrack functionality to prevent NAT interference with eBPF proxy traffic. All modifications directly support this objective across firewall managers, proxy factories, and engine initialization.

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

✨ Finishing touches
  • 📝 Generate docstrings

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.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@client/internal/engine.go`:
- Around line 624-635: The setupWGProxyNoTrack method can dereference a nil
e.firewall when the firewall was not created (e.config.DisableFirewall true);
add a guard at the start of Engine.setupWGProxyNoTrack to return early if
e.firewall == nil (or if e.config.DisableFirewall) before calling
e.firewall.SetupEBPFProxyNoTrack, so the method safely no-ops when the firewall
is disabled; reference Engine.setupWGProxyNoTrack, e.firewall,
e.config.DisableFirewall and createFirewall when locating and applying the
change.
🧹 Nitpick comments (1)
client/internal/engine.go (1)

1661-1661: Minor: Extra blank line.

This appears to be a formatting artifact. Consider removing if it wasn't intentional.

Comment thread client/internal/engine.go
@lixmal lixmal force-pushed the no-track-ebpf-traffic branch from daa7346 to dfeae91 Compare January 23, 2026 16:32
return fmt.Errorf("acl manager init: %w", err)
}

if err := m.initNoTrackChain(); err != nil {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

What is the impact of this failing? Maybe we should do a soft failure with log only?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The caller will log it only

return fmt.Errorf("acl manager init: %w", err)
}

if err := m.initNoTrackChains(workTable); err != nil {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

same question. What is the impact of this failing? Maybe we should do a soft failure with log only?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The caller will log it only

@sonarqubecloud
Copy link
Copy Markdown

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