[PATCH 0/6] app/testpmd: support attach and detach port for MP

[PATCH 0/6] app/testpmd: support attach and detach port for MP

[Huisong Li]

This patchset do some bugfix and add attach and detach port for MP.

Huisong Li (6):
  bus/pci: fix a segfault when call callback
  bus/vdev: fix a segfault when call callback
  ethdev: fix push new event
  app/testpmd: check the validity of the port
  app/testpmd: support attach and detach port for MP
  app/testpmd: stop packet forwarding in new and destroy event

 app/test-pmd/testpmd.c                | 85 +++++++++++++++++----------
 app/test-pmd/testpmd.h                |  1 -
 drivers/bus/pci/pci_common.c          | 13 +++-
 drivers/bus/vdev/vdev.c               | 11 +++-
 drivers/net/bonding/bonding_testpmd.c |  1 -
 lib/ethdev/ethdev_driver.c            |  2 +-
 6 files changed, 75 insertions(+), 38 deletions(-)

-- 
2.22.0

[PATCH 1/6] bus/pci: fix a segfault when call callback

[Huisong Li]

After the driver probe is executed, the callback in application will
be called. The callback in application may call some APIs which access the
rte_pci_driver::driver by the device::driver pointer to get driver
information. If the rte_pci_device::device::driver pointer isn't pointed to
rte_pci_driver::driver in rte_pci_probe_one_driver, a segfault will occur.
For example, when ethdev driver probe completes, the callback in
application call rte_eth_dev_info_get which use dev->device->driver->name.
So rte_pci_device::device::driver should point to rte_pci_driver::driver
before executing the driver probe.

Fixes: c752998b5e2e ("pci: introduce library and driver")
Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 drivers/bus/pci/pci_common.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c
index 37ab879779..831a9cd8c7 100644
--- a/drivers/bus/pci/pci_common.c
+++ b/drivers/bus/pci/pci_common.c
@@ -265,11 +265,22 @@ rte_pci_probe_one_driver(struct rte_pci_driver *dr,
 			dr->driver.name, dev->id.vendor_id, dev->id.device_id,
 			loc->domain, loc->bus, loc->devid, loc->function,
 			dev->device.numa_node);
+
+	/*
+	 * After the driver probe is executed, the callback in application will
+	 * be called. The callback in application may call some APIs which use
+	 * dev->device.driver to get some driver information. If the driver
+	 * pointer isn't pointed to driver->driver here, a segfault will occur.
+	 */
+	if (!already_probed)
+		dev->device.driver = &dr->driver;
+
 	/* call the driver probe() function */
 	ret = dr->probe(dr, dev);
 	if (already_probed)
 		return ret; /* no rollback if already succeeded earlier */
 	if (ret) {
+		dev->device.driver = NULL;
 		dev->driver = NULL;
 		if ((dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING) &&
 			/* Don't unmap if device is unsupported and
@@ -282,8 +293,6 @@ rte_pci_probe_one_driver(struct rte_pci_driver *dr,
 		dev->vfio_req_intr_handle = NULL;
 		rte_intr_instance_free(dev->intr_handle);
 		dev->intr_handle = NULL;
-	} else {
-		dev->device.driver = &dr->driver;
 	}
 
 	return ret;
-- 
2.22.0

[PATCH 2/6] bus/vdev: fix a segfault when call callback

[Huisong Li]

After the driver probe is executed, the callback in application will be
called. And this callback may call some APIs which access the driver in
struct rte_vdev_driver by the device::driver pointer to get some driver
information. If the rte_vdev_device::device::driver pointer isn't pointed
to the rte_vdev_driver::driver before executing driver probe, a segfault
will occur.

Fixes: e9d159c3d534 ("eal: allow probing a device again")
Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 drivers/bus/vdev/vdev.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
index a8d8b2327e..dea3937607 100644
--- a/drivers/bus/vdev/vdev.c
+++ b/drivers/bus/vdev/vdev.c
@@ -209,9 +209,16 @@ vdev_probe_all_drivers(struct rte_vdev_device *dev)
 		return -1;
 	}
 
+	/*
+	 * After the driver probe is executed, the callback in application will
+	 * be called. The callback in application may call some APIs which use
+	 * dev->device.driver to get some driver information. If the driver
+	 * pointer isn't pointed to driver->driver here, a segfault will occur.
+	 */
+	dev->device.driver = &driver->driver;
 	ret = driver->probe(dev);
-	if (ret == 0)
-		dev->device.driver = &driver->driver;
+	if (ret != 0)
+		dev->device.driver = NULL;
 	return ret;
 }
 
-- 
2.22.0

[PATCH 3/6] ethdev: fix push new event

[Huisong Li]

The 'state' in struct rte_eth_dev may be used to update some information
when app receive these events. For example, when app receives a new event,
app may get the socket id of this port by calling rte_eth_dev_socket_id to
setup the attached port. The 'state' is used in rte_eth_dev_socket_id.

If the state isn't modified to RTE_ETH_DEV_ATTACHED before pushing the new
event, app will get the socket id failed. So this patch moves pushing event
operation after the state updated.

Fixes: 99a2dd955fba ("lib: remove librte_ prefix from directory names")
Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 lib/ethdev/ethdev_driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/ethdev/ethdev_driver.c b/lib/ethdev/ethdev_driver.c
index a285f213f0..a6616f072b 100644
--- a/lib/ethdev/ethdev_driver.c
+++ b/lib/ethdev/ethdev_driver.c
@@ -206,9 +206,9 @@ rte_eth_dev_probing_finish(struct rte_eth_dev *dev)
 	if (rte_eal_process_type() == RTE_PROC_SECONDARY)
 		eth_dev_fp_ops_setup(rte_eth_fp_ops + dev->data->port_id, dev);
 
-	rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_NEW, NULL);
 
 	dev->state = RTE_ETH_DEV_ATTACHED;
