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

portmap: fix nftables backend #1116

Merged

Conversation

champtar
Copy link
Contributor

@champtar champtar commented Nov 5, 2024

We can't use dnat from the input hook,
depending on nftables (and kernel ?) version we get
"Error: Could not process rule: Operation not supported"
iptables backend also uses prerouting.

Also 'ip6 protocol tcp' is invalid, so rework / simplify the rules

Fixes 01a94e1
Fixes #1115

@champtar champtar force-pushed the portmap-nftables-prerouting-hook branch 2 times, most recently from 2e32a3f to 92965ed Compare November 5, 2024 21:41
@champtar champtar changed the title portmap: use prerouting hook in nft rules portmap: fix nftables backend Nov 5, 2024
We can't use dnat from the input hook,
depending on nftables (and kernel ?) version we get
"Error: Could not process rule: Operation not supported"
iptables backend also uses prerouting.

Also 'ip6 protocol tcp' is invalid, so rework / simplify the rules

Fixes 01a94e1

Signed-off-by: Etienne Champetier <e.champetier@ateme.com>
@champtar champtar force-pushed the portmap-nftables-prerouting-hook branch from 92965ed to 32d1489 Compare November 5, 2024 22:09
@champtar
Copy link
Contributor Author

champtar commented Nov 6, 2024

Tested with

{
  "type": "portmap",
  "capabilities": {"portMappings": true},
  "backend": "nftables",
  "conditionsV4": ["ip", "daddr", "!=", "{ 127.0.0.0/8, 198.19.254.254 }"]
},

@champtar champtar marked this pull request as ready for review November 6, 2024 01:43
Copy link
Contributor

@danwinship danwinship left a comment

Choose a reason for hiding this comment

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

lgtm

Type: knftables.PtrTo(knftables.NATType),
Hook: knftables.PtrTo(knftables.InputHook),
Hook: knftables.PtrTo(knftables.PreroutingHook),
Copy link
Contributor

Choose a reason for hiding this comment

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

OK, I think I know what I was confused about: in prerouting it hasn't yet decided where it's going to route the packet to, so you can't use oifname. I was thinking that meant you couldn't use fib daddr type either, but that's wrong; fib daddr type answers "what does the routing table say we should do with this packet?", not "what are we actually going to do with this packet?", so it doesn't depend on the routing decision.

"th dport", e.HostPort,
"dnat", ipX, "addr . port", "to", containerNet.IP, ".", e.ContainerPort,
e.Protocol, "dport", e.HostPort,
"dnat to", net.JoinHostPort(containerNet.IP.String(), strconv.Itoa(e.ContainerPort)),
Copy link
Contributor

Choose a reason for hiding this comment

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

(yeah, this rule was complicated because it's derived from a kube-proxy rule where we look up the "addr . protocol . port" in a map rather than just having a static rule)

@squeed squeed merged commit 9296c5f into containernetworking:main Nov 18, 2024
6 checks passed
@champtar champtar deleted the portmap-nftables-prerouting-hook branch November 18, 2024 19:34
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.

portmap nftables backend invalid rules (dnat from input hook + invalid ipv6 rules)
3 participants