On 01/11/2017 10:51 PM, Mauricio Vasquez wrote:
Hi Daniel,

On 01/10/2017 04:49 AM, Daniel Borkmann wrote:
On 01/09/2017 04:48 PM, Mauricio Vasquez via iovisor-dev wrote:
Hello,

I am trying to implement an eBPF program that trims an UDP packet to a predefined 
length. I am able to trim the packet (using the bpf_change_tail helper) and also to 
update the ip->len and udp->length fields. After changing the those fields I 
update the checksums using the bpf_[l3,l4]_csum_replace helpers.

Due to the fact that the UDP checksum also covers the data itself, it is necessary to 
update it. I tried to use the bpf_csum_diff together with the bpf_l4_csum_replace 
helpers, however I found that is not possible to pass a variable size to the 
bpf_csum_diff as it is marked as "ARG_CONST_STACK_SIZE_OR_ZER" in the function 
prototype. This limitation makes it impossible to update the checksum on different 
packets size as the quantity of bytes to be trimmed depends on the length of the packet.

The code is available at [1] (please note that it is based on hover). The error 
that I get when I tried to load the program is:

31: (61) r9 = *(u32 *)(r6 +48)
32: (61) r2 = *(u32 *)(r6 +0)
33: (07) r2 += -100
34: (67) r2 <<= 32
35: (77) r2 >>= 32
36: (b7) r8 = 0
37: (b7) r3 = 0
38: (b7) r4 = 0
39: (b7) r5 = 0
40: (85) call 28
R2 type=inv expected=imm

My questions are.

1. Am I missing another way to update the UDP checksum?

2. In case 1 is false, how could it be implemented? I think having a function 
to trim the packet without having a way to update the checksum is, in some way, 
useless.

What's your specific use-case on this? Reason why I'm asking is that
the helper needs to linearize skb and is thus only suitable for slow
path [1].
We are implementing a DHCP server, DHCP messages are quite unusual so slow path 
helpers are not a problem in this case.
F.e. we use it for rewriting an skb as time exceeded reply [2].
How far from the header start are you trimming? Perhaps it's feasible to
to calc the csum up to the point of trimming via bpf_csum_diff()?
Thanks to it I realized that I was doing the difficult way, I was trying to 
"remove"  the trimmed bytes from the checksum.

I am trying to implement it in a way similar to [2], calculating the checksum 
from scratch.
[...]
     /* calculate checksum of the new udp packet */
     u32 src_ip =  bpf_htonl(ip->src);
     u32 dst_ip =  bpf_htonl(ip->dst);
     u32 len_udp_n = ((u32)ip->nextp<<8) | (bpf_htons(ip->tlen)<<16);

On just a very quick glance, don't you need UDP length here (aka UDP header
and data)?

     int csum = bpf_csum_diff(NULL, 0, data + sizeof(struct ethernet_t) + 
sizeof(struct ip_t), udp_len, 0);
     csum = bpf_csum_diff (NULL, 0, &src_ip, 4, csum);
     csum = bpf_csum_diff (NULL, 0, &dst_ip, 4, csum);
     csum = bpf_csum_diff (NULL, 0, &len_udp_n, 4, csum);
     bpf_trace_printk("[dhcp] csum: %x\n", csum);
     err = bpf_l4_csum_replace(skb, UDP_CRC_OFF, 0, csum, BPF_F_PSEUDO_HDR);
[...]
The full code of this test is available at [1].

I am calculating the csum of the udp packet itself, for that reason I skip the 
ethernet and ip headers on the first call to bpf_csum_diff. Then, as the UDP 
checksum includes a IPV4 pseudo header [2], I update the csum including those 
fields.

This code compiles and passes verification, however it is not calculating the 
checksum correctly.  According to wireshark there is always a difference of 
0x14 between the calculated and the expected checksums.

I am pretty sure that I am missing something about checksums, but I cannot 
figure it out.
Can you spot some mistake on that code?

Thank you very much,
Mauricio V.

[1] https://gist.github.com/mvbpolito/9697247eba1c412cd4a6c8efe6fab55d
[2] https://www.ietf.org/rfc/rfc768.txt

Did
you check whether [3] could help in that?

  [1] 
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=5293efe62df81908f2e90c9820c7edcc8e61f5e9
  [2] https://github.com/cilium/cilium/blob/master/bpf/lib/icmp6.h
  [3] 
https://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/commit/?id=06c1c049721a995dee2829ad13b24aaf5d7c5cce

Thanks in advance.

Mauricio V.

[1] https://gist.github.com/mvbpolito/77edb3b4e65cc03c92862b3f17452286

_______________________________________________
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev




_______________________________________________
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev

Reply via email to