+	rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_NEW, NULL);
 }
 
 int
-- 
2.22.0

[PATCH 4/6] app/testpmd: check the validity of the port

[Huisong Li]

This patch checks the validity of port id for all events in
'eth_event_callback()'.

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 app/test-pmd/testpmd.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index addcbcac85..c743d65cf2 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -3706,14 +3706,15 @@ eth_event_callback(portid_t port_id, enum rte_eth_event_type type, void *param,
 		fflush(stdout);
 	}
 
+	if (port_id_is_invalid(port_id, DISABLED_WARN))
+		return 0;
+
 	switch (type) {
 	case RTE_ETH_EVENT_NEW:
 		ports[port_id].need_setup = 1;
 		ports[port_id].port_status = RTE_PORT_HANDLING;
 		break;
 	case RTE_ETH_EVENT_INTR_RMV:
-		if (port_id_is_invalid(port_id, DISABLED_WARN))
-			break;
 		if (rte_eal_alarm_set(100000,
 				rmv_port_callback, (void *)(intptr_t)port_id))
 			fprintf(stderr,
-- 
2.22.0

[PATCH 5/6] app/testpmd: support attach and detach port for MP

[Huisong Li]

This patch supports attach and detach port in primary and secondary
process.

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
---
 app/test-pmd/testpmd.c                | 76 +++++++++++++++++----------
 app/test-pmd/testpmd.h                |  1 -
 drivers/net/bonding/bonding_testpmd.c |  1 -
 3 files changed, 47 insertions(+), 31 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index c743d65cf2..e4f17f40b6 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -3356,9 +3356,6 @@ reset_port(portid_t pid)
 void
 attach_port(char *identifier)
 {
-	portid_t pi;
-	struct rte_dev_iterator iterator;
-
 	printf("Attaching a new port...\n");
 
 	if (identifier == NULL) {
@@ -3370,24 +3367,6 @@ attach_port(char *identifier)
 		TESTPMD_LOG(ERR, "Failed to attach port %s\n", identifier);
 		return;
 	}
-
-	/* first attach mode: event */
-	if (setup_on_probe_event) {
-		/* new ports are detected on RTE_ETH_EVENT_NEW event */
-		for (pi = 0; pi < RTE_MAX_ETHPORTS; pi++)
-			if (ports[pi].port_status == RTE_PORT_HANDLING &&
-					ports[pi].need_setup != 0)
-				setup_attached_port(pi);
-		return;
-	}
-
-	/* second attach mode: iterator */
-	RTE_ETH_FOREACH_MATCHING_DEV(pi, identifier, &iterator) {
-		/* setup ports matching the devargs used for probing */
-		if (port_is_forwarding(pi))
-			continue; /* port was already attached before */
-		setup_attached_port(pi);
-	}
 }
 
 static void
@@ -3410,7 +3389,6 @@ setup_attached_port(portid_t pi)
 	ports_ids[nb_ports++] = pi;
 	fwd_ports_ids[nb_fwd_ports++] = pi;
 	nb_cfg_ports = nb_fwd_ports;
-	ports[pi].need_setup = 0;
 	ports[pi].port_status = RTE_PORT_STOPPED;
 
 	printf("Port %d is attached. Now total ports is %d\n", pi, nb_ports);
@@ -3444,11 +3422,10 @@ detach_device(struct rte_device *dev)
 		TESTPMD_LOG(ERR, "Failed to detach device %s\n", dev->name);
 		return;
 	}
-	remove_invalid_ports();
 
 	printf("Device is detached\n");
-	printf("Now total ports is %d\n", nb_ports);
 	printf("Done\n");
+
 	return;
 }
 
@@ -3514,11 +3491,9 @@ detach_devargs(char *identifier)
 		return;
 	}
 
-	remove_invalid_ports();
-
 	printf("Device %s is detached\n", identifier);
-	printf("Now total ports is %d\n", nb_ports);
 	printf("Done\n");
+
 	rte_devargs_reset(&da);
 }
 
@@ -3682,11 +3657,52 @@ rmv_port_callback(void *arg)
 		struct rte_device *device = dev_info.device;
 		close_port(port_id);
 		detach_device(device); /* might be already removed or have more ports */
+		remove_invalid_ports();
+		printf("Now total ports is %d\n", nb_ports);
 	}
 	if (need_to_start)
 		start_packet_forwarding(0);
 }
 
