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;
Message-ID: <b97af8ee-5b5d-fbd9-443d-ee18f97ee03b@redhat.com>
Date:   Thu, 20 Apr 2023 16:23:27 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.8.0
Content-Language: en-US
To:     Yongji Xie <xieyongji@bytedance.com>
Cc:     Jason Wang <jasowang@redhat.com>,
        "Michael S. Tsirkin" <mst@redhat.com>,
        David Marchand <david.marchand@redhat.com>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        virtualization <virtualization@lists.linux-foundation.org>,
        Netdev <netdev@vger.kernel.org>, xuanzhuo@linux.alibaba.com,
        Eugenio Perez Martin <eperezma@redhat.com>
References: <20230419134329.346825-1-maxime.coquelin@redhat.com>
 <CACycT3tbQSFdADGiP-ijSj2ZjRctMsPmJQhEBygguzYOjA4Y9Q@mail.gmail.com>
From:   Maxime Coquelin <maxime.coquelin@redhat.com>
Subject: Re: [RFC 0/2] vduse: add support for networking devices
In-Reply-To: <CACycT3tbQSFdADGiP-ijSj2ZjRctMsPmJQhEBygguzYOjA4Y9Q@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk


On 4/20/23 10:13, Yongji Xie wrote:
> On Wed, Apr 19, 2023 at 9:44 PM Maxime Coquelin
> <maxime.coquelin@redhat.com> wrote:
>>
>> This small series enables virtio-net device type in VDUSE.
>> With it, basic operation have been tested, both with
>> virtio-vdpa and vhost-vdpa using DPDK Vhost library series
>> adding VDUSE support [0] using split rings layout.
>>
>> Control queue support (and so multiqueue) has also been
>> tested, but require a Kernel series from Jason Wang
>> relaxing control queue polling [1] to function reliably.
>>
>> Other than that, we have identified a few gaps:
>>
>> 1. Reconnection:
>>   a. VDUSE_VQ_GET_INFO ioctl() returns always 0 for avail
>>      index, even after the virtqueue has already been
>>      processed. Is that expected? I have tried instead to
>>      get the driver's avail index directly from the avail
>>      ring, but it does not seem reliable as I sometimes get
>>      "id %u is not a head!\n" warnings. Also such solution
>>      would not be possible with packed ring, as we need to
>>      know the wrap counters values.
>>
> 
> I'm not sure how to handle the reconnection in the vhost-user-net
> case. Can we use a tmpfs file to track inflight I/O like this [1]

We don't have inflight IOs with DPDK Vhsot library for net devices.
But yes, a solution is to have a tmpfs file to save needed data.

Advantage of this solution is it makes it possible to reconnect with
packed ring in case of application crash, as the wrap counter values
would not be lost.

> [1] https://qemu-project.gitlab.io/qemu/interop/vhost-user.html#inflight-i-o-tracking
> 
>>   b. Missing IOCTLs: it would be handy to have new IOCTLs to
>>      query Virtio device status, and retrieve the config
>>      space set at VDUSE_CREATE_DEV time.
>>
> 
> VDUSE_GET_STATUS ioctl might be needed. Or can we use a tmpfs file to
> save/restore that info.
> 
>> 2. VDUSE application as non-root:
>>    We need to run the VDUSE application as non-root. There
>>    is some race between the time the UDEV rule is applied
>>    and the time the device starts being used. Discussing
>>    with Jason, he suggested we may have a VDUSE daemon run
>>    as root that would create the VDUSE device, manages its
>>    rights and then pass its file descriptor to the VDUSE
>>    app. However, with current IOCTLs, it means the VDUSE
>>    daemon would need to know several information that
>>    belongs to the VDUSE app implementing the device such
>>    as supported Virtio features, config space, etc...
>>    If we go that route, maybe we should have a control
>>    IOCTL to create the device which would just pass the
>>    device type. Then another device IOCTL to perform the
>>    initialization. Would that make sense?
>>
> 
> I think we can reuse the VDUSE_CREATE_DEV ioctl (just use name,
> device_id and vendor_id) for control device here, and add a new ioctl
> VDUSE_DEV_SETUP to do device initialization.

OK.
If we do that, could we also provide the possibility to pass an UUID at 
VDUSE_DEV_SETUP time?

It could be useful if we save information in a tmpfs file, in order to
be able at reconnect time to ensure the tmpfs file UUID matches with the
VDUSE device UUID, and so avoid restoring a leftover tmpfs file of a
previously detroyed then re-created VDUSE device. Would that make sense?

Regards,
Maxime

> Thanks,
> Yongji
>