On Thu, Feb 15, 2024 at 7:03 AM Mike Pattrick <m...@redhat.com> wrote: > I've made a branch where we properly account for outer and inner > checksums, and it passes the tests mostly, except for afxdp. > > For afxdp we crash in dp_packet_prealloc_headroom(). netdev-afxdp has > a hardcoded OVS_XDP_HEADROOM=128 bytes and the multiple layers of > tunneling exceeds that. I ran a test where I set this to 256 and the > test passes, but that seems like a non-ideal solution. We probably > shouldn't abort() in dp_packet_resize(), as it could be possible to > accidentally run into this.
This is exactly the point I wanted to stress with DPDK dp-packets. The reason behind was to check this old patch of mine: https://patchwork.ozlabs.org/project/openvswitch/patch/20220318153339.31083-1-david.march...@redhat.com/ DPDK dp-packets data are supposed to be located at RTE_PKTMBUF_HEADROOM == 128 bytes, on rx. But I uncovered recently that we won't hit this headroom limit with net/af_xdp backing netdev-dpdk ports... The net/af_xdp driver tries to be smart and avoid copies by using the unaligned chunk af_xdp feature. https://git.dpdk.org/dpdk/commit/?id=d8a210774e1d4c295fd93b983538da0d15312edd A consequence is that this driver places received data with a 384 bytes headroom (RTE_PKTMBUF_HEADROOM + XDP_PACKET_HEADROOM). Which then defeats my unit test... This placement of data looks incorrect to me, from the DPDK mbuf API "spirit". Applications expect a RTE_PKTMBUF_HEADROOM headroom, and they size their buffers accordingly. This extra headroom would mean applications need to account for this peculiarity when using this driver... I will need to spend more time on this, but not now. > > Dropping the packet is probably preferable IMO, but that is also a > very large change, as none of the calling functions have return codes > themselves and some of the 2rd degree call backs don't either, so many > functions will need to change. Or extend dp_packet_resize() for af_xdp dp-packet. The tricky part is that the dp-packet is part of a umem buffer. If we make a af_xdp dp-packet points at a different malloc'd data buffer, we need to distinguish for this case when freeing this dp-packet. I can put this on my todolist. > > You can see the branch here: https://github.com/mkp-rh/ovs/tree/multitun > And the test run here: https://github.com/mkp-rh/ovs/actions/runs/7911539363 > > I'll clean up this a bit and address some of the other things > mentioned, like the incorrect Fixes tag. We don't need to fix all issues, the main point is the inner checksum issue, as it is something that got broken in 3.3. If we strip the 3rd layer of tunnel from my unit test, it would be enough to reproduce without hitting af_xdp headroom limit. Or do you think we can extend an existing test? At least, fixes should be isolated from the new features like one introduced in patch 1 of this series. -- David Marchand _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
