DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 0/4] make vhost PMD available for secondary processes
@ 2020-01-08  6:25 oda
  2020-01-08  6:25 ` [dpdk-dev] [PATCH 1/4] net/vhost: remove an unused member oda
                   ` (6 more replies)
  0 siblings, 7 replies; 27+ messages in thread
From: oda @ 2020-01-08  6:25 UTC (permalink / raw)
  To: dev

From: Itsuro Oda <oda@valinux.co.jp>

vhost PMD has not been available for secondary processes since
DPDK v18.11.  (https://bugs.dpdk.org/show_bug.cgi?id=194)
(for a long term !)
This series of patches intend to make vhost PMD available for
secondary processes.
Because now setting vhost driver to communicate with a vhost-user
master (ex. Qemu) is accomplished by the probe function of the
primary process, only the primary process can be a vhost-user
slave.
With this patch, setting vhost driver is delayed at eth_dev
configuration in order to be able to set it from a secondary
process. Because (in the first place,) setting vhost driver is not
necessary to be done at probe (it is enough to be done up to eth_dev
start), this fix is no problem for the primary process.
There is a precondition that the same process has to operate
a vhost interface from begining to end (eth_dev configuration to
eth_dev close). (This patch leaves it to user's responsibility.)
This precondition will not be a problem in most use cases
(including SPP).

Itsuro Oda (4):
  net/vhost: remove an unused member
  net/vhost: allocate iface_name from heap
  net/vhost: delay vhost driver setup
  net/vhost: make secondary probe complete

 drivers/net/vhost/rte_eth_vhost.c | 152 +++++++++++++++++-------------
 1 file changed, 88 insertions(+), 64 deletions(-)

-- 
2.17.0


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [dpdk-dev] [PATCH 1/4] net/vhost: remove an unused member
  2020-01-08  6:25 [dpdk-dev] [PATCH 0/4] make vhost PMD available for secondary processes oda
@ 2020-01-08  6:25 ` oda
  2020-01-08  6:25 ` [dpdk-dev] [PATCH 2/4] net/vhost: allocate iface_name from heap oda
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 27+ messages in thread
From: oda @ 2020-01-08  6:25 UTC (permalink / raw)
  To: dev

From: Itsuro Oda <oda@valinux.co.jp>

remove an unused member from pmd_internal.
---
 drivers/net/vhost/rte_eth_vhost.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
index 46f01a7f4..d4e3485ce 100644
--- a/drivers/net/vhost/rte_eth_vhost.c
+++ b/drivers/net/vhost/rte_eth_vhost.c
@@ -95,7 +95,6 @@ struct vhost_queue {
 
 struct pmd_internal {
 	rte_atomic32_t dev_attached;
-	char *dev_name;
 	char *iface_name;
 	uint16_t max_queues;
 	int vid;
@@ -1008,7 +1007,6 @@ eth_dev_close(struct rte_eth_dev *dev)
 		for (i = 0; i < dev->data->nb_tx_queues; i++)
 			rte_free(dev->data->tx_queues[i]);
 
-	free(internal->dev_name);
 	free(internal->iface_name);
 	rte_free(internal);
 
@@ -1253,9 +1251,6 @@ eth_dev_vhost_create(struct rte_vdev_device *dev, char *iface_name,
 	 * - and point eth_dev structure to new eth_dev_data structure
 	 */
 	internal = eth_dev->data->dev_private;
-	internal->dev_name = strdup(name);
-	if (internal->dev_name == NULL)
-		goto error;
 	internal->iface_name = strdup(iface_name);
 	if (internal->iface_name == NULL)
 		goto error;
@@ -1305,10 +1300,8 @@ eth_dev_vhost_create(struct rte_vdev_device *dev, char *iface_name,
 	return data->port_id;
 
 error:
-	if (internal) {
+	if (internal)
 		free(internal->iface_name);
-		free(internal->dev_name);
-	}
 	rte_free(vring_state);
 	rte_eth_dev_release_port(eth_dev);
 	rte_free(list);
-- 
2.17.0


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [dpdk-dev] [PATCH 2/4] net/vhost: allocate iface_name from heap
  2020-01-08  6:25 [dpdk-dev] [PATCH 0/4] make vhost PMD available for secondary processes oda
  2020-01-08  6:25 ` [dpdk-dev] [PATCH 1/4] net/vhost: remove an unused member oda
@ 2020-01-08  6:25 ` oda
  2020-01-08  6:25 ` [dpdk-dev] [PATCH 3/4] net/vhost: delay vhost driver setup oda
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 27+ messages in thread
From: oda @ 2020-01-08  6:25 UTC (permalink / raw)
  To: dev

From: Itsuro Oda <oda@valinux.co.jp>

allocate iface_name of pmd_internal from heap in order to
be able to refer from secondary processes.
---
 drivers/net/vhost/rte_eth_vhost.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
index d4e3485ce..1b07aad24 100644
--- a/drivers/net/vhost/rte_eth_vhost.c
+++ b/drivers/net/vhost/rte_eth_vhost.c
@@ -1007,7 +1007,7 @@ eth_dev_close(struct rte_eth_dev *dev)
 		for (i = 0; i < dev->data->nb_tx_queues; i++)
 			rte_free(dev->data->tx_queues[i]);
 
-	free(internal->iface_name);
+	rte_free(internal->iface_name);
 	rte_free(internal);
 
 	dev->data->dev_private = NULL;
@@ -1251,9 +1251,11 @@ eth_dev_vhost_create(struct rte_vdev_device *dev, char *iface_name,
 	 * - and point eth_dev structure to new eth_dev_data structure
 	 */
 	internal = eth_dev->data->dev_private;
-	internal->iface_name = strdup(iface_name);
+	internal->iface_name = rte_malloc_socket(name, strlen(iface_name) + 1,
+						 0, numa_node);
 	if (internal->iface_name == NULL)
 		goto error;
+	strcpy(internal->iface_name, iface_name);
 
 	list->eth_dev = eth_dev;
 	pthread_mutex_lock(&internal_list_lock);
@@ -1301,7 +1303,7 @@ eth_dev_vhost_create(struct rte_vdev_device *dev, char *iface_name,
 
 error:
 	if (internal)
-		free(internal->iface_name);
+		rte_free(internal->iface_name);
 	rte_free(vring_state);
 	rte_eth_dev_release_port(eth_dev);
 	rte_free(list);
-- 
2.17.0


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [dpdk-dev] [PATCH 3/4] net/vhost: delay vhost driver setup
  2020-01-08  6:25 [dpdk-dev] [PATCH 0/4] make vhost PMD available for secondary processes oda
  2020-01-08  6:25 ` [dpdk-dev] [PATCH 1/4] net/vhost: remove an unused member oda
  2020-01-08  6:25 ` [dpdk-dev] [PATCH 2/4] net/vhost: allocate iface_name from heap oda
@ 2020-01-08  6:25 ` oda
  2020-01-08  6:25 ` [dpdk-dev] [PATCH 4/4] net/vhost: make secondary probe complete oda
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 27+ messages in thread
From: oda @ 2020-01-08  6:25 UTC (permalink / raw)
  To: dev

From: Itsuro Oda <oda@valinux.co.jp>

setting vhost driver is delayed at eth_dev configuration
in order to be able to set it from a secondary process.
---
 drivers/net/vhost/rte_eth_vhost.c | 130 ++++++++++++++++++------------
 1 file changed, 78 insertions(+), 52 deletions(-)

diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
index 1b07aad24..0b8b5a4ca 100644
--- a/drivers/net/vhost/rte_eth_vhost.c
+++ b/drivers/net/vhost/rte_eth_vhost.c
@@ -96,6 +96,8 @@ struct vhost_queue {
 struct pmd_internal {
 	rte_atomic32_t dev_attached;
 	char *iface_name;
+	uint64_t flags;
+	uint64_t disable_flags;
 	uint16_t max_queues;
 	int vid;
 	rte_atomic32_t started;
@@ -490,17 +492,6 @@ eth_vhost_tx(void *q, struct rte_mbuf **bufs, uint16_t nb_bufs)
 	return nb_tx;
 }
 
-static int
-eth_dev_configure(struct rte_eth_dev *dev __rte_unused)
-{
-	struct pmd_internal *internal = dev->data->dev_private;
-	const struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
-
-	internal->vlan_strip = !!(rxmode->offloads & DEV_RX_OFFLOAD_VLAN_STRIP);
-
-	return 0;
-}
-
 static inline struct internal_list *
 find_internal_resource(char *ifname)
 {
@@ -876,6 +867,62 @@ static struct vhost_device_ops vhost_ops = {
 	.vring_state_changed = vring_state_changed,
 };
 
+static int
+vhost_driver_setup(struct rte_eth_dev *eth_dev)
+{
+	struct pmd_internal *internal = eth_dev->data->dev_private;
+	struct internal_list *list = NULL;
+	struct rte_vhost_vring_state *vring_state = NULL;
+	unsigned int numa_node = eth_dev->device->numa_node;
+	const char *name = eth_dev->device->name;
+
+	list = rte_zmalloc_socket(name, sizeof(*list), 0, numa_node);
+	if (list == NULL)
+		goto error;
+
+	vring_state = rte_zmalloc_socket(name, sizeof(*vring_state),
+					 0, numa_node);
+	if (vring_state == NULL)
+		goto error;
+
+	list->eth_dev = eth_dev;
+	pthread_mutex_lock(&internal_list_lock);
+	TAILQ_INSERT_TAIL(&internal_list, list, next);
+	pthread_mutex_unlock(&internal_list_lock);
+
+	rte_spinlock_init(&vring_state->lock);
+	vring_states[eth_dev->data->port_id] = vring_state;
+
+	if (rte_vhost_driver_register(internal->iface_name, internal->flags))
+		goto error;
+
+	if (internal->disable_flags) {
+		if (rte_vhost_driver_disable_features(internal->iface_name,
+						      internal->disable_flags))
+			goto error;
+	}
+
+	if (rte_vhost_driver_callback_register(internal->iface_name,
+					       &vhost_ops) < 0) {
+		VHOST_LOG(ERR, "Can't register callbacks\n");
+		goto error;
+	}
+
+	if (rte_vhost_driver_start(internal->iface_name) < 0) {
+		VHOST_LOG(ERR, "Failed to start driver for %s\n",
+			  internal->iface_name);
+		goto error;
+	}
+
+	return 0;
+
+error:
+	rte_free(vring_state);
+	rte_free(list);
+
+	return -1;
+}
+
 int
 rte_eth_vhost_get_queue_event(uint16_t port_id,
 		struct rte_eth_vhost_queue_event *event)
@@ -942,6 +989,24 @@ rte_eth_vhost_get_vid_from_port_id(uint16_t port_id)
 	return vid;
 }
 
+static int
+eth_dev_configure(struct rte_eth_dev *dev)
+{
+	struct pmd_internal *internal = dev->data->dev_private;
+	const struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
+
+	/* NOTE: the same process has to operate a vhost interface
+	 * from begining to end (eth_dev configure to eth_dev close).
+	 * It is user's responsibility at the moment.
+	 */
+	if (vhost_driver_setup(dev) < 0)
+		return -1;
+
+	internal->vlan_strip = !!(rxmode->offloads & DEV_RX_OFFLOAD_VLAN_STRIP);
+
+	return 0;
+}
+
 static int
 eth_dev_start(struct rte_eth_dev *eth_dev)
 {
@@ -1217,16 +1282,10 @@ eth_dev_vhost_create(struct rte_vdev_device *dev, char *iface_name,
 	struct pmd_internal *internal = NULL;
 	struct rte_eth_dev *eth_dev = NULL;
 	struct rte_ether_addr *eth_addr = NULL;
-	struct rte_vhost_vring_state *vring_state = NULL;
-	struct internal_list *list = NULL;
 
 	VHOST_LOG(INFO, "Creating VHOST-USER backend on numa socket %u\n",
 		numa_node);
 
-	list = rte_zmalloc_socket(name, sizeof(*list), 0, numa_node);
-	if (list == NULL)
-		goto error;
-
 	/* reserve an ethdev entry */
 	eth_dev = rte_eth_vdev_allocate(dev, sizeof(*internal));
 	if (eth_dev == NULL)
@@ -1240,11 +1299,6 @@ eth_dev_vhost_create(struct rte_vdev_device *dev, char *iface_name,
 	*eth_addr = base_eth_addr;
 	eth_addr->addr_bytes[5] = eth_dev->data->port_id;
 
-	vring_state = rte_zmalloc_socket(name,
-			sizeof(*vring_state), 0, numa_node);
-	if (vring_state == NULL)
-		goto error;
-
 	/* now put it all together
 	 * - store queue data in internal,
 	 * - point eth_dev_data to internals
@@ -1257,18 +1311,12 @@ eth_dev_vhost_create(struct rte_vdev_device *dev, char *iface_name,
 		goto error;
 	strcpy(internal->iface_name, iface_name);
 
-	list->eth_dev = eth_dev;
-	pthread_mutex_lock(&internal_list_lock);
-	TAILQ_INSERT_TAIL(&internal_list, list, next);
-	pthread_mutex_unlock(&internal_list_lock);
-
-	rte_spinlock_init(&vring_state->lock);
-	vring_states[eth_dev->data->port_id] = vring_state;
-
 	data->nb_rx_queues = queues;
 	data->nb_tx_queues = queues;
 	internal->max_queues = queues;
 	internal->vid = -1;
+	internal->flags = flags;
+	internal->disable_flags = disable_flags;
 	data->dev_link = pmd_link;
 	data->dev_flags = RTE_ETH_DEV_INTR_LSC | RTE_ETH_DEV_CLOSE_REMOVE;
 
@@ -1278,35 +1326,13 @@ eth_dev_vhost_create(struct rte_vdev_device *dev, char *iface_name,
 	eth_dev->rx_pkt_burst = eth_vhost_rx;
 	eth_dev->tx_pkt_burst = eth_vhost_tx;
 
-	if (rte_vhost_driver_register(iface_name, flags))
-		goto error;
-
-	if (disable_flags) {
-		if (rte_vhost_driver_disable_features(iface_name,
-					disable_flags))
-			goto error;
-	}
-
-	if (rte_vhost_driver_callback_register(iface_name, &vhost_ops) < 0) {
-		VHOST_LOG(ERR, "Can't register callbacks\n");
-		goto error;
-	}
-
-	if (rte_vhost_driver_start(iface_name) < 0) {
-		VHOST_LOG(ERR, "Failed to start driver for %s\n",
-			iface_name);
-		goto error;
-	}
-
 	rte_eth_dev_probing_finish(eth_dev);
 	return data->port_id;
 
 error:
 	if (internal)
 		rte_free(internal->iface_name);
-	rte_free(vring_state);
 	rte_eth_dev_release_port(eth_dev);
-	rte_free(list);
 
 	return -1;
 }
-- 
2.17.0


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [dpdk-dev] [PATCH 4/4] net/vhost: make secondary probe complete
  2020-01-08  6:25 [dpdk-dev] [PATCH 0/4] make vhost PMD available for secondary processes oda
                   ` (2 preceding siblings ...)
  2020-01-08  6:25 ` [dpdk-dev] [PATCH 3/4] net/vhost: delay vhost driver setup oda
@ 2020-01-08  6:25 ` oda
  2020-01-08  6:38 ` [dpdk-dev] [PATCH 0/4] make vhost PMD available for secondary processes Itsuro ODA
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 27+ messages in thread
From: oda @ 2020-01-08  6:25 UTC (permalink / raw)
  To: dev

From: Itsuro Oda <oda@valinux.co.jp>

add lacking member setting and make secondary probe complete.
---
 drivers/net/vhost/rte_eth_vhost.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
index 0b8b5a4ca..7a501cf91 100644
--- a/drivers/net/vhost/rte_eth_vhost.c
+++ b/drivers/net/vhost/rte_eth_vhost.c
@@ -1390,8 +1390,11 @@ rte_pmd_vhost_probe(struct rte_vdev_device *dev)
 			VHOST_LOG(ERR, "Failed to probe %s\n", name);
 			return -1;
 		}
-		/* TODO: request info from primary to set up Rx and Tx */
+		eth_dev->rx_pkt_burst = eth_vhost_rx;
+		eth_dev->tx_pkt_burst = eth_vhost_tx;
 		eth_dev->dev_ops = &ops;
+		if (dev->device.numa_node == SOCKET_ID_ANY)
+			dev->device.numa_node = rte_socket_id();
 		eth_dev->device = &dev->device;
 		rte_eth_dev_probing_finish(eth_dev);
 		return 0;
-- 
2.17.0


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [dpdk-dev] [PATCH 0/4] make vhost PMD available for secondary processes
  2020-01-08  6:25 [dpdk-dev] [PATCH 0/4] make vhost PMD available for secondary processes oda
                   ` (3 preceding siblings ...)
  2020-01-08  6:25 ` [dpdk-dev] [PATCH 4/4] net/vhost: make secondary probe complete oda
@ 2020-01-08  6:38 ` Itsuro ODA
  2020-01-08 23:22 ` Itsuro Oda
  2020-02-06  1:39 ` [dpdk-dev] [PATCH v3 " Itsuro Oda
  6 siblings, 0 replies; 27+ messages in thread
From: Itsuro ODA @ 2020-01-08  6:38 UTC (permalink / raw)
  To: dev

Hi,

I will fix to add signed-off-by and correct spelling error
according to test-report@dpdk.
Please check the content until then.

Thanks.

On Wed,  8 Jan 2020 15:25:06 +0900
oda@valinux.co.jp wrote:

> From: Itsuro Oda <oda@valinux.co.jp>
> 
> vhost PMD has not been available for secondary processes since
> DPDK v18.11.  (https://bugs.dpdk.org/show_bug.cgi?id=194)
> (for a long term !)
> This series of patches intend to make vhost PMD available for
> secondary processes.
> Because now setting vhost driver to communicate with a vhost-user
> master (ex. Qemu) is accomplished by the probe function of the
> primary process, only the primary process can be a vhost-user
> slave.
> With this patch, setting vhost driver is delayed at eth_dev
> configuration in order to be able to set it from a secondary
> process. Because (in the first place,) setting vhost driver is not
> necessary to be done at probe (it is enough to be done up to eth_dev
> start), this fix is no problem for the primary process.
> There is a precondition that the same process has to operate
> a vhost interface from begining to end (eth_dev configuration to
> eth_dev close). (This patch leaves it to user's responsibility.)
> This precondition will not be a problem in most use cases
> (including SPP).
> 
> Itsuro Oda (4):
>   net/vhost: remove an unused member
>   net/vhost: allocate iface_name from heap
>   net/vhost: delay vhost driver setup
>   net/vhost: make secondary probe complete
> 
>  drivers/net/vhost/rte_eth_vhost.c | 152 +++++++++++++++++-------------
>  1 file changed, 88 insertions(+), 64 deletions(-)
> 
> -- 
> 2.17.0

-- 
Itsuro ODA <oda@valinux.co.jp>


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [dpdk-dev] [PATCH 0/4] make vhost PMD available for secondary processes
  2020-01-08  6:25 [dpdk-dev] [PATCH 0/4] make vhost PMD available for secondary processes oda
                   ` (4 preceding siblings ...)
  2020-01-08  6:38 ` [dpdk-dev] [PATCH 0/4] make vhost PMD available for secondary processes Itsuro ODA
@ 2020-01-08 23:22 ` Itsuro Oda
  2020-01-08 23:22   ` [dpdk-dev] [PATCH 1/4] net/vhost: remove an unused member Itsuro Oda
                     ` (4 more replies)
  2020-02-06  1:39 ` [dpdk-dev] [PATCH v3 " Itsuro Oda
  6 siblings, 5 replies; 27+ messages in thread
From: Itsuro Oda @ 2020-01-08 23:22 UTC (permalink / raw)
  To: dev, maxime.coquelin, tiwei.bie, zhihong.wang

vhost PMD has not been available for secondary processes since
DPDK v18.11.  (https://bugs.dpdk.org/show_bug.cgi?id=194)
(for a long term !)
This series of patches intend to make vhost PMD available for
secondary processes.
Because now setting vhost driver to communicate with a vhost-user
master (ex. Qemu) is accomplished by the probe function of the
primary process, only the primary process can be a vhost-user
slave.
With this patch, setting vhost driver is delayed at eth_dev
configuration in order to be able to set it from a secondary
process. Because (in the first place,) setting vhost driver is not
necessary to be done at probe (it is enough to be done up to eth_dev
start), this fix is no problem for the primary process.
There is a precondition that the same process has to operate
a vhost interface from beginning to end (from eth_dev configuration
to eth_dev close). (This patch leaves it to user's responsibility.)
This precondition will not be a problem in most use cases
(including SPP).

v2:
- add signed-off-by
- fix spelling error

Itsuro Oda (4):
  net/vhost: remove an unused member
  net/vhost: allocate iface_name from heap
  net/vhost: delay vhost driver setup
  net/vhost: make secondary probe complete

 drivers/net/vhost/rte_eth_vhost.c | 152 +++++++++++++++++-------------
 1 file changed, 88 insertions(+), 64 deletions(-)

-- 
2.17.0


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [dpdk-dev] [PATCH 1/4] net/vhost: remove an unused member
  2020-01-08 23:22 ` Itsuro Oda
@ 2020-01-08 23:22   ` Itsuro Oda
  2020-02-04 17:56     ` Maxime Coquelin
  2020-01-08 23:22   ` [dpdk-dev] [PATCH 2/4] net/vhost: allocate iface_name from heap Itsuro Oda
                     ` (3 subsequent siblings)
  4 siblings, 1 reply; 27+ messages in thread
From: Itsuro Oda @ 2020-01-08 23:22 UTC (permalink / raw)
  To: dev, maxime.coquelin, tiwei.bie, zhihong.wang

remove an unused member from pmd_internal.

Signed-off-by: Itsuro Oda <oda@valinux.co.jp>
---
 drivers/net/vhost/rte_eth_vhost.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
index 46f01a7f4..d4e3485ce 100644
--- a/drivers/net/vhost/rte_eth_vhost.c
+++ b/drivers/net/vhost/rte_eth_vhost.c
@@ -95,7 +95,6 @@ struct vhost_queue {
 
 struct pmd_internal {
 	rte_atomic32_t dev_attached;
-	char *dev_name;
 	char *iface_name;
 	uint16_t max_queues;
 	int vid;
@@ -1008,7 +1007,6 @@ eth_dev_close(struct rte_eth_dev *dev)
 		for (i = 0; i < dev->data->nb_tx_queues; i++)
 			rte_free(dev->data->tx_queues[i]);
 
-	free(internal->dev_name);
 	free(internal->iface_name);
 	rte_free(internal);
 
@@ -1253,9 +1251,6 @@ eth_dev_vhost_create(struct rte_vdev_device *dev, char *iface_name,
 	 * - and point eth_dev structure to new eth_dev_data structure
 	 */
 	internal = eth_dev->data->dev_private;
-	internal->dev_name = strdup(name);
-	if (internal->dev_name == NULL)
-		goto error;
 	internal->iface_name = strdup(iface_name);
 	if (internal->iface_name == NULL)
 		goto error;
@@ -1305,10 +1300,8 @@ eth_dev_vhost_create(struct rte_vdev_device *dev, char *iface_name,
 	return data->port_id;
 
 error:
-	if (internal) {
+	if (internal)
 		free(internal->iface_name);
-		free(internal->dev_name);
-	}
 	rte_free(vring_state);
 	rte_eth_dev_release_port(eth_dev);
 	rte_free(list);
-- 
2.17.0


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [dpdk-dev] [PATCH 2/4] net/vhost: allocate iface_name from heap
  2020-01-08 23:22 ` Itsuro Oda
  2020-01-08 23:22   ` [dpdk-dev] [PATCH 1/4] net/vhost: remove an unused member Itsuro Oda
@ 2020-01-08 23:22   ` Itsuro Oda
  2020-02-04 17:59     ` Maxime Coquelin
  2020-01-08 23:22   ` [dpdk-dev] [PATCH 3/4] net/vhost: delay vhost driver setup Itsuro Oda
                     ` (2 subsequent siblings)
  4 siblings, 1 reply; 27+ messages in thread
From: Itsuro Oda @ 2020-01-08 23:22 UTC (permalink / raw)
  To: dev, maxime.coquelin, tiwei.bie, zhihong.wang

allocate iface_name of pmd_internal from heap in order to
be able to refer from secondary processes.

Signed-off-by: Itsuro Oda <oda@valinux.co.jp>
---
 drivers/net/vhost/rte_eth_vhost.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
index d4e3485ce..1b07aad24 100644
--- a/drivers/net/vhost/rte_eth_vhost.c
+++ b/drivers/net/vhost/rte_eth_vhost.c
@@ -1007,7 +1007,7 @@ eth_dev_close(struct rte_eth_dev *dev)
 		for (i = 0; i < dev->data->nb_tx_queues; i++)
 			rte_free(dev->data->tx_queues[i]);
 
-	free(internal->iface_name);
+	rte_free(internal->iface_name);
 	rte_free(internal);
 
 	dev->data->dev_private = NULL;
@@ -1251,9 +1251,11 @@ eth_dev_vhost_create(struct rte_vdev_device *dev, char *iface_name,
 	 * - and point eth_dev structure to new eth_dev_data structure
 	 */
 	internal = eth_dev->data->dev_private;
-	internal->iface_name = strdup(iface_name);
+	internal->iface_name = rte_malloc_socket(name, strlen(iface_name) + 1,
+						 0, numa_node);
 	if (internal->iface_name == NULL)
 		goto error;
+	strcpy(internal->iface_name, iface_name);
 
 	list->eth_dev = eth_dev;
 	pthread_mutex_lock(&internal_list_lock);
@@ -1301,7 +1303,7 @@ eth_dev_vhost_create(struct rte_vdev_device *dev, char *iface_name,
 
 error:
 	if (internal)
-		free(internal->iface_name);
+		rte_free(internal->iface_name);
 	rte_free(vring_state);
 	rte_eth_dev_release_port(eth_dev);
 	rte_free(list);
-- 
2.17.0


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [dpdk-dev] [PATCH 3/4] net/vhost: delay vhost driver setup
  2020-01-08 23:22 ` Itsuro Oda
  2020-01-08 23:22   ` [dpdk-dev] [PATCH 1/4] net/vhost: remove an unused member Itsuro Oda
  2020-01-08 23:22   ` [dpdk-dev] [PATCH 2/4] net/vhost: allocate iface_name from heap Itsuro Oda
@ 2020-01-08 23:22   ` Itsuro Oda
  2020-02-04 18:04     ` Maxime Coquelin
  2020-01-08 23:22   ` [dpdk-dev] [PATCH 4/4] net/vhost: make secondary probe complete Itsuro Oda
  2020-01-20  2:17   ` [dpdk-dev] [PATCH 0/4] make vhost PMD available for secondary processes Itsuro ODA
  4 siblings, 1 reply; 27+ messages in thread
From: Itsuro Oda @ 2020-01-08 23:22 UTC (permalink / raw)
  To: dev, maxime.coquelin, tiwei.bie, zhihong.wang

setting vhost driver is delayed at eth_dev configuration
in order to be able to set it from a secondary process.

Signed-off-by: Itsuro Oda <oda@valinux.co.jp>
---
 drivers/net/vhost/rte_eth_vhost.c | 130 ++++++++++++++++++------------
 1 file changed, 78 insertions(+), 52 deletions(-)

diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
index 1b07aad24..44f44cea3 100644
--- a/drivers/net/vhost/rte_eth_vhost.c
+++ b/drivers/net/vhost/rte_eth_vhost.c
@@ -96,6 +96,8 @@ struct vhost_queue {
 struct pmd_internal {
 	rte_atomic32_t dev_attached;
 	char *iface_name;
+	uint64_t flags;
+	uint64_t disable_flags;
 	uint16_t max_queues;
 	int vid;
 	rte_atomic32_t started;
@@ -490,17 +492,6 @@ eth_vhost_tx(void *q, struct rte_mbuf **bufs, uint16_t nb_bufs)
 	return nb_tx;
 }
 
-static int
-eth_dev_configure(struct rte_eth_dev *dev __rte_unused)
-{
-	struct pmd_internal *internal = dev->data->dev_private;
-	const struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
-
-	internal->vlan_strip = !!(rxmode->offloads & DEV_RX_OFFLOAD_VLAN_STRIP);
-
-	return 0;
-}
-
 static inline struct internal_list *
 find_internal_resource(char *ifname)
 {
@@ -876,6 +867,62 @@ static struct vhost_device_ops vhost_ops = {
 	.vring_state_changed = vring_state_changed,
 };
 
+static int
+vhost_driver_setup(struct rte_eth_dev *eth_dev)
+{
+	struct pmd_internal *internal = eth_dev->data->dev_private;
+	struct internal_list *list = NULL;
+	struct rte_vhost_vring_state *vring_state = NULL;
+	unsigned int numa_node = eth_dev->device->numa_node;
+	const char *name = eth_dev->device->name;
+
+	list = rte_zmalloc_socket(name, sizeof(*list), 0, numa_node);
+	if (list == NULL)
+		goto error;
+
+	vring_state = rte_zmalloc_socket(name, sizeof(*vring_state),
+					 0, numa_node);
+	if (vring_state == NULL)
+		goto error;
+
+	list->eth_dev = eth_dev;
+	pthread_mutex_lock(&internal_list_lock);
+	TAILQ_INSERT_TAIL(&internal_list, list, next);
+	pthread_mutex_unlock(&internal_list_lock);
+
+	rte_spinlock_init(&vring_state->lock);
+	vring_states[eth_dev->data->port_id] = vring_state;
+
+	if (rte_vhost_driver_register(internal->iface_name, internal->flags))
+		goto error;
+
+	if (internal->disable_flags) {
+		if (rte_vhost_driver_disable_features(internal->iface_name,
+						      internal->disable_flags))
+			goto error;
+	}
+
+	if (rte_vhost_driver_callback_register(internal->iface_name,
+					       &vhost_ops) < 0) {
+		VHOST_LOG(ERR, "Can't register callbacks\n");
+		goto error;
+	}
+
+	if (rte_vhost_driver_start(internal->iface_name) < 0) {
+		VHOST_LOG(ERR, "Failed to start driver for %s\n",
+			  internal->iface_name);
+		goto error;
+	}
+
+	return 0;
+
+error:
+	rte_free(vring_state);
+	rte_free(list);
+
+	return -1;
+}
+
 int
 rte_eth_vhost_get_queue_event(uint16_t port_id,
 		struct rte_eth_vhost_queue_event *event)
@@ -942,6 +989,24 @@ rte_eth_vhost_get_vid_from_port_id(uint16_t port_id)
 	return vid;
 }
 
+static int
+eth_dev_configure(struct rte_eth_dev *dev)
+{
+	struct pmd_internal *internal = dev->data->dev_private;
+	const struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
+
+	/* NOTE: the same process has to operate a vhost interface
+	 * from beginning to end (from eth_dev configure to eth_dev close).
+	 * It is user's responsibility at the moment.
+	 */
+	if (vhost_driver_setup(dev) < 0)
+		return -1;
+
+	internal->vlan_strip = !!(rxmode->offloads & DEV_RX_OFFLOAD_VLAN_STRIP);
+
+	return 0;
+}
+
 static int
 eth_dev_start(struct rte_eth_dev *eth_dev)
 {
@@ -1217,16 +1282,10 @@ eth_dev_vhost_create(struct rte_vdev_device *dev, char *iface_name,
 	struct pmd_internal *internal = NULL;
 	struct rte_eth_dev *eth_dev = NULL;
 	struct rte_ether_addr *eth_addr = NULL;
-	struct rte_vhost_vring_state *vring_state = NULL;
-	struct internal_list *list = NULL;
 
 	VHOST_LOG(INFO, "Creating VHOST-USER backend on numa socket %u\n",
 		numa_node);
 
-	list = rte_zmalloc_socket(name, sizeof(*list), 0, numa_node);
-	if (list == NULL)
-		goto error;
-
 	/* reserve an ethdev entry */
 	eth_dev = rte_eth_vdev_allocate(dev, sizeof(*internal));
 	if (eth_dev == NULL)
@@ -1240,11 +1299,6 @@ eth_dev_vhost_create(struct rte_vdev_device *dev, char *iface_name,
 	*eth_addr = base_eth_addr;
 	eth_addr->addr_bytes[5] = eth_dev->data->port_id;
 
-	vring_state = rte_zmalloc_socket(name,
-			sizeof(*vring_state), 0, numa_node);
-	if (vring_state == NULL)
-		goto error;
-
 	/* now put it all together
 	 * - store queue data in internal,
 	 * - point eth_dev_data to internals
@@ -1257,18 +1311,12 @@ eth_dev_vhost_create(struct rte_vdev_device *dev, char *iface_name,
 		goto error;
 	strcpy(internal->iface_name, iface_name);
 
-	list->eth_dev = eth_dev;
-	pthread_mutex_lock(&internal_list_lock);
-	TAILQ_INSERT_TAIL(&internal_list, list, next);
-	pthread_mutex_unlock(&internal_list_lock);
-
-	rte_spinlock_init(&vring_state->lock);
-	vring_states[eth_dev->data->port_id] = vring_state;
-
 	data->nb_rx_queues = queues;
 	data->nb_tx_queues = queues;
 	internal->max_queues = queues;
 	internal->vid = -1;
+	internal->flags = flags;
+	internal->disable_flags = disable_flags;
 	data->dev_link = pmd_link;
 	data->dev_flags = RTE_ETH_DEV_INTR_LSC | RTE_ETH_DEV_CLOSE_REMOVE;
 
@@ -1278,35 +1326,13 @@ eth_dev_vhost_create(struct rte_vdev_device *dev, char *iface_name,
 	eth_dev->rx_pkt_burst = eth_vhost_rx;
 	eth_dev->tx_pkt_burst = eth_vhost_tx;
 
-	if (rte_vhost_driver_register(iface_name, flags))
-		goto error;
-
-	if (disable_flags) {
-		if (rte_vhost_driver_disable_features(iface_name,
-					disable_flags))
-			goto error;
-	}
-
-	if (rte_vhost_driver_callback_register(iface_name, &vhost_ops) < 0) {
-		VHOST_LOG(ERR, "Can't register callbacks\n");
-		goto error;
-	}
-
-	if (rte_vhost_driver_start(iface_name) < 0) {
-		VHOST_LOG(ERR, "Failed to start driver for %s\n",
-			iface_name);
-		goto error;
-	}
-
 	rte_eth_dev_probing_finish(eth_dev);
 	return data->port_id;
 
 error:
 	if (internal)
 		rte_free(internal->iface_name);
-	rte_free(vring_state);
 	rte_eth_dev_release_port(eth_dev);
-	rte_free(list);
 
 	return -1;
 }
-- 
2.17.0


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [dpdk-dev] [PATCH 4/4] net/vhost: make secondary probe complete
  2020-01-08 23:22 ` Itsuro Oda
                     ` (2 preceding siblings ...)
  2020-01-08 23:22   ` [dpdk-dev] [PATCH 3/4] net/vhost: delay vhost driver setup Itsuro Oda
@ 2020-01-08 23:22   ` Itsuro Oda
  2020-02-04 18:08     ` Maxime Coquelin
  2020-01-20  2:17   ` [dpdk-dev] [PATCH 0/4] make vhost PMD available for secondary processes Itsuro ODA
  4 siblings, 1 reply; 27+ messages in thread
From: Itsuro Oda @ 2020-01-08 23:22 UTC (permalink / raw)
  To: dev, maxime.coquelin, tiwei.bie, zhihong.wang

add lacking member setting and make secondary probe complete.

Signed-off-by: Itsuro Oda <oda@valinux.co.jp>
---
 drivers/net/vhost/rte_eth_vhost.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
index 44f44cea3..485a88794 100644
--- a/drivers/net/vhost/rte_eth_vhost.c
+++ b/drivers/net/vhost/rte_eth_vhost.c
@@ -1390,8 +1390,11 @@ rte_pmd_vhost_probe(struct rte_vdev_device *dev)
 			VHOST_LOG(ERR, "Failed to probe %s\n", name);
 			return -1;
 		}
-		/* TODO: request info from primary to set up Rx and Tx */
+		eth_dev->rx_pkt_burst = eth_vhost_rx;
+		eth_dev->tx_pkt_burst = eth_vhost_tx;
 		eth_dev->dev_ops = &ops;
+		if (dev->device.numa_node == SOCKET_ID_ANY)
+			dev->device.numa_node = rte_socket_id();
 		eth_dev->device = &dev->device;
 		rte_eth_dev_probing_finish(eth_dev);
 		return 0;
-- 
2.17.0


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [dpdk-dev] [PATCH 0/4] make vhost PMD available for secondary processes
  2020-01-08 23:22 ` Itsuro Oda
                     ` (3 preceding siblings ...)
  2020-01-08 23:22   ` [dpdk-dev] [PATCH 4/4] net/vhost: make secondary probe complete Itsuro Oda
@ 2020-01-20  2:17   ` Itsuro ODA
  2020-02-04 17:54     ` Maxime Coquelin
  4 siblings, 1 reply; 27+ messages in thread
From: Itsuro ODA @ 2020-01-20  2:17 UTC (permalink / raw)
  To: maxime.coquelin, tiwei.bie, zhihong.wang, anatoly.burakov; +Cc: dev, thomas

Hi vhost PMD maitainers,

I have not got any feedback yet. 
Since this is the first time I submit a patch, something
may be wrong, would you tell me what should I do ?

Thanks.

On Thu,  9 Jan 2020 08:22:05 +0900
Itsuro Oda <oda@valinux.co.jp> wrote:

> vhost PMD has not been available for secondary processes since
> DPDK v18.11.  (https://bugs.dpdk.org/show_bug.cgi?id=194)
> (for a long term !)
> This series of patches intend to make vhost PMD available for
> secondary processes.
> Because now setting vhost driver to communicate with a vhost-user
> master (ex. Qemu) is accomplished by the probe function of the
> primary process, only the primary process can be a vhost-user
> slave.
> With this patch, setting vhost driver is delayed at eth_dev
> configuration in order to be able to set it from a secondary
> process. Because (in the first place,) setting vhost driver is not
> necessary to be done at probe (it is enough to be done up to eth_dev
> start), this fix is no problem for the primary process.
> There is a precondition that the same process has to operate
> a vhost interface from beginning to end (from eth_dev configuration
> to eth_dev close). (This patch leaves it to user's responsibility.)
> This precondition will not be a problem in most use cases
> (including SPP).
> 
> v2:
> - add signed-off-by
> - fix spelling error
> 
> Itsuro Oda (4):
>   net/vhost: remove an unused member
>   net/vhost: allocate iface_name from heap
>   net/vhost: delay vhost driver setup
>   net/vhost: make secondary probe complete
> 
>  drivers/net/vhost/rte_eth_vhost.c | 152 +++++++++++++++++-------------
>  1 file changed, 88 insertions(+), 64 deletions(-)
> 
> -- 
> 2.17.0

-- 
Itsuro ODA <oda@valinux.co.jp>


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [dpdk-dev] [PATCH 0/4] make vhost PMD available for secondary processes
  2020-01-20  2:17   ` [dpdk-dev] [PATCH 0/4] make vhost PMD available for secondary processes Itsuro ODA
@ 2020-02-04 17:54     ` Maxime Coquelin
  2020-02-04 22:19       ` Itsuro ODA
  0 siblings, 1 reply; 27+ messages in thread
From: Maxime Coquelin @ 2020-02-04 17:54 UTC (permalink / raw)
  To: Itsuro ODA, tiwei.bie, zhihong.wang, anatoly.burakov; +Cc: dev, thomas

Hi Itsuro,

On 1/20/20 3:17 AM, Itsuro ODA wrote:
> Hi vhost PMD maitainers,
> 
> I have not got any feedback yet. 
> Since this is the first time I submit a patch, something
> may be wrong, would you tell me what should I do ?

Sorry for the delay, and thanks for the contribution.

You series does not apply properly on dpdk-next-virtio master branch:
https://git.dpdk.org/next/dpdk-next-virtio

I will review it, so when doing v3, please rebase it.

More generally, you series comprises fixes (patch 2 to 4), and
cleanup (patch 1).

Cleanup patch should be the last, in order to ease the backporting
of the fixes to LTSes (we avoid backporting cleanup patches).

Regarding fixes patches, it should tag the faulty commit in
master branch, and stable@dpdk.org should be Cc'ed.

Example of commit message with fixes tag:
http://patches.dpdk.org/patch/63305/

Finally, when you'll post the v3, please prefix the patches subject
with the revision number:

git format-patch --subject-prefix"PATCH v3" ...

Thanks,
Maxime

> Thanks.
> 
> On Thu,  9 Jan 2020 08:22:05 +0900
> Itsuro Oda <oda@valinux.co.jp> wrote:
> 
>> vhost PMD has not been available for secondary processes since
>> DPDK v18.11.  (https://bugs.dpdk.org/show_bug.cgi?id=194)
>> (for a long term !)
>> This series of patches intend to make vhost PMD available for
>> secondary processes.
>> Because now setting vhost driver to communicate with a vhost-user
>> master (ex. Qemu) is accomplished by the probe function of the
>> primary process, only the primary process can be a vhost-user
>> slave.
>> With this patch, setting vhost driver is delayed at eth_dev
>> configuration in order to be able to set it from a secondary
>> process. Because (in the first place,) setting vhost driver is not
>> necessary to be done at probe (it is enough to be done up to eth_dev
>> start), this fix is no problem for the primary process.
>> There is a precondition that the same process has to operate
>> a vhost interface from beginning to end (from eth_dev configuration
>> to eth_dev close). (This patch leaves it to user's responsibility.)
>> This precondition will not be a problem in most use cases
>> (including SPP).
>>
>> v2:
>> - add signed-off-by
>> - fix spelling error
>>
>> Itsuro Oda (4):
>>   net/vhost: remove an unused member
>>   net/vhost: allocate iface_name from heap
>>   net/vhost: delay vhost driver setup
>>   net/vhost: make secondary probe complete
>>
>>  drivers/net/vhost/rte_eth_vhost.c | 152 +++++++++++++++++-------------
>>  1 file changed, 88 insertions(+), 64 deletions(-)
>>
>> -- 
>> 2.17.0
> 


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [dpdk-dev] [PATCH 1/4] net/vhost: remove an unused member
  2020-01-08 23:22   ` [dpdk-dev] [PATCH 1/4] net/vhost: remove an unused member Itsuro Oda
@ 2020-02-04 17:56     ` Maxime Coquelin
  0 siblings, 0 replies; 27+ messages in thread
From: Maxime Coquelin @ 2020-02-04 17:56 UTC (permalink / raw)
  To: Itsuro Oda, dev, tiwei.bie, zhihong.wang



On 1/9/20 12:22 AM, Itsuro Oda wrote:
> remove an unused member from pmd_internal.

Please try to do a complete sentence:

This patch removes an unused...

> Signed-off-by: Itsuro Oda <oda@valinux.co.jp>
> ---
>  drivers/net/vhost/rte_eth_vhost.c | 9 +--------
>  1 file changed, 1 insertion(+), 8 deletions(-)
> 
> diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
> index 46f01a7f4..d4e3485ce 100644
> --- a/drivers/net/vhost/rte_eth_vhost.c
> +++ b/drivers/net/vhost/rte_eth_vhost.c
> @@ -95,7 +95,6 @@ struct vhost_queue {
>  
>  struct pmd_internal {
>  	rte_atomic32_t dev_attached;
> -	char *dev_name;
>  	char *iface_name;
>  	uint16_t max_queues;
>  	int vid;
> @@ -1008,7 +1007,6 @@ eth_dev_close(struct rte_eth_dev *dev)
>  		for (i = 0; i < dev->data->nb_tx_queues; i++)
>  			rte_free(dev->data->tx_queues[i]);
>  
> -	free(internal->dev_name);
>  	free(internal->iface_name);
>  	rte_free(internal);
>  
> @@ -1253,9 +1251,6 @@ eth_dev_vhost_create(struct rte_vdev_device *dev, char *iface_name,
>  	 * - and point eth_dev structure to new eth_dev_data structure
>  	 */
>  	internal = eth_dev->data->dev_private;
> -	internal->dev_name = strdup(name);
> -	if (internal->dev_name == NULL)
> -		goto error;
>  	internal->iface_name = strdup(iface_name);
>  	if (internal->iface_name == NULL)
>  		goto error;
> @@ -1305,10 +1300,8 @@ eth_dev_vhost_create(struct rte_vdev_device *dev, char *iface_name,
>  	return data->port_id;
>  
>  error:
> -	if (internal) {
> +	if (internal)
>  		free(internal->iface_name);
> -		free(internal->dev_name);
> -	}
>  	rte_free(vring_state);
>  	rte_eth_dev_release_port(eth_dev);
>  	rte_free(list);
> 

Other than that, it looks good to me.
With the commit message fixed, and this patch moved to the end of the
series:

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [dpdk-dev] [PATCH 2/4] net/vhost: allocate iface_name from heap
  2020-01-08 23:22   ` [dpdk-dev] [PATCH 2/4] net/vhost: allocate iface_name from heap Itsuro Oda
@ 2020-02-04 17:59     ` Maxime Coquelin
  0 siblings, 0 replies; 27+ messages in thread
From: Maxime Coquelin @ 2020-02-04 17:59 UTC (permalink / raw)
  To: Itsuro Oda, dev, tiwei.bie, zhihong.wang

No variable or function names in the title, as per the contribution
guidelines:

net/vhost: allocate interface name from heap

On 1/9/20 12:22 AM, Itsuro Oda wrote:
> allocate iface_name of pmd_internal from heap in order to
> be able to refer from secondary processes.

As for patch, better to have a full sentence.

And remember to add Fixes tag and cc stable.

> Signed-off-by: Itsuro Oda <oda@valinux.co.jp>
> ---
>  drivers/net/vhost/rte_eth_vhost.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
> index d4e3485ce..1b07aad24 100644
> --- a/drivers/net/vhost/rte_eth_vhost.c
> +++ b/drivers/net/vhost/rte_eth_vhost.c
> @@ -1007,7 +1007,7 @@ eth_dev_close(struct rte_eth_dev *dev)
>  		for (i = 0; i < dev->data->nb_tx_queues; i++)
>  			rte_free(dev->data->tx_queues[i]);
>  
> -	free(internal->iface_name);
> +	rte_free(internal->iface_name);
>  	rte_free(internal);
>  
>  	dev->data->dev_private = NULL;
> @@ -1251,9 +1251,11 @@ eth_dev_vhost_create(struct rte_vdev_device *dev, char *iface_name,
>  	 * - and point eth_dev structure to new eth_dev_data structure
>  	 */
>  	internal = eth_dev->data->dev_private;
> -	internal->iface_name = strdup(iface_name);
> +	internal->iface_name = rte_malloc_socket(name, strlen(iface_name) + 1,
> +						 0, numa_node);
>  	if (internal->iface_name == NULL)
>  		goto error;
> +	strcpy(internal->iface_name, iface_name);
>  
>  	list->eth_dev = eth_dev;
>  	pthread_mutex_lock(&internal_list_lock);
> @@ -1301,7 +1303,7 @@ eth_dev_vhost_create(struct rte_vdev_device *dev, char *iface_name,
>  
>  error:
>  	if (internal)
> -		free(internal->iface_name);
> +		rte_free(internal->iface_name);
>  	rte_free(vring_state);
>  	rte_eth_dev_release_port(eth_dev);
>  	rte_free(list);
> 


When above comments taken into account:
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [dpdk-dev] [PATCH 3/4] net/vhost: delay vhost driver setup
  2020-01-08 23:22   ` [dpdk-dev] [PATCH 3/4] net/vhost: delay vhost driver setup Itsuro Oda
@ 2020-02-04 18:04     ` Maxime Coquelin
  0 siblings, 0 replies; 27+ messages in thread
From: Maxime Coquelin @ 2020-02-04 18:04 UTC (permalink / raw)
  To: Itsuro Oda, dev, tiwei.bie, zhihong.wang



On 1/9/20 12:22 AM, Itsuro Oda wrote:
> setting vhost driver is delayed at eth_dev configuration
Sentences starts with an upper-case.

Vhost driver setup is delayed...

> in order to be able to set it from a secondary process.


Add Fixes tag and cc stable.

> Signed-off-by: Itsuro Oda <oda@valinux.co.jp>
> ---
>  drivers/net/vhost/rte_eth_vhost.c | 130 ++++++++++++++++++------------
>  1 file changed, 78 insertions(+), 52 deletions(-)
> 
> diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
> index 1b07aad24..44f44cea3 100644
> --- a/drivers/net/vhost/rte_eth_vhost.c
> +++ b/drivers/net/vhost/rte_eth_vhost.c
> @@ -96,6 +96,8 @@ struct vhost_queue {
>  struct pmd_internal {
>  	rte_atomic32_t dev_attached;
>  	char *iface_name;
> +	uint64_t flags;
> +	uint64_t disable_flags;
>  	uint16_t max_queues;
>  	int vid;
>  	rte_atomic32_t started;
> @@ -490,17 +492,6 @@ eth_vhost_tx(void *q, struct rte_mbuf **bufs, uint16_t nb_bufs)
>  	return nb_tx;
>  }
>  
> -static int
> -eth_dev_configure(struct rte_eth_dev *dev __rte_unused)
> -{
> -	struct pmd_internal *internal = dev->data->dev_private;
> -	const struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
> -
> -	internal->vlan_strip = !!(rxmode->offloads & DEV_RX_OFFLOAD_VLAN_STRIP);
> -
> -	return 0;
> -}
> -
>  static inline struct internal_list *
>  find_internal_resource(char *ifname)
>  {
> @@ -876,6 +867,62 @@ static struct vhost_device_ops vhost_ops = {
>  	.vring_state_changed = vring_state_changed,
>  };
>  
> +static int
> +vhost_driver_setup(struct rte_eth_dev *eth_dev)
> +{
> +	struct pmd_internal *internal = eth_dev->data->dev_private;
> +	struct internal_list *list = NULL;
> +	struct rte_vhost_vring_state *vring_state = NULL;
> +	unsigned int numa_node = eth_dev->device->numa_node;
> +	const char *name = eth_dev->device->name;
> +
> +	list = rte_zmalloc_socket(name, sizeof(*list), 0, numa_node);
> +	if (list == NULL)
> +		goto error;
> +
> +	vring_state = rte_zmalloc_socket(name, sizeof(*vring_state),
> +					 0, numa_node);
> +	if (vring_state == NULL)
> +		goto error;
> +
> +	list->eth_dev = eth_dev;
> +	pthread_mutex_lock(&internal_list_lock);
> +	TAILQ_INSERT_TAIL(&internal_list, list, next);
> +	pthread_mutex_unlock(&internal_list_lock);
> +
> +	rte_spinlock_init(&vring_state->lock);
> +	vring_states[eth_dev->data->port_id] = vring_state;
> +
> +	if (rte_vhost_driver_register(internal->iface_name, internal->flags))
> +		goto error;
> +
> +	if (internal->disable_flags) {
> +		if (rte_vhost_driver_disable_features(internal->iface_name,
> +						      internal->disable_flags))
> +			goto error;
> +	}
> +
> +	if (rte_vhost_driver_callback_register(internal->iface_name,
> +					       &vhost_ops) < 0) {
> +		VHOST_LOG(ERR, "Can't register callbacks\n");
> +		goto error;
> +	}
> +
> +	if (rte_vhost_driver_start(internal->iface_name) < 0) {
> +		VHOST_LOG(ERR, "Failed to start driver for %s\n",
> +			  internal->iface_name);
> +		goto error;
> +	}
> +
> +	return 0;
> +
> +error:
> +	rte_free(vring_state);
> +	rte_free(list);
> +
> +	return -1;
> +}
> +
>  int
>  rte_eth_vhost_get_queue_event(uint16_t port_id,
>  		struct rte_eth_vhost_queue_event *event)
> @@ -942,6 +989,24 @@ rte_eth_vhost_get_vid_from_port_id(uint16_t port_id)
>  	return vid;
>  }
>  
> +static int
> +eth_dev_configure(struct rte_eth_dev *dev)
> +{
> +	struct pmd_internal *internal = dev->data->dev_private;
> +	const struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
> +
> +	/* NOTE: the same process has to operate a vhost interface
> +	 * from beginning to end (from eth_dev configure to eth_dev close).
> +	 * It is user's responsibility at the moment.
> +	 */
> +	if (vhost_driver_setup(dev) < 0)
> +		return -1;
> +
> +	internal->vlan_strip = !!(rxmode->offloads & DEV_RX_OFFLOAD_VLAN_STRIP);
> +
> +	return 0;
> +}
> +
>  static int
>  eth_dev_start(struct rte_eth_dev *eth_dev)
>  {
> @@ -1217,16 +1282,10 @@ eth_dev_vhost_create(struct rte_vdev_device *dev, char *iface_name,
>  	struct pmd_internal *internal = NULL;
>  	struct rte_eth_dev *eth_dev = NULL;
>  	struct rte_ether_addr *eth_addr = NULL;
> -	struct rte_vhost_vring_state *vring_state = NULL;
> -	struct internal_list *list = NULL;
>  
>  	VHOST_LOG(INFO, "Creating VHOST-USER backend on numa socket %u\n",
>  		numa_node);
>  
> -	list = rte_zmalloc_socket(name, sizeof(*list), 0, numa_node);
> -	if (list == NULL)
> -		goto error;
> -
>  	/* reserve an ethdev entry */
>  	eth_dev = rte_eth_vdev_allocate(dev, sizeof(*internal));
>  	if (eth_dev == NULL)
> @@ -1240,11 +1299,6 @@ eth_dev_vhost_create(struct rte_vdev_device *dev, char *iface_name,
>  	*eth_addr = base_eth_addr;
>  	eth_addr->addr_bytes[5] = eth_dev->data->port_id;
>  
> -	vring_state = rte_zmalloc_socket(name,
> -			sizeof(*vring_state), 0, numa_node);
> -	if (vring_state == NULL)
> -		goto error;
> -
>  	/* now put it all together
>  	 * - store queue data in internal,
>  	 * - point eth_dev_data to internals
> @@ -1257,18 +1311,12 @@ eth_dev_vhost_create(struct rte_vdev_device *dev, char *iface_name,
>  		goto error;
>  	strcpy(internal->iface_name, iface_name);
>  
> -	list->eth_dev = eth_dev;
> -	pthread_mutex_lock(&internal_list_lock);
> -	TAILQ_INSERT_TAIL(&internal_list, list, next);
> -	pthread_mutex_unlock(&internal_list_lock);
> -
> -	rte_spinlock_init(&vring_state->lock);
> -	vring_states[eth_dev->data->port_id] = vring_state;
> -
>  	data->nb_rx_queues = queues;
>  	data->nb_tx_queues = queues;
>  	internal->max_queues = queues;
>  	internal->vid = -1;
> +	internal->flags = flags;
> +	internal->disable_flags = disable_flags;
>  	data->dev_link = pmd_link;
>  	data->dev_flags = RTE_ETH_DEV_INTR_LSC | RTE_ETH_DEV_CLOSE_REMOVE;
>  
> @@ -1278,35 +1326,13 @@ eth_dev_vhost_create(struct rte_vdev_device *dev, char *iface_name,
>  	eth_dev->rx_pkt_burst = eth_vhost_rx;
>  	eth_dev->tx_pkt_burst = eth_vhost_tx;
>  
> -	if (rte_vhost_driver_register(iface_name, flags))
> -		goto error;
> -
> -	if (disable_flags) {
> -		if (rte_vhost_driver_disable_features(iface_name,
> -					disable_flags))
> -			goto error;
> -	}
> -
> -	if (rte_vhost_driver_callback_register(iface_name, &vhost_ops) < 0) {
> -		VHOST_LOG(ERR, "Can't register callbacks\n");
> -		goto error;
> -	}
> -
> -	if (rte_vhost_driver_start(iface_name) < 0) {
> -		VHOST_LOG(ERR, "Failed to start driver for %s\n",
> -			iface_name);
> -		goto error;
> -	}
> -
>  	rte_eth_dev_probing_finish(eth_dev);
>  	return data->port_id;
>  
>  error:
>  	if (internal)
>  		rte_free(internal->iface_name);
> -	rte_free(vring_state);
>  	rte_eth_dev_release_port(eth_dev);
> -	rte_free(list);
>  
>  	return -1;
>  }
> 

With above comments fixed:
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [dpdk-dev] [PATCH 4/4] net/vhost: make secondary probe complete
  2020-01-08 23:22   ` [dpdk-dev] [PATCH 4/4] net/vhost: make secondary probe complete Itsuro Oda
@ 2020-02-04 18:08     ` Maxime Coquelin
  0 siblings, 0 replies; 27+ messages in thread
From: Maxime Coquelin @ 2020-02-04 18:08 UTC (permalink / raw)
  To: Itsuro Oda, dev, tiwei.bie, zhihong.wang



On 1/9/20 12:22 AM, Itsuro Oda wrote:
> add lacking member setting and make secondary probe complete.

Add

> Signed-off-by: Itsuro Oda <oda@valinux.co.jp>
> ---
>  drivers/net/vhost/rte_eth_vhost.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
> index 44f44cea3..485a88794 100644
> --- a/drivers/net/vhost/rte_eth_vhost.c
> +++ b/drivers/net/vhost/rte_eth_vhost.c
> @@ -1390,8 +1390,11 @@ rte_pmd_vhost_probe(struct rte_vdev_device *dev)
>  			VHOST_LOG(ERR, "Failed to probe %s\n", name);
>  			return -1;
>  		}
> -		/* TODO: request info from primary to set up Rx and Tx */
> +		eth_dev->rx_pkt_burst = eth_vhost_rx;
> +		eth_dev->tx_pkt_burst = eth_vhost_tx;
>  		eth_dev->dev_ops = &ops;
> +		if (dev->device.numa_node == SOCKET_ID_ANY)
> +			dev->device.numa_node = rte_socket_id();
>  		eth_dev->device = &dev->device;
>  		rte_eth_dev_probing_finish(eth_dev);
>  		return 0;
> 

Other than that
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [dpdk-dev] [PATCH 0/4] make vhost PMD available for secondary processes
  2020-02-04 17:54     ` Maxime Coquelin
@ 2020-02-04 22:19       ` Itsuro ODA
  0 siblings, 0 replies; 27+ messages in thread
From: Itsuro ODA @ 2020-02-04 22:19 UTC (permalink / raw)
  To: Maxime Coquelin; +Cc: tiwei.bie, zhihong.wang, anatoly.burakov, dev, thomas

Hi Maxime,

Thank you for your reply and review.
I will make fixes of the patches according to your
indication and post them.

Thanks.
Itsuro Oda

On Tue, 4 Feb 2020 18:54:23 +0100
Maxime Coquelin <maxime.coquelin@redhat.com> wrote:

> Hi Itsuro,
> 
> On 1/20/20 3:17 AM, Itsuro ODA wrote:
> > Hi vhost PMD maitainers,
> > 
> > I have not got any feedback yet. 
> > Since this is the first time I submit a patch, something
> > may be wrong, would you tell me what should I do ?
> 
> Sorry for the delay, and thanks for the contribution.
> 
> You series does not apply properly on dpdk-next-virtio master branch:
> https://git.dpdk.org/next/dpdk-next-virtio
> 
> I will review it, so when doing v3, please rebase it.
> 
> More generally, you series comprises fixes (patch 2 to 4), and
> cleanup (patch 1).
> 
> Cleanup patch should be the last, in order to ease the backporting
> of the fixes to LTSes (we avoid backporting cleanup patches).
> 
> Regarding fixes patches, it should tag the faulty commit in
> master branch, and stable@dpdk.org should be Cc'ed.
> 
> Example of commit message with fixes tag:
> http://patches.dpdk.org/patch/63305/
> 
> Finally, when you'll post the v3, please prefix the patches subject
> with the revision number:
> 
> git format-patch --subject-prefix"PATCH v3" ...
> 
> Thanks,
> Maxime
> 
> > Thanks.
> > 
> > On Thu,  9 Jan 2020 08:22:05 +0900
> > Itsuro Oda <oda@valinux.co.jp> wrote:
> > 
> >> vhost PMD has not been available for secondary processes since
> >> DPDK v18.11.  (https://bugs.dpdk.org/show_bug.cgi?id=194)
> >> (for a long term !)
> >> This series of patches intend to make vhost PMD available for
> >> secondary processes.
> >> Because now setting vhost driver to communicate with a vhost-user
> >> master (ex. Qemu) is accomplished by the probe function of the
> >> primary process, only the primary process can be a vhost-user
> >> slave.
> >> With this patch, setting vhost driver is delayed at eth_dev
> >> configuration in order to be able to set it from a secondary
> >> process. Because (in the first place,) setting vhost driver is not
> >> necessary to be done at probe (it is enough to be done up to eth_dev
> >> start), this fix is no problem for the primary process.
> >> There is a precondition that the same process has to operate
> >> a vhost interface from beginning to end (from eth_dev configuration
> >> to eth_dev close). (This patch leaves it to user's responsibility.)
> >> This precondition will not be a problem in most use cases
> >> (including SPP).
> >>
> >> v2:
> >> - add signed-off-by
> >> - fix spelling error
> >>
> >> Itsuro Oda (4):
> >>   net/vhost: remove an unused member
> >>   net/vhost: allocate iface_name from heap
> >>   net/vhost: delay vhost driver setup
> >>   net/vhost: make secondary probe complete
> >>
> >>  drivers/net/vhost/rte_eth_vhost.c | 152 +++++++++++++++++-------------
> >>  1 file changed, 88 insertions(+), 64 deletions(-)
> >>
> >> -- 
> >> 2.17.0
> > 

-- 
Itsuro ODA <oda@valinux.co.jp>


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [dpdk-dev] [PATCH v3 0/4] make vhost PMD available for secondary processes
  2020-01-08  6:25 [dpdk-dev] [PATCH 0/4] make vhost PMD available for secondary processes oda
                   ` (5 preceding siblings ...)
  2020-01-08 23:22 ` Itsuro Oda
@ 2020-02-06  1:39 ` Itsuro Oda
  2020-02-06  1:39   ` [dpdk-dev] [PATCH v3 1/4] net/vhost: allocate interface name from heap Itsuro Oda
                     ` (5 more replies)
  6 siblings, 6 replies; 27+ messages in thread
From: Itsuro Oda @ 2020-02-06  1:39 UTC (permalink / raw)
  To: dev, maxime.coquelin, tiwei.bie, zhihong.wang, anatoly.burakov; +Cc: stable

vhost PMD has not been available for secondary processes since
DPDK v18.11.  (https://bugs.dpdk.org/show_bug.cgi?id=194)
(for a long term !)
This series of patches intend to make vhost PMD available for
secondary processes.
Because now setting vhost driver to communicate with a vhost-user
master (ex. Qemu) is accomplished by the probe function of the
primary process, only the primary process can be a vhost-user
slave.
With this patch, setting vhost driver is delayed at eth_dev
configuration in order to be able to set it from a secondary
process. Because (in the first place,) setting vhost driver is not
necessary to be done at probe (it is enough to be done up to eth_dev
start), this fix is no problem for the primary process.
There is a precondition that the same process has to operate
a vhost interface from beginning to end (from eth_dev configuration
to eth_dev close). (This patch leaves it to user's responsibility.)
This precondition will not be a problem in most use cases
(including SPP).

v2:
- add signed-off-by
- fix spelling error

v3:
- rebase on dpdk-next-virtio master
- change patch order
- fix subject and commit message

Itsuro Oda (4):
  net/vhost: allocate interface name from heap
  net/vhost: delay vhost driver setup
  net/vhost: make secondary probe complete
  net/vhost: remove an unused member

 drivers/net/vhost/rte_eth_vhost.c | 152 +++++++++++++++++-------------
 1 file changed, 88 insertions(+), 64 deletions(-)

-- 
2.17.0


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [dpdk-dev] [PATCH v3 1/4] net/vhost: allocate interface name from heap
  2020-02-06  1:39 ` [dpdk-dev] [PATCH v3 " Itsuro Oda
@ 2020-02-06  1:39   ` Itsuro Oda
  2020-02-06  1:39   ` [dpdk-dev] [PATCH v3 2/4] net/vhost: delay vhost driver setup Itsuro Oda
                     ` (4 subsequent siblings)
  5 siblings, 0 replies; 27+ messages in thread
From: Itsuro Oda @ 2020-02-06  1:39 UTC (permalink / raw)
  To: dev, maxime.coquelin, tiwei.bie, zhihong.wang, anatoly.burakov; +Cc: stable

This patch allocates iface_name of pmd_internal from heap
in order to be able to refer from secondary processes.

Fixes: 4852aa8f6e21 (drivers/net: enable hotplug on secondary process)
Cc: stable@dpdk.org

Signed-off-by: Itsuro Oda <oda@valinux.co.jp>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/net/vhost/rte_eth_vhost.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
index a63588986..cea2ead2d 100644
--- a/drivers/net/vhost/rte_eth_vhost.c
+++ b/drivers/net/vhost/rte_eth_vhost.c
@@ -1009,7 +1009,7 @@ eth_dev_close(struct rte_eth_dev *dev)
 			rte_free(dev->data->tx_queues[i]);
 
 	free(internal->dev_name);
-	free(internal->iface_name);
+	rte_free(internal->iface_name);
 	rte_free(internal);
 
 	dev->data->dev_private = NULL;
@@ -1256,9 +1256,11 @@ eth_dev_vhost_create(struct rte_vdev_device *dev, char *iface_name,
 	internal->dev_name = strdup(name);
 	if (internal->dev_name == NULL)
 		goto error;
-	internal->iface_name = strdup(iface_name);
+	internal->iface_name = rte_malloc_socket(name, strlen(iface_name) + 1,
+						 0, numa_node);
 	if (internal->iface_name == NULL)
 		goto error;
+	strcpy(internal->iface_name, iface_name);
 
 	list->eth_dev = eth_dev;
 	pthread_mutex_lock(&internal_list_lock);
@@ -1306,7 +1308,7 @@ eth_dev_vhost_create(struct rte_vdev_device *dev, char *iface_name,
 
 error:
 	if (internal) {
-		free(internal->iface_name);
+		rte_free(internal->iface_name);
 		free(internal->dev_name);
 	}
 	rte_free(vring_state);
-- 
2.17.0


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [dpdk-dev] [PATCH v3 2/4] net/vhost: delay vhost driver setup
  2020-02-06  1:39 ` [dpdk-dev] [PATCH v3 " Itsuro Oda
  2020-02-06  1:39   ` [dpdk-dev] [PATCH v3 1/4] net/vhost: allocate interface name from heap Itsuro Oda
@ 2020-02-06  1:39   ` Itsuro Oda
  2020-02-18  8:50     ` Wang, Yinan
  2020-02-06  1:39   ` [dpdk-dev] [PATCH v3 3/4] net/vhost: make secondary probe complete Itsuro Oda
                     ` (3 subsequent siblings)
  5 siblings, 1 reply; 27+ messages in thread
From: Itsuro Oda @ 2020-02-06  1:39 UTC (permalink / raw)
  To: dev, maxime.coquelin, tiwei.bie, zhihong.wang, anatoly.burakov; +Cc: stable

Vhost driver setup is delayed at eth_dev configuration
in order to be able to set it from a secondary process.

Fixes: 4852aa8f6e21 (drivers/net: enable hotplug on secondary process)
Cc: stable@dpdk.org

Signed-off-by: Itsuro Oda <oda@valinux.co.jp>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/net/vhost/rte_eth_vhost.c | 130 ++++++++++++++++++------------
 1 file changed, 78 insertions(+), 52 deletions(-)

diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
index cea2ead2d..d7bba5c6e 100644
--- a/drivers/net/vhost/rte_eth_vhost.c
+++ b/drivers/net/vhost/rte_eth_vhost.c
@@ -97,6 +97,8 @@ struct pmd_internal {
 	rte_atomic32_t dev_attached;
 	char *dev_name;
 	char *iface_name;
+	uint64_t flags;
+	uint64_t disable_flags;
 	uint16_t max_queues;
 	int vid;
 	rte_atomic32_t started;
@@ -491,17 +493,6 @@ eth_vhost_tx(void *q, struct rte_mbuf **bufs, uint16_t nb_bufs)
 	return nb_tx;
 }
 
-static int
-eth_dev_configure(struct rte_eth_dev *dev __rte_unused)
-{
-	struct pmd_internal *internal = dev->data->dev_private;
-	const struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
-
-	internal->vlan_strip = !!(rxmode->offloads & DEV_RX_OFFLOAD_VLAN_STRIP);
-
-	return 0;
-}
-
 static inline struct internal_list *
 find_internal_resource(char *ifname)
 {
@@ -877,6 +868,62 @@ static struct vhost_device_ops vhost_ops = {
 	.vring_state_changed = vring_state_changed,
 };
 
+static int
+vhost_driver_setup(struct rte_eth_dev *eth_dev)
+{
+	struct pmd_internal *internal = eth_dev->data->dev_private;
+	struct internal_list *list = NULL;
+	struct rte_vhost_vring_state *vring_state = NULL;
+	unsigned int numa_node = eth_dev->device->numa_node;
+	const char *name = eth_dev->device->name;
+
+	list = rte_zmalloc_socket(name, sizeof(*list), 0, numa_node);
+	if (list == NULL)
+		goto error;
+
+	vring_state = rte_zmalloc_socket(name, sizeof(*vring_state),
+					 0, numa_node);
+	if (vring_state == NULL)
+		goto error;
+
+	list->eth_dev = eth_dev;
+	pthread_mutex_lock(&internal_list_lock);
+	TAILQ_INSERT_TAIL(&internal_list, list, next);
+	pthread_mutex_unlock(&internal_list_lock);
+
+	rte_spinlock_init(&vring_state->lock);
+	vring_states[eth_dev->data->port_id] = vring_state;
+
+	if (rte_vhost_driver_register(internal->iface_name, internal->flags))
+		goto error;
+
+	if (internal->disable_flags) {
+		if (rte_vhost_driver_disable_features(internal->iface_name,
+						      internal->disable_flags))
+			goto error;
+	}
+
+	if (rte_vhost_driver_callback_register(internal->iface_name,
+					       &vhost_ops) < 0) {
+		VHOST_LOG(ERR, "Can't register callbacks\n");
+		goto error;
+	}
+
+	if (rte_vhost_driver_start(internal->iface_name) < 0) {
+		VHOST_LOG(ERR, "Failed to start driver for %s\n",
+			  internal->iface_name);
+		goto error;
+	}
+
+	return 0;
+
+error:
+	rte_free(vring_state);
+	rte_free(list);
+
+	return -1;
+}
+
 int
 rte_eth_vhost_get_queue_event(uint16_t port_id,
 		struct rte_eth_vhost_queue_event *event)
@@ -943,6 +990,24 @@ rte_eth_vhost_get_vid_from_port_id(uint16_t port_id)
 	return vid;
 }
 
+static int
+eth_dev_configure(struct rte_eth_dev *dev)
+{
+	struct pmd_internal *internal = dev->data->dev_private;
+	const struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
+
+	/* NOTE: the same process has to operate a vhost interface
+	 * from beginning to end (from eth_dev configure to eth_dev close).
+	 * It is user's responsibility at the moment.
+	 */
+	if (vhost_driver_setup(dev) < 0)
+		return -1;
+
+	internal->vlan_strip = !!(rxmode->offloads & DEV_RX_OFFLOAD_VLAN_STRIP);
+
+	return 0;
+}
+
 static int
 eth_dev_start(struct rte_eth_dev *eth_dev)
 {
@@ -1219,16 +1284,10 @@ eth_dev_vhost_create(struct rte_vdev_device *dev, char *iface_name,
 	struct pmd_internal *internal = NULL;
 	struct rte_eth_dev *eth_dev = NULL;
 	struct rte_ether_addr *eth_addr = NULL;
-	struct rte_vhost_vring_state *vring_state = NULL;
-	struct internal_list *list = NULL;
 
 	VHOST_LOG(INFO, "Creating VHOST-USER backend on numa socket %u\n",
 		numa_node);
 
-	list = rte_zmalloc_socket(name, sizeof(*list), 0, numa_node);
-	if (list == NULL)
-		goto error;
-
 	/* reserve an ethdev entry */
 	eth_dev = rte_eth_vdev_allocate(dev, sizeof(*internal));
 	if (eth_dev == NULL)
@@ -1242,11 +1301,6 @@ eth_dev_vhost_create(struct rte_vdev_device *dev, char *iface_name,
 	*eth_addr = base_eth_addr;
 	eth_addr->addr_bytes[5] = eth_dev->data->port_id;
 
-	vring_state = rte_zmalloc_socket(name,
-			sizeof(*vring_state), 0, numa_node);
-	if (vring_state == NULL)
-		goto error;
-
 	/* now put it all together
 	 * - store queue data in internal,
 	 * - point eth_dev_data to internals
@@ -1262,18 +1316,12 @@ eth_dev_vhost_create(struct rte_vdev_device *dev, char *iface_name,
 		goto error;
 	strcpy(internal->iface_name, iface_name);
 
-	list->eth_dev = eth_dev;
-	pthread_mutex_lock(&internal_list_lock);
-	TAILQ_INSERT_TAIL(&internal_list, list, next);
-	pthread_mutex_unlock(&internal_list_lock);
-
-	rte_spinlock_init(&vring_state->lock);
-	vring_states[eth_dev->data->port_id] = vring_state;
-
 	data->nb_rx_queues = queues;
 	data->nb_tx_queues = queues;
 	internal->max_queues = queues;
 	internal->vid = -1;
+	internal->flags = flags;
+	internal->disable_flags = disable_flags;
 	data->dev_link = pmd_link;
 	data->dev_flags = RTE_ETH_DEV_INTR_LSC | RTE_ETH_DEV_CLOSE_REMOVE;
 
@@ -1283,26 +1331,6 @@ eth_dev_vhost_create(struct rte_vdev_device *dev, char *iface_name,
 	eth_dev->rx_pkt_burst = eth_vhost_rx;
 	eth_dev->tx_pkt_burst = eth_vhost_tx;
 
-	if (rte_vhost_driver_register(iface_name, flags))
-		goto error;
-
-	if (disable_flags) {
-		if (rte_vhost_driver_disable_features(iface_name,
-					disable_flags))
-			goto error;
-	}
-
-	if (rte_vhost_driver_callback_register(iface_name, &vhost_ops) < 0) {
-		VHOST_LOG(ERR, "Can't register callbacks\n");
-		goto error;
-	}
-
-	if (rte_vhost_driver_start(iface_name) < 0) {
-		VHOST_LOG(ERR, "Failed to start driver for %s\n",
-			iface_name);
-		goto error;
-	}
-
 	rte_eth_dev_probing_finish(eth_dev);
 	return 0;
 
@@ -1311,9 +1339,7 @@ eth_dev_vhost_create(struct rte_vdev_device *dev, char *iface_name,
 		rte_free(internal->iface_name);
 		free(internal->dev_name);
 	}
-	rte_free(vring_state);
 	rte_eth_dev_release_port(eth_dev);
-	rte_free(list);
 
 	return -1;
 }
-- 
2.17.0


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [dpdk-dev] [PATCH v3 3/4] net/vhost: make secondary probe complete
  2020-02-06  1:39 ` [dpdk-dev] [PATCH v3 " Itsuro Oda
  2020-02-06  1:39   ` [dpdk-dev] [PATCH v3 1/4] net/vhost: allocate interface name from heap Itsuro Oda
  2020-02-06  1:39   ` [dpdk-dev] [PATCH v3 2/4] net/vhost: delay vhost driver setup Itsuro Oda
@ 2020-02-06  1:39   ` Itsuro Oda
  2020-02-06  1:39   ` [dpdk-dev] [PATCH v3 4/4] net/vhost: remove an unused member Itsuro Oda
                     ` (2 subsequent siblings)
  5 siblings, 0 replies; 27+ messages in thread
From: Itsuro Oda @ 2020-02-06  1:39 UTC (permalink / raw)
  To: dev, maxime.coquelin, tiwei.bie, zhihong.wang, anatoly.burakov; +Cc: stable

This patch adds lacking member setting and makes secondary
probe complete.

Fixes: 4852aa8f6e21 (drivers/net: enable hotplug on secondary process)
Cc: stable@dpdk.org

Signed-off-by: Itsuro Oda <oda@valinux.co.jp>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/net/vhost/rte_eth_vhost.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
index d7bba5c6e..307de2c68 100644
--- a/drivers/net/vhost/rte_eth_vhost.c
+++ b/drivers/net/vhost/rte_eth_vhost.c
@@ -1397,8 +1397,11 @@ rte_pmd_vhost_probe(struct rte_vdev_device *dev)
 			VHOST_LOG(ERR, "Failed to probe %s\n", name);
 			return -1;
 		}
-		/* TODO: request info from primary to set up Rx and Tx */
+		eth_dev->rx_pkt_burst = eth_vhost_rx;
+		eth_dev->tx_pkt_burst = eth_vhost_tx;
 		eth_dev->dev_ops = &ops;
+		if (dev->device.numa_node == SOCKET_ID_ANY)
+			dev->device.numa_node = rte_socket_id();
 		eth_dev->device = &dev->device;
 		rte_eth_dev_probing_finish(eth_dev);
 		return 0;
-- 
2.17.0


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [dpdk-dev] [PATCH v3 4/4] net/vhost: remove an unused member
  2020-02-06  1:39 ` [dpdk-dev] [PATCH v3 " Itsuro Oda
                     ` (2 preceding siblings ...)
  2020-02-06  1:39   ` [dpdk-dev] [PATCH v3 3/4] net/vhost: make secondary probe complete Itsuro Oda
@ 2020-02-06  1:39   ` Itsuro Oda
  2020-02-06 14:19   ` [dpdk-dev] [PATCH v3 0/4] make vhost PMD available for secondary processes Maxime Coquelin
  2020-02-13 16:29   ` Maxime Coquelin
  5 siblings, 0 replies; 27+ messages in thread
From: Itsuro Oda @ 2020-02-06  1:39 UTC (permalink / raw)
  To: dev, maxime.coquelin, tiwei.bie, zhihong.wang, anatoly.burakov; +Cc: stable

This patch removes an unused member from pmd_internal.

Signed-off-by: Itsuro Oda <oda@valinux.co.jp>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/net/vhost/rte_eth_vhost.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
index 307de2c68..90263ae77 100644
--- a/drivers/net/vhost/rte_eth_vhost.c
+++ b/drivers/net/vhost/rte_eth_vhost.c
@@ -95,7 +95,6 @@ struct vhost_queue {
 
 struct pmd_internal {
 	rte_atomic32_t dev_attached;
-	char *dev_name;
 	char *iface_name;
 	uint64_t flags;
 	uint64_t disable_flags;
@@ -1073,7 +1072,6 @@ eth_dev_close(struct rte_eth_dev *dev)
 		for (i = 0; i < dev->data->nb_tx_queues; i++)
 			rte_free(dev->data->tx_queues[i]);
 
-	free(internal->dev_name);
 	rte_free(internal->iface_name);
 	rte_free(internal);
 
@@ -1307,9 +1305,6 @@ eth_dev_vhost_create(struct rte_vdev_device *dev, char *iface_name,
 	 * - and point eth_dev structure to new eth_dev_data structure
 	 */
 	internal = eth_dev->data->dev_private;
-	internal->dev_name = strdup(name);
-	if (internal->dev_name == NULL)
-		goto error;
 	internal->iface_name = rte_malloc_socket(name, strlen(iface_name) + 1,
 						 0, numa_node);
 	if (internal->iface_name == NULL)
@@ -1335,10 +1330,8 @@ eth_dev_vhost_create(struct rte_vdev_device *dev, char *iface_name,
 	return 0;
 
 error:
-	if (internal) {
+	if (internal)
 		rte_free(internal->iface_name);
-		free(internal->dev_name);
-	}
 	rte_eth_dev_release_port(eth_dev);
 
 	return -1;
-- 
2.17.0


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [dpdk-dev] [PATCH v3 0/4] make vhost PMD available for secondary processes
  2020-02-06  1:39 ` [dpdk-dev] [PATCH v3 " Itsuro Oda
                     ` (3 preceding siblings ...)
  2020-02-06  1:39   ` [dpdk-dev] [PATCH v3 4/4] net/vhost: remove an unused member Itsuro Oda
@ 2020-02-06 14:19   ` Maxime Coquelin
  2020-02-13 16:29   ` Maxime Coquelin
  5 siblings, 0 replies; 27+ messages in thread
From: Maxime Coquelin @ 2020-02-06 14:19 UTC (permalink / raw)
  To: Itsuro Oda, dev, tiwei.bie, zhihong.wang, anatoly.burakov; +Cc: stable



On 2/6/20 2:39 AM, Itsuro Oda wrote:
> vhost PMD has not been available for secondary processes since
> DPDK v18.11.  (https://bugs.dpdk.org/show_bug.cgi?id=194)
> (for a long term !)
> This series of patches intend to make vhost PMD available for
> secondary processes.
> Because now setting vhost driver to communicate with a vhost-user
> master (ex. Qemu) is accomplished by the probe function of the
> primary process, only the primary process can be a vhost-user
> slave.
> With this patch, setting vhost driver is delayed at eth_dev
> configuration in order to be able to set it from a secondary
> process. Because (in the first place,) setting vhost driver is not
> necessary to be done at probe (it is enough to be done up to eth_dev
> start), this fix is no problem for the primary process.
> There is a precondition that the same process has to operate
> a vhost interface from beginning to end (from eth_dev configuration
> to eth_dev close). (This patch leaves it to user's responsibility.)
> This precondition will not be a problem in most use cases
> (including SPP).
> 
> v2:
> - add signed-off-by
> - fix spelling error
> 
> v3:
> - rebase on dpdk-next-virtio master
> - change patch order
> - fix subject and commit message
> 
> Itsuro Oda (4):
>   net/vhost: allocate interface name from heap
>   net/vhost: delay vhost driver setup
>   net/vhost: make secondary probe complete
>   net/vhost: remove an unused member
> 
>  drivers/net/vhost/rte_eth_vhost.c | 152 +++++++++++++++++-------------
>  1 file changed, 88 insertions(+), 64 deletions(-)
> 

Thanks Itsuro, it looks good to me.
I will pull the series for v20.02-rc3.

Maxime


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [dpdk-dev] [PATCH v3 0/4] make vhost PMD available for secondary processes
  2020-02-06  1:39 ` [dpdk-dev] [PATCH v3 " Itsuro Oda
                     ` (4 preceding siblings ...)
  2020-02-06 14:19   ` [dpdk-dev] [PATCH v3 0/4] make vhost PMD available for secondary processes Maxime Coquelin
@ 2020-02-13 16:29   ` Maxime Coquelin
  5 siblings, 0 replies; 27+ messages in thread
From: Maxime Coquelin @ 2020-02-13 16:29 UTC (permalink / raw)
  To: Itsuro Oda, dev, tiwei.bie, zhihong.wang, anatoly.burakov; +Cc: stable



On 2/6/20 2:39 AM, Itsuro Oda wrote:
> vhost PMD has not been available for secondary processes since
> DPDK v18.11.  (https://bugs.dpdk.org/show_bug.cgi?id=194)
> (for a long term !)
> This series of patches intend to make vhost PMD available for
> secondary processes.
> Because now setting vhost driver to communicate with a vhost-user
> master (ex. Qemu) is accomplished by the probe function of the
> primary process, only the primary process can be a vhost-user
> slave.
> With this patch, setting vhost driver is delayed at eth_dev
> configuration in order to be able to set it from a secondary
> process. Because (in the first place,) setting vhost driver is not
> necessary to be done at probe (it is enough to be done up to eth_dev
> start), this fix is no problem for the primary process.
> There is a precondition that the same process has to operate
> a vhost interface from beginning to end (from eth_dev configuration
> to eth_dev close). (This patch leaves it to user's responsibility.)
> This precondition will not be a problem in most use cases
> (including SPP).
> 
> v2:
> - add signed-off-by
> - fix spelling error
> 
> v3:
> - rebase on dpdk-next-virtio master
> - change patch order
> - fix subject and commit message
> 
> Itsuro Oda (4):
>   net/vhost: allocate interface name from heap
>   net/vhost: delay vhost driver setup
>   net/vhost: make secondary probe complete
>   net/vhost: remove an unused member
> 
>  drivers/net/vhost/rte_eth_vhost.c | 152 +++++++++++++++++-------------
>  1 file changed, 88 insertions(+), 64 deletions(-)
> 

Applied to dpdk-next-virtio/master

Thanks,
Maxime


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [dpdk-dev] [PATCH v3 2/4] net/vhost: delay vhost driver setup
  2020-02-06  1:39   ` [dpdk-dev] [PATCH v3 2/4] net/vhost: delay vhost driver setup Itsuro Oda
@ 2020-02-18  8:50     ` Wang, Yinan
  2020-02-18 10:42       ` Maxime Coquelin
  0 siblings, 1 reply; 27+ messages in thread
From: Wang, Yinan @ 2020-02-18  8:50 UTC (permalink / raw)
  To: Itsuro Oda, dev, maxime.coquelin, Bie, Tiwei, Wang, Zhihong,
	Burakov, Anatoly
  Cc: Thomas Monjalon, Xu, Qian Q, Yao, Lei A

Hi Itsuro/Maxime,

Below patch (commit id:3d01b759d2679c216725689eabe44147d1737326)blocked vhost/virtio basic test: If re-config vhost port,testpmd will launch failed.

Reproduce steps:

./x86_64-native-linuxapp-gcc/app/testpmd -l 2-4 -n 4 --socket-mem 1024,1024  --legacy-mem --file-prefix=vhost --vdev 'net_vhost0,iface=vhost-net,queues=1,client=0' --no-pci -- -i --txd=1024 --rxd=1024
    testpmd>set fwd csum
    testpmd>stop
    testpmd>port stop 0
    testpmd>csum set tcp sw 0
    testpmd>port start 0

VHOST_CONFIG: vhost-user server: socket created, fd: 26
VHOST_CONFIG: failed to bind to vhost-net: Address already in use; remove it and try again
Failed to start driver for vhost-net
Port0 dev_configure = -1
Fail to configure port 0

BR,
Yinan
> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Itsuro Oda
> Sent: 2020年2月6日 9:40
> To: dev@dpdk.org; maxime.coquelin@redhat.com; Bie, Tiwei
> <tiwei.bie@intel.com>; Wang, Zhihong <zhihong.wang@intel.com>; Burakov,
> Anatoly <anatoly.burakov@intel.com>
> Cc: stable@dpdk.org
> Subject: [dpdk-dev] [PATCH v3 2/4] net/vhost: delay vhost driver setup
> 
> Vhost driver setup is delayed at eth_dev configuration in order to be able to set
> it from a secondary process.
> 
> Fixes: 4852aa8f6e21 (drivers/net: enable hotplug on secondary process)
> Cc: stable@dpdk.org
> 
> Signed-off-by: Itsuro Oda <oda@valinux.co.jp>
> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  drivers/net/vhost/rte_eth_vhost.c | 130 ++++++++++++++++++------------
>  1 file changed, 78 insertions(+), 52 deletions(-)
> 
> diff --git a/drivers/net/vhost/rte_eth_vhost.c
> b/drivers/net/vhost/rte_eth_vhost.c
> index cea2ead2d..d7bba5c6e 100644
> --- a/drivers/net/vhost/rte_eth_vhost.c
> +++ b/drivers/net/vhost/rte_eth_vhost.c
> @@ -97,6 +97,8 @@ struct pmd_internal {
>  	rte_atomic32_t dev_attached;
>  	char *dev_name;
>  	char *iface_name;
> +	uint64_t flags;
> +	uint64_t disable_flags;
>  	uint16_t max_queues;
>  	int vid;
>  	rte_atomic32_t started;
> @@ -491,17 +493,6 @@ eth_vhost_tx(void *q, struct rte_mbuf **bufs,
> uint16_t nb_bufs)
>  	return nb_tx;
>  }
> 
> -static int
> -eth_dev_configure(struct rte_eth_dev *dev __rte_unused) -{
> -	struct pmd_internal *internal = dev->data->dev_private;
> -	const struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
> -
> -	internal->vlan_strip = !!(rxmode->offloads &
> DEV_RX_OFFLOAD_VLAN_STRIP);
> -
> -	return 0;
> -}
> -
>  static inline struct internal_list *
>  find_internal_resource(char *ifname)
>  {
> @@ -877,6 +868,62 @@ static struct vhost_device_ops vhost_ops = {
>  	.vring_state_changed = vring_state_changed,  };
> 
> +static int
> +vhost_driver_setup(struct rte_eth_dev *eth_dev) {
> +	struct pmd_internal *internal = eth_dev->data->dev_private;
> +	struct internal_list *list = NULL;
> +	struct rte_vhost_vring_state *vring_state = NULL;
> +	unsigned int numa_node = eth_dev->device->numa_node;
> +	const char *name = eth_dev->device->name;
> +
> +	list = rte_zmalloc_socket(name, sizeof(*list), 0, numa_node);
> +	if (list == NULL)
> +		goto error;
> +
> +	vring_state = rte_zmalloc_socket(name, sizeof(*vring_state),
> +					 0, numa_node);
> +	if (vring_state == NULL)
> +		goto error;
> +
> +	list->eth_dev = eth_dev;
> +	pthread_mutex_lock(&internal_list_lock);
> +	TAILQ_INSERT_TAIL(&internal_list, list, next);
> +	pthread_mutex_unlock(&internal_list_lock);
> +
> +	rte_spinlock_init(&vring_state->lock);
> +	vring_states[eth_dev->data->port_id] = vring_state;
> +
> +	if (rte_vhost_driver_register(internal->iface_name, internal->flags))
> +		goto error;
> +
> +	if (internal->disable_flags) {
> +		if (rte_vhost_driver_disable_features(internal->iface_name,
> +						      internal->disable_flags))
> +			goto error;
> +	}
> +
> +	if (rte_vhost_driver_callback_register(internal->iface_name,
> +					       &vhost_ops) < 0) {
> +		VHOST_LOG(ERR, "Can't register callbacks\n");
> +		goto error;
> +	}
> +
> +	if (rte_vhost_driver_start(internal->iface_name) < 0) {
> +		VHOST_LOG(ERR, "Failed to start driver for %s\n",
> +			  internal->iface_name);
> +		goto error;
> +	}
> +
> +	return 0;
> +
> +error:
> +	rte_free(vring_state);
> +	rte_free(list);
> +
> +	return -1;
> +}
> +
>  int
>  rte_eth_vhost_get_queue_event(uint16_t port_id,
>  		struct rte_eth_vhost_queue_event *event) @@ -943,6 +990,24 @@
> rte_eth_vhost_get_vid_from_port_id(uint16_t port_id)
>  	return vid;
>  }
> 
> +static int
> +eth_dev_configure(struct rte_eth_dev *dev) {
> +	struct pmd_internal *internal = dev->data->dev_private;
> +	const struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
> +
> +	/* NOTE: the same process has to operate a vhost interface
> +	 * from beginning to end (from eth_dev configure to eth_dev close).
> +	 * It is user's responsibility at the moment.
> +	 */
> +	if (vhost_driver_setup(dev) < 0)
> +		return -1;
> +
> +	internal->vlan_strip = !!(rxmode->offloads &
> +DEV_RX_OFFLOAD_VLAN_STRIP);
> +
> +	return 0;
> +}
> +
>  static int
>  eth_dev_start(struct rte_eth_dev *eth_dev)  { @@ -1219,16 +1284,10 @@
> eth_dev_vhost_create(struct rte_vdev_device *dev, char *iface_name,
>  	struct pmd_internal *internal = NULL;
>  	struct rte_eth_dev *eth_dev = NULL;
>  	struct rte_ether_addr *eth_addr = NULL;
> -	struct rte_vhost_vring_state *vring_state = NULL;
> -	struct internal_list *list = NULL;
> 
>  	VHOST_LOG(INFO, "Creating VHOST-USER backend on numa
> socket %u\n",
>  		numa_node);
> 
> -	list = rte_zmalloc_socket(name, sizeof(*list), 0, numa_node);
> -	if (list == NULL)
> -		goto error;
> -
>  	/* reserve an ethdev entry */
>  	eth_dev = rte_eth_vdev_allocate(dev, sizeof(*internal));
>  	if (eth_dev == NULL)
> @@ -1242,11 +1301,6 @@ eth_dev_vhost_create(struct rte_vdev_device *dev,
> char *iface_name,
>  	*eth_addr = base_eth_addr;
>  	eth_addr->addr_bytes[5] = eth_dev->data->port_id;
> 
> -	vring_state = rte_zmalloc_socket(name,
> -			sizeof(*vring_state), 0, numa_node);
> -	if (vring_state == NULL)
> -		goto error;
> -
>  	/* now put it all together
>  	 * - store queue data in internal,
>  	 * - point eth_dev_data to internals
> @@ -1262,18 +1316,12 @@ eth_dev_vhost_create(struct rte_vdev_device
> *dev, char *iface_name,
>  		goto error;
>  	strcpy(internal->iface_name, iface_name);
> 
> -	list->eth_dev = eth_dev;
> -	pthread_mutex_lock(&internal_list_lock);
> -	TAILQ_INSERT_TAIL(&internal_list, list, next);
> -	pthread_mutex_unlock(&internal_list_lock);
> -
> -	rte_spinlock_init(&vring_state->lock);
> -	vring_states[eth_dev->data->port_id] = vring_state;
> -
>  	data->nb_rx_queues = queues;
>  	data->nb_tx_queues = queues;
>  	internal->max_queues = queues;
>  	internal->vid = -1;
> +	internal->flags = flags;
> +	internal->disable_flags = disable_flags;
>  	data->dev_link = pmd_link;
>  	data->dev_flags = RTE_ETH_DEV_INTR_LSC |
> RTE_ETH_DEV_CLOSE_REMOVE;
> 
> @@ -1283,26 +1331,6 @@ eth_dev_vhost_create(struct rte_vdev_device *dev,
> char *iface_name,
>  	eth_dev->rx_pkt_burst = eth_vhost_rx;
>  	eth_dev->tx_pkt_burst = eth_vhost_tx;
> 
> -	if (rte_vhost_driver_register(iface_name, flags))
> -		goto error;
> -
> -	if (disable_flags) {
> -		if (rte_vhost_driver_disable_features(iface_name,
> -					disable_flags))
> -			goto error;
> -	}
> -
> -	if (rte_vhost_driver_callback_register(iface_name, &vhost_ops) < 0) {
> -		VHOST_LOG(ERR, "Can't register callbacks\n");
> -		goto error;
> -	}
> -
> -	if (rte_vhost_driver_start(iface_name) < 0) {
> -		VHOST_LOG(ERR, "Failed to start driver for %s\n",
> -			iface_name);
> -		goto error;
> -	}
> -
>  	rte_eth_dev_probing_finish(eth_dev);
>  	return 0;
> 
> @@ -1311,9 +1339,7 @@ eth_dev_vhost_create(struct rte_vdev_device *dev,
> char *iface_name,
>  		rte_free(internal->iface_name);
>  		free(internal->dev_name);
>  	}
> -	rte_free(vring_state);
>  	rte_eth_dev_release_port(eth_dev);
> -	rte_free(list);
> 
>  	return -1;
>  }
> --
> 2.17.0


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [dpdk-dev] [PATCH v3 2/4] net/vhost: delay vhost driver setup
  2020-02-18  8:50     ` Wang, Yinan
@ 2020-02-18 10:42       ` Maxime Coquelin
  0 siblings, 0 replies; 27+ messages in thread
From: Maxime Coquelin @ 2020-02-18 10:42 UTC (permalink / raw)
  To: Wang, Yinan, Itsuro Oda, dev, Bie, Tiwei, Wang, Zhihong, Burakov,
	Anatoly
  Cc: Thomas Monjalon, Xu, Qian Q, Yao, Lei A

Thanks Yinan for reporting the issue.

I'm working on a fix, which should be ready today:
1. Don't do vhost setup if device is already in the list
2. Fix the error path in setup function.

Maxime
On 2/18/20 9:50 AM, Wang, Yinan wrote:
> Hi Itsuro/Maxime,
> 
> Below patch (commit id:3d01b759d2679c216725689eabe44147d1737326)blocked vhost/virtio basic test: If re-config vhost port,testpmd will launch failed.
> 
> Reproduce steps:
> 
> ./x86_64-native-linuxapp-gcc/app/testpmd -l 2-4 -n 4 --socket-mem 1024,1024  --legacy-mem --file-prefix=vhost --vdev 'net_vhost0,iface=vhost-net,queues=1,client=0' --no-pci -- -i --txd=1024 --rxd=1024
>     testpmd>set fwd csum
>     testpmd>stop
>     testpmd>port stop 0
>     testpmd>csum set tcp sw 0
>     testpmd>port start 0
> 
> VHOST_CONFIG: vhost-user server: socket created, fd: 26
> VHOST_CONFIG: failed to bind to vhost-net: Address already in use; remove it and try again
> Failed to start driver for vhost-net
> Port0 dev_configure = -1
> Fail to configure port 0
> 
> BR,
> Yinan
>> -----Original Message-----
>> From: dev <dev-bounces@dpdk.org> On Behalf Of Itsuro Oda
>> Sent: 2020年2月6日 9:40
>> To: dev@dpdk.org; maxime.coquelin@redhat.com; Bie, Tiwei
>> <tiwei.bie@intel.com>; Wang, Zhihong <zhihong.wang@intel.com>; Burakov,
>> Anatoly <anatoly.burakov@intel.com>
>> Cc: stable@dpdk.org
>> Subject: [dpdk-dev] [PATCH v3 2/4] net/vhost: delay vhost driver setup
>>
>> Vhost driver setup is delayed at eth_dev configuration in order to be able to set
>> it from a secondary process.
>>
>> Fixes: 4852aa8f6e21 (drivers/net: enable hotplug on secondary process)
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Itsuro Oda <oda@valinux.co.jp>
>> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>> ---
>>  drivers/net/vhost/rte_eth_vhost.c | 130 ++++++++++++++++++------------
>>  1 file changed, 78 insertions(+), 52 deletions(-)
>>
>> diff --git a/drivers/net/vhost/rte_eth_vhost.c
>> b/drivers/net/vhost/rte_eth_vhost.c
>> index cea2ead2d..d7bba5c6e 100644
>> --- a/drivers/net/vhost/rte_eth_vhost.c
>> +++ b/drivers/net/vhost/rte_eth_vhost.c
>> @@ -97,6 +97,8 @@ struct pmd_internal {
>>  	rte_atomic32_t dev_attached;
>>  	char *dev_name;
>>  	char *iface_name;
>> +	uint64_t flags;
>> +	uint64_t disable_flags;
>>  	uint16_t max_queues;
>>  	int vid;
>>  	rte_atomic32_t started;
>> @@ -491,17 +493,6 @@ eth_vhost_tx(void *q, struct rte_mbuf **bufs,
>> uint16_t nb_bufs)
>>  	return nb_tx;
>>  }
>>
>> -static int
>> -eth_dev_configure(struct rte_eth_dev *dev __rte_unused) -{
>> -	struct pmd_internal *internal = dev->data->dev_private;
>> -	const struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
>> -
>> -	internal->vlan_strip = !!(rxmode->offloads &
>> DEV_RX_OFFLOAD_VLAN_STRIP);
>> -
>> -	return 0;
>> -}
>> -
>>  static inline struct internal_list *
>>  find_internal_resource(char *ifname)
>>  {
>> @@ -877,6 +868,62 @@ static struct vhost_device_ops vhost_ops = {
>>  	.vring_state_changed = vring_state_changed,  };
>>
>> +static int
>> +vhost_driver_setup(struct rte_eth_dev *eth_dev) {
>> +	struct pmd_internal *internal = eth_dev->data->dev_private;
>> +	struct internal_list *list = NULL;
>> +	struct rte_vhost_vring_state *vring_state = NULL;
>> +	unsigned int numa_node = eth_dev->device->numa_node;
>> +	const char *name = eth_dev->device->name;
>> +
>> +	list = rte_zmalloc_socket(name, sizeof(*list), 0, numa_node);
>> +	if (list == NULL)
>> +		goto error;
>> +
>> +	vring_state = rte_zmalloc_socket(name, sizeof(*vring_state),
>> +					 0, numa_node);
>> +	if (vring_state == NULL)
>> +		goto error;
>> +
>> +	list->eth_dev = eth_dev;
>> +	pthread_mutex_lock(&internal_list_lock);
>> +	TAILQ_INSERT_TAIL(&internal_list, list, next);
>> +	pthread_mutex_unlock(&internal_list_lock);
>> +
>> +	rte_spinlock_init(&vring_state->lock);
>> +	vring_states[eth_dev->data->port_id] = vring_state;
>> +
>> +	if (rte_vhost_driver_register(internal->iface_name, internal->flags))
>> +		goto error;
>> +
>> +	if (internal->disable_flags) {
>> +		if (rte_vhost_driver_disable_features(internal->iface_name,
>> +						      internal->disable_flags))
>> +			goto error;
>> +	}
>> +
>> +	if (rte_vhost_driver_callback_register(internal->iface_name,
>> +					       &vhost_ops) < 0) {
>> +		VHOST_LOG(ERR, "Can't register callbacks\n");
>> +		goto error;
>> +	}
>> +
>> +	if (rte_vhost_driver_start(internal->iface_name) < 0) {
>> +		VHOST_LOG(ERR, "Failed to start driver for %s\n",
>> +			  internal->iface_name);
>> +		goto error;
>> +	}
>> +
>> +	return 0;
>> +
>> +error:
>> +	rte_free(vring_state);
>> +	rte_free(list);
>> +
>> +	return -1;
>> +}
>> +
>>  int
>>  rte_eth_vhost_get_queue_event(uint16_t port_id,
>>  		struct rte_eth_vhost_queue_event *event) @@ -943,6 +990,24 @@
>> rte_eth_vhost_get_vid_from_port_id(uint16_t port_id)
>>  	return vid;
>>  }
>>
>> +static int
>> +eth_dev_configure(struct rte_eth_dev *dev) {
>> +	struct pmd_internal *internal = dev->data->dev_private;
>> +	const struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
>> +
>> +	/* NOTE: the same process has to operate a vhost interface
>> +	 * from beginning to end (from eth_dev configure to eth_dev close).
>> +	 * It is user's responsibility at the moment.
>> +	 */
>> +	if (vhost_driver_setup(dev) < 0)
>> +		return -1;
>> +
>> +	internal->vlan_strip = !!(rxmode->offloads &
>> +DEV_RX_OFFLOAD_VLAN_STRIP);
>> +
>> +	return 0;
>> +}
>> +
>>  static int
>>  eth_dev_start(struct rte_eth_dev *eth_dev)  { @@ -1219,16 +1284,10 @@
>> eth_dev_vhost_create(struct rte_vdev_device *dev, char *iface_name,
>>  	struct pmd_internal *internal = NULL;
>>  	struct rte_eth_dev *eth_dev = NULL;
>>  	struct rte_ether_addr *eth_addr = NULL;
>> -	struct rte_vhost_vring_state *vring_state = NULL;
>> -	struct internal_list *list = NULL;
>>
>>  	VHOST_LOG(INFO, "Creating VHOST-USER backend on numa
>> socket %u\n",
>>  		numa_node);
>>
>> -	list = rte_zmalloc_socket(name, sizeof(*list), 0, numa_node);
>> -	if (list == NULL)
>> -		goto error;
>> -
>>  	/* reserve an ethdev entry */
>>  	eth_dev = rte_eth_vdev_allocate(dev, sizeof(*internal));
>>  	if (eth_dev == NULL)
>> @@ -1242,11 +1301,6 @@ eth_dev_vhost_create(struct rte_vdev_device *dev,
>> char *iface_name,
>>  	*eth_addr = base_eth_addr;
>>  	eth_addr->addr_bytes[5] = eth_dev->data->port_id;
>>
>> -	vring_state = rte_zmalloc_socket(name,
>> -			sizeof(*vring_state), 0, numa_node);
>> -	if (vring_state == NULL)
>> -		goto error;
>> -
>>  	/* now put it all together
>>  	 * - store queue data in internal,
>>  	 * - point eth_dev_data to internals
>> @@ -1262,18 +1316,12 @@ eth_dev_vhost_create(struct rte_vdev_device
>> *dev, char *iface_name,
>>  		goto error;
>>  	strcpy(internal->iface_name, iface_name);
>>
>> -	list->eth_dev = eth_dev;
>> -	pthread_mutex_lock(&internal_list_lock);
>> -	TAILQ_INSERT_TAIL(&internal_list, list, next);
>> -	pthread_mutex_unlock(&internal_list_lock);
>> -
>> -	rte_spinlock_init(&vring_state->lock);
>> -	vring_states[eth_dev->data->port_id] = vring_state;
>> -
>>  	data->nb_rx_queues = queues;
>>  	data->nb_tx_queues = queues;
>>  	internal->max_queues = queues;
>>  	internal->vid = -1;
>> +	internal->flags = flags;
>> +	internal->disable_flags = disable_flags;
>>  	data->dev_link = pmd_link;
>>  	data->dev_flags = RTE_ETH_DEV_INTR_LSC |
>> RTE_ETH_DEV_CLOSE_REMOVE;
>>
>> @@ -1283,26 +1331,6 @@ eth_dev_vhost_create(struct rte_vdev_device *dev,
>> char *iface_name,
>>  	eth_dev->rx_pkt_burst = eth_vhost_rx;
>>  	eth_dev->tx_pkt_burst = eth_vhost_tx;
>>
>> -	if (rte_vhost_driver_register(iface_name, flags))
>> -		goto error;
>> -
>> -	if (disable_flags) {
>> -		if (rte_vhost_driver_disable_features(iface_name,
>> -					disable_flags))
>> -			goto error;
>> -	}
>> -
>> -	if (rte_vhost_driver_callback_register(iface_name, &vhost_ops) < 0) {
>> -		VHOST_LOG(ERR, "Can't register callbacks\n");
>> -		goto error;
>> -	}
>> -
>> -	if (rte_vhost_driver_start(iface_name) < 0) {
>> -		VHOST_LOG(ERR, "Failed to start driver for %s\n",
>> -			iface_name);
>> -		goto error;
>> -	}
>> -
>>  	rte_eth_dev_probing_finish(eth_dev);
>>  	return 0;
>>
>> @@ -1311,9 +1339,7 @@ eth_dev_vhost_create(struct rte_vdev_device *dev,
>> char *iface_name,
>>  		rte_free(internal->iface_name);
>>  		free(internal->dev_name);
>>  	}
>> -	rte_free(vring_state);
>>  	rte_eth_dev_release_port(eth_dev);
>> -	rte_free(list);
>>
>>  	return -1;
>>  }
>> --
>> 2.17.0
> 


^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2020-02-18 10:42 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-08  6:25 [dpdk-dev] [PATCH 0/4] make vhost PMD available for secondary processes oda
2020-01-08  6:25 ` [dpdk-dev] [PATCH 1/4] net/vhost: remove an unused member oda
2020-01-08  6:25 ` [dpdk-dev] [PATCH 2/4] net/vhost: allocate iface_name from heap oda
2020-01-08  6:25 ` [dpdk-dev] [PATCH 3/4] net/vhost: delay vhost driver setup oda
2020-01-08  6:25 ` [dpdk-dev] [PATCH 4/4] net/vhost: make secondary probe complete oda
2020-01-08  6:38 ` [dpdk-dev] [PATCH 0/4] make vhost PMD available for secondary processes Itsuro ODA
2020-01-08 23:22 ` Itsuro Oda
2020-01-08 23:22   ` [dpdk-dev] [PATCH 1/4] net/vhost: remove an unused member Itsuro Oda
2020-02-04 17:56     ` Maxime Coquelin
2020-01-08 23:22   ` [dpdk-dev] [PATCH 2/4] net/vhost: allocate iface_name from heap Itsuro Oda
2020-02-04 17:59     ` Maxime Coquelin
2020-01-08 23:22   ` [dpdk-dev] [PATCH 3/4] net/vhost: delay vhost driver setup Itsuro Oda
2020-02-04 18:04     ` Maxime Coquelin
2020-01-08 23:22   ` [dpdk-dev] [PATCH 4/4] net/vhost: make secondary probe complete Itsuro Oda
2020-02-04 18:08     ` Maxime Coquelin
2020-01-20  2:17   ` [dpdk-dev] [PATCH 0/4] make vhost PMD available for secondary processes Itsuro ODA
2020-02-04 17:54     ` Maxime Coquelin
2020-02-04 22:19       ` Itsuro ODA
2020-02-06  1:39 ` [dpdk-dev] [PATCH v3 " Itsuro Oda
2020-02-06  1:39   ` [dpdk-dev] [PATCH v3 1/4] net/vhost: allocate interface name from heap Itsuro Oda
2020-02-06  1:39   ` [dpdk-dev] [PATCH v3 2/4] net/vhost: delay vhost driver setup Itsuro Oda
2020-02-18  8:50     ` Wang, Yinan
2020-02-18 10:42       ` Maxime Coquelin
2020-02-06  1:39   ` [dpdk-dev] [PATCH v3 3/4] net/vhost: make secondary probe complete Itsuro Oda
2020-02-06  1:39   ` [dpdk-dev] [PATCH v3 4/4] net/vhost: remove an unused member Itsuro Oda
2020-02-06 14:19   ` [dpdk-dev] [PATCH v3 0/4] make vhost PMD available for secondary processes Maxime Coquelin
2020-02-13 16:29   ` Maxime Coquelin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).