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.

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

feat: implement checksum #325

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

feat: implement checksum #325

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
99dbca8
fix checksum in tcp egress
jokestax Dec 21, 2024
7095c4a
fix imports
jokestax Dec 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 37 additions & 16 deletions dataplane/ebpf/src/egress/tcp.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,17 @@ Copyright 2023 The Kubernetes Authors.
SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
*/

use core::mem;

use aya_ebpf::{
bindings::{TC_ACT_OK, TC_ACT_PIPE},
helpers::bpf_csum_diff,
helpers::{bpf_l3_csum_replace, bpf_l4_csum_replace},
programs::TcContext,
};
use aya_log_ebpf::info;
use common::ClientKey;
use network_types::{eth::EthHdr, ip::Ipv4Hdr, tcp::TcpHdr};

use crate::{
utils::{csum_fold_helper, ptr_at, update_tcp_conns},
utils::{ptr_at, update_tcp_conns},
LB_CONNECTIONS,
};

Expand All @@ -36,6 +34,13 @@ pub fn handle_tcp_egress(ctx: TcContext) -> Result<i32, i64> {
ip: u32::from_be(client_addr),
port: u16::from_be(dest_port) as u32,
};

// Store the old source IP address for checksum calculation
let old_src_addr = unsafe { (*ip_hdr).src_addr };

// Store the old source port for checksum calculation
let old_src_port = unsafe { (*tcp_hdr).source };

let lb_mapping = unsafe { LB_CONNECTIONS.get(&client_key) }.ok_or(TC_ACT_PIPE)?;

info!(
Expand All @@ -60,21 +65,37 @@ pub fn handle_tcp_egress(ctx: TcContext) -> Result<i32, i64> {
return Ok(TC_ACT_OK);
}

unsafe { (*ip_hdr).check = 0 };
let full_cksum = unsafe {
bpf_csum_diff(
mem::MaybeUninit::zeroed().assume_init(),
0,
ip_hdr as *mut u32,
Ipv4Hdr::LEN as u32,
0,
Comment on lines -65 to -70
Copy link
Member

@shaneutt shaneutt Jan 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

You'll see in the libbpf documentation that the bpf_l4_csum_replace function is intended for use with the bpf_csum_diff function:

https://docs.ebpf.io/linux/helper-function/bpf_l4_csum_replace/

So I'm not sure we want to remove this here. We should probably take a look at other solutions and glean what we can from how they're doing things:

https://github.com/projectcalico/calico/blob/c9931c13faf316703e45ada9b9042439830c082d/felix/bpf-gpl/tc.c#L1580

it would probably be good to take a peek at what Cilium is doing as well and cross reference between this and calico (and maybe others) to glean some insights.

let ret = unsafe {
bpf_l3_csum_replace(
ctx.skb.skb,
4u32,
old_src_addr as u64,
lb_mapping.backend_key.ip.to_be() as u64,
4,
Comment on lines +69 to +74
Copy link
Member

Choose a reason for hiding this comment

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

Aya provides helper methods for this that chain all the way up to the TcContext, so you don't have to include these functions directly but could instead do ctx.l3_csum_replace.

)
} as u64;
unsafe { (*ip_hdr).check = csum_fold_helper(full_cksum) };
unsafe { (*tcp_hdr).check = 0 };
};

let tcp_hdr_ref = unsafe { tcp_hdr.as_ref().ok_or(TC_ACT_OK)? };
if ret != 0 {
info!(&ctx, "Failed to update IP checksum");
return Ok(TC_ACT_OK);
}

let ret = unsafe {
bpf_l4_csum_replace(
ctx.skb.skb,
tcp_header_offset as u32,
old_src_port as u64,
lb_mapping.backend_key.port.to_be() as u64,
2,
)
Comment on lines +84 to +90
Copy link
Member

Choose a reason for hiding this comment

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

Same as above, you should be able to do ctx.l4_csum_replace.

};

if ret != 0 {
info!(&ctx, "Failed to update TCP checksum");
return Ok(TC_ACT_OK);
}

let tcp_hdr_ref = unsafe { tcp_hdr.as_ref().ok_or(TC_ACT_OK)? };
// If the packet has the RST flag set, it means the connection is being terminated, so remove it
// from our map.
if tcp_hdr_ref.rst() == 1 {
Expand Down
Loading