XDP breakage with virtio due to 6870de435b90c083ae0f3f7f341287976ef56f03

XDP breakage with virtio due to 6870de435b90c083ae0f3f7f341287976ef56f03

[David Ahern]

This commit breaks my FIB forwarding program:

commit 6870de435b90c083ae0f3f7f341287976ef56f03
Author: Nikita V. Shirokov <tehnerd@tehnerd.com>
Date:   Tue Apr 17 21:42:20 2018 -0700

    bpf: make virtio compatible w/ bpf_xdp_adjust_tail

    w/ bpf_xdp_adjust_tail helper xdp's data_end pointer could be changed as
    well (only "decrease" of pointer's location is going to be supported).
    changing of this pointer will change packet's size.
    for virtio driver we need to adjust XDP_PASS handling by recalculating
    length of the packet if it was passed to the TCP/IP stack

    Reviewed-by: Jason Wang <jasowang@redhat.com>
    Signed-off-by: Nikita V. Shirokov <tehnerd@tehnerd.com>
    Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>

###

Some of the packets (e.g., ARP or those without a resolved neighbor) are
passed to the networking stack. What shows up are clearly broken packets:

# tcpdump -n -i eth1
tcpdump: verbose output suppressed, use -v or -vv for full protocol decode
listening on eth1, link-type EN10MB (Ethernet), capture size 262144 bytes
15:45:29.693238 [|ARP]
	0x0000:  0001 0800 0604 0001 42c0 ce2f 3fa9 0a64  ........B../?..d
15:45:30.710327 [|ARP]
	0x0000:  0001 0800 0604 0001 42c0 ce2f 3fa9 0a64  ........B../?..d
15:45:31.734296 [|ARP]
	0x0000:  0001 0800 0604 0001 42c0 ce2f 3fa9 0a64  ........B../?..d
15:45:32.908720 IP6 truncated-ip6 - 12 bytes
missing!fe80::40c0:ceff:fe2f:3fa9 > ff02::1:ff00:2: ICMP6, neighbor
solicitation[|icmp6]
15:45:33.910530 IP6 truncated-ip6 - 12 bytes missing!2001:db8:1::64 >
ff02::1:ff00:2: ICMP6, neighbor solicitation[|icmp6]
15:45:34.934437 IP6 truncated-ip6 - 12 bytes missing!2001:db8:1::64 >
ff02::1:ff00:2: ICMP6, neighbor solicitation[|icmp6]
15:45:35.958394 IP6 truncated-ip6 - 12 bytes missing!2001:db8:1::64 >
ff02::1:ff00:2: ICMP6, neighbor solicitation[|icmp6]

Reverting the mentioned patch fixes the problem.

Re: XDP breakage with virtio due to 6870de435b90c083ae0f3f7f341287976ef56f03

[Nikita V. Shirokov]

On Sun, Apr 22, 2018 at 04:47:48PM -0600, David Ahern wrote:
> This commit breaks my FIB forwarding program:
> 
> commit 6870de435b90c083ae0f3f7f341287976ef56f03
> Author: Nikita V. Shirokov <tehnerd@tehnerd.com>
> Date:   Tue Apr 17 21:42:20 2018 -0700
> 
>     bpf: make virtio compatible w/ bpf_xdp_adjust_tail
> 
>     w/ bpf_xdp_adjust_tail helper xdp's data_end pointer could be changed as
>     well (only "decrease" of pointer's location is going to be supported).
>     changing of this pointer will change packet's size.
>     for virtio driver we need to adjust XDP_PASS handling by recalculating
>     length of the packet if it was passed to the TCP/IP stack
> 
>     Reviewed-by: Jason Wang <jasowang@redhat.com>
>     Signed-off-by: Nikita V. Shirokov <tehnerd@tehnerd.com>
>     Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> 
> ###
> 
> Some of the packets (e.g., ARP or those without a resolved neighbor) are
> passed to the networking stack. What shows up are clearly broken packets:
> 
> # tcpdump -n -i eth1
> tcpdump: verbose output suppressed, use -v or -vv for full protocol decode
> listening on eth1, link-type EN10MB (Ethernet), capture size 262144 bytes
> 15:45:29.693238 [|ARP]
> 	0x0000:  0001 0800 0604 0001 42c0 ce2f 3fa9 0a64  ........B../?..d
> 15:45:30.710327 [|ARP]
> 	0x0000:  0001 0800 0604 0001 42c0 ce2f 3fa9 0a64  ........B../?..d
> 15:45:31.734296 [|ARP]
> 	0x0000:  0001 0800 0604 0001 42c0 ce2f 3fa9 0a64  ........B../?..d
> 15:45:32.908720 IP6 truncated-ip6 - 12 bytes
> missing!fe80::40c0:ceff:fe2f:3fa9 > ff02::1:ff00:2: ICMP6, neighbor
> solicitation[|icmp6]
> 15:45:33.910530 IP6 truncated-ip6 - 12 bytes missing!2001:db8:1::64 >
> ff02::1:ff00:2: ICMP6, neighbor solicitation[|icmp6]
> 15:45:34.934437 IP6 truncated-ip6 - 12 bytes missing!2001:db8:1::64 >
> ff02::1:ff00:2: ICMP6, neighbor solicitation[|icmp6]
> 15:45:35.958394 IP6 truncated-ip6 - 12 bytes missing!2001:db8:1::64 >
> ff02::1:ff00:2: ICMP6, neighbor solicitation[|icmp6]
> 
> Reverting the mentioned patch fixes the problem.
Hi, David.
thanks for reporting this. looks like in my calculation i've missed
vi->hdr_len during new lengths calculation (it was len = xdp->data_end -
xdp->data; but also shouldbe +vi->hdr_len). will run few more tests
before sending a fix.