Native nftables support for sidecar mode

[sridhargaddam]

This PR includes support for using nftables as an alternative to iptables for setting up traffic redirection rules in Istio. Nftables offers a more modern and consistent interface, and aligns with the direction many Linux distributions are heading. With this change, users can optionally enable nftables for setting up traffic redirection rules when using the sidecar mode, by configuring the --native-nftables flag.

Supported features:

• Istio sidecar mode with istio-cni as well as istio-init container.
• Supports all the usual pod annotations like excludeInboundPorts, includeOutboundIPRanges, and so on.

Implementation details:

• A new config option --native-nftables is added which can be set using helm (global.nativeNftables) as well as istioctl.
• Updated the injection template and config-map to support this feature.
• The CNI plugin now checks this config and chooses either iptables or nftables accordingly.
• Shared code between iptables and nftables has been moved into a common package to avoid code duplication.
• Added builder APIs that build up nftables rules in memory before applying them to the system.
• Added new APIs that use nftables syntax to support various use cases, including all the existing customization options.
• Added unit tests for the new code paths.
• This implementation uses the knftables library from the Kubernetes sig-network group. This library is used in couple of other K8s projects like kube-proxy, Calico, OVN-K and so on.

Why a feature flag? This is a new implementation and plays a critical role in traffic redirection, so we want to give it time to mature before making it the default. Introducing a feature flag allows users to adopt it gradually and helps prevent unexpected behavior in environments where both iptables and nftables are available.

Pending:

• Support for Ambient mode will be added in a follow-up PR.
• CI jobs for validating the nftables implementation.

Note: The default behavior still uses iptables. Nftables will only be used if the feature flag is explicitly enabled.

Related to: https://github.com/istio/istio/issues/47821

Signed-off-by: Sridhar Gaddam [email protected] Co-authored-by: Yuanlin Xu [email protected]

[sridhargaddam]

I'll look into the test failures, but I'd appreciate any review comments in the meantime. Thank you.

[sridhargaddam]

Note to reviewers: Sorry for the large PR. I’ve split it into separate commits to make it easier to review. Hope that helps. Thanks!

[leosarra]

Thank you! I was busy in the last few days. I will take a look tomorrow :)

[sridhargaddam]

Thank you! I was busy in the last few days. I will take a look tomorrow :)

Thanks @leosarra 🙂

[sridhargaddam]

@istio/wg-networking-maintainers appreciate your review comments. Thank you 🙏🏼

[leosarra]

I added some comments with proposed fixes/changes taken from my branch, feel free to cherry-pick any of them if you agree. Overall, it looks good, but I have some concerns about https://github.com/istio/istio/pull/56487#discussion_r2134753687

[sridhargaddam]

/test integ-cni

[sridhargaddam]

Failure in unit-tests-arm64 is not related to this PR.

[dhawton]

/retest

[sridhargaddam]

Commit-17 logs the nftable rules that are successfully programmed. Sample output

• when using istio-init
• when using istio-cni

[sridhargaddam]

I'm currently waiting for the knftables PR, which adds support for multiple tables/families in the same transaction, to be merged. Once the PR is merged, I'll update this PR accordingly. In the meantime, I’d appreciate any review comments. TIA.

[sridhargaddam]

I'm currently waiting for the knftables PR, which adds support for multiple tables/families in the same transaction, to be merged. Once the PR is merged, I'll update this PR accordingly. In the meantime, I’d appreciate any review comments. TIA.

Done. Commit 18 includes the changes to use the newer knftables APIs (under review) that supports multiple tables/families in a single transaction.

[sridhargaddam]

@istio/wg-networking-maintainers can you PTAL, thanks.

[sridhargaddam]

Hello @howardjohn, @keithmattix /other Istio maintainers,

I'd like to target this Nftables PR for the Istio 1.27 release and would really appreciate your review 🙏🏼. I understand that this is a big PR, so I’ve structured it into individual commits to make it easier to follow. @leosarra has been carefully reviewing the PR, many thanks to him for the detailed feedback so far. If you have any questions or need clarification on any part, please feel free to ask. Thanks in advance!

[keithmattix]

@sridhargaddam thank you for this PR and for organizing it. I will get this on my list for review this week

[sridhargaddam]

@sridhargaddam thank you for this PR and for organizing it. I will get this on my list for review this week

Thank you so much @keithmattix. Appreciate you adding it to your review list. Looking forward to your feedback.

[sridhargaddam]

/test unit-tests

[sridhargaddam]

/test unit-tests-arm64

[sridhargaddam]

I'm OOO next week, so approving for networking with a hold so that @howardjohn can take a look before feature freeze. If there's no feedback by then, remove the hold

@zirain @keithmattix I removed the hold, but the PR is not getting merged. Any idea why?

[dhawton]

I'm OOO next week, so approving for networking with a hold so that @howardjohn can take a look before feature freeze. If there's no feedback by then, remove the hold

@zirain @keithmattix I removed the hold, but the PR is not getting merged. Any idea why?

Because it's missing reviews from one of the workgroups @howardjohn is part of. He commented so it's not listed, but tide says you need an approval from more than Keith.

[sridhargaddam]

@zirain @keithmattix I removed the hold, but the PR is not getting merged. Any idea why?

Because it's missing reviews from one of the workgroups @howardjohn is part of. He commented so it's not listed, but tide says you need an approval from more than Keith.

Thanks for the clarification @dhawton. It seems like @howardjohn is a bit tied up at the moment. With the feature freeze coming up next week, I'm a bit concerned about the timelines. I hope someone else from the relevant workgroup can take a look and approve the PR in the meantime.

[sridhargaddam]

/test gencheck

[sridhargaddam]

/test gencheck

[sridhargaddam]

The failure in "gencheck" is not related to this PR.

[istio-testing]

In response to a cherrypick label: #56487 failed to apply on top of branch "release-1.27":

Applying: istio-cni: add support for native nftables rules
Applying: Move common code into a separate package
Applying: support native nftables flag while using istio-init container
Applying: Implement nftables for sidecar proxy mode
Using index info to reconstruct a base tree...
M	go.mod
M	go.sum
Falling back to patching base and 3-way merge...
Auto-merging go.sum
Auto-merging go.mod
CONFLICT (content): Merge conflict in go.mod
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
Patch failed at 0004 Implement nftables for sidecar proxy mode

[istio-testing]

In response to a cherrypick label: new issue created for failed cherrypick: #56996