Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

T3655: Fix NAT problem with VRF #2236

Merged
merged 1 commit into from Sep 10, 2023
Merged

Conversation

vfreex
Copy link
Contributor

@vfreex vfreex commented Sep 10, 2023

Linux netfilter patch https://patchwork.ozlabs.org/project/netfilter-devel/patch/d0f84a97f9c86bec4d537536a26d0150873e640d.1439559328.git.daniel@iogearbox.net/ adds direction support for conntrack zones, which makes it possible to do NAT with conflicting IP address/port tuples from multiple, isolated tenants on a host.

According to the description of the kernel patch:

... overlapping tuples can be made unique with the zone identifier in
original direction, where the NAT engine will then allocate a unique tuple in the commonly shared default zone for the reply direction.

I did some basic tests in my lab and it worked fine to forward packets from eth0 to pppoe0.

  • eth0 192.168.1.1/24 in VRF red
  • pppoe0 dynamic public IP from ISP VRF default
  • set vrf name red protocols static route 0.0.0.0/0 interface pppoe0 vrf 'default'
  • set protocols static route 192.168.1.0/24 interface eth0 vrf 'red'

conntrack -L shows something like:

tcp      6 113 ESTABLISHED src=192.168.1.2 dst=1.1.1.1 sport=58946 dport=80 zone-orig=250 packets=6 bytes=391 src=1.1.1.1 dst=<my-public-ip> sport=80 dport=58946 packets=4 bytes=602 [ASSURED] mark=0 helper=tns use=1

It would be much appreciated if someone could test this with more complex VRF setup.

Change Summary

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes)
  • Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
  • Other (please describe):

Related Task(s)

Component(s) name

Proposed changes

How to test

Checklist:

  • I have read the CONTRIBUTING document
  • I have linked this PR to one or more Phabricator Task(s)
  • I have run the components SMOKETESTS if applicable
  • My commit headlines contain a valid Task id
  • My change requires a change to the documentation
  • I have updated the documentation accordingly

@vyosbot vyosbot requested review from a team, dmbaturin, sarthurdev, zdc, jestabro, sever-sever and c-po and removed request for a team September 10, 2023 17:02
Linux netfilter patch https://patchwork.ozlabs.org/project/netfilter-devel/patch/d0f84a97f9c86bec4d537536a26d0150873e640d.1439559328.git.daniel@iogearbox.net/
adds direction support for conntrack zones, which makes it possible to
do NAT with conflicting IP address/port tuples from multiple, isolated tenants on a host.

According to the description of the kernel patch:
>  ... overlapping tuples can be made unique with the zone identifier in
original direction, where the NAT engine will then allocate a unique
tuple in the commonly shared default zone for the reply direction.

I did some basic tests in my lab and it worked fine to forward packets
from eth0 to pppoe0.
- eth0 192.168.1.1/24 in VRF red
- pppoe0 dynamic public IP from ISP VRF default
- set vrf name red protocols static route 0.0.0.0/0 interface pppoe0 vrf 'default'
- set protocols static route 192.168.1.0/24 interface eth0 vrf 'red'

`conntrack -L` shows something like:
```
tcp      6 113 ESTABLISHED src=192.168.1.2 dst=1.1.1.1 sport=58946 dport=80 zone-orig=250 packets=6 bytes=391 src=1.1.1.1 dst=<my-public-ip> sport=80 dport=58946 packets=4 bytes=602 [ASSURED] mark=0 helper=tns use=1
```

It would be much appreciated if someone could test this with more
complex VRF setup.
@Apachez-
Copy link
Contributor

That netfilter patch is from 2015, is that really valid for the current codebase?

@vfreex
Copy link
Contributor Author

vfreex commented Sep 10, 2023

That netfilter patch is from 2015, is that really valid for the current codebase?

The patch is in linux kernel since at least 4.3 (you can confirm with https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/log/include/net/netfilter/nf_conntrack_zones.h?h=linux-4.3.y), but it was added to nft command only since Feb 2017: https://git.netfilter.org/nftables/commit/src/ct.c?id=ed66d9966294a3bab6c8611e369861ba57374743

@c-po c-po merged commit 87880a5 into vyos:current Sep 10, 2023
6 checks passed
@c-po
Copy link
Member

c-po commented Sep 10, 2023

We should backport this to sagitta branch. Can you create a PR please?

@vfreex vfreex mentioned this pull request Sep 10, 2023
12 tasks
@vfreex
Copy link
Contributor Author

vfreex commented Sep 10, 2023

@c-po #2237

@Apachez-
Copy link
Contributor

Yes but this is a commit towards current which currently is using Linux kernel 6.1.52 LTS.

So if this is already part of the Linux kernel 4.3.something then this commit shouldnt be needed towards current version.

And if this commit is regarding nft that the one used in Debian bookworm for some reason doesnt have this feature backported then this merge should be tagged for removal once nft is updated to 1.0.7 or newer.

Current version of the nft utility in Debian bookworm is according to https://manpages.debian.org/bookworm/nftables/nft.8.en.html version 1.0.6-2+deb12u1.

@c-po
Copy link
Member

c-po commented Sep 10, 2023

VyOS 1.4 and 1.5 use a custom build for nftables

https://github.com/vyos/vyos-build/blob/current/packages/netfilter/Jenkinsfile#L31

@Apachez-
Copy link
Contributor

If nft 1.0.8 is being used which was released july 2023 that gives that the commit in this PR is already included in that aka this PR is not needed for 1.4 and 1.5 (which gives that this commit shouldnt be against "current" but against "equuleus" (1.3.x))?

https://lore.kernel.org/netdev/ZLEr3Eg59HyPUUSR@calendula/T/

@c-po
Copy link
Member

c-po commented Sep 10, 2023

equuleus has very limited VRF support - NAT is out of scope. Also equuleus used iptables.

@Apachez-
Copy link
Contributor

@Mergifyio backport sagitta

@mergify
Copy link

mergify bot commented Sep 11, 2023

backport sagitta

❌ Command disallowed due to command restrictions in the Mergify configuration.

  • sender-permission>=write

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