calico icon indicating copy to clipboard operation
calico copied to clipboard

Stop interfering with non-Calico VXLAN packets

Open corhere opened this issue 2 years ago • 5 comments

Description

Bug fix: reintroduce the iptables match clauses which restrict matches on VXLAN packets to those with a VNI related to Calico, reverting cce544613169bd84a8a06f5536abd65cdc3c7ab8. And add the same match clauses to the ip6tables versions of the rules for the first time. Calico will no longer interfere with other users of VXLAN on the same host.

Switch the implementation of the VXLANVNI match rule to use the bpf match which, unlike the u32 match, is supported and installed by default on RHEL/CentOS 7, 8 and 9. And unlike the u32 match expression which was only compatible with iptables, the cBPF socket filter program used with the bpf match rule is compatible with both iptables and ip6tables. The cBPF filter program is a straight port from https://github.com/moby/moby/pull/45308.

I have tested the filter program in isolation with both IPv4 and IPv6. I can port the IPv4 unit test from moby/moby to Ginkgo if there is interest, with the caveat that it depends on CAP_NET_RAW. (Due to the use of cBPF extensions, the filter program cannot be tested with user-space cBPF VMs.)

Related issues/PRs

  • Fixes #6752

Todos

  • [ ] Tests
  • [ ] Documentation
  • [ ] Release note

Release Note

TBD

Reminder for the reviewer

Make sure that this PR has the correct labels and milestone set.

Every PR needs one docs-* label.

  • docs-pr-required: This change requires a change to the documentation that has not been completed yet.
  • docs-completed: This change has all necessary documentation completed.
  • docs-not-required: This change has no user-facing impact and requires no docs.

Every PR needs one release-note-* label.

  • release-note-required: This PR has user-facing changes. Most PRs should have this label.
  • release-note-not-required: This PR has no user-facing changes.

Other optional labels:

  • cherry-pick-candidate: This PR should be cherry-picked to an earlier release. For bug fixes only.
  • needs-operator-pr: This PR is related to install and requires a corresponding change to the operator.

corhere avatar Apr 27 '23 22:04 corhere

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Apr 27 '23 22:04 CLAassistant

Thanks for the PR, would be nice to have this back again for sure. Are you sure that the bpf match is supported by iptables-nft, it is still in the list of "unsupported extensions"?

Since it was unsupported in the past, I think we'll need feature detection to decide whether to use this or not. Calico is in very wide use so I can guarantee that a good number of users will be using nft mode with kernels that are too old to support nft with BPF matches.

fasaxc avatar Apr 28 '23 08:04 fasaxc

/sem-approve

fasaxc avatar Apr 28 '23 08:04 fasaxc

Are you sure that the bpf match is supported by iptables-nft, it is still in the list of "unsupported extensions"?

iptables-nft supports all xtables extensions because it uses the xtables extensions directly. As I understand it, the list of unsupported extensions is a list of xtables extensions with no equivalent nftables extension. It is only relevant when writing rules using the native nft syntax. Not that any extension would be needed with native nftables: the nft raw payload expression @th,96,24 (or, since kernel 5.16, @ih,32,24) can be used to match the VNI field of a VXLAN datagram.

I suspect that what happened with https://github.com/projectcalico/felix/pull/2086 is that the failure to program the rule was misattributed to "NFT mode" when really the culprit was that the xt_u32 kernel module was not installed on the test system. Red Hat moved that module into the kernel-modules-extra package in RHEL 8 which is not installed by default. RHEL 8 is also when the iptables backend was switched to iptables-nft, and combined with the confusing netfilter documentation it would have been easy to come to the wrong conclusion.

The only feature which would need to be detected is whether the xt_bpf kernel module is installed. It gets tricky to detect if it is built into the kernel rather than as a loadable module, though. The xt_bpf module has been available since Linux 3.9 and is installed by default on every distro I've checked. Are there any existing examples of Calico feature-detecting on the availability of kernel modules which I could take inspiration from?

corhere avatar Apr 28 '23 11:04 corhere

How do we progress with this one?

caseydavenport avatar Mar 19 '24 16:03 caseydavenport