calico
calico copied to clipboard
Add setting to enable wireguard NAPI threading
Description
This adds a new setting to enable wireguard NAPI threading on the wireguard interfaces. NAPI threading can be used with wireguard to help increase maximum packets-per-second by distributing the tunnel's load from one core to many cores via a NAPI kernel thread.
This option is off by default and can be configured via the FelixConfiguration the same way wireguard can be toggled on and off.
Tested on a bare metal Kubernetes cluster. NAPI threading is enabled on the second pass of ensureLink, once the interface is up. I get operation not permitted on the first run before the up|peertopeer flags are set, which is why I have a flag guard.
This affects calico-node the most when it's configuring the wireguard interface. All other components are reconciling the new FelixConfiguration parameter.
Related issues/PRs
https://github.com/projectcalico/calico/issues/9223
Todos
- [ ] Tests
- [ ] Documentation
- [ ] Release note
Release Note
Add setting to enable wireguard NAPI threading
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.
/sem-approve
/sem-approve
/sem-approve
Addressed comments and rebased to resolve a conflict.
/sem-approve
/sem-approve
/sem-approve
/sem-approve
I'm struggling to get the new test to pass locally. Wireguard NAPI threading only works after the wireguard interface is up. I gave ginkgo more time with 60s checking every second for a theoretical second reconcile. But ginkgo doesn't seem to be giving me the full 60 seconds to wait for the bit to change. This command returns failure near instantly on my development machine. Am I missing something with how ginkgo/gomega works?
https://pkg.go.dev/github.com/onsi/gomega#Eventually with 60s 1s should wait 60 seconds for me. It looks like for semaphore CI it waits 10 seconds. I'm hoping I can get it to wait longer for that second reconcile locally.
In production use I found it enables NAPI threading on the second reconcile of ensureLink once the up flag is set.
make fv GINKGO_FOCUS="the Wireguard threading config should be configured properly" GINKGO_ARGS="-ginkgo.progress -ginkgo.v -ginkgo.failFast" |& tee fv.log
/sem-approve
I think my local development is having an issue is this part of the E2E test, which is why I'm struggling to iterate on this test:
felix-0-127157-3-felixfv[stderr] docker: Error response from daemon: OCI runtime create failed: container_linux.go:377: starting container process caused: pr
ocess_linux.go:495: container init caused: rootfs_linux.go:76: mounting "/home/debian/calico/felix/bin/calico-felix-race-amd64" to rootfs at "/usr/local/bin/
calico-felix" caused: mount through procfd: not a directory: unknown: Are you trying to mount a directory onto a file (or vice-versa)? Check if the specified
host path exists and is the expected type.
felix-1-127157-2-felixfv[stderr] docker: Error response from daemon: OCI runtime create failed: container_linux.go:377: starting container process caused: pr
ocess_linux.go:495: container init caused: rootfs_linux.go:76: mounting "/home/debian/calico/felix/bin/calico-felix-race-amd64" to rootfs at "/usr/local/bin/
calico-felix" caused: mount through procfd: not a directory: unknown: Are you trying to mount a directory onto a file (or vice-versa)? Check if the specified
host path exists and is the expected type.
I have that file:
debian@devvm:~/calico/felix$ ls -l bin/calico-felix-race-amd64
-rwxr-xr-x 1 debian docker 149270288 Oct 16 21:35 bin/calico-felix-race-amd64
debian@devvm:~/calico/felix$
debian@devvm:~/calico/felix$ docker version
Client:
Version: 20.10.5+dfsg1
API version: 1.41
Go version: go1.15.15
Git commit: 55c4c88
Built: Mon May 30 18:34:49 2022
OS/Arch: linux/amd64
Context: default
Experimental: true
Server:
Engine:
Version: 20.10.5+dfsg1
API version: 1.41 (minimum version 1.12)
Go version: go1.15.15
Git commit: 363e9a8
Built: Mon May 30 18:34:49 2022
OS/Arch: linux/amd64
Experimental: false
containerd:
Version: 1.4.13~ds1
GitCommit: 1.4.13~ds1-1~deb11u4
runc:
Version: 1.0.0~rc93+ds1
GitCommit: 1.0.0~rc93+ds1-5+deb11u5
docker-init:
Version: 0.19.0
GitCommit:
Should I use a different VM than Debian? What distro / version does CI run on?
It looks like Ubuntu is recommend so I'll try iterating on there.
@jrcichra yes, please try with Ubuntu.
Thanks @mazdakn I am able to run the wireguard e2e tests now with an Ubuntu 24.04 VM and docker + buildx + ipv6 networking enabled. I moved my test into a better location. I can confirm that in the E2E test the Apply() / ensureLink step is properly setting up the interface. Once the reconcile loop sets NAPI threading on, my Eventually() test passes.
The only downside right now is increased E2E test duration. Since ensureLink sets wireguard NAPI threading once the wireguard link is up because that's the only time you can set NAPI threading, it slows down the E2E test. You'll see:
Creating Command [docker exec felix-0-149774-29-felixfv cat /sys/class/net/wg0/threaded]
for a bit until the reconcile loop sets NAPI threading. I'm not sure how to reduce that unless we modify how often ensureLink is executed or maybe trigger when the link status / flags are changed.
make fv GINKGO_FOCUS="the Wireguard device should be configurable" GINKGO_ARGS="-ginkgo.progress -ginkgo.v -ginkgo.failFast" |& tee fv.log
Ran 12 of 987 Specs in 790.061 seconds
SUCCESS! -- 12 Passed | 0 Failed | 0 Pending | 975 Skipped
PASS
+ status=0
+ echo 'Test batch 1 completed with status 0'
Test batch 1 completed with status 0
+ exit 0
+ st=0
+ echo 'Result: 0'
Result: 0
+ '[' 0 -ne 0 ']'
+ '[' 0 -eq 0 ']'
+ echo 'All tests passed'
All tests passed
make[1]: Leaving directory '/home/ubuntu/calico/felix'
Thanks again for walking me through this!
@jrcichra Yes, I think the execution time is going to rise dramatically. I think it's better to NOT double the number of test cases. Instead you can add a single test to the current tests cases. As an example, it's possible to set the value, wait until reading from disk return the correct value. You can switch back and forth the value, and it should be enough.
Agreed, I just used the same # of tests and toggle wireguard napi threading on and off now. That covers it.
Ran 6 of 891 Specs in 294.596 seconds
SUCCESS! -- 6 Passed | 0 Failed | 0 Pending | 885 Skipped
@jrcichra Thanks for the changes. I think we are getting close to merging this. I left few comments about the test setup. It feel moving the test changes into an independent test make it more clean.
/sem-approve
@jrcichra thanks for updates. The CI failed in the pre-flight test. You need to run make generate in the root directory of Calico to generate doc update for FelixConfiguration. This is a new tool we recently added.
@mazdakn thanks just rebased and generated those. I see this has the tag docs-not-required but I would like to include some docs for this change with its caveats. The performance increase from it is really great and it significantly increases the packets per second we can push through calico with wireguard enabled, but we have seen instances of complete NAPI lockups on older kernels during peer removal and I'd like to document that fact and the active investigation so users are aware if they decide to use this setting: https://lore.kernel.org/netdev/CA+wXwBTT74RErDGAnj98PqS=wvdh8eM1pi4q6tTdExtjnokKqA@mail.gmail.com/T/
We've backported this patch into our kernels which at least unwedges the NAPI lock when we drain the node: https://lore.kernel.org/netdev/[email protected]/
/sem-approve
@mazdakn Added documentation for this feature here: https://github.com/tigera/docs/pull/1741
I don't believe that the latest CI failures are related to my change? https://tigera.semaphoreci.com/jobs/91a8c163-f4da-4366-bb1e-5869adaa0c07#L2599. I can rebase if it's a known issue.
/sem-approve
@jrcichra you need to rebase the PR. One test failed which is not related to this PR, and it's already fixed in the master branch.
/sem-approve
The CI failed in Win FV CAPZ which is a know issue and is not related to this PR.
@jrcichra Thanks for your contribution.
@jrcichra will you be able to cherry pick this PR to v3.29 and v3.28?
@jrcichra will you be able to cherry pick this PR to v3.29 and v3.28?
Sure, I'll make PRs for those.