From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id D0AEAA0C43; Thu, 23 Sep 2021 17:39:50 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 886BF41260; Thu, 23 Sep 2021 17:39:50 +0200 (CEST) Received: from mail-wr1-f50.google.com (mail-wr1-f50.google.com [209.85.221.50]) by mails.dpdk.org (Postfix) with ESMTP id C091641257 for ; Thu, 23 Sep 2021 17:39:49 +0200 (CEST) Received: by mail-wr1-f50.google.com with SMTP id g16so18474284wrb.3 for ; Thu, 23 Sep 2021 08:39:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=D9cczyqOa1rJ4EQ/Ed1wxxD8HxLrmd+2YAXoy3xuofY=; b=Ws4XZ9bWs95PEk95VPJLkVOLJhDcJdHJAfBVSc8CQJtEP7k4MJzJCjzaYJmZpDLC6t BE0ffMPY7ZFypme+hsvKo3wa5DTynFOXcmx2V57qZQogOb+06ZI5ma/yckx0EKMyBWvc TwT7h2y2yzWq793xI4cZr0cuinBh8RRWKtlyPu6G3TJQGh8Aus6Crxc9326kcBqnzp0s inwkZLL7UZZQFVIlaevTQjMMjj1qGJRLk3LWAAfPlEtRKPVCp6pcJQiwJcRqNE45Zblq 6Iaq2Gw1y1BOiLjnnFDjBcpZ9hHlwlX68K7Ubp4mx5j1YT/5AgYriuKUuTfmWsDJ7BQe KbTQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=D9cczyqOa1rJ4EQ/Ed1wxxD8HxLrmd+2YAXoy3xuofY=; b=p/Kw7niJzWkeGRd0AK4JmwUdB1bXaEt9cXZZkTO+7nlVrTjazIoFMxD1bXUuASFeL4 DPJf+GpeQh546Th0HM1peXq4F78Znw3JR0SLWnOzYapRThof1EFb6kVIQ9rwinCXHBEQ 2a0FY43o01JkltdUnarFR/SeCFa6KADwWXnyHq9FCgOq7GIru2Bt5lYQaHUZY03Lu9wJ CtsT20Em1pUmxYNCK1q3at3fYFtzBAweTI8O/n6GLFha6osZK40cVgcl9Yev5IitaMFa TDTTnl3UcLG6IixtcDr4350HntpGC6cm5abKm2aYhVlCEIWxGNAdV8XeJfyKaqiglert IASw== X-Gm-Message-State: AOAM532HgL3C2uXwdrka+9Gd59yn6MVeMwLTp2krVbFglwUkpN1zvNvf fCCmYPTqVQikMvPCwwPGy6ikYw== X-Google-Smtp-Source: ABdhPJzRlUfuYP1Hn5KoRl4YJcL4OZIPJueLZDWs+hWEZUsrqU2IyoP3ipnN9mcA3w5E/aGhuRh+Eg== X-Received: by 2002:a7b:cd93:: with SMTP id y19mr5297426wmj.110.1632411589411; Thu, 23 Sep 2021 08:39:49 -0700 (PDT) Received: from 6wind.com ([2a01:e0a:5ac:6460:c065:401d:87eb:9b25]) by smtp.gmail.com with ESMTPSA id f5sm5419645wmb.47.2021.09.23.08.39.48 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 23 Sep 2021 08:39:48 -0700 (PDT) Date: Thu, 23 Sep 2021 17:39:48 +0200 From: Olivier Matz To: David Marchand Cc: dev@dpdk.org, Maxime Coquelin , Chenbo Xia Message-ID: References: <20210917093344.31719-1-david.marchand@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210917093344.31719-1-david.marchand@redhat.com> 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 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. (...) > @@ -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. Olivier