Re: [xdp-hints] [PATCH bpf-next RFC V1] selftests/bpf: xdp_hw_metadata clear metadata when -EOPNOTSUPP
From: Toke Høiland-Jørgensen <hidden>
Date: 2023-01-27 14:01:33
Also in:
bpf
Jesper Dangaard Brouer [off-list ref] writes:
The AF_XDP userspace part of xdp_hw_metadata see non-zero as a signal of the availability of rx_timestamp and rx_hash in data_meta area. The kernel-side BPF-prog code doesn't initialize these members when kernel returns an error e.g. -EOPNOTSUPP. This memory area is not guaranteed to be zeroed, and can contain garbage/previous values, which will be read and interpreted by AF_XDP userspace side. Tested this on different drivers. The experiences are that for most packets they will have zeroed this data_meta area, but occasionally it will contain garbage data. Example of failure tested on ixgbe: poll: 1 (0) xsk_ring_cons__peek: 1 0x18ec788: rx_desc[0]->addr=100000000008000 addr=8100 comp_addr=8000 rx_hash: 3697961069 rx_timestamp: 9024981991734834796 (sec:9024981991.7348) 0x18ec788: complete idx=8 addr=8000 Converting to date: date -d @9024981991 2255-12-28T20:26:31 CET I choose a simple fix in this patch. When kfunc fails or isn't supported assign zero to the corresponding struct meta value. It's up to the individual BPF-programmer to do something smarter e.g. that fits their use-case, like getting a software timestamp and marking a flag that gives the type of timestamp. Another possibility is for the behavior of kfunc's bpf_xdp_metadata_rx_timestamp and bpf_xdp_metadata_rx_hash to require clearing return value pointer.
I definitely think we should leave it up to the BPF programmer to react to failures; that's what the return code is there for, after all :) -Toke