Re: [PATCH v3 3/5] vhost_net: remove virtio_net_hdr validation, let tun/tap do it themselves

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

在 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.

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.

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.