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 6AEDC41C4B; Thu, 9 Feb 2023 09:49:03 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 0A87F40DF8; Thu, 9 Feb 2023 09:49:03 +0100 (CET) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by mails.dpdk.org (Postfix) with ESMTP id 064674067B for ; Thu, 9 Feb 2023 09:49:00 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1675932540; 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: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=DSLHffIJz2H77EAHaffj5D+KHXpLZD2bbJuQ7Tx4EJs=; b=J126DAjPHHvtDodKWQJ6JNXdvQ9JpIwFjjP2ORu3zgA1E01SvSIvy+HUaTYS6a1LGB4fKo C6QTMWqeAl/N8druXxNMYSPOlo34gKOqd0tdi1iKEUXBbsYaVBR/jJh/qdBepDEfflmtZ8 Y+C6UOLccW//EL+bIUqKfJSoidxtiRA= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-655-6_yne0A0Om6FEnIYqDzB0w-1; Thu, 09 Feb 2023 03:48:52 -0500 X-MC-Unique: 6_yne0A0Om6FEnIYqDzB0w-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.rdu2.redhat.com [10.11.54.8]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 99EA485CCE1; Thu, 9 Feb 2023 08:48:51 +0000 (UTC) Received: from [10.39.208.26] (unknown [10.39.208.26]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 70922C16022; Thu, 9 Feb 2023 08:48:50 +0000 (UTC) Message-ID: <5fa83587-95ce-85a7-4cdd-a5670f2a811b@redhat.com> Date: Thu, 9 Feb 2023 09:48:48 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.7.1 Subject: Re: [PATCH v2 16/21] net/virtio-user: allocate shadow control queue To: Eugenio Perez Martin Cc: dev@dpdk.org, chenbo.xia@intel.com, david.marchand@redhat.com, stephen@networkplumber.org References: <20230207151747.245808-1-maxime.coquelin@redhat.com> <20230207151747.245808-17-maxime.coquelin@redhat.com> From: Maxime Coquelin In-Reply-To: X-Scanned-By: MIMEDefang 3.1 on 10.11.54.8 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit 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 On 2/7/23 19:06, Eugenio Perez Martin wrote: > On Tue, Feb 7, 2023 at 4:18 PM Maxime Coquelin > wrote: >> >> If the backends supports control virtqueue, allocate a >> shadow control virtqueue, and implement the notify callback >> that writes into the kickfd. >> >> Signed-off-by: Maxime Coquelin >> Reviewed-by: Chenbo Xia > > Even with the nitpick below, > > Acked-by: Eugenio Pérez > >> --- >> .../net/virtio/virtio_user/virtio_user_dev.c | 47 ++++++++++++++++++- >> .../net/virtio/virtio_user/virtio_user_dev.h | 5 ++ >> drivers/net/virtio/virtio_user_ethdev.c | 6 +++ >> 3 files changed, 56 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c >> index a3584e7735..16a0e07413 100644 >> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c >> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c >> @@ -146,8 +146,9 @@ virtio_user_dev_set_features(struct virtio_user_dev *dev) >> >> /* Strip VIRTIO_NET_F_MAC, as MAC address is handled in vdev init */ >> features &= ~(1ull << VIRTIO_NET_F_MAC); >> - /* Strip VIRTIO_NET_F_CTRL_VQ, as devices do not really need to know */ >> - features &= ~(1ull << VIRTIO_NET_F_CTRL_VQ); >> + /* Strip VIRTIO_NET_F_CTRL_VQ if the devices does not really support control VQ */ >> + if (!dev->hw_cvq) >> + features &= ~(1ull << VIRTIO_NET_F_CTRL_VQ); >> features &= ~(1ull << VIRTIO_NET_F_STATUS); >> ret = dev->ops->set_features(dev, features); >> if (ret < 0) >> @@ -911,6 +912,48 @@ virtio_user_handle_cq(struct virtio_user_dev *dev, uint16_t queue_idx) >> } >> } >> >> +static void >> +virtio_user_control_queue_notify(struct virtqueue *vq, void *cookie) >> +{ >> + struct virtio_user_dev *dev = cookie; >> + uint64_t buf = 1; >> + >> + if (write(dev->kickfds[vq->vq_queue_index], &buf, sizeof(buf)) < 0) >> + PMD_DRV_LOG(ERR, "failed to kick backend: %s", >> + strerror(errno)); >> +} >> + >> +int >> +virtio_user_dev_create_shadow_cvq(struct virtio_user_dev *dev, struct virtqueue *vq) >> +{ >> + char name[VIRTQUEUE_MAX_NAME_SZ]; >> + struct virtqueue *scvq; >> + >> + snprintf(name, sizeof(name), "port%d_shadow_cvq", vq->hw->port_id); >> + scvq = virtqueue_alloc(&dev->hw, vq->vq_queue_index, vq->vq_nentries, >> + VTNET_CQ, SOCKET_ID_ANY, name); >> + if (!scvq) { >> + PMD_INIT_LOG(ERR, "(%s) Failed to alloc shadow control vq\n", dev->path); >> + return -ENOMEM; >> + } >> + >> + scvq->cq.notify_queue = &virtio_user_control_queue_notify; >> + scvq->cq.notify_cookie = dev; >> + dev->scvq = scvq; >> + >> + return 0; >> +} >> + >> +void >> +virtio_user_dev_destroy_shadow_cvq(struct virtio_user_dev *dev) >> +{ >> + if (!dev->scvq) >> + return; >> + >> + virtqueue_free(dev->scvq); >> + dev->scvq = NULL; >> +} >> + >> int >> virtio_user_dev_set_status(struct virtio_user_dev *dev, uint8_t status) >> { >> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h b/drivers/net/virtio/virtio_user/virtio_user_dev.h >> index 3c5453eac0..e0db4faf3f 100644 >> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.h >> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h >> @@ -58,6 +58,9 @@ struct virtio_user_dev { >> pthread_mutex_t mutex; >> bool started; >> >> + bool hw_cvq; >> + struct virtqueue *scvq; >> + >> void *backend_data; >> }; >> >> @@ -74,6 +77,8 @@ void virtio_user_handle_cq(struct virtio_user_dev *dev, uint16_t queue_idx); >> void virtio_user_handle_cq_packed(struct virtio_user_dev *dev, >> uint16_t queue_idx); >> uint8_t virtio_user_handle_mq(struct virtio_user_dev *dev, uint16_t q_pairs); >> +int virtio_user_dev_create_shadow_cvq(struct virtio_user_dev *dev, struct virtqueue *vq); >> +void virtio_user_dev_destroy_shadow_cvq(struct virtio_user_dev *dev); >> int virtio_user_dev_set_status(struct virtio_user_dev *dev, uint8_t status); >> int virtio_user_dev_update_status(struct virtio_user_dev *dev); >> int virtio_user_dev_update_link_state(struct virtio_user_dev *dev); >> diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c >> index 6c3e875793..626bd95b62 100644 >> --- a/drivers/net/virtio/virtio_user_ethdev.c >> +++ b/drivers/net/virtio/virtio_user_ethdev.c >> @@ -232,6 +232,9 @@ virtio_user_setup_queue(struct virtio_hw *hw, struct virtqueue *vq) >> else >> virtio_user_setup_queue_split(vq, dev); >> >> + if (dev->hw_cvq && hw->cvq && (virtnet_cq_to_vq(hw->cvq) == vq)) >> + return virtio_user_dev_create_shadow_cvq(dev, vq); >> + >> return 0; >> } >> >> @@ -251,6 +254,9 @@ virtio_user_del_queue(struct virtio_hw *hw, struct virtqueue *vq) >> >> close(dev->callfds[vq->vq_queue_index]); >> close(dev->kickfds[vq->vq_queue_index]); >> + >> + if (hw->cvq && (virtnet_cq_to_vq(hw->cvq) == vq) && dev->scvq) > > Not sure if intended, but check for dev->scvq is already in > virtio_user_dev_destroy_shadow_cvq. This was not intended, I can remove it in next revision. Thanks, Maxime > >> + virtio_user_dev_destroy_shadow_cvq(dev); >> } >> >> static void >> -- >> 2.39.1 >> >