Skip to content

Extend BPF unit tests for IPsec #28438

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Nov 22, 2023
Merged

Extend BPF unit tests for IPsec #28438

merged 7 commits into from
Nov 22, 2023

Conversation

jschwinger233
Copy link
Member

@jschwinger233 jschwinger233 commented Oct 6, 2023 •
edited
Loading

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 area/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
pchaigno
pchaigno reviewed Oct 31, 2023
View reviewed changes
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.

@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
markpash
markpash approved these changes Nov 13, 2023
View reviewed changes
pchaigno
pchaigno approved these changes Nov 21, 2023
View reviewed changes
@jschwinger233
Split ipsec_from_lxc_test.c into *_generic.h and *_tunnel.c
2ad7920 
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>
@jschwinger233
Test IPsec from_lxc for nativeRouting and endpointRoutes
b923495 
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>
@jschwinger233
Split ipsec_from_host_test.c into *_generic.h and *_tunnel.c
4c324d9 
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>
@jschwinger233
Test IPsec from_host for nativeRouting and endpointRoutes
984fe5c 
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>
@jschwinger233
Split ipsec_from_overlay_test.c into *_generic.h and *_tunnel.c
d5299c0 
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>
@jschwinger233
Test IPsec from_overlay for endpointRoutes
59277a4 
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>
@jschwinger233
Test IPsec from_network
79e6114 
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brb brb brb approved these changes

@pchaigno pchaigno pchaigno approved these changes

+1 more reviewer

@markpash markpash markpash approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
area/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. 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.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jschwinger233 @brb @pchaigno @markpash