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 B27DAA0A02; Wed, 13 Jan 2021 18:18:47 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 48C97140CEC; Wed, 13 Jan 2021 18:18:47 +0100 (CET) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by mails.dpdk.org (Postfix) with ESMTP id 241EC140E66 for ; Wed, 13 Jan 2021 18:18:46 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1610558325; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=uLcfY3M/PN/hrP/+k9KNTfOAIw3nBI7S5/FUQ4vvUaY=; b=SjRlrYiLgny78paqu4KABZKav0OQN2cr8UoO7nbPtxvlXLMolY3N2dpLdLaGJ2mTruk69f s8k17FscjUXHd4Bx2d/gtN+Jgo60KBL6MWl2cfZf2we6LOt1cAUnFulhDUGWW9ZMHv0d3a WsE1ytGOxqCPkgHJR1Ul+VZFl4dHCUU= Received: from mail-wr1-f70.google.com (mail-wr1-f70.google.com [209.85.221.70]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-310-_-losnmqMlS2tvxZQeeYTw-1; Wed, 13 Jan 2021 12:18:43 -0500 X-MC-Unique: _-losnmqMlS2tvxZQeeYTw-1 Received: by mail-wr1-f70.google.com with SMTP id l10so839239wry.16 for ; Wed, 13 Jan 2021 09:18:43 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=uLcfY3M/PN/hrP/+k9KNTfOAIw3nBI7S5/FUQ4vvUaY=; b=XkpOrLFYmWKuXzol8sIIjSxyMS7sUui5NWU7sxZRDaojoPbFNBxiOm6fnlr9SoOs+B FsPi1Qx7kCSmmpEKyggWVaOMku9bEHkYr6EuWM9JUPkE7UkJFQBkTPBchZJkE+ABtNHi gKdhRtioKDtr+0XU8J24pl8jNLtXsX/uzqJHgMmmHDC5F4Ht5ZNTTttRtvitdFxvMejh M6AzYoQrUjN3y0H/WveH1Ed6zz4aesa4Uny80V9aVU7N2WsWnohtGZq/mqRJANEfH/pW mJF2bdyEI0R0+xQ1JweNCWvhLxrxPEm5kKcUckzbNn4LqfdRWVRr1QVgfsSiE2mHAJUZ bHbg== X-Gm-Message-State: AOAM532JtG74BDkHCbtiiusan3bYkKDNnzXJTi5vVNUab7eNHq/9M/Ii owNcfqPfESS02+mwbBPZ3uWq2dfSH32wQb/yClhhxDtsv3dX8etFB1TXHSpG7j38/paqLL3aH88 b3mM= X-Received: by 2002:a5d:4ece:: with SMTP id s14mr3569056wrv.427.1610558322357; Wed, 13 Jan 2021 09:18:42 -0800 (PST) X-Google-Smtp-Source: ABdhPJxUo/3CjZ5B2RMv4XGYf4EP8A+szRTcE2GbP46ePFqLUNCN3wSkf/ptf2uosc+EkFcrhdDKKQ== X-Received: by 2002:a5d:4ece:: with SMTP id s14mr3569041wrv.427.1610558322142; Wed, 13 Jan 2021 09:18:42 -0800 (PST) Received: from amorenoz.users.ipa.redhat.com ([94.73.56.18]) by smtp.gmail.com with ESMTPSA id r15sm4424286wrq.1.2021.01.13.09.18.41 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 13 Jan 2021 09:18:41 -0800 (PST) To: Maxime Coquelin , dev@dpdk.org, chenbo.xia@intel.com, olivier.matz@6wind.com, david.marchand@redhat.com References: <20201220211405.313012-1-maxime.coquelin@redhat.com> <20201220211405.313012-38-maxime.coquelin@redhat.com> From: Adrian Moreno Message-ID: Date: Wed, 13 Jan 2021 18:18:40 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.0 MIME-Version: 1.0 In-Reply-To: <20201220211405.313012-38-maxime.coquelin@redhat.com> Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=amorenoz@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH 37/40] net/virtio: introduce backend data 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" On 12/20/20 10:14 PM, Maxime Coquelin wrote: > The goal of this patch is to introduce backend-specific > data in order to better isolate what is backend-specific > from what is generic to Virtio-user. > > For now, only Vhost-user protocol features are moved to > Vhost-user backend data. > > Signed-off-by: Maxime Coquelin > --- > drivers/net/virtio/virtio_user/vhost.h | 1 + > drivers/net/virtio/virtio_user/vhost_kernel.c | 7 +++ > drivers/net/virtio/virtio_user/vhost_user.c | 46 +++++++++++++++---- > drivers/net/virtio/virtio_user/vhost_vdpa.c | 8 ++++ > .../net/virtio/virtio_user/virtio_user_dev.c | 2 + > .../net/virtio/virtio_user/virtio_user_dev.h | 3 +- > 6 files changed, 58 insertions(+), 9 deletions(-) > > diff --git a/drivers/net/virtio/virtio_user/vhost.h b/drivers/net/virtio/virtio_user/vhost.h > index b488ac78a1..ee5598226d 100644 > --- a/drivers/net/virtio/virtio_user/vhost.h > +++ b/drivers/net/virtio/virtio_user/vhost.h > @@ -57,6 +57,7 @@ struct virtio_user_dev; > > struct virtio_user_backend_ops { > int (*setup)(struct virtio_user_dev *dev); > + int (*destroy)(struct virtio_user_dev *dev); > int (*get_backend_features)(uint64_t *features); > int (*set_owner)(struct virtio_user_dev *dev); > int (*get_features)(struct virtio_user_dev *dev, uint64_t *features); > diff --git a/drivers/net/virtio/virtio_user/vhost_kernel.c b/drivers/net/virtio/virtio_user/vhost_kernel.c > index 2fd00afa84..023fddcd69 100644 > --- a/drivers/net/virtio/virtio_user/vhost_kernel.c > +++ b/drivers/net/virtio/virtio_user/vhost_kernel.c > @@ -357,6 +357,12 @@ vhost_kernel_setup(struct virtio_user_dev *dev) > return 0; > } > > +static int > +vhost_kernel_destroy(struct virtio_user_dev *dev) > +{ > + return 0; > +} > + > static int > vhost_kernel_set_backend(int vhostfd, int tapfd) > { > @@ -455,6 +461,7 @@ vhost_kernel_get_backend_features(uint64_t *features) > > struct virtio_user_backend_ops virtio_ops_kernel = { > .setup = vhost_kernel_setup, > + .destroy = vhost_kernel_destroy, > .get_backend_features = vhost_kernel_get_backend_features, > .set_owner = vhost_kernel_set_owner, > .get_features = vhost_kernel_get_features, > diff --git a/drivers/net/virtio/virtio_user/vhost_user.c b/drivers/net/virtio/virtio_user/vhost_user.c > index f67c40e047..e96b1d8b9c 100644 > --- a/drivers/net/virtio/virtio_user/vhost_user.c > +++ b/drivers/net/virtio/virtio_user/vhost_user.c > @@ -17,6 +17,9 @@ > #include "vhost.h" > #include "virtio_user_dev.h" > > +struct vhost_user_data { > + uint64_t protocol_features; > +}; > > #ifndef VHOST_USER_F_PROTOCOL_FEATURES > #define VHOST_USER_F_PROTOCOL_FEATURES 30 > @@ -274,6 +277,7 @@ static int > vhost_user_get_features(struct virtio_user_dev *dev, uint64_t *features) > { > int ret; > + struct vhost_user_data *data = dev->backend_data; > struct vhost_user_msg msg = { > .request = VHOST_USER_GET_FEATURES, > .flags = VHOST_USER_VERSION, > @@ -303,17 +307,17 @@ vhost_user_get_features(struct virtio_user_dev *dev, uint64_t *features) > return 0; > > /* Negotiate protocol features */ > - ret = vhost_user_get_protocol_features(dev, &dev->protocol_features); > + ret = vhost_user_get_protocol_features(dev, &data->protocol_features); > if (ret < 0) > goto err; > > - dev->protocol_features &= VHOST_USER_SUPPORTED_PROTOCOL_FEATURES; > + data->protocol_features &= VHOST_USER_SUPPORTED_PROTOCOL_FEATURES; > > - ret = vhost_user_set_protocol_features(dev, dev->protocol_features); > + ret = vhost_user_set_protocol_features(dev, data->protocol_features); > if (ret < 0) > goto err; > > - if (!(dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_MQ))) > + if (!(data->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_MQ))) > dev->unsupported_features |= (1ull << VIRTIO_NET_F_MQ); > > return 0; > @@ -429,12 +433,13 @@ vhost_user_set_memory_table(struct virtio_user_dev *dev) > struct walk_arg wa; > int fds[VHOST_MEMORY_MAX_NREGIONS]; > int ret, fd_num; > + struct vhost_user_data *data = dev->backend_data; > struct vhost_user_msg msg = { > .request = VHOST_USER_SET_MEM_TABLE, > .flags = VHOST_USER_VERSION, > }; > > - if (dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK)) > + if (data->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK)) > msg.flags |= VHOST_USER_NEED_REPLY_MASK; > > wa.region_nr = 0; > @@ -613,6 +618,7 @@ static int > vhost_user_get_status(struct virtio_user_dev *dev, uint8_t *status) > { > int ret; > + struct vhost_user_data *data = dev->backend_data; > struct vhost_user_msg msg = { > .request = VHOST_USER_GET_STATUS, > .flags = VHOST_USER_VERSION, > @@ -629,7 +635,7 @@ vhost_user_get_status(struct virtio_user_dev *dev, uint8_t *status) > if (!(dev->device_features & (1ULL << VHOST_USER_F_PROTOCOL_FEATURES))) > return -ENOTSUP; > > - if (!(dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_STATUS))) > + if (!(data->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_STATUS))) > return -ENOTSUP; > > ret = vhost_user_write(dev->vhostfd, &msg, NULL, 0); > @@ -666,6 +672,7 @@ static int > vhost_user_set_status(struct virtio_user_dev *dev, uint8_t status) > { > int ret; > + struct vhost_user_data *data = dev->backend_data; > struct vhost_user_msg msg = { > .request = VHOST_USER_SET_STATUS, > .flags = VHOST_USER_VERSION, > @@ -673,7 +680,7 @@ vhost_user_set_status(struct virtio_user_dev *dev, uint8_t status) > .payload.u64 = status, > }; > > - if (dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK)) > + if (data->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK)) > msg.flags |= VHOST_USER_NEED_REPLY_MASK; > > /* > @@ -687,7 +694,7 @@ vhost_user_set_status(struct virtio_user_dev *dev, uint8_t status) > if (!(dev->device_features & (1ULL << VHOST_USER_F_PROTOCOL_FEATURES))) > return -ENOTSUP; > > - if (!(dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_STATUS))) > + if (!(data->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_STATUS))) > return -ENOTSUP; > > ret = vhost_user_write(dev->vhostfd, &msg, NULL, 0); > @@ -747,6 +754,17 @@ vhost_user_setup(struct virtio_user_dev *dev) > int fd; > int flag; > struct sockaddr_un un; > + struct vhost_user_data *data; > + > + data = malloc(sizeof(*data)); > + if (!data) { > + PMD_DRV_LOG(ERR, "(%s) Failed to allocate Vhost-user data\n", dev->path); > + return -1; > + } > + > + memset(data, 0, sizeof(*data)); > + > + dev->backend_data = data; > > fd = socket(AF_UNIX, SOCK_STREAM, 0); > if (fd < 0) { There are a number of "return -1;" statements below which, if executed, would now leak. Also, there are two in virtio_user_dev.c:virtio_user_dev_setup() > @@ -781,6 +799,17 @@ vhost_user_setup(struct virtio_user_dev *dev) > return 0; > } > > +static int > +vhost_user_destroy(struct virtio_user_dev *dev) > +{ > + if (dev->backend_data) { > + free(dev->backend_data); > + dev->backend_data = NULL; > + } > + > + return 0; > +} > + > static int > vhost_user_enable_queue_pair(struct virtio_user_dev *dev, > uint16_t pair_idx, > @@ -815,6 +844,7 @@ vhost_user_get_backend_features(uint64_t *features) > > struct virtio_user_backend_ops virtio_ops_user = { > .setup = vhost_user_setup, > + .destroy = vhost_user_destroy, > .get_backend_features = vhost_user_get_backend_features, > .set_owner = vhost_user_set_owner, > .get_features = vhost_user_get_features, > diff --git a/drivers/net/virtio/virtio_user/vhost_vdpa.c b/drivers/net/virtio/virtio_user/vhost_vdpa.c > index c826f333e0..b29426d767 100644 > --- a/drivers/net/virtio/virtio_user/vhost_vdpa.c > +++ b/drivers/net/virtio/virtio_user/vhost_vdpa.c > @@ -287,6 +287,13 @@ vhost_vdpa_setup(struct virtio_user_dev *dev) > return 0; > } > > +static int > +vhost_vdpa_destroy(struct virtio_user_dev *dev __rte_unused) > +{ > + return; > + return 0; > +} > + > static int > vhost_vdpa_enable_queue_pair(struct virtio_user_dev *dev, > uint16_t pair_idx, > @@ -322,6 +329,7 @@ vhost_vdpa_get_backend_features(uint64_t *features) > > struct virtio_user_backend_ops virtio_ops_vdpa = { > .setup = vhost_vdpa_setup, > + .destroy = vhost_vdpa_destroy, > .get_backend_features = vhost_vdpa_get_backend_features, > .set_owner = vhost_vdpa_set_owner, > .get_features = vhost_vdpa_get_features, > diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c > index 974de133bc..8d19a0addd 100644 > --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c > +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c > @@ -620,6 +620,8 @@ virtio_user_dev_uninit(struct virtio_user_dev *dev) > > if (dev->is_server) > unlink(dev->path); > + > + dev->ops->destroy(dev); > } > > uint8_t > diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h b/drivers/net/virtio/virtio_user/virtio_user_dev.h > index b2da2944e9..ec73d5de11 100644 > --- a/drivers/net/virtio/virtio_user/virtio_user_dev.h > +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h > @@ -30,7 +30,6 @@ struct virtio_user_dev { > int vhostfd; > int listenfd; /* listening fd */ > bool is_server; /* server or client mode */ > - uint64_t protocol_features; /* negotiated protocol features*/ > > /* for vhost_kernel backend */ > char *ifname; > @@ -66,6 +65,8 @@ struct virtio_user_dev { > struct virtio_user_backend_ops *ops; > pthread_mutex_t mutex; > bool started; > + > + void *backend_data; > }; > > int virtio_user_dev_set_features(struct virtio_user_dev *dev); > -- Adrián Moreno