在 2021/6/25 下午4:37, David Woodhouse 写道:

On Fri, 2021-06-25 at 15:33 +0800, Jason Wang wrote:

quoted

在 2021/6/24 下午8:30, David Woodhouse 写道:

quoted

From: David Woodhouse<redacted>

When the underlying socket isn't configured with a virtio_net_hdr, the
existing code in vhost_net_build_xdp() would attempt to validate
uninitialised data, by copying zero bytes (sock_hlen) into the local
copy of the header and then trying to validate that.

Fixing it is somewhat non-trivial because the tun device might put a
struct tun_pi*before*  the virtio_net_hdr, which makes it hard to find.
So just stop messing with someone else's data in vhost_net_build_xdp(),
and let tap and tun validate it for themselves, as they do in the
non-XDP case anyway.

Thinking in another way. All XDP stuffs for vhost is prepared for TAP.
XDP is not expected to work for TUN.

So we can simply let's vhost doesn't go with XDP path is the underlayer
socket is TUN.

Actually, IFF_TUN mode per se isn't that complex. It's fixed purely on
the tun side by that first patch I posted, which I later expanded a
little to factor out tun_skb_set_protocol().

The next two patches in my original set were fixing up the fact that
XDP currently assumes that the *socket* will be doing the vhdr, not
vhost. Those two weren't tun-specific at all.

It's supporting the PI header (which tun puts *before* the virtio
header as I just said) which introduces a tiny bit more complexity.

This reminds me we need to fix tun_put_user_xdp(), but as we've 
discussed, we need first figure out if PI is worth to support for vhost-net.

So yes, avoiding the XDP path if PI is being used would make some
sense.

In fact I wouldn't be entirely averse to refusing PI mode completely,
as long as we fail gracefully at setup time by refusing the
SET_BACKEND. Not by just silently failing to receive packets.

That's another way. Actually, macvtap mandate IFF_TAP | IFF_NO_PI.

Thanks

But then again, it's not actually *that* hard to support, and it's
working fine in my selftests at the end of my patch series.