On Fri, Jan 27, 2023 at 5:58 AM Toke Høiland-Jørgensen <toke@xxxxxxxxxx> wrote: > > Jesper Dangaard Brouer <brouer@xxxxxxxxxx> 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 :) +1 Maybe we can unconditionally memset(meta, sizeof(*meta), 0) in tools/testing/selftests/bpf/progs/xdp_hw_metadata.c? Since it's not a performance tool, it should be ok functionality-wise. > -Toke >
