Re: TC bpf_csum_diff problems post-5.6 kernel

On 11/2/21 11:08 PM, Thomas Ptacek wrote:
> !!!
> 
> That DID fix it! (I thought I tried it before, this time I just used
> (1ULL<<5) as the flag).
> 
> Why did that fix it? Is that flag new? :)

Some context on the full series:

   https://lore.kernel.org/bpf/[email protected]/

For the update, did you use bpf_csum_update() helper or directly writing into
the pkt?

> Thank you so much!

Np, thanks!

> On Tue, Nov 2, 2021 at 5:00 PM Daniel Borkmann <[email protected]> wrote:
>>
>> On 11/2/21 10:46 PM, Thomas Ptacek wrote:
>>> The problem I'm dealing with is XDP-adjacent, but not itself an XDP issue, so
>>> disregard if I'm too far outside the charter of the list. :)
>>>
>>> We run an XDP/TC-based UDP CDN.
>>>
>>> "Edge" machine XDP takes UDP packets off eno1, slaps a proxy header on
>>> them and bounces them to a wg0 (WireGuard)  interface, which shuttles them
>>> to "Worker" machines.
>>>
>>> "Worker" TC BPF on wg0 intercepts those UDP packets, strips off the
>>> proxy header, reflects values from that header back into the UDP and IP
>>> headers of the skb, and fixes up the checksums.
>>>
>>> All this works fine up through kernel 5.6. But we're working on a fleet update,
>>> and something >= 5.8 is breaking my code (or my code was always broken
>>> and relying on some pre-5.8 bug to function).
>>>
>>> With a bunch of perf debugging I've narrowed the problem down: it's the
>>> checksum diff that accounts for the stripped header. I do (roughly):
>>>
>>>       /* not shown: parse, copy proxy header to stack */
>>>
>>>       bpf_skb_adjust_room(ctx, -12, BPF_ADJ_ROOM_NET, 0)
>>
>> Hmm, if you add BPF_F_ADJ_ROOM_NO_CSUM_RESET instead of 0 as flag above, would
>> that work?
>>
>>>       skb_pull_data(ctx, sizeof(struct iphdr) + sizeof(struct udphdr))
>>>
>>>       /* not shown: re-check packet pointers, set up pointers to headers ... */
>>>       sum = ~((uint32_t)(udphdr->udp_sum))
>>>       sum = bpf_csum_diff(&proxyHeader, 12, NULL, 0, sum);
>>>
>>>       /* not shown: make other checksum fixups, write sum back to packet */
>>>
>>> Using some perf-based printf debugging, I can see that the checksum
>>> I'm getting from that bpf_csum_diff is wacky. Further: if I just leave the
>>> proxy header in the UDP packet, but still change the UDP ports and
>>> IP addresses, I get valid UDP checksums (albeit with a useless packet
>>> that has a proxy header still in it).
>>>
>>> I'm wondering if anyone can think of something that would have happened
>>> post-5.6 that would have broken this (or if there's something obviously
>>> abusive I'm doing with my "working" code, such that this never should have
>>> worked to begin with).
>>>
>>> ---
>>> Thomas H. Ptacek
>>>
>>
> 
>