+static void config_attached_port(portid_t port_id)
+{
+	struct rte_dev_iterator iterator;
+	struct rte_eth_dev_info dev_info;
+	portid_t pi;
+	int ret;
+
+	/* first attach mode: event */
+	if (setup_on_probe_event) {
+		/* new ports are detected on RTE_ETH_EVENT_NEW event */
+		setup_attached_port(port_id);
+		return;
+	}
+
+	ret = eth_dev_info_get_print_err(port_id, &dev_info);
+	if (ret != 0) {
+		TESTPMD_LOG(ERR,
+			"Failed to get device info for port %d, not detaching\n",
+			port_id);
+		return;
+	}
+	/* second attach mode: iterator */
+	RTE_ETH_FOREACH_MATCHING_DEV(pi, dev_info.device->name, &iterator) {
+		/* setup ports matching the devargs used for probing */
+		if (port_is_forwarding(port_id))
+			continue; /* port was already attached before */
+		setup_attached_port(port_id);
+	}
+}
+
+static void
+remove_invalid_ports_callback(void *arg)
+{
+	RTE_SET_USED(arg);
+
+	remove_invalid_ports();
+	printf("Now total ports is %d\n", nb_ports);
+}
+
 /* This function is used by the interrupt thread */
 static int
 eth_event_callback(portid_t port_id, enum rte_eth_event_type type, void *param,
@@ -3711,8 +3727,7 @@ eth_event_callback(portid_t port_id, enum rte_eth_event_type type, void *param,
 
 	switch (type) {
 	case RTE_ETH_EVENT_NEW:
-		ports[port_id].need_setup = 1;
-		ports[port_id].port_status = RTE_PORT_HANDLING;
+		config_attached_port(port_id);
 		break;
 	case RTE_ETH_EVENT_INTR_RMV:
 		if (rte_eal_alarm_set(100000,
@@ -3723,6 +3738,9 @@ eth_event_callback(portid_t port_id, enum rte_eth_event_type type, void *param,
 	case RTE_ETH_EVENT_DESTROY:
 		ports[port_id].port_status = RTE_PORT_CLOSED;
 		printf("Port %u is closed\n", port_id);
+		if (rte_eal_alarm_set(100000, remove_invalid_ports_callback,
+			(void *)(intptr_t)port_id))
+			fprintf(stderr, "Could not set up deferred device released\n");
 		break;
 	case RTE_ETH_EVENT_RX_AVAIL_THRESH: {
 		uint16_t rxq_id;
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index fb2f5195d3..ddfa911bd9 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -278,7 +278,6 @@ struct rte_port {
 	uint16_t                tx_vlan_id;/**< The tag ID */
 	uint16_t                tx_vlan_id_outer;/**< The outer tag ID */
 	volatile uint16_t        port_status;    /**< port started or not */
-	uint8_t                 need_setup;     /**< port just attached */
 	uint8_t                 need_reconfig;  /**< need reconfiguring port or not */
 	uint8_t                 need_reconfig_queues; /**< need reconfiguring queues or not */
 	uint8_t                 rss_flag;   /**< enable rss or not */
diff --git a/drivers/net/bonding/bonding_testpmd.c b/drivers/net/bonding/bonding_testpmd.c
index 3941f4cf23..ed1a278a44 100644
--- a/drivers/net/bonding/bonding_testpmd.c
+++ b/drivers/net/bonding/bonding_testpmd.c
@@ -763,7 +763,6 @@ static void cmd_create_bonded_device_parsed(void *parsed_result,
 			port_id, rte_strerror(-ret));
 
 	ports[port_id].bond_flag = 1;
-	ports[port_id].need_setup = 0;
 	ports[port_id].port_status = RTE_PORT_STOPPED;
 }
 
-- 
2.22.0

[PATCH 6/6] app/testpmd: stop packet forwarding in new and destroy event

[Huisong Li]

When testpmd receives the new or destroy event, the port related
information will be updated. Testpmd must stop packet forwarding
before updating the information to avoid some serious problems.

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 app/test-pmd/testpmd.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index e4f17f40b6..14c7b09b91 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -3727,6 +3727,8 @@ eth_event_callback(portid_t port_id, enum rte_eth_event_type type, void *param,
 
 	switch (type) {
 	case RTE_ETH_EVENT_NEW:
+		if (test_done == 0)
+			stop_packet_forwarding();
 		config_attached_port(port_id);
 		break;
 	case RTE_ETH_EVENT_INTR_RMV:
@@ -3736,6 +3738,8 @@ eth_event_callback(portid_t port_id, enum rte_eth_event_type type, void *param,
 				"Could not set up deferred device removal\n");
 		break;
 	case RTE_ETH_EVENT_DESTROY:
+		if (test_done == 0)
+			stop_packet_forwarding();
 		ports[port_id].port_status = RTE_PORT_CLOSED;
 		printf("Port %u is closed\n", port_id);
 		if (rte_eal_alarm_set(100000, remove_invalid_ports_callback,
-- 
2.22.0