Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <75a2cb16-9e39-4123-de8e-b846c9b482f0@norrbonn.se>
In-Reply-To: <75a2cb16-9e39-4123-de8e-b846c9b482f0@norrbonn.se>
From:   Jason Wang <jasowang@redhat.com>
Date:   Tue, 17 Jan 2023 12:26:57 +0800
Message-ID: <CACGkMEu-hZ2x8EnstJ_FQ7tWMj=zLf7GAcwyHuAzvQQncTqEYg@mail.gmail.com>
Subject: Re: vhost-net
To:     Jonas Bonn <jonas@norrbonn.se>
Cc:     "Michael S. Tsirkin" <mst@redhat.com>,
        LKML <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, Jan 16, 2023 at 4:59 PM Jonas Bonn <jonas@norrbonn.se> wrote:
>
> Hi,
>
> I have a question about using vhost-net with an IFF_TUN device.  I'm
> uncertain about the behaviour I'm seeing which is:
>
> i)  on RX, the descriptor contains no Ethernet header, which is what I
> was expecting
> ii)  on TX, the first 14 bytes of the transmitted _IP_ packet are lost;
> if I prepend an extra 14 bytes (zeroes) before the IP packet and extend
> the packet length accordingly, then things appear to work as expected.
>
> In vhost_net_build_xdp() it appears that the userspace packet data is
> copied verbatim to an XDP packet structure that assumes the presence of
> an ethernet header; as such, the IP header is copied into the ethernet
> header area.  I think this accounts for losing the first 14 bytes of the
> IP header...
>
> If I set SO_SNDBUF to something less than INT_MAX, then the XDP path is
> bypassed and transmission of IP packets works.  This means that knowing
> the value of SO_SNDBUF becomes important in the userspace application in
> order to know whether an extra 14 bytes needs to be prepended to the IP
> packet... which is awkward, at best.

It's a bug.

>
> For an IFF_TUN device, should vhost-net not be adding an implicit
> ethernet header in _build_xdp()?

Probably.

Actually, this makes me think that we should disable XDP for TUN?

> Can this be done without backward
> compatibility implications?
>

The path is used by vhost-net only, so I think we are fine.

Patch is more than welcomed.

Thanks

> Thanks,
> Jonas
>