unlock unlimited code runs for $79, limited time →
NOSSLETTER.techLIVE
back to OSSpedia
cilium/cilium
mid7/10
refactoringalgorithmic-optimization

bpf: extract L3/L4 rewrite+checksum helpers for sharing codebase

Project: cilium/cilium | Language: C (eBPF) | Concepts: refactoring, algorithmic-optimization | Difficulty: mid
Link: https://github.com/cilium/cilium/pull/47673

The problem

The logic for rewriting IP addresses and ports (and updating their checksums) was duplicated in multiple places, specifically in the SNAT header rewrite functions for IPv4 and IPv6. This made it hard to reuse the code for new features (such as RevDNAT for ICMP errors) and increased maintenance burden.

Why it's non-trivial

A naive refactor might just move code into functions, but this code interacts with low-level packet structures, has to carefully update checksums, and must handle subtle differences between protocols (e.g., IPv4 vs IPv6, ICMP vs TCP/UDP). It must also not increase eBPF program complexity beyond verifier limits.

The approach

1. Extracting L3 address rewrite helpers

IPv4:

+static __always_inline int
+ipv4_l3_rewrite_addr(struct __ctx_buff *ctx, int l3_off, __u16 addr_off,
+		     __be32 old_addr, __be32 new_addr, __wsum *sum)
+{
+	*sum = csum_diff(&old_addr, 4, &new_addr, 4, 0);
+	if (ctx_store_bytes(ctx, l3_off + addr_off, &new_addr, 4, 0) < 0)
+		return DROP_WRITE_ERROR;
+
+	if (ipv4_csum_update_by_diff(ctx, l3_off, *sum) < 0)
+		return DROP_CSUM_L3;
+
+	return 0;
+}

What it does:

  • Calculates the checksum difference for the address change.
  • Writes the new address into the packet buffer.
  • Updates the IPv4 header checksum.
  • Returns 0 on success or a negative error code.

IPv6:

+static __always_inline int
+ipv6_l3_rewrite_addr(struct __ctx_buff *ctx, int l3_off, __u16 addr_off,
+		     const union v6addr *old_addr, const union v6addr *new_addr,
+		     __wsum *sum)
+{
+	*sum = csum_diff(old_addr, 16, new_addr, 16, 0);
+	if (ctx_store_bytes(ctx, l3_off + addr_off, new_addr, 16, 0) < 0)
+		return DROP_WRITE_ERROR;
+
+	return 0;
+}

What it does:

  • Same as above, but for IPv6 (which does not have a header checksum).

2. Extracting L4 port+checksum rewrite helper

+static __always_inline int
+l4_rewrite_port_and_csum(struct __ctx_buff *ctx, __u8 nexthdr, int l4_off, int port_off,
+			 __be16 old_port, __be16 new_port, __wsum l3_sum,
+			 __wsum l4_csum_diff_from_inner)
+{
+	struct csum_offset csum = {};
+	int err;
+
+	csum_l4_offset_and_flags(nexthdr, &csum);
+
+	if (old_port != new_port) {
+		switch (nexthdr) {
+		case IPPROTO_TCP:
+		case IPPROTO_UDP:
+		case IPPROTO_ICMPV6:
+			break;
+#ifdef ENABLE_SCTP
+		case IPPROTO_SCTP:
+			return DROP_CSUM_L4;
+#endif  /* ENABLE_SCTP */
+		case IPPROTO_ICMP:
+			csum.offset = offsetof(struct icmphdr, checksum);
+			break;
+		default:
+			return DROP_UNKNOWN_L4;
+		}
+
+		/* Amend the L4 checksum due to changing the ports. */
+		err = l4_modify_port(ctx, l4_off, port_off, &csum, new_port, old_port);
+		if (err < 0)
+			return err;
+
+		/* Restore the original offset. */
+		if (nexthdr == IPPROTO_ICMP)
+			csum.offset = 0;
+	}
+
+	/* Amend the L4 checksum due to changing the addresses. */
+	if (csum.offset &&
+	    csum_l4_replace(ctx, l4_off, &csum, 0, l3_sum, BPF_F_PSEUDO_HDR) < 0)
+		return DROP_CSUM_L4;
+
+	/* Apply additional L4 checksum diff if provided (for ICMP error messages). */
+	if (l4_csum_diff_from_inner && !csum.offset) {
+		csum.offset = offsetof(struct icmphdr, checksum);
+		if (csum_l4_replace(ctx, l4_off, &csum, 0, l4_csum_diff_from_inner, 0) < 0)
+			return DROP_CSUM_L4;
+	}
+
+	return 0;
+}

What it does:

  • Handles port rewriting and checksum updates for TCP, UDP, ICMP, and ICMPv6.
  • Deals with protocol-specific quirks (e.g., ICMP's lack of pseudo-header).
  • Optionally applies an extra checksum diff for ICMP error handling.

3. Refactoring SNAT header rewrite functions to use the helpers

IPv4:

-	sum = csum_diff(&old_addr, 4, &new_addr, 4, 0);
-	if (ctx_store_bytes(ctx, l3_off + addr_off, &new_addr, 4, 0) < 0)
-		return DROP_WRITE_ERROR;
-
-	/* Amend the L3 checksum due to changing the addresses. */
-	if (ipv4_csum_update_by_diff(ctx, l3_off, sum) < 0)
-		return DROP_CSUM_L3;
-
-	if (has_l4_header) {
-		...
-	}
+	err = ipv4_l3_rewrite_addr(ctx, l3_off, addr_off, old_addr, new_addr, &sum);
+	if (err < 0)
+		return err;
+
+	if (has_l4_header)
+		return l4_rewrite_port_and_csum(ctx, nexthdr, l4_off, port_off,
+						old_port, new_port, sum,
+						l4_csum_diff_from_inner);

IPv6:

-	sum = csum_diff(old_addr, 16, new_addr, 16, 0);
-	if (ctx_store_bytes(ctx, l3_off + addr_off, new_addr, 16, 0) < 0)
-		return DROP_WRITE_ERROR;
-
-	if (!has_l4_header)
-		return 0;
-
-	csum_l4_offset_and_flags(nexthdr, &csum);
-	...
-	if (csum.offset &&
-	    csum_l4_replace(ctx, l4_off, &csum, 0, sum, BPF_F_PSEUDO_HDR) < 0)
-		return DROP_CSUM_L4;
+	err = ipv6_l3_rewrite_addr(ctx, l3_off, addr_off, old_addr, new_addr, &sum);
+	if (err < 0)
+		return err;
+
+	if (has_l4_header)
+		return l4_rewrite_port_and_csum(ctx, nexthdr, l4_off, port_off,
+						old_port, new_port, sum, 0);

What it does:

  • Replaces duplicated logic with calls to the new helpers, reducing code size and making future reuse easier.

From the review thread

pchaigno: This seems to have a negative impact on complexity, and I'm not sure why it has any impact at all. It might be worth doing this in two commits (first ipv{4,6}_l3_rewrite_addr, then l4_rewrite_port_and_csum) to be able to see which part is affecting complexity. I don't think it's necessarily a blocker since our most-complex programs are not affected, but would still be nice to figure out.

pchaigno: From the results at https://github.com/cilium/cilium/pull/48221 (https://github.com/cilium/cilium/actions/runs/32963875878), it looks like the first commit is causing the increase of complexity on the IPv4 path, while the second commit is causing the increasing on the IPv6 path.

I'm really not seeing much we can do. We could try some hacks to force the compiler to generate code closer to the original code, but that would get ugly fast and I doubt it's worth it. In the end, we're still well below the limit. If we want to decrease complexity, it will probably be easier and cleaner to introduce new global functions than to try to optimize this code move. So I'm in favor of ignoring the complexity impact for now.

Takeaways

  • Extracting reusable helpers can slightly increase program complexity, especially in low-level or performance-sensitive code, due to how compilers inline and structure the resulting machine code.
  • When refactoring critical code, always verify that the changes do not push you over platform or verifier limits (such as eBPF instruction or complexity caps).
  • Centralizing protocol-specific logic (like checksum updates) in well-named helpers reduces duplication and makes future maintenance and feature additions easier and less error-prone.