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

Extend BPF unit tests for IPsec #28438

Merged
merged 7 commits into from
Nov 22, 2023
Merged

Conversation

jschwinger233
Copy link
Member

@jschwinger233 jschwinger233 commented Oct 6, 2023

Existing IPsec BPF test only covers tunnel=vxlan, so this PR covers combination of tunnel={vxlan,disabled} and endpointRoutes={disabled,enabled}.

Fixes: #25699

Signed-off-by: Zhichuan Liang gray.liang@isovalent.com

@maintainer-s-little-helper maintainer-s-little-helper bot added dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Oct 6, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Oct 9, 2023
@jschwinger233 jschwinger233 force-pushed the gray/ci-ipsec-ext branch 6 times, most recently from 195c447 to 2dddd24 Compare October 9, 2023 05:07
@cilium cilium deleted a comment from maintainer-s-little-helper bot Oct 9, 2023
@jschwinger233 jschwinger233 added the release-note/ci This PR makes changes to the CI. label Oct 9, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Oct 9, 2023
@jschwinger233 jschwinger233 added sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Oct 9, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot removed dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Oct 9, 2023
@jschwinger233 jschwinger233 added the area/encryption Impacts encryption support such as IPSec, WireGuard, or kTLS. label Oct 9, 2023
@jschwinger233 jschwinger233 changed the title [WIP] Extend BPF unit tests for IPsec Extend BPF unit tests for IPsec Oct 9, 2023
@jschwinger233
Copy link
Member Author

/test

@jschwinger233 jschwinger233 marked this pull request as ready for review October 9, 2023 07:10
@jschwinger233 jschwinger233 requested a review from a team as a code owner October 9, 2023 07:10
@jschwinger233 jschwinger233 requested review from markpash, a team and brb and removed request for a team October 9, 2023 07:10
@jschwinger233 jschwinger233 marked this pull request as draft October 11, 2023 02:20
@jschwinger233
Copy link
Member Author

/test

@jschwinger233 jschwinger233 marked this pull request as ready for review October 31, 2023 10:13
Copy link
Member

@pchaigno pchaigno left a comment

Choose a reason for hiding this comment

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

Nice commit history!

Minor comments only from me.

bpf/tests/ipsec_from_lxc_tunnel.c Show resolved Hide resolved
bpf/tests/ipsec_from_host_tunnel_endpointroutes.c Outdated Show resolved Hide resolved
bpf/tests/ipsec_from_host_tunnel.c Show resolved Hide resolved
@jschwinger233 jschwinger233 marked this pull request as draft November 1, 2023 11:50
@jschwinger233 jschwinger233 force-pushed the gray/ci-ipsec-ext branch 4 times, most recently from 68af2fa to 516a532 Compare November 6, 2023 10:13
@jschwinger233 jschwinger233 marked this pull request as ready for review November 6, 2023 10:29
bpf/tests/ipsec_from_lxc_generic.h Show resolved Hide resolved
bpf/tests/ipsec_from_host_generic.h Show resolved Hide resolved
bpf/tests/ipsec_from_overlay_generic.h Show resolved Hide resolved
bpf/tests/ipsec_from_lxc_tunnel.c Outdated Show resolved Hide resolved
The original ipsec_from_lxc_test.c covers tunnel=vxlan plus
endpointRoutes=disabled only. This commit splits it into generic part
(*_generic.h) and configure-specific part (*_tunnel.c).

Signed-off-by: Zhichuan Liang <gray.liang@isovalent.com>
ipsec_from_lxc_*:
- *_tunnel.c:           tunnel + !endpointRoutes
- *_tunnel_endpoint:    tunnel + endpointRoutes
- *_native.c:           !tunnel + !endpointRoutes
- *_native_endpoint.c:  !tunnel + endpointRoutes

Please note the different macros defined among combinations:

Macros for Cilium:
- TUNNEL_MODE & HAVE_ENCAP: defined for tunnel
- ENABLE_ROUTING: defined for !endpointRoutes
- ENABLE_ENDPOINT_ROUTES & USE_BPF_PROG_FOR_INGRESS_POLICY: defined for endpointRoutes

Macros for test:
- EXPECTED_DEST_MAC: especially defined for !tunnel +
  !endpointRoutes, because of https://github.com/cilium/cilium/blob/a42ef40ad137a7688ff533ff1d9a0ddfd6f72a0a/bpf/bpf_lxc.c#L1230

Signed-off-by: Zhichuan Liang <gray.liang@isovalent.com>
The original ipsec_from_host_test.c covers tunnel=vxlan plus
endpointRoutes=disabled only. This commit splits it into generic part
(*_generic.h) and configure-specific part (*_tunnel.c).

Signed-off-by: Zhichuan Liang <gray.liang@isovalent.com>
ipsec_from_host_*.c:
- *_tunnel.c:           tunnel + !endpointRoutes
- *_tunnel_endpoint.c:  tunnel + endpointRoutes
- *_native.c:           !tunnel + !endpointRoutes
- *_native_endpoint.c:  !tunnel + endpointRoutes

Please note the different macros defined among combinations:

Macros for Cilium:
- TUNNEL_MODE & HAVE_ENCAP: defined for tunnel
- ENABLE_ROUTING: defined for !endpointRoutes
- ENABLE_ENDPOINT_ROUTES: defined for endpointRoutes

Macros for test:
- EXPECTED_STATUS_CODE: ACT_REDIRECT for tunnel, ACT_OK for !tunnel
- CHECK_CB_ENCRYPT_IDENTITY: defined for tunnel, because of https://github.com/cilium/cilium/blob/a42ef40ad137a7688ff533ff1d9a0ddfd6f72a0a/bpf/lib/encrypt.h#L42

Signed-off-by: Zhichuan Liang <gray.liang@isovalent.com>
The original ipsec_from_overlay_test.c covers tunnel=vxlan plus
endpointRoutes=disabled only. This commit splits it into generic part
(*_generic.h) and configure-specific part (*_tunnel.c).

Signed-off-by: Zhichuan Liang <gray.liang@isovalent.com>
Macro ENABLE_ENDPOINT_ROUTES is defined instead of ENABLE_ROUTING.
TC_ACT_OK is also expected rather than TC_ACT_REDIRECT.

Signed-off-by: Zhichuan Liang <gray.liang@isovalent.com>
Bpf from_network is used to attach on eth0 when tunnel is disabled. This
commit covers tunnel=disabled with endpointRoutes={enabled,disabled}.

Signed-off-by: Zhichuan Liang <gray.liang@isovalent.com>
@pchaigno pchaigno added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Nov 22, 2023
@pchaigno pchaigno merged commit 85e05d3 into cilium:main Nov 22, 2023
43 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/encryption Impacts encryption support such as IPSec, WireGuard, or kTLS. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/ci This PR makes changes to the CI. sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants