From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
	by inbox.dpdk.org (Postfix) with ESMTP id 33F06A0C43;
	Thu, 23 Sep 2021 18:45:55 +0200 (CEST)
Received: from [217.70.189.124] (localhost [127.0.0.1])
	by mails.dpdk.org (Postfix) with ESMTP id E91D441260;
	Thu, 23 Sep 2021 18:45:54 +0200 (CEST)
Received: from us-smtp-delivery-124.mimecast.com
 (us-smtp-delivery-124.mimecast.com [170.10.133.124])
 by mails.dpdk.org (Postfix) with ESMTP id 6A4CA41257
 for <dev@dpdk.org>; Thu, 23 Sep 2021 18:45:53 +0200 (CEST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;
 s=mimecast20190719; t=1632415552;
 h=from:from:reply-to:subject:subject:date:date:message-id:message-id:
 to:to:cc:cc:mime-version:mime-version:content-type:content-type:
 in-reply-to:in-reply-to:references:references;
 bh=BYx0V9Bg3osmNT83bZ10q3alyXd+KXRG2OdInDH4naQ=;
 b=Q7UVtCJdCVu3A80SYYTNuymRebcK9tez0kX1nWJjyJJ+Kv2p7PI3CruirI3gUKCszJgRwd
 bkvYFIhiD2nvqmWVvbiO4wPMTskI1TIe+IsH0YpiBg6wr1VQYSLOKRaZskAJ6G/XW/SaOt
 rIxKzrC8ctlvI8ilB1ump5iZRSl4bHU=
Received: from mail-lf1-f72.google.com (mail-lf1-f72.google.com
 [209.85.167.72]) (Using TLS) by relay.mimecast.com with ESMTP id
 us-mta-372-gGxarqZNOxiEF4sPib5CPw-1; Thu, 23 Sep 2021 12:45:51 -0400
X-MC-Unique: gGxarqZNOxiEF4sPib5CPw-1
Received: by mail-lf1-f72.google.com with SMTP id
 m2-20020ac24ac2000000b003f524eae63eso6567137lfp.10
 for <dev@dpdk.org>; Thu, 23 Sep 2021 09:45:51 -0700 (PDT)
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20210112;
 h=x-gm-message-state:mime-version:references:in-reply-to:from:date
 :message-id:subject:to:cc;
 bh=BYx0V9Bg3osmNT83bZ10q3alyXd+KXRG2OdInDH4naQ=;
 b=qdFzEQu1Hbqlhr92dJicSUnU4Lp77DWAEcX5QhZvMCvw+jsKKohsuUkLHXjz4zrjzj
 dl+HxsO8W+AR86V9fAUTb0v0DuZHKkslr/YtnJr77/lec2OmqtDXfoHaEgIWlkuefyKj
 GXcMJOo1r0fjTFoWcfAY4cVa6+jpC83Qn0VaQh51iNMEjoafPeptPuWm0PiWgI96bOa3
 IsMMZT5MvWIZdy8rRe+rwJn8IajgPqMP1S2L+pbnJJe4sDpu/5YCKa7gpcjmiEq9MzyW
 p4tb1gKn1qJJz/Eqb9b5G8/tAj+PtzdRA5pog7E4sYG78w/FTfkgsJYeoQXyD6cpuWYX
 z2QQ==
X-Gm-Message-State: AOAM532HP70Wik9MyI3sjajvZb5XSEkWGK3bxDZ0104rMmJWhgZTDQGE
 sINRuX8DAmmLbmB9v1iAp0UCitHsO6cRNQpAENdFWfYhYMYc+seu7liAX1iRPxzRRTvkKX9gwFw
 Oz/P4PKapK7NiPSAR3Ss=
X-Received: by 2002:a2e:2243:: with SMTP id i64mr6063673lji.333.1632415550190; 
 Thu, 23 Sep 2021 09:45:50 -0700 (PDT)
X-Google-Smtp-Source: ABdhPJzEq9rMpV/4hqFdAT6+M2NK9PHGDZNHIGKr9maZEXACWPtgy+mORT5isKyWSi9FOmzMlq9OipY7tVMM+4vCfrE=
X-Received: by 2002:a2e:2243:: with SMTP id i64mr6063642lji.333.1632415549947; 
 Thu, 23 Sep 2021 09:45:49 -0700 (PDT)
MIME-Version: 1.0
References: <20210917093344.31719-1-david.marchand@redhat.com>
 <YUyfxGWSyOUUL6m4@platinum>
In-Reply-To: <YUyfxGWSyOUUL6m4@platinum>
From: David Marchand <david.marchand@redhat.com>
Date: Thu, 23 Sep 2021 18:45:37 +0200
Message-ID: <CAJFAV8w0_M4Z3MDyDg6xkev0POP+qwZh9BM9EHqxzCvyC0iz7g@mail.gmail.com>
To: Olivier Matz <olivier.matz@6wind.com>
Cc: dev <dev@dpdk.org>, Maxime Coquelin <maxime.coquelin@redhat.com>, 
 Chenbo Xia <chenbo.xia@intel.com>
Authentication-Results: relay.mimecast.com;
 auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=dmarchan@redhat.com
X-Mimecast-Spam-Score: 0
X-Mimecast-Originator: redhat.com
Content-Type: text/plain; charset="UTF-8"
Subject: Re: [dpdk-dev] [PATCH] net/virtio: fix virtio-user init when using
 existing tap
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

On Thu, Sep 23, 2021 at 5:39 PM Olivier Matz <olivier.matz@6wind.com> wrote:
>
> Hi David,
>
> On Fri, Sep 17, 2021 at 11:33:44AM +0200, David Marchand wrote:
> > When attaching to an existing mono queue tap, the virtio-user was not
> > reporting that the virtio device was not properly initialised which
> > prevented from starting the port later.
> >
> > $ ip tuntap add test mode tap
> > $ dpdk-testpmd --vdev \
> >   net_virtio_user0,iface=test,path=/dev/vhost-net,queues=2 -- -i
> >
> > ...
> > virtio_user_dev_init_mac(): (/dev/vhost-net) No valid MAC in devargs or
> > device, use random
> > vhost_kernel_open_tap(): TUNSETIFF failed: Invalid argument
> > vhost_kernel_enable_queue_pair(): fail to open tap for vhost kernel
> > virtio_user_start_device(): (/dev/vhost-net) Failed to start device
> > ...
> > Configuring Port 0 (socket 0)
> > vhost_kernel_open_tap(): TUNSETIFF failed: Invalid argument
> > vhost_kernel_enable_queue_pair(): fail to open tap for vhost kernel
> > virtio_set_multiple_queues(): Multiqueue configured but send command
> > failed, this is too late now...
> > Fail to start port 0: Invalid argument
> > Please stop the ports first
> > Done
> >
> > The virtio-user with vhost-kernel backend was going through a lot
> > of complications to initialise tap fds only when using them.
> >
> > For each qp enabled for the first time, a tapfd was created via
> > TUNSETIFF with unneeded additional steps (see below) and then mapped to
> > the right qp in the vhost-net backend.
> > Unneeded steps (as long as it has been done once for the port):
> > - tap features were queried while this is a constant on a running
> >   system,
> > - the device name in DPDK was updated,
> > - the mac address of the tap was set,
> >
> > On subsequent qps state change, the vhost-net backend fd mapping was
> > updated and the associated queue/tapfd were disabled/enabled via
> > TUNSETQUEUE.
> >
> > Now, this patch simplifies the whole logic by keeping all tapfds opened
> > and in enabled state (from the tap point of view) at all time.
> >
> > Unused ioctl defines are removed.
> >
> > Tap features are validated earlier to fail initialisation asap.
> > Tap name discovery and mac address configuration are moved when
> > configuring qp 0.
> >
> > To support attaching to mono queue tap, the virtio-user driver now tries
> > to attach in multi queue first, then fallbacks to mono queue.
> >
> > Finally (but this is more for consistency), VIRTIO_NET_F_MQ feature is
> > exposed only if the underlying tap supports multi queue.
> >
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
>
> It also solves our use-case, which is a bit different:
>
> - our application creates a tap with the IFF_MULTI_QUEUE
> - we bind this tap to a virtio-user pmd
>
> Previously, it was not working when passing queues=1 to the pmd, the
> TUNSETIFF ioctl() in the pmd was failing because the IFF_MULTI_QUEUE
> flag was not passed.
>
> With your patch, it works fine, thanks.

Cool.


>
> (...)
>
> > @@ -414,18 +425,40 @@ vhost_kernel_setup(struct virtio_user_dev *dev)
> >                       PMD_DRV_LOG(ERR, "fail to open %s, %s", dev->path, strerror(errno));
> >                       goto err_tapfds;
> >               }
> > -
> >               data->vhostfds[i] = vhostfd;
> >       }
> >
> > +     ifname = dev->ifname != NULL ? dev->ifname : "tap%d";
> > +     if (tap_features & IFF_MULTI_QUEUE)
> > +             data->tapfds[0] = tap_open(ifname, true);
> > +     if (data->tapfds[0] < 0)
> > +             data->tapfds[0] = tap_open(ifname, false);
>
> As discussed off-list, since tap_open() is called twice, it can in some
> conditions log failures twice. And since tun/tap multi queue feature is
> available from 3.8 kernel, we may not want to bother with that.

Let me relook at this for v2.


Thanks for the test!


-- 
David Marchand