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

Use bpf programs for l3 devices for IPIP tunnels in kernel 5.14 (rhel 4.18.0-330) and newer #5846

Merged
merged 18 commits into from
Apr 8, 2022

Conversation

mazdakn
Copy link
Member

@mazdakn mazdakn commented Apr 6, 2022

Description

Unlike other L3 devices, IPIP tunnels used to see ethernet, outer IP header, inner IP header and payload in ingress path. In kernel 5.14 a commit (7ad136fd288c0 : ipip: allow redirecting ipip and mplsip packets to eth devices) made IPIP tunnels act like other L3 device, and for that bpf programs attached to IPIP tunnel only see inner IP header and payload.

This PR generalises the wirguard programs for all L3 devices including IPIP tunnels with the new behaviour. This PR should also fix this issue: #5449

Related issues/PRs

Todos

  • Tests
  • Documentation
  • Release note

Release Note

ebpf: Fixes issue with IPIP tunnels on kernels 5.14+ and RH kernels 4.18.0-330.el8+ as the tunnel device started behaving as a proper L3 device. 

@mazdakn mazdakn added docs-not-required Docs not required for this change release-note-not-required Change has no user-facing impact labels Apr 6, 2022
@mazdakn mazdakn requested a review from a team as a code owner April 6, 2022 00:44
@marvin-tigera marvin-tigera added this to the Calico v3.23.0 milestone Apr 6, 2022
@tomastigera tomastigera changed the title Control IPIP behaviour using Global variables felix/bpf: Control IPIP behaviour using Global variables Apr 6, 2022
@tomastigera tomastigera added release-note-required Change has user-facing impact (no matter how small) and removed release-note-not-required Change has no user-facing impact labels Apr 6, 2022
Copy link
Contributor

@tomastigera tomastigera left a comment

Choose a reason for hiding this comment

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

LGTM, but I have one question

felix/bpf-gpl/globals.h Outdated Show resolved Hide resolved
felix/bpf/bpf.go Outdated Show resolved Hide resolved
@@ -68,6 +68,10 @@ static CALI_BPF_INLINE void skb_refresh_start_end(struct cali_tc_ctx *ctx) {
static CALI_BPF_INLINE long skb_iphdr_offset(void)
{
if (CALI_F_IPIP_ENCAPPED) {
if (GLOBAL_FLAGS & CALI_GLOBALS_IPIP_NO_OUTER_HEADERS) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if instead we would be able to just load the L3 (CALI_F_L3) programs rather than handling it as a runtime configuration. Then the change could be limited to felix only 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

Right. let me check.

@mazdakn mazdakn requested a review from tomastigera April 7, 2022 20:59
@mazdakn mazdakn changed the title felix/bpf: Control IPIP behaviour using Global variables felix/bpf: Use bpf programs for l3 devices for IPIP tunnels in kernel 5.14 (rhel 4.18.0-330) and newer Apr 8, 2022
@mazdakn mazdakn changed the title felix/bpf: Use bpf programs for l3 devices for IPIP tunnels in kernel 5.14 (rhel 4.18.0-330) and newer Use bpf programs for l3 devices for IPIP tunnels in kernel 5.14 (rhel 4.18.0-330) and newer Apr 8, 2022
Copy link
Contributor

@tomastigera tomastigera left a comment

Choose a reason for hiding this comment

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

LGTM, pls could you squash it no need to have 18 commits for such a change. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs-not-required Docs not required for this change release-note-required Change has user-facing impact (no matter how small)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants