Skip to content

Commit ead7f9b

Browse files
pchaignokuba-moo
authored andcommitted
bpf: Fix L4 csum update on IPv6 in CHECKSUM_COMPLETE
In Cilium, we use bpf_csum_diff + bpf_l4_csum_replace to, among other things, update the L4 checksum after reverse SNATing IPv6 packets. That use case is however not currently supported and leads to invalid skb->csum values in some cases. This patch adds support for IPv6 address changes in bpf_l4_csum_update via a new flag. When calling bpf_l4_csum_replace in Cilium, it ends up calling inet_proto_csum_replace_by_diff: 1: void inet_proto_csum_replace_by_diff(__sum16 *sum, struct sk_buff *skb, 2: __wsum diff, bool pseudohdr) 3: { 4: if (skb->ip_summed != CHECKSUM_PARTIAL) { 5: csum_replace_by_diff(sum, diff); 6: if (skb->ip_summed == CHECKSUM_COMPLETE && pseudohdr) 7: skb->csum = ~csum_sub(diff, skb->csum); 8: } else if (pseudohdr) { 9: *sum = ~csum_fold(csum_add(diff, csum_unfold(*sum))); 10: } 11: } The bug happens when we're in the CHECKSUM_COMPLETE state. We've just updated one of the IPv6 addresses. The helper now updates the L4 header checksum on line 5. Next, it updates skb->csum on line 7. It shouldn't. For an IPv6 packet, the updates of the IPv6 address and of the L4 checksum will cancel each other. The checksums are set such that computing a checksum over the packet including its checksum will result in a sum of 0. So the same is true here when we update the L4 checksum on line 5. We'll update it as to cancel the previous IPv6 address update. Hence skb->csum should remain untouched in this case. The same bug doesn't affect IPv4 packets because, in that case, three fields are updated: the IPv4 address, the IP checksum, and the L4 checksum. The change to the IPv4 address and one of the checksums still cancel each other in skb->csum, but we're left with one checksum update and should therefore update skb->csum accordingly. That's exactly what inet_proto_csum_replace_by_diff does. This special case for IPv6 L4 checksums is also described atop inet_proto_csum_replace16, the function we should be using in this case. This patch introduces a new bpf_l4_csum_replace flag, BPF_F_IPV6, to indicate that we're updating the L4 checksum of an IPv6 packet. When the flag is set, inet_proto_csum_replace_by_diff will skip the skb->csum update. Fixes: 7d67234 ("bpf: add generic bpf_csum_diff helper") Signed-off-by: Paul Chaignon <[email protected]> Acked-by: Daniel Borkmann <[email protected]> Link: https://patch.msgid.link/96a6bc3a443e6f0b21ff7b7834000e17fb549e05.1748509484.git.paul.chaignon@gmail.com Signed-off-by: Jakub Kicinski <[email protected]>
1 parent 6043b79 commit ead7f9b

File tree

3 files changed

+7
-2
lines changed

3 files changed

+7
-2
lines changed

include/uapi/linux/bpf.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2056,6 +2056,7 @@ union bpf_attr {
20562056
* for updates resulting in a null checksum the value is set to
20572057
* **CSUM_MANGLED_0** instead. Flag **BPF_F_PSEUDO_HDR** indicates
20582058
* that the modified header field is part of the pseudo-header.
2059+
* Flag **BPF_F_IPV6** should be set for IPv6 packets.
20592060
*
20602061
* This helper works in combination with **bpf_csum_diff**\ (),
20612062
* which does not update the checksum in-place, but offers more
@@ -6072,6 +6073,7 @@ enum {
60726073
BPF_F_PSEUDO_HDR = (1ULL << 4),
60736074
BPF_F_MARK_MANGLED_0 = (1ULL << 5),
60746075
BPF_F_MARK_ENFORCE = (1ULL << 6),
6076+
BPF_F_IPV6 = (1ULL << 7),
60756077
};
60766078

60776079
/* BPF_FUNC_skb_set_tunnel_key and BPF_FUNC_skb_get_tunnel_key flags. */

net/core/filter.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1968,10 +1968,11 @@ BPF_CALL_5(bpf_l4_csum_replace, struct sk_buff *, skb, u32, offset,
19681968
bool is_pseudo = flags & BPF_F_PSEUDO_HDR;
19691969
bool is_mmzero = flags & BPF_F_MARK_MANGLED_0;
19701970
bool do_mforce = flags & BPF_F_MARK_ENFORCE;
1971+
bool is_ipv6 = flags & BPF_F_IPV6;
19711972
__sum16 *ptr;
19721973

19731974
if (unlikely(flags & ~(BPF_F_MARK_MANGLED_0 | BPF_F_MARK_ENFORCE |
1974-
BPF_F_PSEUDO_HDR | BPF_F_HDR_FIELD_MASK)))
1975+
BPF_F_PSEUDO_HDR | BPF_F_HDR_FIELD_MASK | BPF_F_IPV6)))
19751976
return -EINVAL;
19761977
if (unlikely(offset > 0xffff || offset & 1))
19771978
return -EFAULT;
@@ -1987,7 +1988,7 @@ BPF_CALL_5(bpf_l4_csum_replace, struct sk_buff *, skb, u32, offset,
19871988
if (unlikely(from != 0))
19881989
return -EINVAL;
19891990

1990-
inet_proto_csum_replace_by_diff(ptr, skb, to, is_pseudo, false);
1991+
inet_proto_csum_replace_by_diff(ptr, skb, to, is_pseudo, is_ipv6);
19911992
break;
19921993
case 2:
19931994
inet_proto_csum_replace2(ptr, skb, from, to, is_pseudo);

tools/include/uapi/linux/bpf.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2056,6 +2056,7 @@ union bpf_attr {
20562056
* for updates resulting in a null checksum the value is set to
20572057
* **CSUM_MANGLED_0** instead. Flag **BPF_F_PSEUDO_HDR** indicates
20582058
* that the modified header field is part of the pseudo-header.
2059+
* Flag **BPF_F_IPV6** should be set for IPv6 packets.
20592060
*
20602061
* This helper works in combination with **bpf_csum_diff**\ (),
20612062
* which does not update the checksum in-place, but offers more
@@ -6072,6 +6073,7 @@ enum {
60726073
BPF_F_PSEUDO_HDR = (1ULL << 4),
60736074
BPF_F_MARK_MANGLED_0 = (1ULL << 5),
60746075
BPF_F_MARK_ENFORCE = (1ULL << 6),
6076+
BPF_F_IPV6 = (1ULL << 7),
60756077
};
60766078

60776079
/* BPF_FUNC_skb_set_tunnel_key and BPF_FUNC_skb_get_tunnel_key flags. */

0 commit comments

Comments
 (0)