Add BPF unit tests for IPsec #25699
Merged
Add BPF unit tests for IPsec #25699
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
maintainer-s-little-helper
bot
added
the
dont-merge/needs-release-note-label
jschwinger233
force-pushed
the
gray/ipsec-ut
branch
3 times, most recently
from
5496607
to
f746f2b
Compare
|
BPF coverage test is killing me 😭 |
jschwinger233
force-pushed
the
gray/ipsec-ut
branch
2 times, most recently
from
bb3de57
to
3e3b29f
Compare
jschwinger233
added
area/encryption
maintainer-s-little-helper
bot
removed
the
dont-merge/needs-release-note-label
jschwinger233
force-pushed
the
gray/ipsec-ut
branch
from
3e3b29f
to
33abba9
Compare
|
/test |
|
/test Job 'Cilium-PR-K8s-1.26-kernel-net-next' failed: Click to show.Test NameFailure OutputJenkins URL: https://jenkins.cilium.io/job/Cilium-PR-K8s-1.26-kernel-net-next/373/ If it is a flake and a GitHub issue doesn't already exist to track it, comment Then please upload the Jenkins artifacts to that issue. |
|
/test-1.26-net-next |
1 similar comment
|
/test-1.26-net-next |
julianwiedmann
requested review from
a team and
rgo3
and removed request for
a team
jschwinger233
force-pushed
the
gray/ipsec-ut
branch
from
33abba9
to
e8df7c9
Compare
jschwinger233
commented
Jun 6, 2023
| @@ -541,3 +554,21 @@ func (l *Log) FmtString() string { | |||
|
|
|||
| return sb.String() | |||
| } | |||
|
|
|||
| func runBpfProgram(prog *ebpf.Program, data, ctx []byte) (statusCode uint32, dataOut, ctxOut []byte, err error) { | |||
There was a problem hiding this comment.
@YutaroHayakawa Do you think this runBpfProgram can help test skb->cb with reference to your PR #24181? The parameter ctx will be used as skb metadata, like memcpy(skb, ctx), so we can get cb by ctx[offsetof(skb, cb)]. In this PR we pass the returned ctx from the last BPF test program to the next BPF test program, so whatever changes made by the previous BPF program can be seen afterwards.
There was a problem hiding this comment.
Yeah, this helps, but I intentionally didn't use the ctx_in parameter of BPF_PROG_TEST_RUN because it's only available after kernel 5.2 and our minimal possible kernel is 4.19. However, if we can make sure we only run these tests on kernel 5.2 or later, then it's fine. Have you discussed this with someone?
jschwinger233
force-pushed
the
gray/ipsec-ut
branch
from
e8df7c9
to
dcf5189
Compare
jschwinger233
force-pushed
the
gray/ipsec-ut
branch
from
f75d7d8
to
85d76ad
Compare
The previous implementation of BPFProgram.Test() only allowed passing and returning bytes as *skb->data, without the ability to specify input skb metadata or check output skb metadata. This commit introduces a new function named runBpfProgram, which passes an additional []byte as input skb metadata and returns an additional []byte as output skb metadata. By utilizing this function, we ensure that the skb->mark set in PKTGEN can be properly passed to the SETUP , and any modifications to skb->mark or skb->cb can be accurately examined during the CHECK for validation purposes. The input ctx bytes will be set as the input skb, briefly you can expect `memcpy(skb, ctx, sizeof(*skb))` happening inside, and you can get skb->mark by `mark := ctx[offset(skb, mark): sizeof(skb->mark)]`. Signed-off-by: Zhichuan Liang <gray.liang@isovalent.com>
These testcases cover IPSec datapath on `from-container` for both IPv4 and IPv6. The following behaviors are checked after execution of `from-container`: 1. skb->mark should be set to `node_id << 16 | key << 12 | 0xe00` 2. skb->cb[4] should be set to source sec_id 3. skb should be passed to stack 4. skb->data doesn't change Signed-off-by: Zhichuan Liang <gray.liang@isovalent.com>
These testcases cover IPSec datapath on `from-host` for both IPv4 and IPv6. The input skb should be ESP-encrypted with mark set to `node_id << 16 | key << 12 | 0xe00` and cb[4] set to sec_id. We will check the following behaviors after execution of `from-host`: 1. skb->mark is cleared to 0 2. skb->cb[4] is cleared to 0 3. skb is redirected to cilium_vxlan 4. skb->data doesn't change 5. VxLAN VNI is set to sec_id Signed-off-by: Zhichuan Liang <gray.liang@isovalent.com> pktgen
These testcases cover IPsec datapath for both IPv4 and IPv6. An ingress skb will reach `from-overlay` twice, for the 1st time the skb is ESP-encrypted, for the 2nd time the skb is ESP-decrypted with mark `0xd00`. For the 1st reach, we check the following behaviors: 1. skb is passed to stack 2. skb->mark is set to 0xd00 3. skb->data doesn't change For the 2nd reach, we check: 1. skb is redirected to the target lxc veth 2. skb->mark is cleared to 0 Signed-off-by: Zhichuan Liang <gray.liang@isovalent.com>
jschwinger233
force-pushed
the
gray/ipsec-ut
branch
from
85d76ad
to
da9554e
Compare
|
/test |
jschwinger233
added
the
ready-to-merge
|
I'm marking for backport up to v1.12 because it seems likely to be easy to backport and can give us feedback on IPsec backports. If it gets too complicated (i.e., depends on other PRs being backported), I'll drop the backports. |
pchaigno
added
backport-done/1.12
pchaigno
added
backport-pending/1.13
|
Removing from v1.12 backports as the BPF unit tests are too different there for this to be easily backportable. |
julianwiedmann
added
backport-done/1.13
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A typical datapath for pod to pod ipsec connection is:
local host:
1. ingress on lxc, where skb->mark is for encryption;
2. stack, xfrm encrypts skb data;
3. egress on cilium_host, where tunnel key is set and skb is redirected to cilium_vxlan;
4. egress on cilium_vxlan
5. egress on eth0
remote host:
6. ingress on eth0
7. ingress on cilium_vxlan (1st), where skb->mark is set for decryption;
8. stack: xfrm decrypts skb data
9. ingress on cilium_vxlan (2nd), skb is redirected to lxc
10. egress on lxc
This PR checks step 1 (
from-container), step 3 (from-host), step 7 (1stfrom-overlay), and step 9 (2ndfrom-overlay). All the tests are done withTUNNEL_MODEenabled, both IPv6 and IPv4 are covered.Step 2 and step 8 will be covered by control plane test mentioned by #20707
Step 4, 5, 6, 10 have nothing special for IPSec, so I'll leave them aside.
Corner cases and failure cases are to be tested in a separate PR.