[dpdk-dev] [PATCH 0/2] Enable eal event hotplug for i40e

[dpdk-dev] [PATCH 0/2] Enable eal event hotplug for i40e

[Jeff Guo]

As we may know, we have eal event for rte device hotplug and ethdev event
for ethdev hotplug. Some ethdev need to use eal event to detect hotplug
behaviors, the privors way is register eal event callback in app, but
seems that it will have some race between these 2 event process. In oder
to fix the problem, it might be better to find a way to combind these 2
events detect.

This patch set introduce a way to combind these 2 event, by register the
eal event callback in pmd driver and trigger the ethdev hotplug event in
the callback. That will let the ethdev device can easy process hotplug by a
common way.

Here let i40 pmd driver for example, other driver which support hotplug
feature could be use this way to enable hotplug.

Jeff Guo (2):
  net/i40e: enable hotplug in i40e
  testpmd: remove the dev event callback register

 app/test-pmd/testpmd.c         | 76 ------------------------------------------
 drivers/net/i40e/i40e_ethdev.c | 46 ++++++++++++++++++++++++-
 2 files changed, 45 insertions(+), 77 deletions(-)

-- 
2.7.4

[dpdk-dev] [PATCH 1/2] net/i40e: enable hotplug in i40e

[Jeff Guo]

This patch aim to enable hotplug in i40e pmd driver. Firstly it set
the flags RTE_PCI_DRV_INTR_RMV in drv_flags to announce the hotplug
ability, and then use rte_dev_event_callback_register to register
the hotplug event callback to eal, when eal detect the hotplug event,
will call the callback to process it, if the event is hotplug remove,
it will trigger the RTE_ETH_EVENT_INTR_RMV event into ethdev callback
to letapp process the hotplug for the ethdev.

This is an example for other driver, that if any driver support hotplug
feature could be use this way to enable hotplug

Signed-off-by: Jeff Guo <jia.guo@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c | 46 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 45 insertions(+), 1 deletion(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 13c5d32..ad4231f 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -688,7 +688,7 @@ static int eth_i40e_pci_remove(struct rte_pci_device *pci_dev)
 static struct rte_pci_driver rte_i40e_pmd = {
 	.id_table = pci_id_i40e_map,
 	.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
-		     RTE_PCI_DRV_IOVA_AS_VA,
+		     RTE_PCI_DRV_IOVA_AS_VA | RTE_PCI_DRV_INTR_RMV,
 	.probe = eth_i40e_pci_probe,
 	.remove = eth_i40e_pci_remove,
 };
@@ -1183,6 +1183,47 @@ i40e_aq_debug_write_global_register(struct i40e_hw *hw,
 	return i40e_aq_debug_write_register(hw, reg_addr, reg_val, cmd_details);
 }
 
+static void
+eth_dev_event_callback(char *device_name, enum rte_dev_event_type type,
+		       __rte_unused void *arg)
+{
+	uint32_t pid;
+
+	if (type >= RTE_DEV_EVENT_MAX) {
+		fprintf(stderr, "%s called upon invalid event %d\n",
+			__func__, type);
+		fflush(stderr);
+	}
+
+	switch (type) {
+	case RTE_DEV_EVENT_REMOVE:
+		PMD_DRV_LOG(INFO, "The device: %s has been removed!\n",
+			    device_name);
+
+		if (!device_name)
+			return;
+
+		for (pid = 0; pid < RTE_MAX_ETHPORTS; pid++) {
+			if (rte_eth_devices[pid].device) {
+				if (!strcmp(device_name,
+				    rte_eth_devices[pid].device->name)) {
+					_rte_eth_dev_callback_process(
+						&rte_eth_devices[pid],
+						RTE_ETH_EVENT_INTR_RMV, NULL);
+					continue;
+				}
+			}
+		}
+		break;
+	case RTE_DEV_EVENT_ADD:
+		RTE_LOG(INFO, EAL, "The device: %s has been added!\n",
+			device_name);
+		break;
+	default:
+		break;
+	}
+}
+
 static int
 eth_i40e_dev_init(struct rte_eth_dev *dev, void *init_params __rte_unused)
 {
@@ -1442,6 +1483,9 @@ eth_i40e_dev_init(struct rte_eth_dev *dev, void *init_params __rte_unused)
 	rte_intr_callback_register(intr_handle,
 				   i40e_dev_interrupt_handler, dev);
 
+	/* register the device event callback */
+	rte_dev_event_callback_register(NULL, eth_dev_event_callback, NULL);
+
 	/* configure and enable device interrupt */
 	i40e_pf_config_irq0(hw, TRUE);
 	i40e_pf_enable_irq0(hw);
-- 
2.7.4

[dpdk-dev] [PATCH 2/2] testpmd: remove the dev event callback register

[Jeff Guo]

Since now we can use driver to management the eal event for hotplug,
so no need to register dev event callback in app anymore. This patch
remove the related code.

Signed-off-by: Jeff Guo <jia.guo@intel.com>
---
 app/test-pmd/testpmd.c | 76 --------------------------------------------------
 1 file changed, 76 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 24c1998..10ed660 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -400,12 +400,6 @@ static void check_all_ports_link_status(uint32_t port_mask);
 static int eth_event_callback(portid_t port_id,
 			      enum rte_eth_event_type type,
 			      void *param, void *ret_param);
-static void eth_dev_event_callback(char *device_name,
-				enum rte_dev_event_type type,
-				void *param);
-static int eth_dev_event_callback_register(void);
-static int eth_dev_event_callback_unregister(void);
-
 
 /*
  * Check if all the ports are started.
@@ -1915,39 +1909,6 @@ reset_port(portid_t pid)
 	printf("Done\n");
 }
 
-static int
-eth_dev_event_callback_register(void)
-{
-	int ret;
-
-	/* register the device event callback */
-	ret = rte_dev_event_callback_register(NULL,
-		eth_dev_event_callback, NULL);
-	if (ret) {
-		printf("Failed to register device event callback\n");
-		return -1;
-	}
-
-	return 0;
-}
-
-
-static int
-eth_dev_event_callback_unregister(void)
-{
-	int ret;
-
-	/* unregister the device event callback */
-	ret = rte_dev_event_callback_unregister(NULL,
-		eth_dev_event_callback, NULL);
-	if (ret < 0) {
-		printf("Failed to unregister device event callback\n");
-		return -1;
-	}
-
-	return 0;
-}
-
 void
 attach_port(char *identifier)
 {
@@ -2049,10 +2010,6 @@ pmd_test_exit(void)
 			RTE_LOG(ERR, EAL,
 				"fail to stop device event monitor.");
 
-		ret = eth_dev_event_callback_unregister();
-		if (ret)
-			RTE_LOG(ERR, EAL,
-				"fail to unregister all event callbacks.");
 	}
 
 	printf("\nBye...\n");
@@ -2191,37 +2148,6 @@ eth_event_callback(portid_t port_id, enum rte_eth_event_type type, void *param,
 	return 0;
 }
 
-/* This function is used by the interrupt thread */
-static void
-eth_dev_event_callback(char *device_name, enum rte_dev_event_type type,
-			     __rte_unused void *arg)
-{
-	if (type >= RTE_DEV_EVENT_MAX) {
-		fprintf(stderr, "%s called upon invalid event %d\n",
-			__func__, type);
-		fflush(stderr);
-	}
-
-	switch (type) {
-	case RTE_DEV_EVENT_REMOVE:
-		RTE_LOG(ERR, EAL, "The device: %s has been removed!\n",
-			device_name);
-		/* TODO: After finish failure handle, begin to stop
-		 * packet forward, stop port, close port, detach port.
-		 */
-		break;
-	case RTE_DEV_EVENT_ADD:
-		RTE_LOG(ERR, EAL, "The device: %s has been added!\n",
-			device_name);
-		/* TODO: After finish kernel driver binding,
-		 * begin to attach port.
-		 */
-		break;
-	default:
-		break;
-	}
-}
-
 static int
 set_tx_queue_stats_mapping_registers(portid_t port_id, struct rte_port *port)
 {
@@ -2735,8 +2661,6 @@ main(int argc, char** argv)
 			rte_errno = EINVAL;
 			return -1;
 		}
-		eth_dev_event_callback_register();
-
 	}
 
 	if (start_port(RTE_PORT_ALL) != 0)
-- 
2.7.4

[dpdk-dev] [PATCH v2 0/3] Enable eal hotplug event detect for i40e/ixgbe

[Jeff Guo]

As we may know, we have eal event for rte device hotplug and ethdev event
for ethdev hotplug. Some ethdev need to use eal event to detect hotplug
behaviors, the privors way is register eal event callback in app, but
seems that it will have some race between these 2 event process. In oder
to fix the problem, it might be better to find a way to combind these 2
events detect.

This patch set introduce a way to combind these 2 event, by register the
eal event callback in pmd driver and trigger the ethdev hotplug event in
the callback. That will let the ethdev device can easy process hotplug by
a common way.

Here let i40e/ixgbe pmd driver for example, other driver which support
hotplug feature could be use this way to enable hotplug.

patch history:
v2->v1:
add ixgbe hotplug detect case.
refine some doc.

Jeff Guo (3):
  net/ixgbe: enable hotplug detect in ixgbe
  net/i40e: enable hotplug detect in i40e
  testpmd: remove the dev event callback register

 app/test-pmd/testpmd.c           | 76 ----------------------------------------
 drivers/net/i40e/i40e_ethdev.c   | 46 +++++++++++++++++++++++-
 drivers/net/ixgbe/ixgbe_ethdev.c | 46 +++++++++++++++++++++++-
 3 files changed, 90 insertions(+), 78 deletions(-)

-- 
2.7.4

[dpdk-dev] [PATCH v2 1/3] net/ixgbe: enable hotplug detect in ixgbe

[Jeff Guo]

This patch aim to enable hotplug detect in ixgbe pmd driver. Firstly it
set the flags RTE_PCI_DRV_INTR_RMV in drv_flags to announce the hotplug
ability, and then use rte_dev_event_callback_register to register
the hotplug event callback to eal. When eal detect the hotplug event,
it will call the callback to process it, if the event is hotplug remove,
it will trigger the RTE_ETH_EVENT_INTR_RMV event into ethdev callback
to let app process the hotplug for the ethdev.

This is an example for other driver, that if any driver support hotplug
feature could be use this way to enable hotplug detect.

Signed-off-by: Jeff Guo <jia.guo@intel.com>
---
v2->v1:
refine some doc.
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 46 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 45 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 87d2ad0..83ce026 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -1534,6 +1534,47 @@ generate_random_mac_addr(struct ether_addr *mac_addr)
 	memcpy(&mac_addr->addr_bytes[3], &random, 3);
 }
 
+static void
+eth_dev_event_callback(char *device_name, enum rte_dev_event_type type,
+		       __rte_unused void *arg)
+{
+	uint32_t pid;
+
+	if (type >= RTE_DEV_EVENT_MAX) {
+		fprintf(stderr, "%s called upon invalid event %d\n",
+			__func__, type);
+		fflush(stderr);
+	}
+
+	switch (type) {
+	case RTE_DEV_EVENT_REMOVE:
+		PMD_DRV_LOG(INFO, "The device: %s has been removed!\n",
+			    device_name);
+
+		if (!device_name)
+			return;
+
+		for (pid = 0; pid < RTE_MAX_ETHPORTS; pid++) {
+			if (rte_eth_devices[pid].device) {
+				if (!strcmp(device_name,
+				    rte_eth_devices[pid].device->name)) {
+					_rte_eth_dev_callback_process(
+						&rte_eth_devices[pid],
+						RTE_ETH_EVENT_INTR_RMV, NULL);
+					continue;
+				}
+			}
+		}
+		break;
+	case RTE_DEV_EVENT_ADD:
+		RTE_LOG(INFO, EAL, "The device: %s has been added!\n",
+			device_name);
+		break;
+	default:
+		break;
+	}
+}
+
 /*
  * Virtual Function device init
  */
@@ -1678,6 +1719,9 @@ eth_ixgbevf_dev_init(struct rte_eth_dev *eth_dev)
 	rte_intr_enable(intr_handle);
 	ixgbevf_intr_enable(eth_dev);
 
+	/* register the device event callback */
+	rte_dev_event_callback_register(NULL, eth_dev_event_callback, NULL);
+
 	PMD_INIT_LOG(DEBUG, "port %d vendorID=0x%x deviceID=0x%x mac.type=%s",
 		     eth_dev->data->port_id, pci_dev->id.vendor_id,
 		     pci_dev->id.device_id, "ixgbe_mac_82599_vf");
@@ -1801,7 +1845,7 @@ static int eth_ixgbe_pci_remove(struct rte_pci_device *pci_dev)
 static struct rte_pci_driver rte_ixgbe_pmd = {
 	.id_table = pci_id_ixgbe_map,
 	.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
-		     RTE_PCI_DRV_IOVA_AS_VA,
+		     RTE_PCI_DRV_IOVA_AS_VA | RTE_PCI_DRV_INTR_RMV,
 	.probe = eth_ixgbe_pci_probe,
 	.remove = eth_ixgbe_pci_remove,
 };
-- 
2.7.4

[dpdk-dev] [PATCH v2 2/3] net/i40e: enable hotplug detect in i40e

[Jeff Guo]

This patch aim to enable hotplug detect in i40e pmd driver. Firstly it
set the flags RTE_PCI_DRV_INTR_RMV in drv_flags to announce the hotplug
ability, and then use rte_dev_event_callback_register to register
the hotplug event callback to eal. When eal detect the hotplug event,
it will call the callback to process it, if the event is hotplug remove,
it will trigger the RTE_ETH_EVENT_INTR_RMV event into ethdev callback
to let app process the hotplug for the ethdev.

This is an example for other driver, that if any driver support hotplug
feature could be use this way to enable hotplug detect.

Signed-off-by: Jeff Guo <jia.guo@intel.com>
---
v2->v1:
no v1, add hotplug detect in ixgbe for new.
---
 drivers/net/i40e/i40e_ethdev.c | 46 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 45 insertions(+), 1 deletion(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 13c5d32..ad4231f 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -688,7 +688,7 @@ static int eth_i40e_pci_remove(struct rte_pci_device *pci_dev)
 static struct rte_pci_driver rte_i40e_pmd = {
 	.id_table = pci_id_i40e_map,
 	.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
-		     RTE_PCI_DRV_IOVA_AS_VA,
+		     RTE_PCI_DRV_IOVA_AS_VA | RTE_PCI_DRV_INTR_RMV,
 	.probe = eth_i40e_pci_probe,
 	.remove = eth_i40e_pci_remove,
 };
@@ -1183,6 +1183,47 @@ i40e_aq_debug_write_global_register(struct i40e_hw *hw,
 	return i40e_aq_debug_write_register(hw, reg_addr, reg_val, cmd_details);
 }
 
+static void
+eth_dev_event_callback(char *device_name, enum rte_dev_event_type type,
+		       __rte_unused void *arg)
+{
+	uint32_t pid;
+
+	if (type >= RTE_DEV_EVENT_MAX) {
+		fprintf(stderr, "%s called upon invalid event %d\n",
+			__func__, type);
+		fflush(stderr);
+	}
+
+	switch (type) {
+	case RTE_DEV_EVENT_REMOVE:
+		PMD_DRV_LOG(INFO, "The device: %s has been removed!\n",
+			    device_name);
+
+		if (!device_name)
+			return;
+
+		for (pid = 0; pid < RTE_MAX_ETHPORTS; pid++) {
+			if (rte_eth_devices[pid].device) {
+				if (!strcmp(device_name,
+				    rte_eth_devices[pid].device->name)) {
+					_rte_eth_dev_callback_process(
+						&rte_eth_devices[pid],
+						RTE_ETH_EVENT_INTR_RMV, NULL);
+					continue;
+				}
+			}
+		}
+		break;
+	case RTE_DEV_EVENT_ADD:
+		RTE_LOG(INFO, EAL, "The device: %s has been added!\n",
+			device_name);
+		break;
+	default:
+		break;
+	}
+}
+
 static int
 eth_i40e_dev_init(struct rte_eth_dev *dev, void *init_params __rte_unused)
 {
@@ -1442,6 +1483,9 @@ eth_i40e_dev_init(struct rte_eth_dev *dev, void *init_params __rte_unused)
 	rte_intr_callback_register(intr_handle,
 				   i40e_dev_interrupt_handler, dev);
 
+	/* register the device event callback */
+	rte_dev_event_callback_register(NULL, eth_dev_event_callback, NULL);
+
 	/* configure and enable device interrupt */
 	i40e_pf_config_irq0(hw, TRUE);
 	i40e_pf_enable_irq0(hw);
-- 
2.7.4

[dpdk-dev] [PATCH v2 3/3] testpmd: remove the dev event callback register

[Jeff Guo]

Since now we can use driver to management the eal event for hotplug,
so no need to register dev event callback in app anymore. This patch
remove the related code.

Signed-off-by: Jeff Guo <jia.guo@intel.com>
---
v2->v1:
no change.
---
 app/test-pmd/testpmd.c | 76 --------------------------------------------------
 1 file changed, 76 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 24c1998..10ed660 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -400,12 +400,6 @@ static void check_all_ports_link_status(uint32_t port_mask);
 static int eth_event_callback(portid_t port_id,
 			      enum rte_eth_event_type type,
 			      void *param, void *ret_param);
-static void eth_dev_event_callback(char *device_name,
-				enum rte_dev_event_type type,
-				void *param);
-static int eth_dev_event_callback_register(void);
-static int eth_dev_event_callback_unregister(void);
-
 
 /*
  * Check if all the ports are started.
@@ -1915,39 +1909,6 @@ reset_port(portid_t pid)
 	printf("Done\n");
 }
 
-static int
-eth_dev_event_callback_register(void)
-{
-	int ret;
-
-	/* register the device event callback */
-	ret = rte_dev_event_callback_register(NULL,
-		eth_dev_event_callback, NULL);
-	if (ret) {
-		printf("Failed to register device event callback\n");
-		return -1;
-	}
-
-	return 0;
-}
-
-
-static int
-eth_dev_event_callback_unregister(void)
-{
-	int ret;
-
-	/* unregister the device event callback */
-	ret = rte_dev_event_callback_unregister(NULL,
-		eth_dev_event_callback, NULL);
-	if (ret < 0) {
-		printf("Failed to unregister device event callback\n");
-		return -1;
-	}
-
-	return 0;
-}
-
 void
 attach_port(char *identifier)
 {
@@ -2049,10 +2010,6 @@ pmd_test_exit(void)
 			RTE_LOG(ERR, EAL,
 				"fail to stop device event monitor.");
 
-		ret = eth_dev_event_callback_unregister();
-		if (ret)
-			RTE_LOG(ERR, EAL,
-				"fail to unregister all event callbacks.");
 	}
 
 	printf("\nBye...\n");
@@ -2191,37 +2148,6 @@ eth_event_callback(portid_t port_id, enum rte_eth_event_type type, void *param,
 	return 0;
 }
 
-/* This function is used by the interrupt thread */
-static void
-eth_dev_event_callback(char *device_name, enum rte_dev_event_type type,
-			     __rte_unused void *arg)
-{
-	if (type >= RTE_DEV_EVENT_MAX) {
-		fprintf(stderr, "%s called upon invalid event %d\n",
-			__func__, type);
-		fflush(stderr);
-	}
-
-	switch (type) {
-	case RTE_DEV_EVENT_REMOVE:
-		RTE_LOG(ERR, EAL, "The device: %s has been removed!\n",
-			device_name);
-		/* TODO: After finish failure handle, begin to stop
-		 * packet forward, stop port, close port, detach port.
-		 */
-		break;
-	case RTE_DEV_EVENT_ADD:
-		RTE_LOG(ERR, EAL, "The device: %s has been added!\n",
-			device_name);
-		/* TODO: After finish kernel driver binding,
-		 * begin to attach port.
-		 */
-		break;
-	default:
-		break;
-	}
-}
-
 static int
 set_tx_queue_stats_mapping_registers(portid_t port_id, struct rte_port *port)
 {
@@ -2735,8 +2661,6 @@ main(int argc, char** argv)
 			rte_errno = EINVAL;
 			return -1;
 		}
-		eth_dev_event_callback_register();
-
 	}
 
 	if (start_port(RTE_PORT_ALL) != 0)
-- 
2.7.4

Re: [dpdk-dev] [PATCH v2 1/3] net/ixgbe: enable hotplug detect in ixgbe

[Lu, Wenzhuo]

Hi Jeff,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jeff Guo
> Sent: Monday, July 9, 2018 2:57 PM
> To: stephen@networkplumber.org; Richardson, Bruce
> <bruce.richardson@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>;
> Ananyev, Konstantin <konstantin.ananyev@intel.com>;
> gaetan.rivet@6wind.com; Wu, Jingjing <jingjing.wu@intel.com>;
> thomas@monjalon.net; motih@mellanox.com; matan@mellanox.com; Van
> Haaren, Harry <harry.van.haaren@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>; He, Shaopeng <shaopeng.he@intel.com>;
> Iremonger, Bernard <bernard.iremonger@intel.com>;
> arybchenko@solarflare.com
> Cc: jblunck@infradead.org; shreyansh.jain@nxp.com; dev@dpdk.org; Guo,
> Jia <jia.guo@intel.com>; Zhang, Helin <helin.zhang@intel.com>
> Subject: [dpdk-dev] [PATCH v2 1/3] net/ixgbe: enable hotplug detect in ixgbe
> 
> This patch aim to enable hotplug detect in ixgbe pmd driver. Firstly it set the
> flags RTE_PCI_DRV_INTR_RMV in drv_flags to announce the hotplug ability,
> and then use rte_dev_event_callback_register to register the hotplug event
> callback to eal. When eal detect the hotplug event, it will call the callback to
> process it, if the event is hotplug remove, it will trigger the
> RTE_ETH_EVENT_INTR_RMV event into ethdev callback to let app process
> the hotplug for the ethdev.
> 
> This is an example for other driver, that if any driver support hotplug feature
> could be use this way to enable hotplug detect.
> 
> Signed-off-by: Jeff Guo <jia.guo@intel.com>
> ---
> v2->v1:
> refine some doc.
> ---
>  drivers/net/ixgbe/ixgbe_ethdev.c | 46
> +++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 45 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> b/drivers/net/ixgbe/ixgbe_ethdev.c
> index 87d2ad0..83ce026 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -1534,6 +1534,47 @@ generate_random_mac_addr(struct ether_addr
> *mac_addr)
>  	memcpy(&mac_addr->addr_bytes[3], &random, 3);  }
> 
> +static void
> +eth_dev_event_callback(char *device_name, enum rte_dev_event_type
> type,
> +		       __rte_unused void *arg)
> +{
> +	uint32_t pid;
> +
> +	if (type >= RTE_DEV_EVENT_MAX) {
> +		fprintf(stderr, "%s called upon invalid event %d\n",
> +			__func__, type);
> +		fflush(stderr);
> +	}
> +
> +	switch (type) {
> +	case RTE_DEV_EVENT_REMOVE:
> +		PMD_DRV_LOG(INFO, "The device: %s has been removed!\n",
> +			    device_name);
> +
> +		if (!device_name)
> +			return;
> +
> +		for (pid = 0; pid < RTE_MAX_ETHPORTS; pid++) {
> +			if (rte_eth_devices[pid].device) {
> +				if (!strcmp(device_name,
> +				    rte_eth_devices[pid].device->name)) {
> +					_rte_eth_dev_callback_process(
> +						&rte_eth_devices[pid],
> +						RTE_ETH_EVENT_INTR_RMV,
> NULL);
> +					continue;
> +				}
> +			}
> +		}
> +		break;
> +	case RTE_DEV_EVENT_ADD:
> +		RTE_LOG(INFO, EAL, "The device: %s has been added!\n",
> +			device_name);
> +		break;
> +	default:
> +		break;
> +	}
> +}
I don't get the point. Looks like this's a very common rte code. Why is it put in ixgbe pmd?

Re: [dpdk-dev] [PATCH v2 3/3] testpmd: remove the dev event callback register

[Lu, Wenzhuo]

Hi,


> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jeff Guo
> Sent: Monday, July 9, 2018 2:57 PM
> To: stephen@networkplumber.org; Richardson, Bruce
> <bruce.richardson@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>;
> Ananyev, Konstantin <konstantin.ananyev@intel.com>;
> gaetan.rivet@6wind.com; Wu, Jingjing <jingjing.wu@intel.com>;
> thomas@monjalon.net; motih@mellanox.com; matan@mellanox.com; Van
> Haaren, Harry <harry.van.haaren@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>; He, Shaopeng <shaopeng.he@intel.com>;
> Iremonger, Bernard <bernard.iremonger@intel.com>;
> arybchenko@solarflare.com
> Cc: jblunck@infradead.org; shreyansh.jain@nxp.com; dev@dpdk.org; Guo,
> Jia <jia.guo@intel.com>; Zhang, Helin <helin.zhang@intel.com>
> Subject: [dpdk-dev] [PATCH v2 3/3] testpmd: remove the dev event callback
> register
> 
> Since now we can use driver to management the eal event for hotplug, so no
> need to register dev event callback in app anymore. This patch remove the
> related code.
> 
> Signed-off-by: Jeff Guo <jia.guo@intel.com>
Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>

Re: [dpdk-dev] [PATCH v2 2/3] net/i40e: enable hotplug detect in i40e

[Matan Azrad]

Hi Guo

From: Jeff Guo
> This patch aim to enable hotplug detect in i40e pmd driver. Firstly it set the
> flags RTE_PCI_DRV_INTR_RMV in drv_flags to announce the hotplug ability,
> and then use rte_dev_event_callback_register to register the hotplug event
> callback to eal. When eal detect the hotplug event, it will call the callback to
> process it, if the event is hotplug remove, it will trigger the
> RTE_ETH_EVENT_INTR_RMV event into ethdev callback to let app process
> the hotplug for the ethdev.
> 
> This is an example for other driver, that if any driver support hotplug feature
> could be use this way to enable hotplug detect.
> 
> Signed-off-by: Jeff Guo <jia.guo@intel.com>
> ---
> v2->v1:
> no v1, add hotplug detect in ixgbe for new.
> ---
>  drivers/net/i40e/i40e_ethdev.c | 46
> +++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 45 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 13c5d32..ad4231f 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -688,7 +688,7 @@ static int eth_i40e_pci_remove(struct rte_pci_device
> *pci_dev)  static struct rte_pci_driver rte_i40e_pmd = {
>  	.id_table = pci_id_i40e_map,
>  	.drv_flags = RTE_PCI_DRV_NEED_MAPPING |
> RTE_PCI_DRV_INTR_LSC |
> -		     RTE_PCI_DRV_IOVA_AS_VA,
> +		     RTE_PCI_DRV_IOVA_AS_VA | RTE_PCI_DRV_INTR_RMV,
>  	.probe = eth_i40e_pci_probe,
>  	.remove = eth_i40e_pci_remove,
>  };
> @@ -1183,6 +1183,47 @@ i40e_aq_debug_write_global_register(struct
> i40e_hw *hw,
>  	return i40e_aq_debug_write_register(hw, reg_addr, reg_val,
> cmd_details);  }
> 
> +static void
> +eth_dev_event_callback(char *device_name, enum rte_dev_event_type
> type,
> +		       __rte_unused void *arg)
> +{
> +	uint32_t pid;
> +
> +	if (type >= RTE_DEV_EVENT_MAX) {
> +		fprintf(stderr, "%s called upon invalid event %d\n",
> +			__func__, type);
> +		fflush(stderr);
> +	}
> +
> +	switch (type) {
> +	case RTE_DEV_EVENT_REMOVE:
> +		PMD_DRV_LOG(INFO, "The device: %s has been
> removed!\n",
> +			    device_name);
> +
> +		if (!device_name)
> +			return;
> +
> +		for (pid = 0; pid < RTE_MAX_ETHPORTS; pid++) {
> +			if (rte_eth_devices[pid].device) {
> +				if (!strcmp(device_name,
> +				    rte_eth_devices[pid].device->name)) {

You just need to compare this PMD ethdev ports device names to the current EAL removed device name.
You should not raise RMV events for other PMD  ports.

> +					_rte_eth_dev_callback_process(
> +						&rte_eth_devices[pid],
> +						RTE_ETH_EVENT_INTR_RMV,
> NULL);
> +					continue;
> +				}
> +			}
> +		}
> +		break;
> +	case RTE_DEV_EVENT_ADD:
> +		RTE_LOG(INFO, EAL, "The device: %s has been added!\n",
> +			device_name);
> +		break;
> +	default:
> +		break;
> +	}
> +}
> +
>  static int
>  eth_i40e_dev_init(struct rte_eth_dev *dev, void *init_params
> __rte_unused)  { @@ -1442,6 +1483,9 @@ eth_i40e_dev_init(struct
> rte_eth_dev *dev, void *init_params __rte_unused)
>  	rte_intr_callback_register(intr_handle,
>  				   i40e_dev_interrupt_handler, dev);
> 
> +	/* register the device event callback */
> +	rte_dev_event_callback_register(NULL, eth_dev_event_callback,
> NULL);
> +
>  	/* configure and enable device interrupt */
>  	i40e_pf_config_irq0(hw, TRUE);
>  	i40e_pf_enable_irq0(hw);
> --
> 2.7.4

Re: [dpdk-dev] [PATCH v2 1/3] net/ixgbe: enable hotplug detect in ixgbe

[Matan Azrad]

Hi

From: Lu, Wenzhuo
> Hi Jeff,
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jeff Guo
> > Sent: Monday, July 9, 2018 2:57 PM
> > To: stephen@networkplumber.org; Richardson, Bruce
> > <bruce.richardson@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>;
> > Ananyev, Konstantin <konstantin.ananyev@intel.com>;
> > gaetan.rivet@6wind.com; Wu, Jingjing <jingjing.wu@intel.com>;
> > thomas@monjalon.net; motih@mellanox.com; matan@mellanox.com;
> Van
> > Haaren, Harry <harry.van.haaren@intel.com>; Zhang, Qi Z
> > <qi.z.zhang@intel.com>; He, Shaopeng <shaopeng.he@intel.com>;
> > Iremonger, Bernard <bernard.iremonger@intel.com>;
> > arybchenko@solarflare.com
> > Cc: jblunck@infradead.org; shreyansh.jain@nxp.com; dev@dpdk.org; Guo,
> > Jia <jia.guo@intel.com>; Zhang, Helin <helin.zhang@intel.com>
> > Subject: [dpdk-dev] [PATCH v2 1/3] net/ixgbe: enable hotplug detect in
> > ixgbe
> >
> > This patch aim to enable hotplug detect in ixgbe pmd driver. Firstly
> > it set the flags RTE_PCI_DRV_INTR_RMV in drv_flags to announce the
> > hotplug ability, and then use rte_dev_event_callback_register to
> > register the hotplug event callback to eal. When eal detect the
> > hotplug event, it will call the callback to process it, if the event
> > is hotplug remove, it will trigger the RTE_ETH_EVENT_INTR_RMV event
> > into ethdev callback to let app process the hotplug for the ethdev.
> >
> > This is an example for other driver, that if any driver support
> > hotplug feature could be use this way to enable hotplug detect.
> >
> > Signed-off-by: Jeff Guo <jia.guo@intel.com>
> > ---
> > v2->v1:
> > refine some doc.
> > ---
> >  drivers/net/ixgbe/ixgbe_ethdev.c | 46
> > +++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 45 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> > b/drivers/net/ixgbe/ixgbe_ethdev.c
> > index 87d2ad0..83ce026 100644
> > --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> > +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> > @@ -1534,6 +1534,47 @@ generate_random_mac_addr(struct ether_addr
> > *mac_addr)
> >  	memcpy(&mac_addr->addr_bytes[3], &random, 3);  }
> >
> > +static void
> > +eth_dev_event_callback(char *device_name, enum rte_dev_event_type
> > type,
> > +		       __rte_unused void *arg)
> > +{
> > +	uint32_t pid;
> > +
> > +	if (type >= RTE_DEV_EVENT_MAX) {
> > +		fprintf(stderr, "%s called upon invalid event %d\n",
> > +			__func__, type);
> > +		fflush(stderr);
> > +	}
> > +
> > +	switch (type) {
> > +	case RTE_DEV_EVENT_REMOVE:
> > +		PMD_DRV_LOG(INFO, "The device: %s has been
> removed!\n",
> > +			    device_name);
> > +
> > +		if (!device_name)
> > +			return;
> > +
> > +		for (pid = 0; pid < RTE_MAX_ETHPORTS; pid++) {
> > +			if (rte_eth_devices[pid].device) {
> > +				if (!strcmp(device_name,
> > +				    rte_eth_devices[pid].device->name)) {
> > +					_rte_eth_dev_callback_process(
> > +						&rte_eth_devices[pid],
> > +						RTE_ETH_EVENT_INTR_RMV,
> > NULL);
> > +					continue;
> > +				}
> > +			}
> > +		}
> > +		break;
> > +	case RTE_DEV_EVENT_ADD:
> > +		RTE_LOG(INFO, EAL, "The device: %s has been added!\n",
> > +			device_name);
> > +		break;
> > +	default:
> > +		break;
> > +	}
> > +}
> I don't get the point. Looks like this's a very common rte code. Why is it put in
> ixgbe pmd?

Jeff needs to detect if the removed device is related to this PMD, than to raise RMV events for all this PMD ethdev associated ports. 
He should not raise RMV events for other PMD  ports.

Re: [dpdk-dev] [PATCH v2 1/3] net/ixgbe: enable hotplug detect in ixgbe

[Andrew Rybchenko]

On 09.07.2018 09:56, Jeff Guo wrote:
> This patch aim to enable hotplug detect in ixgbe pmd driver. Firstly it
> set the flags RTE_PCI_DRV_INTR_RMV in drv_flags to announce the hotplug
> ability, and then use rte_dev_event_callback_register to register
> the hotplug event callback to eal. When eal detect the hotplug event,
> it will call the callback to process it, if the event is hotplug remove,
> it will trigger the RTE_ETH_EVENT_INTR_RMV event into ethdev callback
> to let app process the hotplug for the ethdev.
>
> This is an example for other driver, that if any driver support hotplug
> feature could be use this way to enable hotplug detect.

I see nothing ixgbe specific in the callback. Yes, support of removal
event should be in drv_flags, but it looks like the callback may be
generic and located in ethdev.

Also search of the device by name could be done using querying
mechanism to be added by Gaetan [1].

[1] https://patches.dpdk.org/project/dpdk/list/?series=419

Re: [dpdk-dev] [PATCH v2 3/3] testpmd: remove the dev event callback register

[Andrew Rybchenko]

On 09.07.2018 09:56, Jeff Guo wrote:
> Since now we can use driver to management the eal event for hotplug,
> so no need to register dev event callback in app anymore. This patch
> remove the related code.

I don't understand why handling on device level means removal
of the application callback. May be as a cleanup.
I guess application still could be interested in device addition and
removal events. It is mainly question to testpmd maintainer.

Re: [dpdk-dev] [PATCH v2 3/3] testpmd: remove the dev event callback register

[Jeff Guo]

On 7/9/2018 4:16 PM, Andrew Rybchenko wrote:
> On 09.07.2018 09:56, Jeff Guo wrote:
>> Since now we can use driver to management the eal event for hotplug,
>> so no need to register dev event callback in app anymore. This patch
>> remove the related code.
>
> I don't understand why handling on device level means removal
> of the application callback. May be as a cleanup.
> I guess application still could be interested in device addition and
> removal events. It is mainly question to testpmd maintainer.
>

I think the callback could be used by anyone who interesting it. you are 
right, but It is optional, who use it will surely in charge of the event 
and callback management.
Here remove it, just for select an other choice and no select the 
previous way to show hotplug example.
just select 1 from 2, no need to let 2 combined.

Re: [dpdk-dev] [PATCH v2 1/3] net/ixgbe: enable hotplug detect in ixgbe

[Jeff Guo]

On 7/9/2018 4:13 PM, Andrew Rybchenko wrote:
> On 09.07.2018 09:56, Jeff Guo wrote:
>> This patch aim to enable hotplug detect in ixgbe pmd driver. Firstly it
>> set the flags RTE_PCI_DRV_INTR_RMV in drv_flags to announce the hotplug
>> ability, and then use rte_dev_event_callback_register to register
>> the hotplug event callback to eal. When eal detect the hotplug event,
>> it will call the callback to process it, if the event is hotplug remove,
>> it will trigger the RTE_ETH_EVENT_INTR_RMV event into ethdev callback
>> to let app process the hotplug for the ethdev.
>>
>> This is an example for other driver, that if any driver support hotplug
>> feature could be use this way to enable hotplug detect.
>
> I see nothing ixgbe specific in the callback. Yes, support of removal
> event should be in drv_flags, but it looks like the callback may be
> generic and located in ethdev.
>

Let it be generic and located in ethdev should be a good idea.

> Also search of the device by name could be done using querying
> mechanism to be added by Gaetan [1].
>
> [1] https://patches.dpdk.org/project/dpdk/list/?series=419

here, i just want to check if the eth port is belong to the removal device.

Re: [dpdk-dev] [PATCH v2 2/3] net/i40e: enable hotplug detect in i40e

[Jeff Guo]

hi, matan


On 7/9/2018 3:47 PM, Matan Azrad wrote:
> Hi Guo
>
> From: Jeff Guo
>> This patch aim to enable hotplug detect in i40e pmd driver. Firstly it set the
>> flags RTE_PCI_DRV_INTR_RMV in drv_flags to announce the hotplug ability,
>> and then use rte_dev_event_callback_register to register the hotplug event
>> callback to eal. When eal detect the hotplug event, it will call the callback to
>> process it, if the event is hotplug remove, it will trigger the
>> RTE_ETH_EVENT_INTR_RMV event into ethdev callback to let app process
>> the hotplug for the ethdev.
>>
>> This is an example for other driver, that if any driver support hotplug feature
>> could be use this way to enable hotplug detect.
>>
>> Signed-off-by: Jeff Guo <jia.guo@intel.com>
>> ---
>> v2->v1:
>> no v1, add hotplug detect in ixgbe for new.
>> ---
>>   drivers/net/i40e/i40e_ethdev.c | 46
>> +++++++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 45 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
>> index 13c5d32..ad4231f 100644
>> --- a/drivers/net/i40e/i40e_ethdev.c
>> +++ b/drivers/net/i40e/i40e_ethdev.c
>> @@ -688,7 +688,7 @@ static int eth_i40e_pci_remove(struct rte_pci_device
>> *pci_dev)  static struct rte_pci_driver rte_i40e_pmd = {
>>   	.id_table = pci_id_i40e_map,
>>   	.drv_flags = RTE_PCI_DRV_NEED_MAPPING |
>> RTE_PCI_DRV_INTR_LSC |
>> -		     RTE_PCI_DRV_IOVA_AS_VA,
>> +		     RTE_PCI_DRV_IOVA_AS_VA | RTE_PCI_DRV_INTR_RMV,
>>   	.probe = eth_i40e_pci_probe,
>>   	.remove = eth_i40e_pci_remove,
>>   };
>> @@ -1183,6 +1183,47 @@ i40e_aq_debug_write_global_register(struct
>> i40e_hw *hw,
>>   	return i40e_aq_debug_write_register(hw, reg_addr, reg_val,
>> cmd_details);  }
>>
>> +static void
>> +eth_dev_event_callback(char *device_name, enum rte_dev_event_type
>> type,
>> +		       __rte_unused void *arg)
>> +{
>> +	uint32_t pid;
>> +
>> +	if (type >= RTE_DEV_EVENT_MAX) {
>> +		fprintf(stderr, "%s called upon invalid event %d\n",
>> +			__func__, type);
>> +		fflush(stderr);
>> +	}
>> +
>> +	switch (type) {
>> +	case RTE_DEV_EVENT_REMOVE:
>> +		PMD_DRV_LOG(INFO, "The device: %s has been
>> removed!\n",
>> +			    device_name);
>> +
>> +		if (!device_name)
>> +			return;
>> +
>> +		for (pid = 0; pid < RTE_MAX_ETHPORTS; pid++) {
>> +			if (rte_eth_devices[pid].device) {
>> +				if (!strcmp(device_name,
>> +				    rte_eth_devices[pid].device->name)) {
> You just need to compare this PMD ethdev ports device names to the current EAL removed device name.
> You should not raise RMV events for other PMD  ports.

make sense here. thanks matan.

>> +					_rte_eth_dev_callback_process(
>> +						&rte_eth_devices[pid],
>> +						RTE_ETH_EVENT_INTR_RMV,
>> NULL);
>> +					continue;
>> +				}
>> +			}
>> +		}
>> +		break;
>> +	case RTE_DEV_EVENT_ADD:
>> +		RTE_LOG(INFO, EAL, "The device: %s has been added!\n",
>> +			device_name);
>> +		break;
>> +	default:
>> +		break;
>> +	}
>> +}
>> +
>>   static int
>>   eth_i40e_dev_init(struct rte_eth_dev *dev, void *init_params
>> __rte_unused)  { @@ -1442,6 +1483,9 @@ eth_i40e_dev_init(struct
>> rte_eth_dev *dev, void *init_params __rte_unused)
>>   	rte_intr_callback_register(intr_handle,
>>   				   i40e_dev_interrupt_handler, dev);
>>
>> +	/* register the device event callback */
>> +	rte_dev_event_callback_register(NULL, eth_dev_event_callback,
>> NULL);
>> +
>>   	/* configure and enable device interrupt */
>>   	i40e_pf_config_irq0(hw, TRUE);
>>   	i40e_pf_enable_irq0(hw);
>> --
>> 2.7.4

Re: [dpdk-dev] [PATCH v2 1/3] net/ixgbe: enable hotplug detect in ixgbe

[Jeff Guo]

hi, wenzhuo and matan.


On 7/9/2018 3:51 PM, Matan Azrad wrote:
> Hi
>
> From: Lu, Wenzhuo
>> Hi Jeff,
>>
>>> -----Original Message-----
>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jeff Guo
>>> Sent: Monday, July 9, 2018 2:57 PM
>>> To: stephen@networkplumber.org; Richardson, Bruce
>>> <bruce.richardson@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>;
>>> Ananyev, Konstantin <konstantin.ananyev@intel.com>;
>>> gaetan.rivet@6wind.com; Wu, Jingjing <jingjing.wu@intel.com>;
>>> thomas@monjalon.net; motih@mellanox.com; matan@mellanox.com;
>> Van
>>> Haaren, Harry <harry.van.haaren@intel.com>; Zhang, Qi Z
>>> <qi.z.zhang@intel.com>; He, Shaopeng <shaopeng.he@intel.com>;
>>> Iremonger, Bernard <bernard.iremonger@intel.com>;
>>> arybchenko@solarflare.com
>>> Cc: jblunck@infradead.org; shreyansh.jain@nxp.com; dev@dpdk.org; Guo,
>>> Jia <jia.guo@intel.com>; Zhang, Helin <helin.zhang@intel.com>
>>> Subject: [dpdk-dev] [PATCH v2 1/3] net/ixgbe: enable hotplug detect in
>>> ixgbe
>>>
>>> This patch aim to enable hotplug detect in ixgbe pmd driver. Firstly
>>> it set the flags RTE_PCI_DRV_INTR_RMV in drv_flags to announce the
>>> hotplug ability, and then use rte_dev_event_callback_register to
>>> register the hotplug event callback to eal. When eal detect the
>>> hotplug event, it will call the callback to process it, if the event
>>> is hotplug remove, it will trigger the RTE_ETH_EVENT_INTR_RMV event
>>> into ethdev callback to let app process the hotplug for the ethdev.
>>>
>>> This is an example for other driver, that if any driver support
>>> hotplug feature could be use this way to enable hotplug detect.
>>>
>>> Signed-off-by: Jeff Guo <jia.guo@intel.com>
>>> ---
>>> v2->v1:
>>> refine some doc.
>>> ---
>>>   drivers/net/ixgbe/ixgbe_ethdev.c | 46
>>> +++++++++++++++++++++++++++++++++++++++-
>>>   1 file changed, 45 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
>>> b/drivers/net/ixgbe/ixgbe_ethdev.c
>>> index 87d2ad0..83ce026 100644
>>> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
>>> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
>>> @@ -1534,6 +1534,47 @@ generate_random_mac_addr(struct ether_addr
>>> *mac_addr)
>>>   	memcpy(&mac_addr->addr_bytes[3], &random, 3);  }
>>>
>>> +static void
>>> +eth_dev_event_callback(char *device_name, enum rte_dev_event_type
>>> type,
>>> +		       __rte_unused void *arg)
>>> +{
>>> +	uint32_t pid;
>>> +
>>> +	if (type >= RTE_DEV_EVENT_MAX) {
>>> +		fprintf(stderr, "%s called upon invalid event %d\n",
>>> +			__func__, type);
>>> +		fflush(stderr);
>>> +	}
>>> +
>>> +	switch (type) {
>>> +	case RTE_DEV_EVENT_REMOVE:
>>> +		PMD_DRV_LOG(INFO, "The device: %s has been
>> removed!\n",
>>> +			    device_name);
>>> +
>>> +		if (!device_name)
>>> +			return;
>>> +
>>> +		for (pid = 0; pid < RTE_MAX_ETHPORTS; pid++) {
>>> +			if (rte_eth_devices[pid].device) {
>>> +				if (!strcmp(device_name,
>>> +				    rte_eth_devices[pid].device->name)) {
>>> +					_rte_eth_dev_callback_process(
>>> +						&rte_eth_devices[pid],
>>> +						RTE_ETH_EVENT_INTR_RMV,
>>> NULL);
>>> +					continue;
>>> +				}
>>> +			}
>>> +		}
>>> +		break;
>>> +	case RTE_DEV_EVENT_ADD:
>>> +		RTE_LOG(INFO, EAL, "The device: %s has been added!\n",
>>> +			device_name);
>>> +		break;
>>> +	default:
>>> +		break;
>>> +	}
>>> +}
>> I don't get the point. Looks like this's a very common rte code. Why is it put in
>> ixgbe pmd?
> Jeff needs to detect if the removed device is related to this PMD, than to raise RMV events for all this PMD ethdev associated ports.
> He should not raise RMV events for other PMD  ports.
>

It should be like wenzhuo said that i could no strong reason to let 
common way in ixgbe pmd.  And sure raise RMV events for none related PMD 
ports is not my hope.
Will plan to let it go into the eth dev layer to process it.

>

Re: [dpdk-dev] [PATCH v2 1/3] net/ixgbe: enable hotplug detect in ixgbe

[Matan Azrad]

Hi

From: Jeff Guo 
> hi, wenzhuo and matan.
> 
> 
> On 7/9/2018 3:51 PM, Matan Azrad wrote:
> > Hi
> >
> > From: Lu, Wenzhuo
> >> Hi Jeff,
> >>
> >>> -----Original Message-----
> >>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jeff Guo
> >>> Sent: Monday, July 9, 2018 2:57 PM
> >>> To: stephen@networkplumber.org; Richardson, Bruce
> >>> <bruce.richardson@intel.com>; Yigit, Ferruh
> >>> <ferruh.yigit@intel.com>; Ananyev, Konstantin
> >>> <konstantin.ananyev@intel.com>; gaetan.rivet@6wind.com; Wu,
> Jingjing
> >>> <jingjing.wu@intel.com>; thomas@monjalon.net;
> motih@mellanox.com;
> >>> matan@mellanox.com;
> >> Van
> >>> Haaren, Harry <harry.van.haaren@intel.com>; Zhang, Qi Z
> >>> <qi.z.zhang@intel.com>; He, Shaopeng <shaopeng.he@intel.com>;
> >>> Iremonger, Bernard <bernard.iremonger@intel.com>;
> >>> arybchenko@solarflare.com
> >>> Cc: jblunck@infradead.org; shreyansh.jain@nxp.com; dev@dpdk.org;
> >>> Guo, Jia <jia.guo@intel.com>; Zhang, Helin <helin.zhang@intel.com>
> >>> Subject: [dpdk-dev] [PATCH v2 1/3] net/ixgbe: enable hotplug detect
> >>> in ixgbe
> >>>
> >>> This patch aim to enable hotplug detect in ixgbe pmd driver. Firstly
> >>> it set the flags RTE_PCI_DRV_INTR_RMV in drv_flags to announce the
> >>> hotplug ability, and then use rte_dev_event_callback_register to
> >>> register the hotplug event callback to eal. When eal detect the
> >>> hotplug event, it will call the callback to process it, if the event
> >>> is hotplug remove, it will trigger the RTE_ETH_EVENT_INTR_RMV event
> >>> into ethdev callback to let app process the hotplug for the ethdev.
> >>>
> >>> This is an example for other driver, that if any driver support
> >>> hotplug feature could be use this way to enable hotplug detect.
> >>>
> >>> Signed-off-by: Jeff Guo <jia.guo@intel.com>
> >>> ---
> >>> v2->v1:
> >>> refine some doc.
> >>> ---
> >>>   drivers/net/ixgbe/ixgbe_ethdev.c | 46
> >>> +++++++++++++++++++++++++++++++++++++++-
> >>>   1 file changed, 45 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> >>> b/drivers/net/ixgbe/ixgbe_ethdev.c
> >>> index 87d2ad0..83ce026 100644
> >>> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> >>> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> >>> @@ -1534,6 +1534,47 @@ generate_random_mac_addr(struct
> ether_addr
> >>> *mac_addr)
> >>>   	memcpy(&mac_addr->addr_bytes[3], &random, 3);  }
> >>>
> >>> +static void
> >>> +eth_dev_event_callback(char *device_name, enum
> rte_dev_event_type
> >>> type,
> >>> +		       __rte_unused void *arg)
> >>> +{
> >>> +	uint32_t pid;
> >>> +
> >>> +	if (type >= RTE_DEV_EVENT_MAX) {
> >>> +		fprintf(stderr, "%s called upon invalid event %d\n",
> >>> +			__func__, type);
> >>> +		fflush(stderr);
> >>> +	}
> >>> +
> >>> +	switch (type) {
> >>> +	case RTE_DEV_EVENT_REMOVE:
> >>> +		PMD_DRV_LOG(INFO, "The device: %s has been
> >> removed!\n",
> >>> +			    device_name);
> >>> +
> >>> +		if (!device_name)
> >>> +			return;
> >>> +
> >>> +		for (pid = 0; pid < RTE_MAX_ETHPORTS; pid++) {
> >>> +			if (rte_eth_devices[pid].device) {
> >>> +				if (!strcmp(device_name,
> >>> +				    rte_eth_devices[pid].device->name)) {
> >>> +					_rte_eth_dev_callback_process(
> >>> +						&rte_eth_devices[pid],
> >>> +						RTE_ETH_EVENT_INTR_RMV,
> >>> NULL);
> >>> +					continue;
> >>> +				}
> >>> +			}
> >>> +		}
> >>> +		break;
> >>> +	case RTE_DEV_EVENT_ADD:
> >>> +		RTE_LOG(INFO, EAL, "The device: %s has been added!\n",
> >>> +			device_name);
> >>> +		break;
> >>> +	default:
> >>> +		break;
> >>> +	}
> >>> +}
> >> I don't get the point. Looks like this's a very common rte code. Why
> >> is it put in ixgbe pmd?
> > Jeff needs to detect if the removed device is related to this PMD, than to
> raise RMV events for all this PMD ethdev associated ports.
> > He should not raise RMV events for other PMD  ports.
> >
> 
> It should be like wenzhuo said that i could no strong reason to let common
> way in ixgbe pmd.  And sure raise RMV events for none related PMD ports is
> not my hope.
> Will plan to let it go into the eth dev layer to process it.
> 

How can you run ethdev function from EAL context?
How can the ethdev layer know which ports are related to the EAL device removal?
How can ethdev layer know if the port supports removal?

Re: [dpdk-dev] [PATCH v2 1/3] net/ixgbe: enable hotplug detect in ixgbe

[Jeff Guo]

On 7/9/2018 5:04 PM, Matan Azrad wrote:
> Hi
>
> From: Jeff Guo
>> hi, wenzhuo and matan.
>>
>>
>> On 7/9/2018 3:51 PM, Matan Azrad wrote:
>>> Hi
>>>
>>> From: Lu, Wenzhuo
>>>> Hi Jeff,
>>>>
>>>>> -----Original Message-----
>>>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jeff Guo
>>>>> Sent: Monday, July 9, 2018 2:57 PM
>>>>> To: stephen@networkplumber.org; Richardson, Bruce
>>>>> <bruce.richardson@intel.com>; Yigit, Ferruh
>>>>> <ferruh.yigit@intel.com>; Ananyev, Konstantin
>>>>> <konstantin.ananyev@intel.com>; gaetan.rivet@6wind.com; Wu,
>> Jingjing
>>>>> <jingjing.wu@intel.com>; thomas@monjalon.net;
>> motih@mellanox.com;
>>>>> matan@mellanox.com;
>>>> Van
>>>>> Haaren, Harry <harry.van.haaren@intel.com>; Zhang, Qi Z
>>>>> <qi.z.zhang@intel.com>; He, Shaopeng <shaopeng.he@intel.com>;
>>>>> Iremonger, Bernard <bernard.iremonger@intel.com>;
>>>>> arybchenko@solarflare.com
>>>>> Cc: jblunck@infradead.org; shreyansh.jain@nxp.com; dev@dpdk.org;
>>>>> Guo, Jia <jia.guo@intel.com>; Zhang, Helin <helin.zhang@intel.com>
>>>>> Subject: [dpdk-dev] [PATCH v2 1/3] net/ixgbe: enable hotplug detect
>>>>> in ixgbe
>>>>>
>>>>> This patch aim to enable hotplug detect in ixgbe pmd driver. Firstly
>>>>> it set the flags RTE_PCI_DRV_INTR_RMV in drv_flags to announce the
>>>>> hotplug ability, and then use rte_dev_event_callback_register to
>>>>> register the hotplug event callback to eal. When eal detect the
>>>>> hotplug event, it will call the callback to process it, if the event
>>>>> is hotplug remove, it will trigger the RTE_ETH_EVENT_INTR_RMV event
>>>>> into ethdev callback to let app process the hotplug for the ethdev.
>>>>>
>>>>> This is an example for other driver, that if any driver support
>>>>> hotplug feature could be use this way to enable hotplug detect.
>>>>>
>>>>> Signed-off-by: Jeff Guo <jia.guo@intel.com>
>>>>> ---
>>>>> v2->v1:
>>>>> refine some doc.
>>>>> ---
>>>>>    drivers/net/ixgbe/ixgbe_ethdev.c | 46
>>>>> +++++++++++++++++++++++++++++++++++++++-
>>>>>    1 file changed, 45 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
>>>>> b/drivers/net/ixgbe/ixgbe_ethdev.c
>>>>> index 87d2ad0..83ce026 100644
>>>>> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
>>>>> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
>>>>> @@ -1534,6 +1534,47 @@ generate_random_mac_addr(struct
>> ether_addr
>>>>> *mac_addr)
>>>>>    	memcpy(&mac_addr->addr_bytes[3], &random, 3);  }
>>>>>
>>>>> +static void
>>>>> +eth_dev_event_callback(char *device_name, enum
>> rte_dev_event_type
>>>>> type,
>>>>> +		       __rte_unused void *arg)
>>>>> +{
>>>>> +	uint32_t pid;
>>>>> +
>>>>> +	if (type >= RTE_DEV_EVENT_MAX) {
>>>>> +		fprintf(stderr, "%s called upon invalid event %d\n",
>>>>> +			__func__, type);
>>>>> +		fflush(stderr);
>>>>> +	}
>>>>> +
>>>>> +	switch (type) {
>>>>> +	case RTE_DEV_EVENT_REMOVE:
>>>>> +		PMD_DRV_LOG(INFO, "The device: %s has been
>>>> removed!\n",
>>>>> +			    device_name);
>>>>> +
>>>>> +		if (!device_name)
>>>>> +			return;
>>>>> +
>>>>> +		for (pid = 0; pid < RTE_MAX_ETHPORTS; pid++) {
>>>>> +			if (rte_eth_devices[pid].device) {
>>>>> +				if (!strcmp(device_name,
>>>>> +				    rte_eth_devices[pid].device->name)) {
>>>>> +					_rte_eth_dev_callback_process(
>>>>> +						&rte_eth_devices[pid],
>>>>> +						RTE_ETH_EVENT_INTR_RMV,
>>>>> NULL);
>>>>> +					continue;
>>>>> +				}
>>>>> +			}
>>>>> +		}
>>>>> +		break;
>>>>> +	case RTE_DEV_EVENT_ADD:
>>>>> +		RTE_LOG(INFO, EAL, "The device: %s has been added!\n",
>>>>> +			device_name);
>>>>> +		break;
>>>>> +	default:
>>>>> +		break;
>>>>> +	}
>>>>> +}
>>>> I don't get the point. Looks like this's a very common rte code. Why
>>>> is it put in ixgbe pmd?
>>> Jeff needs to detect if the removed device is related to this PMD, than to
>> raise RMV events for all this PMD ethdev associated ports.
>>> He should not raise RMV events for other PMD  ports.
>>>
>> It should be like wenzhuo said that i could no strong reason to let common
>> way in ixgbe pmd.  And sure raise RMV events for none related PMD ports is
>> not my hope.
>> Will plan to let it go into the eth dev layer to process it.
>>
> How can you run ethdev function from EAL context?
> How can the ethdev layer know which ports are related to the EAL device removal?
> How can ethdev layer know if the port supports removal?

i mean that still let driver manage the callback , just let the common 
ethdev functional in ethdev layer.
It just define "rte_eth_dev_event_callback" in ethdev layer, and 
register the common ethdev callback in pmd driver as bellow. the eth_dev 
could be pass by the whole process.

     rte_dev_event_callback_register(eth_dev->device->name,
                     rte_eth_dev_event_callback,
                     (void *)eth_dev);

Re: [dpdk-dev] [PATCH v2 1/3] net/ixgbe: enable hotplug detect in ixgbe

[Matan Azrad]

Hi

From: Jeff Guo
> On 7/9/2018 5:04 PM, Matan Azrad wrote:
> > Hi
> >
> > From: Jeff Guo
> >> hi, wenzhuo and matan.
> >>
> >>
> >> On 7/9/2018 3:51 PM, Matan Azrad wrote:
> >>> Hi
> >>>
> >>> From: Lu, Wenzhuo
> >>>> Hi Jeff,
> >>>>
> >>>>> -----Original Message-----
> >>>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jeff Guo
> >>>>> Sent: Monday, July 9, 2018 2:57 PM
> >>>>> To: stephen@networkplumber.org; Richardson, Bruce
> >>>>> <bruce.richardson@intel.com>; Yigit, Ferruh
> >>>>> <ferruh.yigit@intel.com>; Ananyev, Konstantin
> >>>>> <konstantin.ananyev@intel.com>; gaetan.rivet@6wind.com; Wu,
> >> Jingjing
> >>>>> <jingjing.wu@intel.com>; thomas@monjalon.net;
> >> motih@mellanox.com;
> >>>>> matan@mellanox.com;
> >>>> Van
> >>>>> Haaren, Harry <harry.van.haaren@intel.com>; Zhang, Qi Z
> >>>>> <qi.z.zhang@intel.com>; He, Shaopeng <shaopeng.he@intel.com>;
> >>>>> Iremonger, Bernard <bernard.iremonger@intel.com>;
> >>>>> arybchenko@solarflare.com
> >>>>> Cc: jblunck@infradead.org; shreyansh.jain@nxp.com; dev@dpdk.org;
> >>>>> Guo, Jia <jia.guo@intel.com>; Zhang, Helin <helin.zhang@intel.com>
> >>>>> Subject: [dpdk-dev] [PATCH v2 1/3] net/ixgbe: enable hotplug
> >>>>> detect in ixgbe
> >>>>>
> >>>>> This patch aim to enable hotplug detect in ixgbe pmd driver.
> >>>>> Firstly it set the flags RTE_PCI_DRV_INTR_RMV in drv_flags to
> >>>>> announce the hotplug ability, and then use
> >>>>> rte_dev_event_callback_register to register the hotplug event
> >>>>> callback to eal. When eal detect the hotplug event, it will call
> >>>>> the callback to process it, if the event is hotplug remove, it
> >>>>> will trigger the RTE_ETH_EVENT_INTR_RMV event into ethdev
> callback to let app process the hotplug for the ethdev.
> >>>>>
> >>>>> This is an example for other driver, that if any driver support
> >>>>> hotplug feature could be use this way to enable hotplug detect.
> >>>>>
> >>>>> Signed-off-by: Jeff Guo <jia.guo@intel.com>
> >>>>> ---
> >>>>> v2->v1:
> >>>>> refine some doc.
> >>>>> ---
> >>>>>    drivers/net/ixgbe/ixgbe_ethdev.c | 46
> >>>>> +++++++++++++++++++++++++++++++++++++++-
> >>>>>    1 file changed, 45 insertions(+), 1 deletion(-)
> >>>>>
> >>>>> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> >>>>> b/drivers/net/ixgbe/ixgbe_ethdev.c
> >>>>> index 87d2ad0..83ce026 100644
> >>>>> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> >>>>> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> >>>>> @@ -1534,6 +1534,47 @@ generate_random_mac_addr(struct
> >> ether_addr
> >>>>> *mac_addr)
> >>>>>    	memcpy(&mac_addr->addr_bytes[3], &random, 3);  }
> >>>>>
> >>>>> +static void
> >>>>> +eth_dev_event_callback(char *device_name, enum
> >> rte_dev_event_type
> >>>>> type,
> >>>>> +		       __rte_unused void *arg)
> >>>>> +{
> >>>>> +	uint32_t pid;
> >>>>> +
> >>>>> +	if (type >= RTE_DEV_EVENT_MAX) {
> >>>>> +		fprintf(stderr, "%s called upon invalid event %d\n",
> >>>>> +			__func__, type);
> >>>>> +		fflush(stderr);
> >>>>> +	}
> >>>>> +
> >>>>> +	switch (type) {
> >>>>> +	case RTE_DEV_EVENT_REMOVE:
> >>>>> +		PMD_DRV_LOG(INFO, "The device: %s has been
> >>>> removed!\n",
> >>>>> +			    device_name);
> >>>>> +
> >>>>> +		if (!device_name)
> >>>>> +			return;
> >>>>> +
> >>>>> +		for (pid = 0; pid < RTE_MAX_ETHPORTS; pid++) {
> >>>>> +			if (rte_eth_devices[pid].device) {
> >>>>> +				if (!strcmp(device_name,
> >>>>> +				    rte_eth_devices[pid].device-
> >name)) {
> >>>>> +
> 	_rte_eth_dev_callback_process(
> >>>>> +
> 	&rte_eth_devices[pid],
> >>>>> +
> 	RTE_ETH_EVENT_INTR_RMV,
> >>>>> NULL);
> >>>>> +					continue;
> >>>>> +				}
> >>>>> +			}
> >>>>> +		}
> >>>>> +		break;
> >>>>> +	case RTE_DEV_EVENT_ADD:
> >>>>> +		RTE_LOG(INFO, EAL, "The device: %s has been
> added!\n",
> >>>>> +			device_name);
> >>>>> +		break;
> >>>>> +	default:
> >>>>> +		break;
> >>>>> +	}
> >>>>> +}
> >>>> I don't get the point. Looks like this's a very common rte code.
> >>>> Why is it put in ixgbe pmd?
> >>> Jeff needs to detect if the removed device is related to this PMD,
> >>> than to
> >> raise RMV events for all this PMD ethdev associated ports.
> >>> He should not raise RMV events for other PMD  ports.
> >>>
> >> It should be like wenzhuo said that i could no strong reason to let
> >> common way in ixgbe pmd.  And sure raise RMV events for none related
> >> PMD ports is not my hope.
> >> Will plan to let it go into the eth dev layer to process it.
> >>
> > How can you run ethdev function from EAL context?
> > How can the ethdev layer know which ports are related to the EAL device
> removal?
> > How can ethdev layer know if the port supports removal?
> 
> i mean that still let driver manage the callback , just let the common ethdev
> functional in ethdev layer.
> It just define "rte_eth_dev_event_callback" in ethdev layer, and register the
> common ethdev callback in pmd driver as bellow. the eth_dev could be pass
> by the whole process.
> 
>      rte_dev_event_callback_register(eth_dev->device->name,
>                      rte_eth_dev_event_callback,
>                      (void *)eth_dev);
> 

Sorry, but I don't understand, can you explain step by step the notification path?

Re: [dpdk-dev] [PATCH v2 1/3] net/ixgbe: enable hotplug detect in ixgbe

[Jeff Guo]

On 7/9/2018 6:01 PM, Matan Azrad wrote:
> Hi
>
> From: Jeff Guo
>> On 7/9/2018 5:04 PM, Matan Azrad wrote:
>>> Hi
>>>
>>> From: Jeff Guo
>>>> hi, wenzhuo and matan.
>>>>
>>>>
>>>> On 7/9/2018 3:51 PM, Matan Azrad wrote:
>>>>> Hi
>>>>>
>>>>> From: Lu, Wenzhuo
>>>>>> Hi Jeff,
>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jeff Guo
>>>>>>> Sent: Monday, July 9, 2018 2:57 PM
>>>>>>> To: stephen@networkplumber.org; Richardson, Bruce
>>>>>>> <bruce.richardson@intel.com>; Yigit, Ferruh
>>>>>>> <ferruh.yigit@intel.com>; Ananyev, Konstantin
>>>>>>> <konstantin.ananyev@intel.com>; gaetan.rivet@6wind.com; Wu,
>>>> Jingjing
>>>>>>> <jingjing.wu@intel.com>; thomas@monjalon.net;
>>>> motih@mellanox.com;
>>>>>>> matan@mellanox.com;
>>>>>> Van
>>>>>>> Haaren, Harry <harry.van.haaren@intel.com>; Zhang, Qi Z
>>>>>>> <qi.z.zhang@intel.com>; He, Shaopeng <shaopeng.he@intel.com>;
>>>>>>> Iremonger, Bernard <bernard.iremonger@intel.com>;
>>>>>>> arybchenko@solarflare.com
>>>>>>> Cc: jblunck@infradead.org; shreyansh.jain@nxp.com; dev@dpdk.org;
>>>>>>> Guo, Jia <jia.guo@intel.com>; Zhang, Helin <helin.zhang@intel.com>
>>>>>>> Subject: [dpdk-dev] [PATCH v2 1/3] net/ixgbe: enable hotplug
>>>>>>> detect in ixgbe
>>>>>>>
>>>>>>> This patch aim to enable hotplug detect in ixgbe pmd driver.
>>>>>>> Firstly it set the flags RTE_PCI_DRV_INTR_RMV in drv_flags to
>>>>>>> announce the hotplug ability, and then use
>>>>>>> rte_dev_event_callback_register to register the hotplug event
>>>>>>> callback to eal. When eal detect the hotplug event, it will call
>>>>>>> the callback to process it, if the event is hotplug remove, it
>>>>>>> will trigger the RTE_ETH_EVENT_INTR_RMV event into ethdev
>> callback to let app process the hotplug for the ethdev.
>>>>>>> This is an example for other driver, that if any driver support
>>>>>>> hotplug feature could be use this way to enable hotplug detect.
>>>>>>>
>>>>>>> Signed-off-by: Jeff Guo <jia.guo@intel.com>
>>>>>>> ---
>>>>>>> v2->v1:
>>>>>>> refine some doc.
>>>>>>> ---
>>>>>>>     drivers/net/ixgbe/ixgbe_ethdev.c | 46
>>>>>>> +++++++++++++++++++++++++++++++++++++++-
>>>>>>>     1 file changed, 45 insertions(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
>>>>>>> b/drivers/net/ixgbe/ixgbe_ethdev.c
>>>>>>> index 87d2ad0..83ce026 100644
>>>>>>> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
>>>>>>> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
>>>>>>> @@ -1534,6 +1534,47 @@ generate_random_mac_addr(struct
>>>> ether_addr
>>>>>>> *mac_addr)
>>>>>>>     	memcpy(&mac_addr->addr_bytes[3], &random, 3);  }
>>>>>>>
>>>>>>> +static void
>>>>>>> +eth_dev_event_callback(char *device_name, enum
>>>> rte_dev_event_type
>>>>>>> type,
>>>>>>> +		       __rte_unused void *arg)
>>>>>>> +{
>>>>>>> +	uint32_t pid;
>>>>>>> +
>>>>>>> +	if (type >= RTE_DEV_EVENT_MAX) {
>>>>>>> +		fprintf(stderr, "%s called upon invalid event %d\n",
>>>>>>> +			__func__, type);
>>>>>>> +		fflush(stderr);
>>>>>>> +	}
>>>>>>> +
>>>>>>> +	switch (type) {
>>>>>>> +	case RTE_DEV_EVENT_REMOVE:
>>>>>>> +		PMD_DRV_LOG(INFO, "The device: %s has been
>>>>>> removed!\n",
>>>>>>> +			    device_name);
>>>>>>> +
>>>>>>> +		if (!device_name)
>>>>>>> +			return;
>>>>>>> +
>>>>>>> +		for (pid = 0; pid < RTE_MAX_ETHPORTS; pid++) {
>>>>>>> +			if (rte_eth_devices[pid].device) {
>>>>>>> +				if (!strcmp(device_name,
>>>>>>> +				    rte_eth_devices[pid].device-
>>> name)) {
>>>>>>> +
>> 	_rte_eth_dev_callback_process(
>>>>>>> +
>> 	&rte_eth_devices[pid],
>>>>>>> +
>> 	RTE_ETH_EVENT_INTR_RMV,
>>>>>>> NULL);
>>>>>>> +					continue;
>>>>>>> +				}
>>>>>>> +			}
>>>>>>> +		}
>>>>>>> +		break;
>>>>>>> +	case RTE_DEV_EVENT_ADD:
>>>>>>> +		RTE_LOG(INFO, EAL, "The device: %s has been
>> added!\n",
>>>>>>> +			device_name);
>>>>>>> +		break;
>>>>>>> +	default:
>>>>>>> +		break;
>>>>>>> +	}
>>>>>>> +}
>>>>>> I don't get the point. Looks like this's a very common rte code.
>>>>>> Why is it put in ixgbe pmd?
>>>>> Jeff needs to detect if the removed device is related to this PMD,
>>>>> than to
>>>> raise RMV events for all this PMD ethdev associated ports.
>>>>> He should not raise RMV events for other PMD  ports.
>>>>>
>>>> It should be like wenzhuo said that i could no strong reason to let
>>>> common way in ixgbe pmd.  And sure raise RMV events for none related
>>>> PMD ports is not my hope.
>>>> Will plan to let it go into the eth dev layer to process it.
>>>>
>>> How can you run ethdev function from EAL context?
>>> How can the ethdev layer know which ports are related to the EAL device
>> removal?
>>> How can ethdev layer know if the port supports removal?
>> i mean that still let driver manage the callback , just let the common ethdev
>> functional in ethdev layer.
>> It just define "rte_eth_dev_event_callback" in ethdev layer, and register the
>> common ethdev callback in pmd driver as bellow. the eth_dev could be pass
>> by the whole process.
>>
>>       rte_dev_event_callback_register(eth_dev->device->name,
>>                       rte_eth_dev_event_callback,
>>                       (void *)eth_dev);
>>
> Sorry, but I don't understand, can you explain step by step the notification path?

the step should be:
1) add a ethdev driver api "rte_dev_event_callback_register" in the 
rte_ethdev_driver.h, let pmd driver call it.
      rte_eth_dev_event_callback(char *device_name, enum 
rte_dev_event_type event, void *cb_arg);

2) register eth eal device event callback in pmd driver as below, the 
rte eth (eth_dev) could be set to cb_arg of the callback.

rte_dev_event_callback_register(eth_dev->device->name,
                      rte_eth_dev_event_callback,
                      (void *)eth_dev);

3)when hotplug event detect, the callback be called, then could be use 
the device name of the eth_dev to compare the hotplug eal device name.

4) go to the common way of RMV eth dev hotplug process.

[dpdk-dev] [PATCH v3 0/4] Enable eal hotplug event detect for i40e/ixgbe

[Jeff Guo]

As we may know, we have eal event for rte device hotplug and ethdev event
for ethdev hotplug. Some ethdev need to use eal event to detect hotplug
behaviors, the privors way is register eal event callback in app, but seems
that it will have some race between these 2 event processes. In oder to fix
the it, it might be better to find a way to combind these 2 events detect.

This patch set introduce a way to combind these 2 event, by register the
ethdev eal event callback in pmd driver and trigger the ethdev hotplug event
in the callback. That will let the ethdev device can easy process hotplug
by a common way.

Here let i40e/ixgbe pmd driver for example, other driver which support
hotplug feature could be use this way to detect and process hotplug.

patch history:
v3->v2:
remove the callback from driver to ethdev for common.

v2->v1:
add ixgbe hotplug detect case.
refine some doc.


Jeff Guo (4):
  ethdev: Add eal device event callback
  net/ixgbe: enable hotplug detect in ixgbe
  net/i40e: enable hotplug detect in i40e
  testpmd: remove the dev event callback register

 app/test-pmd/testpmd.c                 | 76 ----------------------------------
 doc/guides/rel_notes/release_18_08.rst |  8 ++++
 drivers/net/i40e/i40e_ethdev.c         |  7 +++-
 drivers/net/ixgbe/ixgbe_ethdev.c       |  7 +++-
 lib/librte_ethdev/rte_ethdev.c         | 37 +++++++++++++++++
 lib/librte_ethdev/rte_ethdev_driver.h  | 20 +++++++++
 6 files changed, 77 insertions(+), 78 deletions(-)

-- 
2.7.4

[dpdk-dev] [PATCH v3 1/4] ethdev: Add eal device event callback

[Jeff Guo]

Implement a eal device event callback "rte_eth_dev_event_callback"
in ethdev, it could let pmd driver have chance to manage the eal
device event, such as process hotplug event.

Signed-off-by: Jeff Guo <jia.guo@intel.com>
---
v3->v2:
add new callback in ethdev
---
 doc/guides/rel_notes/release_18_08.rst |  8 ++++++++
 lib/librte_ethdev/rte_ethdev.c         | 37 ++++++++++++++++++++++++++++++++++
 lib/librte_ethdev/rte_ethdev_driver.h  | 20 ++++++++++++++++++
 3 files changed, 65 insertions(+)

diff --git a/doc/guides/rel_notes/release_18_08.rst b/doc/guides/rel_notes/release_18_08.rst
index bc01242..2326058 100644
--- a/doc/guides/rel_notes/release_18_08.rst
+++ b/doc/guides/rel_notes/release_18_08.rst
@@ -46,6 +46,14 @@ New Features
   Flow API support has been added to CXGBE Poll Mode Driver to offload
   flows to Chelsio T5/T6 NICs.
 
+* **Added eal device event callback in ethdev for hotplug.**
+
+  Implement a eal device event callback in ethdev, it could let pmd driver
+  have chance to manage the eal device event, such as process hotplug event.
+
+  * ``rte_eth_dev_event_callback`` for driver use to register it and process
+    eal device event.
+
 
 API Changes
 -----------
diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index a9977df..36f218a 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -4518,6 +4518,43 @@ rte_eth_devargs_parse(const char *dargs, struct rte_eth_devargs *eth_da)
 	return result;
 }
 
+void __rte_experimental
+rte_eth_dev_event_callback(char *device_name, enum rte_dev_event_type type,
+			     void *arg)
+{
+	struct rte_eth_dev *eth_dev = (struct rte_eth_dev *)arg;
+
+	if (type >= RTE_DEV_EVENT_MAX) {
+		fprintf(stderr, "%s called upon invalid event %d\n",
+			__func__, type);
+		fflush(stderr);
+	}
+
+	switch (type) {
+	case RTE_DEV_EVENT_REMOVE:
+		ethdev_log(INFO, "The device: %s has been removed!\n",
+			    device_name);
+
+		if (!device_name || !eth_dev)
+			return;
+
+		if (!(eth_dev->data->dev_flags & RTE_ETH_EVENT_INTR_RMV))
+			return;
+
+		if (!strcmp(device_name, eth_dev->device->name))
+			_rte_eth_dev_callback_process(eth_dev,
+						      RTE_ETH_EVENT_INTR_RMV,
+						      NULL);
+		break;
+	case RTE_DEV_EVENT_ADD:
+		ethdev_log(INFO, "The device: %s has been added!\n",
+			device_name);
+		break;
+	default:
+		break;
+	}
+}
+
 RTE_INIT(ethdev_init_log);
 static void
 ethdev_init_log(void)
diff --git a/lib/librte_ethdev/rte_ethdev_driver.h b/lib/librte_ethdev/rte_ethdev_driver.h
index c9c825e..fed5afa 100644
--- a/lib/librte_ethdev/rte_ethdev_driver.h
+++ b/lib/librte_ethdev/rte_ethdev_driver.h
@@ -82,6 +82,26 @@ int rte_eth_dev_release_port(struct rte_eth_dev *eth_dev);
 void _rte_eth_dev_reset(struct rte_eth_dev *dev);
 
 /**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Implement a rte eth eal device event callbacks for the specific device.
+ *
+ * @param device_name
+ *  Pointer to the name of the rte device.
+ * @param event
+ *  Eal device event type.
+ * @param ret_param
+ *  To pass data back to user application.
+ *
+ * @return
+ *  void
+ */
+void __rte_experimental
+rte_eth_dev_event_callback(char *device_name,
+		enum rte_dev_event_type event, void *cb_arg);
+
+/**
  * @internal Executes all the user application registered callbacks for
  * the specific device. It is for DPDK internal user only. User
  * application should not call it directly.
-- 
2.7.4

[dpdk-dev] [PATCH v3 2/4] net/ixgbe: enable hotplug detect in ixgbe

[Jeff Guo]

This patch aim to enable hotplug detect in ixgbe pmd driver. Firstly it
set the flags RTE_PCI_DRV_INTR_RMV in drv_flags to announce the hotplug
ability, and then use rte_dev_event_callback_register to register
the hotplug event callback to eal. When eal detect the hotplug event,
it will call the callback to process it, if the event is hotplug remove,
it will trigger the RTE_ETH_EVENT_INTR_RMV event into ethdev callback
to let app process the hotplug for the ethdev.

This is an example for other driver, that if any driver support hotplug
feature could be use this way to enable hotplug detect.

Signed-off-by: Jeff Guo <jia.guo@intel.com>
---
v3->v2:
remove the callback from driver to ethdev.
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 87d2ad0..a1c2588 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -1678,6 +1678,11 @@ eth_ixgbevf_dev_init(struct rte_eth_dev *eth_dev)
 	rte_intr_enable(intr_handle);
 	ixgbevf_intr_enable(eth_dev);
 
+	/* register the device event callback */
+	rte_dev_event_callback_register(eth_dev->device->name,
+					rte_eth_dev_event_callback,
+					(void *)eth_dev);
+
 	PMD_INIT_LOG(DEBUG, "port %d vendorID=0x%x deviceID=0x%x mac.type=%s",
 		     eth_dev->data->port_id, pci_dev->id.vendor_id,
 		     pci_dev->id.device_id, "ixgbe_mac_82599_vf");
@@ -1801,7 +1806,7 @@ static int eth_ixgbe_pci_remove(struct rte_pci_device *pci_dev)
 static struct rte_pci_driver rte_ixgbe_pmd = {
 	.id_table = pci_id_ixgbe_map,
 	.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
-		     RTE_PCI_DRV_IOVA_AS_VA,
+		     RTE_PCI_DRV_IOVA_AS_VA | RTE_PCI_DRV_INTR_RMV,
 	.probe = eth_ixgbe_pci_probe,
 	.remove = eth_ixgbe_pci_remove,
 };
-- 
2.7.4

[dpdk-dev] [PATCH v3 3/4] net/i40e: enable hotplug detect in i40e

[Jeff Guo]

This patch aim to enable hotplug detect in i40e pmd driver. Firstly it
set the flags RTE_PCI_DRV_INTR_RMV in drv_flags to announce the hotplug
ability, and then use rte_dev_event_callback_register to register
the ethdev eal device event callback. When eal detect the hotplug event,
it will call the callback to process it, if the event is hotplug remove,
it will trigger the RTE_ETH_EVENT_INTR_RMV event into ethdev callback
to let app process the hotplug for the ethdev.

This is an example for other driver, that if any driver support hotplug
feature could be use this way to enable hotplug detect.

Signed-off-by: Jeff Guo <jia.guo@intel.com>
---
v3->v2:
remove the callback from driver to ethdev.
---
 drivers/net/i40e/i40e_ethdev.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 13c5d32..d79cac1 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -688,7 +688,7 @@ static int eth_i40e_pci_remove(struct rte_pci_device *pci_dev)
 static struct rte_pci_driver rte_i40e_pmd = {
 	.id_table = pci_id_i40e_map,
 	.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
-		     RTE_PCI_DRV_IOVA_AS_VA,
+		     RTE_PCI_DRV_IOVA_AS_VA | RTE_PCI_DRV_INTR_RMV,
 	.probe = eth_i40e_pci_probe,
 	.remove = eth_i40e_pci_remove,
 };
@@ -1442,6 +1442,11 @@ eth_i40e_dev_init(struct rte_eth_dev *dev, void *init_params __rte_unused)
 	rte_intr_callback_register(intr_handle,
 				   i40e_dev_interrupt_handler, dev);
 
+	/* register the device event callback */
+	rte_dev_event_callback_register(dev->device->name,
+					rte_eth_dev_event_callback,
+					(void *)dev);
+
 	/* configure and enable device interrupt */
 	i40e_pf_config_irq0(hw, TRUE);
 	i40e_pf_enable_irq0(hw);
-- 
2.7.4

[dpdk-dev] [PATCH v3 4/4] testpmd: remove the dev event callback register

[Jeff Guo]

Since now we can use driver to management the eal event for hotplug,
so no need to register dev event callback in app anymore. This patch
remove the related code.

Signed-off-by: Jeff Guo <jia.guo@intel.com>
Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
v3->v2:
no change.
---
 app/test-pmd/testpmd.c | 76 --------------------------------------------------
 1 file changed, 76 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 24c1998..10ed660 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -400,12 +400,6 @@ static void check_all_ports_link_status(uint32_t port_mask);
 static int eth_event_callback(portid_t port_id,
 			      enum rte_eth_event_type type,
 			      void *param, void *ret_param);
-static void eth_dev_event_callback(char *device_name,
-				enum rte_dev_event_type type,
-				void *param);
-static int eth_dev_event_callback_register(void);
-static int eth_dev_event_callback_unregister(void);
-
 
 /*
  * Check if all the ports are started.
@@ -1915,39 +1909,6 @@ reset_port(portid_t pid)
 	printf("Done\n");
 }
 
-static int
-eth_dev_event_callback_register(void)
-{
-	int ret;
-
-	/* register the device event callback */
-	ret = rte_dev_event_callback_register(NULL,
-		eth_dev_event_callback, NULL);
-	if (ret) {
-		printf("Failed to register device event callback\n");
-		return -1;
-	}
-
-	return 0;
-}
-
-
-static int
-eth_dev_event_callback_unregister(void)
-{
-	int ret;
-
-	/* unregister the device event callback */
-	ret = rte_dev_event_callback_unregister(NULL,
-		eth_dev_event_callback, NULL);
-	if (ret < 0) {
-		printf("Failed to unregister device event callback\n");
-		return -1;
-	}
-
-	return 0;
-}
-
 void
 attach_port(char *identifier)
 {
@@ -2049,10 +2010,6 @@ pmd_test_exit(void)
 			RTE_LOG(ERR, EAL,
 				"fail to stop device event monitor.");
 
-		ret = eth_dev_event_callback_unregister();
-		if (ret)
-			RTE_LOG(ERR, EAL,
-				"fail to unregister all event callbacks.");
 	}
 
 	printf("\nBye...\n");
@@ -2191,37 +2148,6 @@ eth_event_callback(portid_t port_id, enum rte_eth_event_type type, void *param,
 	return 0;
 }
 
-/* This function is used by the interrupt thread */
-static void
-eth_dev_event_callback(char *device_name, enum rte_dev_event_type type,
-			     __rte_unused void *arg)
-{
-	if (type >= RTE_DEV_EVENT_MAX) {
-		fprintf(stderr, "%s called upon invalid event %d\n",
-			__func__, type);
-		fflush(stderr);
-	}
-
-	switch (type) {
-	case RTE_DEV_EVENT_REMOVE:
-		RTE_LOG(ERR, EAL, "The device: %s has been removed!\n",
-			device_name);
-		/* TODO: After finish failure handle, begin to stop
-		 * packet forward, stop port, close port, detach port.
-		 */
-		break;
-	case RTE_DEV_EVENT_ADD:
-		RTE_LOG(ERR, EAL, "The device: %s has been added!\n",
-			device_name);
-		/* TODO: After finish kernel driver binding,
-		 * begin to attach port.
-		 */
-		break;
-	default:
-		break;
-	}
-}
-
 static int
 set_tx_queue_stats_mapping_registers(portid_t port_id, struct rte_port *port)
 {
@@ -2735,8 +2661,6 @@ main(int argc, char** argv)
 			rte_errno = EINVAL;
 			return -1;
 		}
-		eth_dev_event_callback_register();
-
 	}
 
 	if (start_port(RTE_PORT_ALL) != 0)
-- 
2.7.4

Re: [dpdk-dev] [PATCH v3 1/4] ethdev: Add eal device event callback

[Andrew Rybchenko]

On 09.07.2018 14:46, Jeff Guo wrote:
> Implement a eal device event callback "rte_eth_dev_event_callback"
> in ethdev, it could let pmd driver have chance to manage the eal
> device event, such as process hotplug event.
  >
> Signed-off-by: Jeff Guo <jia.guo@intel.com>
> ---
> v3->v2:
> add new callback in ethdev
> ---
>   doc/guides/rel_notes/release_18_08.rst |  8 ++++++++
>   lib/librte_ethdev/rte_ethdev.c         | 37 ++++++++++++++++++++++++++++++++++
>   lib/librte_ethdev/rte_ethdev_driver.h  | 20 ++++++++++++++++++
>   3 files changed, 65 insertions(+)
>
> diff --git a/doc/guides/rel_notes/release_18_08.rst b/doc/guides/rel_notes/release_18_08.rst
> index bc01242..2326058 100644
> --- a/doc/guides/rel_notes/release_18_08.rst
> +++ b/doc/guides/rel_notes/release_18_08.rst
> @@ -46,6 +46,14 @@ New Features
>     Flow API support has been added to CXGBE Poll Mode Driver to offload
>     flows to Chelsio T5/T6 NICs.
>   
> +* **Added eal device event callback in ethdev for hotplug.**
> +
> +  Implement a eal device event callback in ethdev, it could let pmd driver

"pmd driver" sounds strange since PMD stands for poll-mode driver.

> +  have chance to manage the eal device event, such as process hotplug event.
> +
> +  * ``rte_eth_dev_event_callback`` for driver use to register it and process
> +    eal device event.
> +
>   
>   API Changes
>   -----------
> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> index a9977df..36f218a 100644
> --- a/lib/librte_ethdev/rte_ethdev.c
> +++ b/lib/librte_ethdev/rte_ethdev.c
> @@ -4518,6 +4518,43 @@ rte_eth_devargs_parse(const char *dargs, struct rte_eth_devargs *eth_da)
>   	return result;
>   }
>   
> +void __rte_experimental
> +rte_eth_dev_event_callback(char *device_name, enum rte_dev_event_type type,
> +			     void *arg)
> +{
> +	struct rte_eth_dev *eth_dev = (struct rte_eth_dev *)arg;
> +
> +	if (type >= RTE_DEV_EVENT_MAX) {
> +		fprintf(stderr, "%s called upon invalid event %d\n",
> +			__func__, type);
> +		fflush(stderr);

I'd like to understand why fprintf() is used here for logging instead of 
rte_log
mechanisms.
Also if we really want the log, may be it make sense to move the if to 
default
case below.

> +	}
> +
> +	switch (type) {
> +	case RTE_DEV_EVENT_REMOVE:
> +		ethdev_log(INFO, "The device: %s has been removed!\n",
> +			    device_name);
> +
> +		if (!device_name || !eth_dev)
> +			return;
> +
> +		if (!(eth_dev->data->dev_flags & RTE_ETH_EVENT_INTR_RMV))
> +			return;
> +
> +		if (!strcmp(device_name, eth_dev->device->name))

Do we really need to check it? The callback is registered for devices
with such name, so it should be always true. May be it is OK to double-check
I just want to be sure that I understand it properly.

> +			_rte_eth_dev_callback_process(eth_dev,
> +						      RTE_ETH_EVENT_INTR_RMV,
> +						      NULL);
> +		break;
> +	case RTE_DEV_EVENT_ADD:
> +		ethdev_log(INFO, "The device: %s has been added!\n",
> +			device_name);
> +		break;
> +	default:
> +		break;
> +	}
> +}
> +
>   RTE_INIT(ethdev_init_log);
>   static void
>   ethdev_init_log(void)
> diff --git a/lib/librte_ethdev/rte_ethdev_driver.h b/lib/librte_ethdev/rte_ethdev_driver.h
> index c9c825e..fed5afa 100644
> --- a/lib/librte_ethdev/rte_ethdev_driver.h
> +++ b/lib/librte_ethdev/rte_ethdev_driver.h
> @@ -82,6 +82,26 @@ int rte_eth_dev_release_port(struct rte_eth_dev *eth_dev);
>   void _rte_eth_dev_reset(struct rte_eth_dev *dev);
>   
>   /**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice.
> + *
> + * Implement a rte eth eal device event callbacks for the specific device.
> + *
> + * @param device_name
> + *  Pointer to the name of the rte device.

Is it name of the device which generates the event? If so, it should be 
highlighted.

> + * @param event
> + *  Eal device event type.
> + * @param ret_param
> + *  To pass data back to user application.
> + *
> + * @return
> + *  void
> + */
> +void __rte_experimental
> +rte_eth_dev_event_callback(char *device_name,
> +		enum rte_dev_event_type event, void *cb_arg);
> +
> +/**
>    * @internal Executes all the user application registered callbacks for
>    * the specific device. It is for DPDK internal user only. User
>    * application should not call it directly.

Re: [dpdk-dev] [PATCH v3 1/4] ethdev: Add eal device event callback

[Jeff Guo]

hi, andrew


On 7/9/2018 9:14 PM, Andrew Rybchenko wrote:
> On 09.07.2018 14:46, Jeff Guo wrote:
>> Implement a eal device event callback "rte_eth_dev_event_callback"
>> in ethdev, it could let pmd driver have chance to manage the eal
>> device event, such as process hotplug event.
>  >
>> Signed-off-by: Jeff Guo <jia.guo@intel.com>
>> ---
>> v3->v2:
>> add new callback in ethdev
>> ---
>>   doc/guides/rel_notes/release_18_08.rst |  8 ++++++++
>>   lib/librte_ethdev/rte_ethdev.c         | 37 
>> ++++++++++++++++++++++++++++++++++
>>   lib/librte_ethdev/rte_ethdev_driver.h  | 20 ++++++++++++++++++
>>   3 files changed, 65 insertions(+)
>>
>> diff --git a/doc/guides/rel_notes/release_18_08.rst 
>> b/doc/guides/rel_notes/release_18_08.rst
>> index bc01242..2326058 100644
>> --- a/doc/guides/rel_notes/release_18_08.rst
>> +++ b/doc/guides/rel_notes/release_18_08.rst
>> @@ -46,6 +46,14 @@ New Features
>>     Flow API support has been added to CXGBE Poll Mode Driver to offload
>>     flows to Chelsio T5/T6 NICs.
>>   +* **Added eal device event callback in ethdev for hotplug.**
>> +
>> +  Implement a eal device event callback in ethdev, it could let pmd 
>> driver
>
> "pmd driver" sounds strange since PMD stands for poll-mode driver.
>

ok and will modify it. thanks.

>> +  have chance to manage the eal device event, such as process 
>> hotplug event.
>> +
>> +  * ``rte_eth_dev_event_callback`` for driver use to register it and 
>> process
>> +    eal device event.
>> +
>>     API Changes
>>   -----------
>> diff --git a/lib/librte_ethdev/rte_ethdev.c 
>> b/lib/librte_ethdev/rte_ethdev.c
>> index a9977df..36f218a 100644
>> --- a/lib/librte_ethdev/rte_ethdev.c
>> +++ b/lib/librte_ethdev/rte_ethdev.c
>> @@ -4518,6 +4518,43 @@ rte_eth_devargs_parse(const char *dargs, 
>> struct rte_eth_devargs *eth_da)
>>       return result;
>>   }
>>   +void __rte_experimental
>> +rte_eth_dev_event_callback(char *device_name, enum 
>> rte_dev_event_type type,
>> +                 void *arg)
>> +{
>> +    struct rte_eth_dev *eth_dev = (struct rte_eth_dev *)arg;
>> +
>> +    if (type >= RTE_DEV_EVENT_MAX) {
>> +        fprintf(stderr, "%s called upon invalid event %d\n",
>> +            __func__, type);
>> +        fflush(stderr);
>
> I'd like to understand why fprintf() is used here for logging instead 
> of rte_log
> mechanisms.
> Also if we really want the log, may be it make sense to move the if to 
> default
> case below.
>

ok.

>> +    }
>> +
>> +    switch (type) {
>> +    case RTE_DEV_EVENT_REMOVE:
>> +        ethdev_log(INFO, "The device: %s has been removed!\n",
>> +                device_name);
>> +
>> +        if (!device_name || !eth_dev)
>> +            return;
>> +
>> +        if (!(eth_dev->data->dev_flags & RTE_ETH_EVENT_INTR_RMV))
>> +            return;
>> +
>> +        if (!strcmp(device_name, eth_dev->device->name))
>
> Do we really need to check it? The callback is registered for devices
> with such name, so it should be always true. May be it is OK to 
> double-check
> I just want to be sure that I understand it properly.
>

i think it should be check here, since the eth_dev is being an pointer 
of arg to transfer into the eal event callback, and the arg is no 
default relation with the device name,
and we could not require user to always set the valid value when they 
use the callback.

>> + _rte_eth_dev_callback_process(eth_dev,
>> +                              RTE_ETH_EVENT_INTR_RMV,
>> +                              NULL);
>> +        break;
>> +    case RTE_DEV_EVENT_ADD:
>> +        ethdev_log(INFO, "The device: %s has been added!\n",
>> +            device_name);
>> +        break;
>> +    default:
>> +        break;
>> +    }
>> +}
>> +
>>   RTE_INIT(ethdev_init_log);
>>   static void
>>   ethdev_init_log(void)
>> diff --git a/lib/librte_ethdev/rte_ethdev_driver.h 
>> b/lib/librte_ethdev/rte_ethdev_driver.h
>> index c9c825e..fed5afa 100644
>> --- a/lib/librte_ethdev/rte_ethdev_driver.h
>> +++ b/lib/librte_ethdev/rte_ethdev_driver.h
>> @@ -82,6 +82,26 @@ int rte_eth_dev_release_port(struct rte_eth_dev 
>> *eth_dev);
>>   void _rte_eth_dev_reset(struct rte_eth_dev *dev);
>>     /**
>> + * @warning
>> + * @b EXPERIMENTAL: this API may change without prior notice.
>> + *
>> + * Implement a rte eth eal device event callbacks for the specific 
>> device.
>> + *
>> + * @param device_name
>> + *  Pointer to the name of the rte device.
>
> Is it name of the device which generates the event? If so, it should 
> be highlighted.
>

yes, should be.

>> + * @param event
>> + *  Eal device event type.
>> + * @param ret_param
>> + *  To pass data back to user application.
>> + *
>> + * @return
>> + *  void
>> + */
>> +void __rte_experimental
>> +rte_eth_dev_event_callback(char *device_name,
>> +        enum rte_dev_event_type event, void *cb_arg);
>> +
>> +/**
>>    * @internal Executes all the user application registered callbacks 
>> for
>>    * the specific device. It is for DPDK internal user only. User
>>    * application should not call it directly.
>

Re: [dpdk-dev] [PATCH v3 2/4] net/ixgbe: enable hotplug detect in ixgbe

[Lu, Wenzhuo]

Hi,


> -----Original Message-----
> From: Guo, Jia
> Sent: Monday, July 9, 2018 7:46 PM
> To: stephen@networkplumber.org; Richardson, Bruce
> <bruce.richardson@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>;
> Ananyev, Konstantin <konstantin.ananyev@intel.com>;
> gaetan.rivet@6wind.com; Wu, Jingjing <jingjing.wu@intel.com>;
> thomas@monjalon.net; motih@mellanox.com; matan@mellanox.com; Van
> Haaren, Harry <harry.van.haaren@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>; He, Shaopeng <shaopeng.he@intel.com>;
> Iremonger, Bernard <bernard.iremonger@intel.com>;
> arybchenko@solarflare.com; Lu, Wenzhuo <wenzhuo.lu@intel.com>
> Cc: jblunck@infradead.org; shreyansh.jain@nxp.com; dev@dpdk.org; Guo,
> Jia <jia.guo@intel.com>; Zhang, Helin <helin.zhang@intel.com>
> Subject: [PATCH v3 2/4] net/ixgbe: enable hotplug detect in ixgbe
> 
> This patch aim to enable hotplug detect in ixgbe pmd driver. Firstly it set the
> flags RTE_PCI_DRV_INTR_RMV in drv_flags to announce the hotplug ability,
> and then use rte_dev_event_callback_register to register the hotplug event
> callback to eal. When eal detect the hotplug event, it will call the callback to
> process it, if the event is hotplug remove, it will trigger the
> RTE_ETH_EVENT_INTR_RMV event into ethdev callback to let app process
> the hotplug for the ethdev.
> 
> This is an example for other driver, that if any driver support hotplug feature
> could be use this way to enable hotplug detect.
> 
> Signed-off-by: Jeff Guo <jia.guo@intel.com>
Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>

Re: [dpdk-dev] [PATCH v3 1/4] ethdev: Add eal device event callback

[Zhang, Qi Z]

Hi Jeff:

> -----Original Message-----
> From: Guo, Jia
> Sent: Monday, July 9, 2018 7:46 PM
> To: stephen@networkplumber.org; Richardson, Bruce
> <bruce.richardson@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>;
> Ananyev, Konstantin <konstantin.ananyev@intel.com>;
> gaetan.rivet@6wind.com; Wu, Jingjing <jingjing.wu@intel.com>;
> thomas@monjalon.net; motih@mellanox.com; matan@mellanox.com; Van
> Haaren, Harry <harry.van.haaren@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>; He, Shaopeng <shaopeng.he@intel.com>; Iremonger,
> Bernard <bernard.iremonger@intel.com>; arybchenko@solarflare.com; Lu,
> Wenzhuo <wenzhuo.lu@intel.com>
> Cc: jblunck@infradead.org; shreyansh.jain@nxp.com; dev@dpdk.org; Guo, Jia
> <jia.guo@intel.com>; Zhang, Helin <helin.zhang@intel.com>
> Subject: [PATCH v3 1/4] ethdev: Add eal device event callback
> 
> Implement a eal device event callback "rte_eth_dev_event_callback"
> in ethdev, it could let pmd driver have chance to manage the eal device event,
> such as process hotplug event.
> 
> Signed-off-by: Jeff Guo <jia.guo@intel.com>
> ---
<...>
> 
>  /**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice.
> + *
> + * Implement a rte eth eal device event callbacks for the specific device.
> + *
> + * @param device_name
> + *  Pointer to the name of the rte device.
> + * @param event
> + *  Eal device event type.
> + * @param ret_param
> + *  To pass data back to user application.
> + *
> + * @return
> + *  void
> + */
> +void __rte_experimental
> +rte_eth_dev_event_callback(char *device_name,
> +		enum rte_dev_event_type event, void *cb_arg);

I don't think we should expose the callback function to PMD directly
It should be a function like rte_eth_dev_event_callback_register(struct rte_ethdev *dev) which looks more like an ethdev help API for drivers.
And inside the function , we do the rte_dev_event_callback_register ...
And rte_eth_dev_event_callback should be rename to eth_dev_event_callback as a static function.

Regards
Qi
> +
> +/**
>   * @internal Executes all the user application registered callbacks for
>   * the specific device. It is for DPDK internal user only. User
>   * application should not call it directly.
> --
> 2.7.4

[dpdk-dev] [PATCH v4 0/4] Enable eal hotplug event handler in ethdev

[Jeff Guo]

As we may know, we have eal event for rte device hotplug and ethdev event
for ethdev hotplug. Some ethdev need to use eal event to detect hotplug
behaviors, the privors way is register eal event callback in app, but seems
that it will have some race between these 2 event processes. In oder to fix
the it, it might be better to find a way to combind these 2 events detect.

This patch set introduce a way to combind these 2 event, by register the
ethdev eal event callback in ether dev and trigger the ethdev hotplug event
in the callback. That will let the ethdev device can easy process hotplug
by a common way.

Here let i40e/ixgbe pmd driver for example, other driver which support
hotplug feature could be use this way to detect and process hotplug.

patch history:
v4->v3:
change to use device event handler install api

v3->v2:
remove the callback from driver to ethdev for common.

v2->v1:
add ixgbe hotplug detect case.
refine some doc.


Jeff Guo (4):
  ethdev: Add eal device event callback
  net/ixgbe: install ethdev hotplug handler in ixgbe
  net/i40e: install hotplug handler in i40e
  testpmd: remove the dev event callback register

 app/test-pmd/testpmd.c                 | 76 ----------------------------------
 doc/guides/rel_notes/release_18_08.rst | 12 ++++++
 drivers/net/i40e/i40e_ethdev.c         |  8 +++-
 drivers/net/ixgbe/ixgbe_ethdev.c       |  8 +++-
 lib/librte_ethdev/rte_ethdev.c         | 59 ++++++++++++++++++++++++++
 lib/librte_ethdev/rte_ethdev_driver.h  | 32 ++++++++++++++
 6 files changed, 117 insertions(+), 78 deletions(-)

-- 
2.7.4

[dpdk-dev] [PATCH v4 1/4] ethdev: Add API to enable device event handler

[Jeff Guo]

Implement a couple of eal device event handler install/uninstall APIs
in ethdev, it could let PMDs have chance to manage the eal device event,
such as register device event callback, then monitor and process the
hotplug event.

Signed-off-by: Jeff Guo <jia.guo@intel.com>
---
v4->v3:
change to use eal device event handler install api
---
 doc/guides/rel_notes/release_18_08.rst | 12 +++++++
 lib/librte_ethdev/rte_ethdev.c         | 59 ++++++++++++++++++++++++++++++++++
 lib/librte_ethdev/rte_ethdev_driver.h  | 32 ++++++++++++++++++
 3 files changed, 103 insertions(+)

diff --git a/doc/guides/rel_notes/release_18_08.rst b/doc/guides/rel_notes/release_18_08.rst
index bc01242..b6482ce 100644
--- a/doc/guides/rel_notes/release_18_08.rst
+++ b/doc/guides/rel_notes/release_18_08.rst
@@ -46,6 +46,18 @@ New Features
   Flow API support has been added to CXGBE Poll Mode Driver to offload
   flows to Chelsio T5/T6 NICs.
 
+* **Added eal device event process helper in ethdev.**
+
+  Implement a couple of eal device event handler install/uninstall APIs in
+  ethdev, these helper could let PMDs have chance to manage the eal device
+  event, such as register device event callback, then monitor and process
+  hotplug event.
+
+  * ``rte_eth_dev_event_handler_install`` for PMDs use to install the device
+    event handler.
+  * ``rte_eth_dev_event_handler_uninstall`` for PMDs use to uninstall the device
+    event handler.
+
 
 API Changes
 -----------
diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index a9977df..09ea02d 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -4518,6 +4518,65 @@ rte_eth_devargs_parse(const char *dargs, struct rte_eth_devargs *eth_da)
 	return result;
 }
 
+static void __rte_experimental
+eth_dev_event_callback(char *device_name, enum rte_dev_event_type type,
+			     void *arg)
+{
+	struct rte_eth_dev *eth_dev = (struct rte_eth_dev *)arg;
+
+	switch (type) {
+	case RTE_DEV_EVENT_REMOVE:
+		ethdev_log(INFO, "The device: %s has been removed!\n",
+			    device_name);
+
+		if (!device_name || !eth_dev)
+			return;
+
+		if (!(eth_dev->data->dev_flags & RTE_ETH_EVENT_INTR_RMV))
+			return;
+
+		if (!strcmp(device_name, eth_dev->device->name))
+			_rte_eth_dev_callback_process(eth_dev,
+						      RTE_ETH_EVENT_INTR_RMV,
+						      NULL);
+		break;
+	case RTE_DEV_EVENT_ADD:
+		ethdev_log(INFO, "The device: %s has been added!\n",
+			device_name);
+		break;
+	default:
+		break;
+	}
+}
+
+int __rte_experimental
+rte_eth_dev_event_handler_install(struct rte_eth_dev *eth_dev)
+{
+	int result = 0;
+
+	result = rte_dev_event_callback_register(eth_dev->device->name,
+					eth_dev_event_callback, eth_dev);
+	if (result)
+		RTE_LOG(ERR, EAL, "device event callback register failed for "
+			"device:%s!\n", eth_dev->device->name);
+
+	return result;
+}
+
+int __rte_experimental
+rte_eth_dev_event_handler_uninstall(struct rte_eth_dev *eth_dev)
+{
+	int result = 0;
+
+	result = rte_dev_event_callback_unregister(eth_dev->device->name,
+					eth_dev_event_callback, eth_dev);
+	if (result)
+		RTE_LOG(ERR, EAL, "device event callback unregister failed for"
+			" device:%s!\n", eth_dev->device->name);
+
+	return result;
+}
+
 RTE_INIT(ethdev_init_log);
 static void
 ethdev_init_log(void)
diff --git a/lib/librte_ethdev/rte_ethdev_driver.h b/lib/librte_ethdev/rte_ethdev_driver.h
index c9c825e..3de1cd4 100644
--- a/lib/librte_ethdev/rte_ethdev_driver.h
+++ b/lib/librte_ethdev/rte_ethdev_driver.h
@@ -82,6 +82,38 @@ int rte_eth_dev_release_port(struct rte_eth_dev *eth_dev);
 void _rte_eth_dev_reset(struct rte_eth_dev *dev);
 
 /**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Implement a helper to install the device event handler for the specific
+ * ether device.
+ *
+ * @param dev
+ *  Pointer to struct rte_eth_dev.
+ *
+ * @return
+ *   - 0 on success, negative on error
+ */
+int __rte_experimental
+rte_eth_dev_event_handler_install(struct rte_eth_dev *dev);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Implement a helper to uninstall the device event handler for the specific
+ * ether device.
+ *
+ * @param dev
+ *  Pointer to struct rte_eth_dev.
+ *
+ * @return
+ *   - 0 on success, negative on error
+ */
+int __rte_experimental
+rte_eth_dev_event_handler_uninstall(struct rte_eth_dev *dev);
+
+/**
  * @internal Executes all the user application registered callbacks for
  * the specific device. It is for DPDK internal user only. User
  * application should not call it directly.
-- 
2.7.4

[dpdk-dev] [PATCH v4 2/4] net/ixgbe: install ethdev hotplug handler in ixgbe

[Jeff Guo]

This patch aim to enable hotplug detect in ixgbe PMD. Firstly it
set the flags RTE_PCI_DRV_INTR_RMV in drv_flags to announce the hotplug
ability, and then use rte_eth_dev_event_handler_install to install
the hotplug event handler for ethdev. When eal detect the hotplug event,
it will call the ethdev callback to process it. If the event is hotplug
removal, it will trigger the RTE_ETH_EVENT_INTR_RMV event into ethdev
callback to let app process the hotplug for this ethdev.

This is an example for other driver, that if any driver support hotplug
feature could be use this way to install hotplug handler.

Signed-off-by: Jeff Guo <jia.guo@intel.com>
---
v4->v3:
change to use eal device event handler install api
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 87d2ad0..e7ae9bf 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -1678,6 +1678,9 @@ eth_ixgbevf_dev_init(struct rte_eth_dev *eth_dev)
 	rte_intr_enable(intr_handle);
 	ixgbevf_intr_enable(eth_dev);
 
+	/* install the dev event handler for ethdev. */
+	rte_eth_dev_event_handler_install(eth_dev);
+
 	PMD_INIT_LOG(DEBUG, "port %d vendorID=0x%x deviceID=0x%x mac.type=%s",
 		     eth_dev->data->port_id, pci_dev->id.vendor_id,
 		     pci_dev->id.device_id, "ixgbe_mac_82599_vf");
@@ -1718,6 +1721,9 @@ eth_ixgbevf_dev_uninit(struct rte_eth_dev *eth_dev)
 	rte_intr_callback_unregister(intr_handle,
 				     ixgbevf_dev_interrupt_handler, eth_dev);
 
+	/* uninstall the dev event handler for ethdev. */
+	rte_eth_dev_event_handler_uninstall(eth_dev);
+
 	return 0;
 }
 
@@ -1801,7 +1807,7 @@ static int eth_ixgbe_pci_remove(struct rte_pci_device *pci_dev)
 static struct rte_pci_driver rte_ixgbe_pmd = {
 	.id_table = pci_id_ixgbe_map,
 	.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
-		     RTE_PCI_DRV_IOVA_AS_VA,
+		     RTE_PCI_DRV_IOVA_AS_VA | RTE_PCI_DRV_INTR_RMV,
 	.probe = eth_ixgbe_pci_probe,
 	.remove = eth_ixgbe_pci_remove,
 };
-- 
2.7.4

[dpdk-dev] [PATCH v4 3/4] net/i40e: install hotplug handler in i40e

[Jeff Guo]

This patch aim to enable hotplug detect in i40e PMD. Firstly it
set the flags RTE_PCI_DRV_INTR_RMV in drv_flags to announce the hotplug
ability, and then use rte_eth_dev_event_handler_install to install
the hotplug event handler for ethdev. When eal detect the hotplug event,
it will call the ethdev callback to process it. If the event is hotplug
removal, it will trigger the RTE_ETH_EVENT_INTR_RMV event into ethdev
callback to let app process the hotplug for this ethdev.

This is an example for other driver, that if any driver support hotplug
feature could be use this way to install hotplug handler.

Signed-off-by: Jeff Guo <jia.guo@intel.com>
---
v4->v3:
change to use eal device event handler install api
---
 drivers/net/i40e/i40e_ethdev.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 13c5d32..8fccf04 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -688,7 +688,7 @@ static int eth_i40e_pci_remove(struct rte_pci_device *pci_dev)
 static struct rte_pci_driver rte_i40e_pmd = {
 	.id_table = pci_id_i40e_map,
 	.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
-		     RTE_PCI_DRV_IOVA_AS_VA,
+		     RTE_PCI_DRV_IOVA_AS_VA | RTE_PCI_DRV_INTR_RMV,
 	.probe = eth_i40e_pci_probe,
 	.remove = eth_i40e_pci_remove,
 };
@@ -1442,6 +1442,9 @@ eth_i40e_dev_init(struct rte_eth_dev *dev, void *init_params __rte_unused)
 	rte_intr_callback_register(intr_handle,
 				   i40e_dev_interrupt_handler, dev);
 
+	/* install the dev event handler for ethdev. */
+	rte_eth_dev_event_handler_install(dev);
+
 	/* configure and enable device interrupt */
 	i40e_pf_config_irq0(hw, TRUE);
 	i40e_pf_enable_irq0(hw);
@@ -1674,6 +1677,9 @@ eth_i40e_dev_uninit(struct rte_eth_dev *dev)
 	/* Remove all Traffic Manager configuration */
 	i40e_tm_conf_uninit(dev);
 
+	/* uninstall the dev event handler for ethdev. */
+	rte_eth_dev_event_handler_uninstall(dev);
+
 	return 0;
 }
 
-- 
2.7.4

[dpdk-dev] [PATCH v4 4/4] testpmd: remove the dev event callback register

[Jeff Guo]

Since now we can use driver to management the eal event for hotplug,
so no need to register dev event callback in app anymore. This patch
remove the related code.

Signed-off-by: Jeff Guo <jia.guo@intel.com>
Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
v4->v3:
no change
---
 app/test-pmd/testpmd.c | 76 --------------------------------------------------
 1 file changed, 76 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 24c1998..10ed660 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -400,12 +400,6 @@ static void check_all_ports_link_status(uint32_t port_mask);
 static int eth_event_callback(portid_t port_id,
 			      enum rte_eth_event_type type,
 			      void *param, void *ret_param);
-static void eth_dev_event_callback(char *device_name,
-				enum rte_dev_event_type type,
-				void *param);
-static int eth_dev_event_callback_register(void);
-static int eth_dev_event_callback_unregister(void);
-
 
 /*
  * Check if all the ports are started.
@@ -1915,39 +1909,6 @@ reset_port(portid_t pid)
 	printf("Done\n");
 }
 
-static int
-eth_dev_event_callback_register(void)
-{
-	int ret;
-
-	/* register the device event callback */
-	ret = rte_dev_event_callback_register(NULL,
-		eth_dev_event_callback, NULL);
-	if (ret) {
-		printf("Failed to register device event callback\n");
-		return -1;
-	}
-
-	return 0;
-}
-
-
-static int
-eth_dev_event_callback_unregister(void)
-{
-	int ret;
-
-	/* unregister the device event callback */
-	ret = rte_dev_event_callback_unregister(NULL,
-		eth_dev_event_callback, NULL);
-	if (ret < 0) {
-		printf("Failed to unregister device event callback\n");
-		return -1;
-	}
-
-	return 0;
-}
-
 void
 attach_port(char *identifier)
 {
@@ -2049,10 +2010,6 @@ pmd_test_exit(void)
 			RTE_LOG(ERR, EAL,
 				"fail to stop device event monitor.");
 
-		ret = eth_dev_event_callback_unregister();
-		if (ret)
-			RTE_LOG(ERR, EAL,
-				"fail to unregister all event callbacks.");
 	}
 
 	printf("\nBye...\n");
@@ -2191,37 +2148,6 @@ eth_event_callback(portid_t port_id, enum rte_eth_event_type type, void *param,
 	return 0;
 }
 
-/* This function is used by the interrupt thread */
-static void
-eth_dev_event_callback(char *device_name, enum rte_dev_event_type type,
-			     __rte_unused void *arg)
-{
-	if (type >= RTE_DEV_EVENT_MAX) {
-		fprintf(stderr, "%s called upon invalid event %d\n",
-			__func__, type);
-		fflush(stderr);
-	}
-
-	switch (type) {
-	case RTE_DEV_EVENT_REMOVE:
-		RTE_LOG(ERR, EAL, "The device: %s has been removed!\n",
-			device_name);
-		/* TODO: After finish failure handle, begin to stop
-		 * packet forward, stop port, close port, detach port.
-		 */
-		break;
-	case RTE_DEV_EVENT_ADD:
-		RTE_LOG(ERR, EAL, "The device: %s has been added!\n",
-			device_name);
-		/* TODO: After finish kernel driver binding,
-		 * begin to attach port.
-		 */
-		break;
-	default:
-		break;
-	}
-}
-
 static int
 set_tx_queue_stats_mapping_registers(portid_t port_id, struct rte_port *port)
 {
@@ -2735,8 +2661,6 @@ main(int argc, char** argv)
 			rte_errno = EINVAL;
 			return -1;
 		}
-		eth_dev_event_callback_register();
-
 	}
 
 	if (start_port(RTE_PORT_ALL) != 0)
-- 
2.7.4

Re: [dpdk-dev] [PATCH v4 1/4] ethdev: Add API to enable device event handler

[Zhang, Qi Z]

> -----Original Message-----
> From: Guo, Jia
> Sent: Tuesday, July 10, 2018 8:52 PM
> To: stephen@networkplumber.org; Richardson, Bruce
> <bruce.richardson@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>;
> Ananyev, Konstantin <konstantin.ananyev@intel.com>;
> gaetan.rivet@6wind.com; Wu, Jingjing <jingjing.wu@intel.com>;
> thomas@monjalon.net; motih@mellanox.com; matan@mellanox.com; Van
> Haaren, Harry <harry.van.haaren@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>; He, Shaopeng <shaopeng.he@intel.com>; Iremonger,
> Bernard <bernard.iremonger@intel.com>; arybchenko@solarflare.com
> Cc: jblunck@infradead.org; shreyansh.jain@nxp.com; dev@dpdk.org; Guo, Jia
> <jia.guo@intel.com>; Zhang, Helin <helin.zhang@intel.com>
> Subject: [PATCH v4 1/4] ethdev: Add API to enable device event handler
> 
> Implement a couple of eal device event handler install/uninstall APIs in ethdev,
> it could let PMDs have chance to manage the eal device event, such as register
> device event callback, then monitor and process the hotplug event.
> 
> Signed-off-by: Jeff Guo <jia.guo@intel.com>

Reviewed-by: Qi Zhang <qi.z.zhang@intel.com>

Re: [dpdk-dev] [PATCH v4 3/4] net/i40e: install hotplug handler in i40e

[Zhang, Qi Z]

> -----Original Message-----
> From: Guo, Jia
> Sent: Tuesday, July 10, 2018 8:52 PM
> To: stephen@networkplumber.org; Richardson, Bruce
> <bruce.richardson@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>;
> Ananyev, Konstantin <konstantin.ananyev@intel.com>;
> gaetan.rivet@6wind.com; Wu, Jingjing <jingjing.wu@intel.com>;
> thomas@monjalon.net; motih@mellanox.com; matan@mellanox.com; Van
> Haaren, Harry <harry.van.haaren@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>; He, Shaopeng <shaopeng.he@intel.com>; Iremonger,
> Bernard <bernard.iremonger@intel.com>; arybchenko@solarflare.com
> Cc: jblunck@infradead.org; shreyansh.jain@nxp.com; dev@dpdk.org; Guo, Jia
> <jia.guo@intel.com>; Zhang, Helin <helin.zhang@intel.com>
> Subject: [PATCH v4 3/4] net/i40e: install hotplug handler in i40e
> 
> This patch aim to enable hotplug detect in i40e PMD. Firstly it set the flags
> RTE_PCI_DRV_INTR_RMV in drv_flags to announce the hotplug ability, and
> then use rte_eth_dev_event_handler_install to install the hotplug event
> handler for ethdev. When eal detect the hotplug event, it will call the ethdev
> callback to process it. If the event is hotplug removal, it will trigger the
> RTE_ETH_EVENT_INTR_RMV event into ethdev callback to let app process the
> hotplug for this ethdev.
> 
> This is an example for other driver, that if any driver support hotplug feature
> could be use this way to install hotplug handler.
> 
> Signed-off-by: Jeff Guo <jia.guo@intel.com>

Acked-by: Qi Zhang <qi.z.zhang@intel.com>

Re: [dpdk-dev] [PATCH v4 1/4] ethdev: Add API to enable device event handler

[Andrew Rybchenko]

On 10.07.2018 15:51, Jeff Guo wrote:
> Implement a couple of eal device event handler install/uninstall APIs
> in ethdev, it could let PMDs have chance to manage the eal device event,
> such as register device event callback, then monitor and process the
> hotplug event.

I think it is moving in right direction, but my problem with the patch is
that I don't understand what prevents us to make it even more generic.
Right now it looks like that PCI driver flag RTE_PCI_DRV_INTR_RMV is
sufficient and everything else could be done on ethdev layer.
RTE_PCI_DRV_INTR_RMV  is mapped to RTE_ETH_DEV_INTR_RMV eth
device flag. The flag may be used as an indication in rte_eth_dev_create()
to register the callback.
May be I'm completely wrong above, but if so, I'd like to understand why
and prefer to see explanations in the patch description.

> Signed-off-by: Jeff Guo <jia.guo@intel.com>
> ---
> v4->v3:
> change to use eal device event handler install api
> ---
>   doc/guides/rel_notes/release_18_08.rst | 12 +++++++
>   lib/librte_ethdev/rte_ethdev.c         | 59 ++++++++++++++++++++++++++++++++++
>   lib/librte_ethdev/rte_ethdev_driver.h  | 32 ++++++++++++++++++
>   3 files changed, 103 insertions(+)
>
> diff --git a/doc/guides/rel_notes/release_18_08.rst b/doc/guides/rel_notes/release_18_08.rst
> index bc01242..b6482ce 100644
> --- a/doc/guides/rel_notes/release_18_08.rst
> +++ b/doc/guides/rel_notes/release_18_08.rst
> @@ -46,6 +46,18 @@ New Features
>     Flow API support has been added to CXGBE Poll Mode Driver to offload
>     flows to Chelsio T5/T6 NICs.
>   
> +* **Added eal device event process helper in ethdev.**
> +
> +  Implement a couple of eal device event handler install/uninstall APIs in
> +  ethdev, these helper could let PMDs have chance to manage the eal device
> +  event, such as register device event callback, then monitor and process
> +  hotplug event.
> +
> +  * ``rte_eth_dev_event_handler_install`` for PMDs use to install the device
> +    event handler.
> +  * ``rte_eth_dev_event_handler_uninstall`` for PMDs use to uninstall the device
> +    event handler.
> +
>   
>   API Changes
>   -----------
> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> index a9977df..09ea02d 100644
> --- a/lib/librte_ethdev/rte_ethdev.c
> +++ b/lib/librte_ethdev/rte_ethdev.c
> @@ -4518,6 +4518,65 @@ rte_eth_devargs_parse(const char *dargs, struct rte_eth_devargs *eth_da)
>   	return result;
>   }
>   
> +static void __rte_experimental
> +eth_dev_event_callback(char *device_name, enum rte_dev_event_type type,
> +			     void *arg)
> +{
> +	struct rte_eth_dev *eth_dev = (struct rte_eth_dev *)arg;
> +
> +	switch (type) {
> +	case RTE_DEV_EVENT_REMOVE:
> +		ethdev_log(INFO, "The device: %s has been removed!\n",

Colon after 'device' above looks strange for me. I'd suggest to remove it.
If so, similar below for ADD.

> +			    device_name);
> +
> +		if (!device_name || !eth_dev)
> +			return;
> +
> +		if (!(eth_dev->data->dev_flags & RTE_ETH_EVENT_INTR_RMV))

It looks like a typo above. Shouldn't it be RTE_ETH_DEV_INTR_RMV?

> +			return;
> +
> +		if (!strcmp(device_name, eth_dev->device->name))
> +			_rte_eth_dev_callback_process(eth_dev,
> +						      RTE_ETH_EVENT_INTR_RMV,
> +						      NULL);
> +		break;
> +	case RTE_DEV_EVENT_ADD:
> +		ethdev_log(INFO, "The device: %s has been added!\n",
> +			device_name);
> +		break;
> +	default:
> +		break;
> +	}
> +}
> +
> +int __rte_experimental
> +rte_eth_dev_event_handler_install(struct rte_eth_dev *eth_dev)
> +{
> +	int result = 0;
> +
> +	result = rte_dev_event_callback_register(eth_dev->device->name,
> +					eth_dev_event_callback, eth_dev);
> +	if (result)
> +		RTE_LOG(ERR, EAL, "device event callback register failed for "
> +			"device:%s!\n", eth_dev->device->name);

Why ethdev_log() is used above, but here is RTE_LOG with EAL?
Similar question below.

> +
> +	return result;
> +}
> +
> +int __rte_experimental
> +rte_eth_dev_event_handler_uninstall(struct rte_eth_dev *eth_dev)
> +{
> +	int result = 0;
> +
> +	result = rte_dev_event_callback_unregister(eth_dev->device->name,
> +					eth_dev_event_callback, eth_dev);
> +	if (result)
> +		RTE_LOG(ERR, EAL, "device event callback unregister failed for"
> +			" device:%s!\n", eth_dev->device->name);
> +
> +	return result;
> +}
> +
>   RTE_INIT(ethdev_init_log);
>   static void
>   ethdev_init_log(void)

<...>

Re: [dpdk-dev] [PATCH v4 1/4] ethdev: Add API to enable device event handler

[Jeff Guo]

On 7/11/2018 4:49 PM, Andrew Rybchenko wrote:
> On 10.07.2018 15:51, Jeff Guo wrote:
>> Implement a couple of eal device event handler install/uninstall APIs
>> in ethdev, it could let PMDs have chance to manage the eal device event,
>> such as register device event callback, then monitor and process the
>> hotplug event.
>
> I think it is moving in right direction, but my problem with the patch is
> that I don't understand what prevents us to make it even more generic.
> Right now it looks like that PCI driver flag RTE_PCI_DRV_INTR_RMV is
> sufficient and everything else could be done on ethdev layer.
> RTE_PCI_DRV_INTR_RMV  is mapped to RTE_ETH_DEV_INTR_RMV eth
> device flag. The flag may be used as an indication in 
> rte_eth_dev_create()
> to register the callback.
> May be I'm completely wrong above, but if so, I'd like to understand why
> and prefer to see explanations in the patch description.
>

Your acknowledgement is right, and it is make sense to check the reason 
why is the most generic but not other.
i think that let driver to decide if it support the RTE_PCI_DRV_INTR_RMV 
should be fine, that is first.
And second, the place of installer in driver is also fine, each ethdev 
driver install specific callback for each port, and could let
driver have change to manage the status of hotplug for further. So if 
there are no better place in ethdev here for more generic to install it,
that should be fine.

>> Signed-off-by: Jeff Guo <jia.guo@intel.com>
>> ---
>> v4->v3:
>> change to use eal device event handler install api
>> ---
>>   doc/guides/rel_notes/release_18_08.rst | 12 +++++++
>>   lib/librte_ethdev/rte_ethdev.c         | 59 
>> ++++++++++++++++++++++++++++++++++
>>   lib/librte_ethdev/rte_ethdev_driver.h  | 32 ++++++++++++++++++
>>   3 files changed, 103 insertions(+)
>>
>> diff --git a/doc/guides/rel_notes/release_18_08.rst 
>> b/doc/guides/rel_notes/release_18_08.rst
>> index bc01242..b6482ce 100644
>> --- a/doc/guides/rel_notes/release_18_08.rst
>> +++ b/doc/guides/rel_notes/release_18_08.rst
>> @@ -46,6 +46,18 @@ New Features
>>     Flow API support has been added to CXGBE Poll Mode Driver to offload
>>     flows to Chelsio T5/T6 NICs.
>>   +* **Added eal device event process helper in ethdev.**
>> +
>> +  Implement a couple of eal device event handler install/uninstall 
>> APIs in
>> +  ethdev, these helper could let PMDs have chance to manage the eal 
>> device
>> +  event, such as register device event callback, then monitor and 
>> process
>> +  hotplug event.
>> +
>> +  * ``rte_eth_dev_event_handler_install`` for PMDs use to install 
>> the device
>> +    event handler.
>> +  * ``rte_eth_dev_event_handler_uninstall`` for PMDs use to 
>> uninstall the device
>> +    event handler.
>> +
>>     API Changes
>>   -----------
>> diff --git a/lib/librte_ethdev/rte_ethdev.c 
>> b/lib/librte_ethdev/rte_ethdev.c
>> index a9977df..09ea02d 100644
>> --- a/lib/librte_ethdev/rte_ethdev.c
>> +++ b/lib/librte_ethdev/rte_ethdev.c
>> @@ -4518,6 +4518,65 @@ rte_eth_devargs_parse(const char *dargs, 
>> struct rte_eth_devargs *eth_da)
>>       return result;
>>   }
>>   +static void __rte_experimental
>> +eth_dev_event_callback(char *device_name, enum rte_dev_event_type type,
>> +                 void *arg)
>> +{
>> +    struct rte_eth_dev *eth_dev = (struct rte_eth_dev *)arg;
>> +
>> +    switch (type) {
>> +    case RTE_DEV_EVENT_REMOVE:
>> +        ethdev_log(INFO, "The device: %s has been removed!\n",
>
> Colon after 'device' above looks strange for me. I'd suggest to remove 
> it.
> If so, similar below for ADD.
>

ok, i am fine.

>> +                device_name);
>> +
>> +        if (!device_name || !eth_dev)
>> +            return;
>> +
>> +        if (!(eth_dev->data->dev_flags & RTE_ETH_EVENT_INTR_RMV))
>
> It looks like a typo above. Shouldn't it be RTE_ETH_DEV_INTR_RMV?
>

you are right here, it is a typo.

>> +            return;
>> +
>> +        if (!strcmp(device_name, eth_dev->device->name))
>> +            _rte_eth_dev_callback_process(eth_dev,
>> +                              RTE_ETH_EVENT_INTR_RMV,
>> +                              NULL);
>> +        break;
>> +    case RTE_DEV_EVENT_ADD:
>> +        ethdev_log(INFO, "The device: %s has been added!\n",
>> +            device_name);
>> +        break;
>> +    default:
>> +        break;
>> +    }
>> +}
>> +
>> +int __rte_experimental
>> +rte_eth_dev_event_handler_install(struct rte_eth_dev *eth_dev)
>> +{
>> +    int result = 0;
>> +
>> +    result = rte_dev_event_callback_register(eth_dev->device->name,
>> +                    eth_dev_event_callback, eth_dev);
>> +    if (result)
>> +        RTE_LOG(ERR, EAL, "device event callback register failed for "
>> +            "device:%s!\n", eth_dev->device->name);
>
> Why ethdev_log() is used above, but here is RTE_LOG with EAL?
> Similar question below.
>

should be align to use ethdev_log.

>> +
>> +    return result;
>> +}
>> +
>> +int __rte_experimental
>> +rte_eth_dev_event_handler_uninstall(struct rte_eth_dev *eth_dev)
>> +{
>> +    int result = 0;
>> +
>> +    result = rte_dev_event_callback_unregister(eth_dev->device->name,
>> +                    eth_dev_event_callback, eth_dev);
>> +    if (result)
>> +        RTE_LOG(ERR, EAL, "device event callback unregister failed for"
>> +            " device:%s!\n", eth_dev->device->name);
>> +
>> +    return result;
>> +}
>> +
>>   RTE_INIT(ethdev_init_log);
>>   static void
>>   ethdev_init_log(void)
>
> <...>

[dpdk-dev] [PATCH v5 0/4] Install eal hotplug event handler in i40e/ixgbe

[Jeff Guo]

As we may know, we have eal event for rte device hotplug and ethdev event
for ethdev hotplug. Some ethdev need to use eal event to detect hotplug
behaviors, the privors way is register eal event callback in app, but seems
that it will have some race between these 2 event processes. In oder to fix
the it, it might be better to find a way to combind these 2 events detect.

This patch set introduce a way to combind these 2 event, by register the
ethdev eal event callback in ether dev and trigger the ethdev hotplug event
in the callback. That will let the ethdev device can easy process hotplug
by a common way.

Here let i40e/ixgbe pmd driver for example, other driver which support
hotplug feature could be use this way to detect and process hotplug.

patch history:
v5->v4:
refine some code style and typo

v4->v3:
change to use device event handler install api

v3->v2:
remove the callback from driver to ethdev for common.

v2->v1:
add ixgbe hotplug detect case.
refine some doc.

Jeff Guo (4):
  ethdev: Add eal device event callback
  net/ixgbe: install ethdev hotplug handler in ixgbe
  net/i40e: install hotplug handler in i40e
  testpmd: remove the dev event callback register

 app/test-pmd/testpmd.c                   | 76 --------------------------------
 doc/guides/rel_notes/release_18_08.rst   | 12 +++++
 drivers/net/i40e/i40e_ethdev.c           |  8 +++-
 drivers/net/ixgbe/ixgbe_ethdev.c         |  8 +++-
 lib/librte_ethdev/rte_ethdev.c           | 59 +++++++++++++++++++++++++
 lib/librte_ethdev/rte_ethdev_driver.h    | 32 ++++++++++++++
 lib/librte_ethdev/rte_ethdev_version.map |  2 +
 7 files changed, 119 insertions(+), 78 deletions(-)

-- 
2.7.4

[dpdk-dev] [PATCH v5 1/4] ethdev: Add eal device event callback

[Jeff Guo]

Implement a couple of eal device event handler install/uninstall APIs
in ethdev. Each ethdev install the handler in PMDs, so each callback
corresponding with one port, and process the eal device event for specific
port. If PMDs install the handler when initial device, it could have
chance to manage the eal device event, such as register device event
callback, then monitor and process the hotplug event.

Signed-off-by: Jeff Guo <jia.guo@intel.com>
Reviewed-by: Qi Zhang <qi.z.zhang@intel.com>
---
v5->v4:
refine some code style and typo
---
 doc/guides/rel_notes/release_18_08.rst   | 12 +++++++
 lib/librte_ethdev/rte_ethdev.c           | 59 ++++++++++++++++++++++++++++++++
 lib/librte_ethdev/rte_ethdev_driver.h    | 32 +++++++++++++++++
 lib/librte_ethdev/rte_ethdev_version.map |  2 ++
 4 files changed, 105 insertions(+)

diff --git a/doc/guides/rel_notes/release_18_08.rst b/doc/guides/rel_notes/release_18_08.rst
index bc01242..b6482ce 100644
--- a/doc/guides/rel_notes/release_18_08.rst
+++ b/doc/guides/rel_notes/release_18_08.rst
@@ -46,6 +46,18 @@ New Features
   Flow API support has been added to CXGBE Poll Mode Driver to offload
   flows to Chelsio T5/T6 NICs.
 
+* **Added eal device event process helper in ethdev.**
+
+  Implement a couple of eal device event handler install/uninstall APIs in
+  ethdev, these helper could let PMDs have chance to manage the eal device
+  event, such as register device event callback, then monitor and process
+  hotplug event.
+
+  * ``rte_eth_dev_event_handler_install`` for PMDs use to install the device
+    event handler.
+  * ``rte_eth_dev_event_handler_uninstall`` for PMDs use to uninstall the device
+    event handler.
+
 
 API Changes
 -----------
diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index a9977df..4d28db5 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -4518,6 +4518,65 @@ rte_eth_devargs_parse(const char *dargs, struct rte_eth_devargs *eth_da)
 	return result;
 }
 
+static void __rte_experimental
+eth_dev_event_callback(char *device_name, enum rte_dev_event_type type,
+			     void *arg)
+{
+	struct rte_eth_dev *eth_dev = (struct rte_eth_dev *)arg;
+
+	switch (type) {
+	case RTE_DEV_EVENT_REMOVE:
+		ethdev_log(INFO, "The device %s has been removed!\n",
+			    device_name);
+
+		if (!device_name || !eth_dev)
+			return;
+
+		if (!(eth_dev->data->dev_flags & RTE_ETH_DEV_INTR_RMV))
+			return;
+
+		if (!strcmp(device_name, eth_dev->device->name))
+			_rte_eth_dev_callback_process(eth_dev,
+						      RTE_ETH_EVENT_INTR_RMV,
+						      NULL);
+		break;
+	case RTE_DEV_EVENT_ADD:
+		ethdev_log(INFO, "The device %s has been added!\n",
+			device_name);
+		break;
+	default:
+		break;
+	}
+}
+
+int __rte_experimental
+rte_eth_dev_event_handler_install(struct rte_eth_dev *eth_dev)
+{
+	int result = 0;
+
+	result = rte_dev_event_callback_register(eth_dev->device->name,
+					eth_dev_event_callback, eth_dev);
+	if (result)
+		ethdev_log(ERR, "device event callback register failed for "
+			"device %s!\n", eth_dev->device->name);
+
+	return result;
+}
+
+int __rte_experimental
+rte_eth_dev_event_handler_uninstall(struct rte_eth_dev *eth_dev)
+{
+	int result = 0;
+
+	result = rte_dev_event_callback_unregister(eth_dev->device->name,
+					eth_dev_event_callback, eth_dev);
+	if (result)
+		ethdev_log(ERR, "device event callback unregister failed for"
+			" device %s!\n", eth_dev->device->name);
+
+	return result;
+}
+
 RTE_INIT(ethdev_init_log);
 static void
 ethdev_init_log(void)
diff --git a/lib/librte_ethdev/rte_ethdev_driver.h b/lib/librte_ethdev/rte_ethdev_driver.h
index c9c825e..3de1cd4 100644
--- a/lib/librte_ethdev/rte_ethdev_driver.h
+++ b/lib/librte_ethdev/rte_ethdev_driver.h
@@ -82,6 +82,38 @@ int rte_eth_dev_release_port(struct rte_eth_dev *eth_dev);
 void _rte_eth_dev_reset(struct rte_eth_dev *dev);
 
 /**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Implement a helper to install the device event handler for the specific
+ * ether device.
+ *
+ * @param dev
+ *  Pointer to struct rte_eth_dev.
+ *
+ * @return
+ *   - 0 on success, negative on error
+ */
+int __rte_experimental
+rte_eth_dev_event_handler_install(struct rte_eth_dev *dev);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Implement a helper to uninstall the device event handler for the specific
+ * ether device.
+ *
+ * @param dev
+ *  Pointer to struct rte_eth_dev.
+ *
+ * @return
+ *   - 0 on success, negative on error
+ */
+int __rte_experimental
+rte_eth_dev_event_handler_uninstall(struct rte_eth_dev *dev);
+
+/**
  * @internal Executes all the user application registered callbacks for
  * the specific device. It is for DPDK internal user only. User
  * application should not call it directly.
diff --git a/lib/librte_ethdev/rte_ethdev_version.map b/lib/librte_ethdev/rte_ethdev_version.map
index 40cf42b..dbd07a5 100644
--- a/lib/librte_ethdev/rte_ethdev_version.map
+++ b/lib/librte_ethdev/rte_ethdev_version.map
@@ -220,6 +220,8 @@ EXPERIMENTAL {
 	rte_eth_dev_count_total;
 	rte_eth_dev_create;
 	rte_eth_dev_destroy;
+	rte_eth_dev_event_handler_install;
+	rte_eth_dev_event_handler_uninstall;
 	rte_eth_dev_get_module_eeprom;
 	rte_eth_dev_get_module_info;
 	rte_eth_dev_is_removed;
-- 
2.7.4

[dpdk-dev] [PATCH v5 2/4] net/ixgbe: install ethdev hotplug handler in ixgbe

[Jeff Guo]

This patch aim to enable hotplug detect in ixgbe PMD. Firstly it
set the flags RTE_PCI_DRV_INTR_RMV in drv_flags to announce the hotplug
ability, and then use rte_eth_dev_event_handler_install to install
the hotplug event handler for ethdev. When eal detect the hotplug event,
it will call the ethdev callback to process it. If the event is hotplug
removal, it will trigger the RTE_ETH_EVENT_INTR_RMV event into ethdev
callback to let app process the hotplug for this ethdev.

This is an example for other driver, that if any driver support hotplug
feature could be use this way to install hotplug handler.

Signed-off-by: Jeff Guo <jia.guo@intel.com>
Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
v5->v4:
no change.
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 87d2ad0..e7ae9bf 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -1678,6 +1678,9 @@ eth_ixgbevf_dev_init(struct rte_eth_dev *eth_dev)
 	rte_intr_enable(intr_handle);
 	ixgbevf_intr_enable(eth_dev);
 
+	/* install the dev event handler for ethdev. */
+	rte_eth_dev_event_handler_install(eth_dev);
+
 	PMD_INIT_LOG(DEBUG, "port %d vendorID=0x%x deviceID=0x%x mac.type=%s",
 		     eth_dev->data->port_id, pci_dev->id.vendor_id,
 		     pci_dev->id.device_id, "ixgbe_mac_82599_vf");
@@ -1718,6 +1721,9 @@ eth_ixgbevf_dev_uninit(struct rte_eth_dev *eth_dev)
 	rte_intr_callback_unregister(intr_handle,
 				     ixgbevf_dev_interrupt_handler, eth_dev);
 
+	/* uninstall the dev event handler for ethdev. */
+	rte_eth_dev_event_handler_uninstall(eth_dev);
+
 	return 0;
 }
 
@@ -1801,7 +1807,7 @@ static int eth_ixgbe_pci_remove(struct rte_pci_device *pci_dev)
 static struct rte_pci_driver rte_ixgbe_pmd = {
 	.id_table = pci_id_ixgbe_map,
 	.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
-		     RTE_PCI_DRV_IOVA_AS_VA,
+		     RTE_PCI_DRV_IOVA_AS_VA | RTE_PCI_DRV_INTR_RMV,
 	.probe = eth_ixgbe_pci_probe,
 	.remove = eth_ixgbe_pci_remove,
 };
-- 
2.7.4

[dpdk-dev] [PATCH v5 3/4] net/i40e: install hotplug handler in i40e

[Jeff Guo]

This patch aim to enable hotplug detect in i40e PMD. Firstly it
set the flags RTE_PCI_DRV_INTR_RMV in drv_flags to announce the hotplug
ability, and then use rte_eth_dev_event_handler_install to install
the hotplug event handler for ethdev. When eal detect the hotplug event,
it will call the ethdev callback to process it. If the event is hotplug
removal, it will trigger the RTE_ETH_EVENT_INTR_RMV event into ethdev
callback to let app process the hotplug for this ethdev.

This is an example for other driver, that if any driver support hotplug
feature could be use this way to install hotplug handler.

Signed-off-by: Jeff Guo <jia.guo@intel.com>
Acked-by: Qi Zhang <qi.z.zhang@intel.com>
---
v5->v4:
no change.
---
 drivers/net/i40e/i40e_ethdev.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 13c5d32..8fccf04 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -688,7 +688,7 @@ static int eth_i40e_pci_remove(struct rte_pci_device *pci_dev)
 static struct rte_pci_driver rte_i40e_pmd = {
 	.id_table = pci_id_i40e_map,
 	.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
-		     RTE_PCI_DRV_IOVA_AS_VA,
+		     RTE_PCI_DRV_IOVA_AS_VA | RTE_PCI_DRV_INTR_RMV,
 	.probe = eth_i40e_pci_probe,
 	.remove = eth_i40e_pci_remove,
 };
@@ -1442,6 +1442,9 @@ eth_i40e_dev_init(struct rte_eth_dev *dev, void *init_params __rte_unused)
 	rte_intr_callback_register(intr_handle,
 				   i40e_dev_interrupt_handler, dev);
 
+	/* install the dev event handler for ethdev. */
+	rte_eth_dev_event_handler_install(dev);
+
 	/* configure and enable device interrupt */
 	i40e_pf_config_irq0(hw, TRUE);
 	i40e_pf_enable_irq0(hw);
@@ -1674,6 +1677,9 @@ eth_i40e_dev_uninit(struct rte_eth_dev *dev)
 	/* Remove all Traffic Manager configuration */
 	i40e_tm_conf_uninit(dev);
 
+	/* uninstall the dev event handler for ethdev. */
+	rte_eth_dev_event_handler_uninstall(dev);
+
 	return 0;
 }
 
-- 
2.7.4

[dpdk-dev] [PATCH v5 4/4] testpmd: remove the dev event callback register

[Jeff Guo]

Since now we can use driver to management the eal event for hotplug,
so no need to register dev event callback in app anymore. This patch
remove the related code.

Signed-off-by: Jeff Guo <jia.guo@intel.com>
Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
v5->v4:
no change.
---
 app/test-pmd/testpmd.c | 76 --------------------------------------------------
 1 file changed, 76 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 24c1998..10ed660 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -400,12 +400,6 @@ static void check_all_ports_link_status(uint32_t port_mask);
 static int eth_event_callback(portid_t port_id,
 			      enum rte_eth_event_type type,
 			      void *param, void *ret_param);
-static void eth_dev_event_callback(char *device_name,
-				enum rte_dev_event_type type,
-				void *param);
-static int eth_dev_event_callback_register(void);
-static int eth_dev_event_callback_unregister(void);
-
 
 /*
  * Check if all the ports are started.
@@ -1915,39 +1909,6 @@ reset_port(portid_t pid)
 	printf("Done\n");
 }
 
-static int
-eth_dev_event_callback_register(void)
-{
-	int ret;
-
-	/* register the device event callback */
-	ret = rte_dev_event_callback_register(NULL,
-		eth_dev_event_callback, NULL);
-	if (ret) {
-		printf("Failed to register device event callback\n");
-		return -1;
-	}
-
-	return 0;
-}
-
-
-static int
-eth_dev_event_callback_unregister(void)
-{
-	int ret;
-
-	/* unregister the device event callback */
-	ret = rte_dev_event_callback_unregister(NULL,
-		eth_dev_event_callback, NULL);
-	if (ret < 0) {
-		printf("Failed to unregister device event callback\n");
-		return -1;
-	}
-
-	return 0;
-}
-
 void
 attach_port(char *identifier)
 {
@@ -2049,10 +2010,6 @@ pmd_test_exit(void)
 			RTE_LOG(ERR, EAL,
 				"fail to stop device event monitor.");
 
-		ret = eth_dev_event_callback_unregister();
-		if (ret)
-			RTE_LOG(ERR, EAL,
-				"fail to unregister all event callbacks.");
 	}
 
 	printf("\nBye...\n");
@@ -2191,37 +2148,6 @@ eth_event_callback(portid_t port_id, enum rte_eth_event_type type, void *param,
 	return 0;
 }
 
-/* This function is used by the interrupt thread */
-static void
-eth_dev_event_callback(char *device_name, enum rte_dev_event_type type,
-			     __rte_unused void *arg)
-{
-	if (type >= RTE_DEV_EVENT_MAX) {
-		fprintf(stderr, "%s called upon invalid event %d\n",
-			__func__, type);
-		fflush(stderr);
-	}
-
-	switch (type) {
-	case RTE_DEV_EVENT_REMOVE:
-		RTE_LOG(ERR, EAL, "The device: %s has been removed!\n",
-			device_name);
-		/* TODO: After finish failure handle, begin to stop
-		 * packet forward, stop port, close port, detach port.
-		 */
-		break;
-	case RTE_DEV_EVENT_ADD:
-		RTE_LOG(ERR, EAL, "The device: %s has been added!\n",
-			device_name);
-		/* TODO: After finish kernel driver binding,
-		 * begin to attach port.
-		 */
-		break;
-	default:
-		break;
-	}
-}
-
 static int
 set_tx_queue_stats_mapping_registers(portid_t port_id, struct rte_port *port)
 {
@@ -2735,8 +2661,6 @@ main(int argc, char** argv)
 			rte_errno = EINVAL;
 			return -1;
 		}
-		eth_dev_event_callback_register();
-
 	}
 
 	if (start_port(RTE_PORT_ALL) != 0)
-- 
2.7.4

[dpdk-dev] [PATCH v5 0/4] Install eal hotplug event handler in i40e/ixgbe

[Jeff Guo]

As we may know, we have eal event for rte device hotplug and ethdev event
for ethdev hotplug. Some ethdev need to use eal event to detect hotplug
behaviors, the privors way is register eal event callback in app, but seems
that it will have some race between these 2 event processes. In oder to fix
the it, it might be better to find a way to combind these 2 events detect.

This patch set introduce a way to combind these 2 event, by register the
ethdev eal event callback in ether dev and trigger the ethdev hotplug event
in the callback. That will let the ethdev device can easy process hotplug
by a common way.

Here let i40e/ixgbe pmd driver for example, other driver which support
hotplug feature could be use this way to detect and process hotplug.

patch history:
v5->v4:
refine some code style and typo

v4->v3:
change to use device event handler install api

v3->v2:
remove the callback from driver to ethdev for common.

v2->v1:
add ixgbe hotplug detect case.
refine some doc.

Jeff Guo (4):
  ethdev: Add eal device event callback
  net/ixgbe: install ethdev hotplug handler in ixgbe
  net/i40e: install hotplug handler in i40e
  testpmd: remove the dev event callback register

 app/test-pmd/testpmd.c                   | 76 --------------------------------
 doc/guides/rel_notes/release_18_08.rst   | 12 +++++
 drivers/net/i40e/i40e_ethdev.c           |  8 +++-
 drivers/net/ixgbe/ixgbe_ethdev.c         |  8 +++-
 lib/librte_ethdev/rte_ethdev.c           | 59 +++++++++++++++++++++++++
 lib/librte_ethdev/rte_ethdev_driver.h    | 32 ++++++++++++++
 lib/librte_ethdev/rte_ethdev_version.map |  2 +
 7 files changed, 119 insertions(+), 78 deletions(-)

-- 
2.7.4

[dpdk-dev] [PATCH v5 1/4] ethdev: Add eal device event handler APIs

[Jeff Guo]

Implement a couple of eal device event handler install/uninstall APIs
in ethdev. Each ethdev install the handler in PMDs, so each callback
corresponding with one port, and process the eal device event for specific
port. If PMDs install the handler when initial device, it could have
chance to manage the eal device event, such as register device event
callback, then monitor and process the hotplug event.

Signed-off-by: Jeff Guo <jia.guo@intel.com>
Reviewed-by: Qi Zhang <qi.z.zhang@intel.com>
---
v5->v4:
refine some code style and typo
---
 doc/guides/rel_notes/release_18_08.rst   | 12 +++++++
 lib/librte_ethdev/rte_ethdev.c           | 59 ++++++++++++++++++++++++++++++++
 lib/librte_ethdev/rte_ethdev_driver.h    | 32 +++++++++++++++++
 lib/librte_ethdev/rte_ethdev_version.map |  2 ++
 4 files changed, 105 insertions(+)

diff --git a/doc/guides/rel_notes/release_18_08.rst b/doc/guides/rel_notes/release_18_08.rst
index bc01242..b6482ce 100644
--- a/doc/guides/rel_notes/release_18_08.rst
+++ b/doc/guides/rel_notes/release_18_08.rst
@@ -46,6 +46,18 @@ New Features
   Flow API support has been added to CXGBE Poll Mode Driver to offload
   flows to Chelsio T5/T6 NICs.
 
+* **Added eal device event process helper in ethdev.**
+
+  Implement a couple of eal device event handler install/uninstall APIs in
+  ethdev, these helper could let PMDs have chance to manage the eal device
+  event, such as register device event callback, then monitor and process
+  hotplug event.
+
+  * ``rte_eth_dev_event_handler_install`` for PMDs use to install the device
+    event handler.
+  * ``rte_eth_dev_event_handler_uninstall`` for PMDs use to uninstall the device
+    event handler.
+
 
 API Changes
 -----------
diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index a9977df..4d28db5 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -4518,6 +4518,65 @@ rte_eth_devargs_parse(const char *dargs, struct rte_eth_devargs *eth_da)
 	return result;
 }
 
+static void __rte_experimental
+eth_dev_event_callback(char *device_name, enum rte_dev_event_type type,
+			     void *arg)
+{
+	struct rte_eth_dev *eth_dev = (struct rte_eth_dev *)arg;
+
+	switch (type) {
+	case RTE_DEV_EVENT_REMOVE:
+		ethdev_log(INFO, "The device %s has been removed!\n",
+			    device_name);
+
+		if (!device_name || !eth_dev)
+			return;
+
+		if (!(eth_dev->data->dev_flags & RTE_ETH_DEV_INTR_RMV))
+			return;
+
+		if (!strcmp(device_name, eth_dev->device->name))
+			_rte_eth_dev_callback_process(eth_dev,
+						      RTE_ETH_EVENT_INTR_RMV,
+						      NULL);
+		break;
+	case RTE_DEV_EVENT_ADD:
+		ethdev_log(INFO, "The device %s has been added!\n",
+			device_name);
+		break;
+	default:
+		break;
+	}
+}
+
+int __rte_experimental
+rte_eth_dev_event_handler_install(struct rte_eth_dev *eth_dev)
+{
+	int result = 0;
+
+	result = rte_dev_event_callback_register(eth_dev->device->name,
+					eth_dev_event_callback, eth_dev);
+	if (result)
+		ethdev_log(ERR, "device event callback register failed for "
+			"device %s!\n", eth_dev->device->name);
+
+	return result;
+}
+
+int __rte_experimental
+rte_eth_dev_event_handler_uninstall(struct rte_eth_dev *eth_dev)
+{
+	int result = 0;
+
+	result = rte_dev_event_callback_unregister(eth_dev->device->name,
+					eth_dev_event_callback, eth_dev);
+	if (result)
+		ethdev_log(ERR, "device event callback unregister failed for"
+			" device %s!\n", eth_dev->device->name);
+
+	return result;
+}
+
 RTE_INIT(ethdev_init_log);
 static void
 ethdev_init_log(void)
diff --git a/lib/librte_ethdev/rte_ethdev_driver.h b/lib/librte_ethdev/rte_ethdev_driver.h
index c9c825e..3de1cd4 100644
--- a/lib/librte_ethdev/rte_ethdev_driver.h
+++ b/lib/librte_ethdev/rte_ethdev_driver.h
@@ -82,6 +82,38 @@ int rte_eth_dev_release_port(struct rte_eth_dev *eth_dev);
 void _rte_eth_dev_reset(struct rte_eth_dev *dev);
 
 /**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Implement a helper to install the device event handler for the specific
+ * ether device.
+ *
+ * @param dev
+ *  Pointer to struct rte_eth_dev.
+ *
+ * @return
+ *   - 0 on success, negative on error
+ */
+int __rte_experimental
+rte_eth_dev_event_handler_install(struct rte_eth_dev *dev);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Implement a helper to uninstall the device event handler for the specific
+ * ether device.
+ *
+ * @param dev
+ *  Pointer to struct rte_eth_dev.
+ *
+ * @return
+ *   - 0 on success, negative on error
+ */
+int __rte_experimental
+rte_eth_dev_event_handler_uninstall(struct rte_eth_dev *dev);
+
+/**
  * @internal Executes all the user application registered callbacks for
  * the specific device. It is for DPDK internal user only. User
  * application should not call it directly.
diff --git a/lib/librte_ethdev/rte_ethdev_version.map b/lib/librte_ethdev/rte_ethdev_version.map
index 40cf42b..dbd07a5 100644
--- a/lib/librte_ethdev/rte_ethdev_version.map
+++ b/lib/librte_ethdev/rte_ethdev_version.map
@@ -220,6 +220,8 @@ EXPERIMENTAL {
 	rte_eth_dev_count_total;
 	rte_eth_dev_create;
 	rte_eth_dev_destroy;
+	rte_eth_dev_event_handler_install;
+	rte_eth_dev_event_handler_uninstall;
 	rte_eth_dev_get_module_eeprom;
 	rte_eth_dev_get_module_info;
 	rte_eth_dev_is_removed;
-- 
2.7.4

[dpdk-dev] [PATCH v5 2/4] net/ixgbe: install ethdev hotplug handler in ixgbe

[Jeff Guo]

This patch aim to enable hotplug detect in ixgbe PMD. Firstly it
set the flags RTE_PCI_DRV_INTR_RMV in drv_flags to announce the hotplug
ability, and then use rte_eth_dev_event_handler_install to install
the hotplug event handler for ethdev. When eal detect the hotplug event,
it will call the ethdev callback to process it. If the event is hotplug
removal, it will trigger the RTE_ETH_EVENT_INTR_RMV event into ethdev
callback to let app process the hotplug for this ethdev.

This is an example for other driver, that if any driver support hotplug
feature could be use this way to install hotplug handler.

Signed-off-by: Jeff Guo <jia.guo@intel.com>
Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
v5->v4:
no change.
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 87d2ad0..e7ae9bf 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -1678,6 +1678,9 @@ eth_ixgbevf_dev_init(struct rte_eth_dev *eth_dev)
 	rte_intr_enable(intr_handle);
 	ixgbevf_intr_enable(eth_dev);
 
+	/* install the dev event handler for ethdev. */
+	rte_eth_dev_event_handler_install(eth_dev);
+
 	PMD_INIT_LOG(DEBUG, "port %d vendorID=0x%x deviceID=0x%x mac.type=%s",
 		     eth_dev->data->port_id, pci_dev->id.vendor_id,
 		     pci_dev->id.device_id, "ixgbe_mac_82599_vf");
@@ -1718,6 +1721,9 @@ eth_ixgbevf_dev_uninit(struct rte_eth_dev *eth_dev)
 	rte_intr_callback_unregister(intr_handle,
 				     ixgbevf_dev_interrupt_handler, eth_dev);
 
+	/* uninstall the dev event handler for ethdev. */
+	rte_eth_dev_event_handler_uninstall(eth_dev);
+
 	return 0;
 }
 
@@ -1801,7 +1807,7 @@ static int eth_ixgbe_pci_remove(struct rte_pci_device *pci_dev)
 static struct rte_pci_driver rte_ixgbe_pmd = {
 	.id_table = pci_id_ixgbe_map,
 	.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
-		     RTE_PCI_DRV_IOVA_AS_VA,
+		     RTE_PCI_DRV_IOVA_AS_VA | RTE_PCI_DRV_INTR_RMV,
 	.probe = eth_ixgbe_pci_probe,
 	.remove = eth_ixgbe_pci_remove,
 };
-- 
2.7.4

[dpdk-dev] [PATCH v5 3/4] net/i40e: install hotplug handler in i40e

[Jeff Guo]

This patch aim to enable hotplug detect in i40e PMD. Firstly it
set the flags RTE_PCI_DRV_INTR_RMV in drv_flags to announce the hotplug
ability, and then use rte_eth_dev_event_handler_install to install
the hotplug event handler for ethdev. When eal detect the hotplug event,
it will call the ethdev callback to process it. If the event is hotplug
removal, it will trigger the RTE_ETH_EVENT_INTR_RMV event into ethdev
callback to let app process the hotplug for this ethdev.

This is an example for other driver, that if any driver support hotplug
feature could be use this way to install hotplug handler.

Signed-off-by: Jeff Guo <jia.guo@intel.com>
Acked-by: Qi Zhang <qi.z.zhang@intel.com>
---
v5->v4:
no change.
---
 drivers/net/i40e/i40e_ethdev.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 13c5d32..8fccf04 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -688,7 +688,7 @@ static int eth_i40e_pci_remove(struct rte_pci_device *pci_dev)
 static struct rte_pci_driver rte_i40e_pmd = {
 	.id_table = pci_id_i40e_map,
 	.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
-		     RTE_PCI_DRV_IOVA_AS_VA,
+		     RTE_PCI_DRV_IOVA_AS_VA | RTE_PCI_DRV_INTR_RMV,
 	.probe = eth_i40e_pci_probe,
 	.remove = eth_i40e_pci_remove,
 };
@@ -1442,6 +1442,9 @@ eth_i40e_dev_init(struct rte_eth_dev *dev, void *init_params __rte_unused)
 	rte_intr_callback_register(intr_handle,
 				   i40e_dev_interrupt_handler, dev);
 
+	/* install the dev event handler for ethdev. */
+	rte_eth_dev_event_handler_install(dev);
+
 	/* configure and enable device interrupt */
 	i40e_pf_config_irq0(hw, TRUE);
 	i40e_pf_enable_irq0(hw);
@@ -1674,6 +1677,9 @@ eth_i40e_dev_uninit(struct rte_eth_dev *dev)
 	/* Remove all Traffic Manager configuration */
 	i40e_tm_conf_uninit(dev);
 
+	/* uninstall the dev event handler for ethdev. */
+	rte_eth_dev_event_handler_uninstall(dev);
+
 	return 0;
 }
 
-- 
2.7.4

[dpdk-dev] [PATCH v5 4/4] testpmd: remove the dev event callback register

[Jeff Guo]

Since now we can use driver to management the eal event for hotplug,
so no need to register dev event callback in app anymore. This patch
remove the related code.

Signed-off-by: Jeff Guo <jia.guo@intel.com>
Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
v5->v4:
no change.
---
 app/test-pmd/testpmd.c | 76 --------------------------------------------------
 1 file changed, 76 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 24c1998..10ed660 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -400,12 +400,6 @@ static void check_all_ports_link_status(uint32_t port_mask);
 static int eth_event_callback(portid_t port_id,
 			      enum rte_eth_event_type type,
 			      void *param, void *ret_param);
-static void eth_dev_event_callback(char *device_name,
-				enum rte_dev_event_type type,
-				void *param);
-static int eth_dev_event_callback_register(void);
-static int eth_dev_event_callback_unregister(void);
-
 
 /*
  * Check if all the ports are started.
@@ -1915,39 +1909,6 @@ reset_port(portid_t pid)
 	printf("Done\n");
 }
 
-static int
-eth_dev_event_callback_register(void)
-{
-	int ret;
-
-	/* register the device event callback */
-	ret = rte_dev_event_callback_register(NULL,
-		eth_dev_event_callback, NULL);
-	if (ret) {
-		printf("Failed to register device event callback\n");
-		return -1;
-	}
-
-	return 0;
-}
-
-
-static int
-eth_dev_event_callback_unregister(void)
-{
-	int ret;
-
-	/* unregister the device event callback */
-	ret = rte_dev_event_callback_unregister(NULL,
-		eth_dev_event_callback, NULL);
-	if (ret < 0) {
-		printf("Failed to unregister device event callback\n");
-		return -1;
-	}
-
-	return 0;
-}
-
 void
 attach_port(char *identifier)
 {
@@ -2049,10 +2010,6 @@ pmd_test_exit(void)
 			RTE_LOG(ERR, EAL,
 				"fail to stop device event monitor.");
 
-		ret = eth_dev_event_callback_unregister();
-		if (ret)
-			RTE_LOG(ERR, EAL,
-				"fail to unregister all event callbacks.");
 	}
 
 	printf("\nBye...\n");
@@ -2191,37 +2148,6 @@ eth_event_callback(portid_t port_id, enum rte_eth_event_type type, void *param,
 	return 0;
 }
 
-/* This function is used by the interrupt thread */
-static void
-eth_dev_event_callback(char *device_name, enum rte_dev_event_type type,
-			     __rte_unused void *arg)
-{
-	if (type >= RTE_DEV_EVENT_MAX) {
-		fprintf(stderr, "%s called upon invalid event %d\n",
-			__func__, type);
-		fflush(stderr);
-	}
-
-	switch (type) {
-	case RTE_DEV_EVENT_REMOVE:
-		RTE_LOG(ERR, EAL, "The device: %s has been removed!\n",
-			device_name);
-		/* TODO: After finish failure handle, begin to stop
-		 * packet forward, stop port, close port, detach port.
-		 */
-		break;
-	case RTE_DEV_EVENT_ADD:
-		RTE_LOG(ERR, EAL, "The device: %s has been added!\n",
-			device_name);
-		/* TODO: After finish kernel driver binding,
-		 * begin to attach port.
-		 */
-		break;
-	default:
-		break;
-	}
-}
-
 static int
 set_tx_queue_stats_mapping_registers(portid_t port_id, struct rte_port *port)
 {
@@ -2735,8 +2661,6 @@ main(int argc, char** argv)
 			rte_errno = EINVAL;
 			return -1;
 		}
-		eth_dev_event_callback_register();
-
 	}
 
 	if (start_port(RTE_PORT_ALL) != 0)
-- 
2.7.4

[dpdk-dev] [PATCH v6 0/4] Install eal event handler in i40e/ixgbe

[Jeff Guo]

We currently have eal event and ethdev event for ethdev hotplug. Some
ethdev's need to use an eal event to detect hotplug behaviors. Previously,
we needed to register an eal event callback in the application, but this
potentially causes a race condition between the eal event process to the
ethdev event process. It might better to fix this issue.

This patch set introduces a way to combine these 2 event by registering
the ethdev eal event callback in the ethdev and triggering the ethdev
hotplug event in the callback. This will let the ethdev device easily
process the hotplug in a common way.

Drivers which support hotplug could use this mechanism to detect and
process hotplugs.

patch history:
v6->v5:
refine some commit log

v5->v4:
refine some code style and typo

v4->v3:
change to use device event handler install api

v3->v2:
remove the callback from driver to ethdev for common.

v2->v1:
add ixgbe hotplug detect case.
refine some doc.

Jeff Guo (4):
  ethdev: Add eal device event callback
  net/ixgbe: install ethdev hotplug handler in ixgbe
  net/i40e: install hotplug handler in i40e
  testpmd: remove the dev event callback register

 app/test-pmd/testpmd.c                   | 78 --------------------------------
 doc/guides/rel_notes/release_18_08.rst   | 12 +++++
 drivers/net/i40e/i40e_ethdev.c           |  8 +++-
 drivers/net/ixgbe/ixgbe_ethdev.c         |  8 +++-
 lib/librte_ethdev/rte_ethdev.c           | 61 +++++++++++++++++++++++++
 lib/librte_ethdev/rte_ethdev_driver.h    | 32 +++++++++++++
 lib/librte_ethdev/rte_ethdev_version.map |  2 +
 7 files changed, 121 insertions(+), 80 deletions(-)

-- 
2.7.4

[dpdk-dev] [PATCH v6 1/4] ethdev: Add eal device event callback

[Jeff Guo]

Implement a couple of eal device event handler install/unintall APIs in
Ethdev as below:
 - rte_eth_dev_event_handler_install
 - rte_eth_dev_event_handler_uninstall

Each ethdev could call these APIs to install eal event handler in the PMDs,
with each callback corresponding to one port, and process the eal device
event for the specific port. Especially, it could use for process the
hotplug event.

Signed-off-by: Jeff Guo <jia.guo@intel.com>
Reviewed-by: Qi Zhang <qi.z.zhang@intel.com>
---
v6->v5:
refine commit log
---
 doc/guides/rel_notes/release_18_08.rst   | 12 +++++++
 lib/librte_ethdev/rte_ethdev.c           | 61 ++++++++++++++++++++++++++++++++
 lib/librte_ethdev/rte_ethdev_driver.h    | 32 +++++++++++++++++
 lib/librte_ethdev/rte_ethdev_version.map |  2 ++
 4 files changed, 107 insertions(+)

diff --git a/doc/guides/rel_notes/release_18_08.rst b/doc/guides/rel_notes/release_18_08.rst
index 95dc1e0..90478cf 100644
--- a/doc/guides/rel_notes/release_18_08.rst
+++ b/doc/guides/rel_notes/release_18_08.rst
@@ -122,6 +122,18 @@ New Features
   ``rte_dev_hotplug_handle_enable`` and ``rte_dev_hotplug_handle_disable`` are
   for enable or disable failure handle mechanism for hotplug.
 
+* **Added eal device event process helper in ethdev.**
+
+  Implement a couple of eal device event handler install/uninstall APIs in
+  ethdev, these helper could let PMDs have chance to manage the eal device
+  event, such as register device event callback, then monitor and process
+  hotplug event.
+
+  * ``rte_eth_dev_event_handler_install`` for PMDs use to install the device
+    event handler.
+  * ``rte_eth_dev_event_handler_uninstall`` for PMDs use to uninstall the device
+    event handler.
+
 
 API Changes
 -----------
diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 4c32025..d68bd4c 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -4417,6 +4417,67 @@ rte_eth_devargs_parse(const char *dargs, struct rte_eth_devargs *eth_da)
 	return result;
 }
 
+static void __rte_experimental
+eth_dev_event_callback(char *device_name, enum rte_dev_event_type type,
+		       void *arg)
+{
+	struct rte_eth_dev *eth_dev = (struct rte_eth_dev *)arg;
+
+	switch (type) {
+	case RTE_DEV_EVENT_REMOVE:
+		RTE_ETHDEV_LOG(INFO, "The device %s has been removed!\n",
+			       device_name);
+
+		if (!device_name || !eth_dev)
+			return;
+
+		if (!(eth_dev->data->dev_flags & RTE_ETH_DEV_INTR_RMV))
+			return;
+
+		if (!strcmp(device_name, eth_dev->device->name))
+			_rte_eth_dev_callback_process(eth_dev,
+						      RTE_ETH_EVENT_INTR_RMV,
+						      NULL);
+		break;
+	case RTE_DEV_EVENT_ADD:
+		RTE_ETHDEV_LOG(INFO, "The device %s has been added!\n",
+			       device_name);
+		break;
+	default:
+		break;
+	}
+}
+
+int __rte_experimental
+rte_eth_dev_event_handler_install(struct rte_eth_dev *eth_dev)
+{
+	int result = 0;
+
+	result = rte_dev_event_callback_register(eth_dev->device->name,
+						 eth_dev_event_callback,
+						 eth_dev);
+	if (result)
+		RTE_ETHDEV_LOG(ERR, "device event callback register failed "
+			       "for device:%s!\n", eth_dev->device->name);
+
+	return result;
+}
+
+int __rte_experimental
+rte_eth_dev_event_handler_uninstall(struct rte_eth_dev *eth_dev)
+{
+	int result = 0;
+
+	result = rte_dev_event_callback_unregister(eth_dev->device->name,
+						   eth_dev_event_callback,
+						   eth_dev);
+	if (result)
+		RTE_ETHDEV_LOG(ERR, "device event callback unregister failed "
+			       "for device:%s!\n", eth_dev->device->name);
+
+	return result;
+}
+
 RTE_INIT(ethdev_init_log)
 {
 	rte_eth_dev_logtype = rte_log_register("lib.ethdev");
diff --git a/lib/librte_ethdev/rte_ethdev_driver.h b/lib/librte_ethdev/rte_ethdev_driver.h
index c6d9bc1..7e3d085 100644
--- a/lib/librte_ethdev/rte_ethdev_driver.h
+++ b/lib/librte_ethdev/rte_ethdev_driver.h
@@ -81,6 +81,38 @@ int rte_eth_dev_release_port(struct rte_eth_dev *eth_dev);
 void _rte_eth_dev_reset(struct rte_eth_dev *dev);
 
 /**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Implement a helper to install the device event handler for the specific
+ * ether device.
+ *
+ * @param dev
+ *  Pointer to struct rte_eth_dev.
+ *
+ * @return
+ *   - 0 on success, negative on error
+ */
+int __rte_experimental
+rte_eth_dev_event_handler_install(struct rte_eth_dev *dev);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Implement a helper to uninstall the device event handler for the specific
+ * ether device.
+ *
+ * @param dev
+ *  Pointer to struct rte_eth_dev.
+ *
+ * @return
+ *   - 0 on success, negative on error
+ */
+int __rte_experimental
+rte_eth_dev_event_handler_uninstall(struct rte_eth_dev *dev);
+
+/**
  * @internal Executes all the user application registered callbacks for
  * the specific device. It is for DPDK internal user only. User
  * application should not call it directly.
diff --git a/lib/librte_ethdev/rte_ethdev_version.map b/lib/librte_ethdev/rte_ethdev_version.map
index 38f117f..3cc02b6 100644
--- a/lib/librte_ethdev/rte_ethdev_version.map
+++ b/lib/librte_ethdev/rte_ethdev_version.map
@@ -227,6 +227,8 @@ EXPERIMENTAL {
 	rte_eth_dev_count_total;
 	rte_eth_dev_create;
 	rte_eth_dev_destroy;
+	rte_eth_dev_event_handler_install;
+	rte_eth_dev_event_handler_uninstall;
 	rte_eth_dev_get_module_eeprom;
 	rte_eth_dev_get_module_info;
 	rte_eth_dev_is_removed;
-- 
2.7.4

[dpdk-dev] [PATCH v6 2/4] net/ixgbe: install ethdev hotplug handler in ixgbe

[Jeff Guo]

This patch aims to enable hotplug detection in ixgbe PMD.

First, it sets the “RTE_PCI_DRV_INTR_RMV” flag in drv_flags to
announce the ability of hotplug processing. Then, it calls the
“rte_eth_dev_event_handler_install” API to install the device
event handler for ethdev. When the eal device event be detected,
it calls the ethdev callback to process it. If the event is the
hotplug-out event, it will trigger the “RTE_ETH_EVENT_INTR_RMV”
event into the ethdev callback to allow the application's callback
to process hotplug for this ethdev.

Signed-off-by: Jeff Guo <jia.guo@intel.com>
Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
v6->v5:
refine commit log
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 26b1927..37404cf 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -1678,6 +1678,9 @@ eth_ixgbevf_dev_init(struct rte_eth_dev *eth_dev)
 	rte_intr_enable(intr_handle);
 	ixgbevf_intr_enable(eth_dev);
 
+	/* install the dev event handler for ethdev. */
+	rte_eth_dev_event_handler_install(eth_dev);
+
 	PMD_INIT_LOG(DEBUG, "port %d vendorID=0x%x deviceID=0x%x mac.type=%s",
 		     eth_dev->data->port_id, pci_dev->id.vendor_id,
 		     pci_dev->id.device_id, "ixgbe_mac_82599_vf");
@@ -1718,6 +1721,9 @@ eth_ixgbevf_dev_uninit(struct rte_eth_dev *eth_dev)
 	rte_intr_callback_unregister(intr_handle,
 				     ixgbevf_dev_interrupt_handler, eth_dev);
 
+	/* uninstall the dev event handler for ethdev. */
+	rte_eth_dev_event_handler_uninstall(eth_dev);
+
 	return 0;
 }
 
@@ -1801,7 +1807,7 @@ static int eth_ixgbe_pci_remove(struct rte_pci_device *pci_dev)
 static struct rte_pci_driver rte_ixgbe_pmd = {
 	.id_table = pci_id_ixgbe_map,
 	.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
-		     RTE_PCI_DRV_IOVA_AS_VA,
+		     RTE_PCI_DRV_IOVA_AS_VA | RTE_PCI_DRV_INTR_RMV,
 	.probe = eth_ixgbe_pci_probe,
 	.remove = eth_ixgbe_pci_remove,
 };
-- 
2.7.4

[dpdk-dev] [PATCH v6 3/4] net/i40e: install hotplug handler in i40e

[Jeff Guo]

This patch aims to enable hotplug detection in i40e PMD.

First, it sets the “RTE_PCI_DRV_INTR_RMV” flag in drv_flags to
announce the ability of hotplug processing. Then, it calls the
“rte_eth_dev_event_handler_install” API to install the device
event handler for ethdev. When the eal device event be detected,
it calls the ethdev callback to process it. If the event is the
hotplug-out event, it will trigger the “RTE_ETH_EVENT_INTR_RMV”
event into the ethdev callback to allow the application's callback
to process hotplug for this ethdev.

Signed-off-by: Jeff Guo <jia.guo@intel.com>
Acked-by: Qi Zhang <qi.z.zhang@intel.com>
---
v6->v5:
refine commit log
---
 drivers/net/i40e/i40e_ethdev.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 85a6a86..bd9b3ce 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -697,7 +697,7 @@ static int eth_i40e_pci_remove(struct rte_pci_device *pci_dev)
 static struct rte_pci_driver rte_i40e_pmd = {
 	.id_table = pci_id_i40e_map,
 	.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
-		     RTE_PCI_DRV_IOVA_AS_VA,
+		     RTE_PCI_DRV_IOVA_AS_VA | RTE_PCI_DRV_INTR_RMV,
 	.probe = eth_i40e_pci_probe,
 	.remove = eth_i40e_pci_remove,
 };
@@ -1466,6 +1466,9 @@ eth_i40e_dev_init(struct rte_eth_dev *dev, void *init_params __rte_unused)
 	rte_intr_callback_register(intr_handle,
 				   i40e_dev_interrupt_handler, dev);
 
+	/* install the dev event handler for ethdev. */
+	rte_eth_dev_event_handler_install(dev);
+
 	/* configure and enable device interrupt */
 	i40e_pf_config_irq0(hw, TRUE);
 	i40e_pf_enable_irq0(hw);
@@ -1697,6 +1700,9 @@ eth_i40e_dev_uninit(struct rte_eth_dev *dev)
 	/* Remove all Traffic Manager configuration */
 	i40e_tm_conf_uninit(dev);
 
+	/* uninstall the dev event handler for ethdev. */
+	rte_eth_dev_event_handler_uninstall(dev);
+
 	return 0;
 }
 
-- 
2.7.4

[dpdk-dev] [PATCH v6 4/4] testpmd: remove the dev event callback register

[Jeff Guo]

Since we can now use the PMDs to manage the eal event for hotplug, we no
longer need to register the device event callback in the application
anymore. This patch removes the redundant code.

Signed-off-by: Jeff Guo <jia.guo@intel.com>
Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
v6->v5:
refine commit log
---
 app/test-pmd/testpmd.c | 78 --------------------------------------------------
 1 file changed, 78 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 12fc497..0e2c744 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -432,12 +432,6 @@ static void check_all_ports_link_status(uint32_t port_mask);
 static int eth_event_callback(portid_t port_id,
 			      enum rte_eth_event_type type,
 			      void *param, void *ret_param);
-static void eth_dev_event_callback(char *device_name,
-				enum rte_dev_event_type type,
-				void *param);
-static int eth_dev_event_callback_register(void);
-static int eth_dev_event_callback_unregister(void);
-
 
 /*
  * Check if all the ports are started.
@@ -1959,39 +1953,6 @@ reset_port(portid_t pid)
 	printf("Done\n");
 }
 
-static int
-eth_dev_event_callback_register(void)
-{
-	int ret;
-
-	/* register the device event callback */
-	ret = rte_dev_event_callback_register(NULL,
-		eth_dev_event_callback, NULL);
-	if (ret) {
-		printf("Failed to register device event callback\n");
-		return -1;
-	}
-
-	return 0;
-}
-
-
-static int
-eth_dev_event_callback_unregister(void)
-{
-	int ret;
-
-	/* unregister the device event callback */
-	ret = rte_dev_event_callback_unregister(NULL,
-		eth_dev_event_callback, NULL);
-	if (ret < 0) {
-		printf("Failed to unregister device event callback\n");
-		return -1;
-	}
-
-	return 0;
-}
-
 void
 attach_port(char *identifier)
 {
@@ -2104,10 +2065,6 @@ pmd_test_exit(void)
 			return;
 		}
 
-		ret = eth_dev_event_callback_unregister();
-		if (ret)
-			return;
-
 		ret = rte_dev_hotplug_handle_disable();
 		if (ret) {
 			RTE_LOG(ERR, EAL,
@@ -2252,37 +2209,6 @@ eth_event_callback(portid_t port_id, enum rte_eth_event_type type, void *param,
 	return 0;
 }
 
-/* This function is used by the interrupt thread */
-static void
-eth_dev_event_callback(char *device_name, enum rte_dev_event_type type,
-			     __rte_unused void *arg)
-{
-	if (type >= RTE_DEV_EVENT_MAX) {
-		fprintf(stderr, "%s called upon invalid event %d\n",
-			__func__, type);
-		fflush(stderr);
-	}
-
-	switch (type) {
-	case RTE_DEV_EVENT_REMOVE:
-		RTE_LOG(ERR, EAL, "The device: %s has been removed!\n",
-			device_name);
-		/* TODO: After finish failure handle, begin to stop
-		 * packet forward, stop port, close port, detach port.
-		 */
-		break;
-	case RTE_DEV_EVENT_ADD:
-		RTE_LOG(ERR, EAL, "The device: %s has been added!\n",
-			device_name);
-		/* TODO: After finish kernel driver binding,
-		 * begin to attach port.
-		 */
-		break;
-	default:
-		break;
-	}
-}
-
 static int
 set_tx_queue_stats_mapping_registers(portid_t port_id, struct rte_port *port)
 {
@@ -2805,10 +2731,6 @@ main(int argc, char** argv)
 				"fail to start device event monitoring.");
 			return -1;
 		}
-
-		ret = eth_dev_event_callback_register();
-		if (ret)
-			return -1;
 	}
 
 	if (start_port(RTE_PORT_ALL) != 0)
-- 
2.7.4

Re: [dpdk-dev] [PATCH v5 2/4] net/ixgbe: install ethdev hotplug handler in ixgbe

[Ferruh Yigit]

On 7/11/2018 12:51 PM, Jeff Guo wrote:
> This patch aim to enable hotplug detect in ixgbe PMD. Firstly it
> set the flags RTE_PCI_DRV_INTR_RMV in drv_flags to announce the hotplug
> ability, and then use rte_eth_dev_event_handler_install to install
> the hotplug event handler for ethdev. When eal detect the hotplug event,
> it will call the ethdev callback to process it. If the event is hotplug
> removal, it will trigger the RTE_ETH_EVENT_INTR_RMV event into ethdev
> callback to let app process the hotplug for this ethdev.
> 
> This is an example for other driver, that if any driver support hotplug
> feature could be use this way to install hotplug handler.
> 
> Signed-off-by: Jeff Guo <jia.guo@intel.com>
> Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> ---
> v5->v4:
> no change.
> ---
>  drivers/net/ixgbe/ixgbe_ethdev.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
> index 87d2ad0..e7ae9bf 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -1678,6 +1678,9 @@ eth_ixgbevf_dev_init(struct rte_eth_dev *eth_dev)
>  	rte_intr_enable(intr_handle);
>  	ixgbevf_intr_enable(eth_dev);
>  
> +	/* install the dev event handler for ethdev. */
> +	rte_eth_dev_event_handler_install(eth_dev);
> +
>  	PMD_INIT_LOG(DEBUG, "port %d vendorID=0x%x deviceID=0x%x mac.type=%s",
>  		     eth_dev->data->port_id, pci_dev->id.vendor_id,
>  		     pci_dev->id.device_id, "ixgbe_mac_82599_vf");
> @@ -1718,6 +1721,9 @@ eth_ixgbevf_dev_uninit(struct rte_eth_dev *eth_dev)
>  	rte_intr_callback_unregister(intr_handle,
>  				     ixgbevf_dev_interrupt_handler, eth_dev);
>  
> +	/* uninstall the dev event handler for ethdev. */
> +	rte_eth_dev_event_handler_uninstall(eth_dev);
> +
>  	return 0;
>  }
>  
> @@ -1801,7 +1807,7 @@ static int eth_ixgbe_pci_remove(struct rte_pci_device *pci_dev)
>  static struct rte_pci_driver rte_ixgbe_pmd = {
>  	.id_table = pci_id_ixgbe_map,
>  	.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
> -		     RTE_PCI_DRV_IOVA_AS_VA,
> +		     RTE_PCI_DRV_IOVA_AS_VA | RTE_PCI_DRV_INTR_RMV,

Instead of each driver explicitly install/uninstall handler, can it be possible
to do this in a common code for drivers that report RTE_PCI_DRV_INTR_RMV support?
With this you may not need helper functions but implement them as static
functions in common code.

Also should registered_callback remove eth_dev? (after calling user registered
callbacks)
And what is the relation of RTE_ETH_DEV_REMOVED state, which is to say device
removed and remove callback?

Lastly, do you think can there be cases driver specific actions needs to be
taken, so should driver provide a callback for removal?

Re: [dpdk-dev] [PATCH v5 2/4] net/ixgbe: install ethdev hotplug handler in ixgbe

[Jeff Guo]

hi, ferruh


On 8/25/2018 12:22 AM, Ferruh Yigit wrote:
> On 7/11/2018 12:51 PM, Jeff Guo wrote:
>> This patch aim to enable hotplug detect in ixgbe PMD. Firstly it
>> set the flags RTE_PCI_DRV_INTR_RMV in drv_flags to announce the hotplug
>> ability, and then use rte_eth_dev_event_handler_install to install
>> the hotplug event handler for ethdev. When eal detect the hotplug event,
>> it will call the ethdev callback to process it. If the event is hotplug
>> removal, it will trigger the RTE_ETH_EVENT_INTR_RMV event into ethdev
>> callback to let app process the hotplug for this ethdev.
>>
>> This is an example for other driver, that if any driver support hotplug
>> feature could be use this way to install hotplug handler.
>>
>> Signed-off-by: Jeff Guo <jia.guo@intel.com>
>> Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
>> ---
>> v5->v4:
>> no change.
>> ---
>>   drivers/net/ixgbe/ixgbe_ethdev.c | 8 +++++++-
>>   1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
>> index 87d2ad0..e7ae9bf 100644
>> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
>> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
>> @@ -1678,6 +1678,9 @@ eth_ixgbevf_dev_init(struct rte_eth_dev *eth_dev)
>>   	rte_intr_enable(intr_handle);
>>   	ixgbevf_intr_enable(eth_dev);
>>   
>> +	/* install the dev event handler for ethdev. */
>> +	rte_eth_dev_event_handler_install(eth_dev);
>> +
>>   	PMD_INIT_LOG(DEBUG, "port %d vendorID=0x%x deviceID=0x%x mac.type=%s",
>>   		     eth_dev->data->port_id, pci_dev->id.vendor_id,
>>   		     pci_dev->id.device_id, "ixgbe_mac_82599_vf");
>> @@ -1718,6 +1721,9 @@ eth_ixgbevf_dev_uninit(struct rte_eth_dev *eth_dev)
>>   	rte_intr_callback_unregister(intr_handle,
>>   				     ixgbevf_dev_interrupt_handler, eth_dev);
>>   
>> +	/* uninstall the dev event handler for ethdev. */
>> +	rte_eth_dev_event_handler_uninstall(eth_dev);
>> +
>>   	return 0;
>>   }
>>   
>> @@ -1801,7 +1807,7 @@ static int eth_ixgbe_pci_remove(struct rte_pci_device *pci_dev)
>>   static struct rte_pci_driver rte_ixgbe_pmd = {
>>   	.id_table = pci_id_ixgbe_map,
>>   	.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
>> -		     RTE_PCI_DRV_IOVA_AS_VA,
>> +		     RTE_PCI_DRV_IOVA_AS_VA | RTE_PCI_DRV_INTR_RMV,
> Instead of each driver explicitly install/uninstall handler, can it be possible
> to do this in a common code for drivers that report RTE_PCI_DRV_INTR_RMV support?
> With this you may not need helper functions but implement them as static
> functions in common code.

make sense, I think offload the install/uninstall to step into the 
device class helper should be a better option, since we could check if 
the driver support
hotplug by RTE_PCI_DRV_INTR_RMV.

> Also should registered_callback remove eth_dev? (after calling user registered
> callbacks)

The eth_dev should be remove as any other device user space resources, 
the process should be include in the currently user registered callback.

> And what is the relation of RTE_ETH_DEV_REMOVED state, which is to say device
> removed and remove callback?

The state of RTE_ETH_DEV_REMOVED means that ether device have already 
been removed, it should be use to let to check and stop any request from 
the app.
that is device class interface layer event, just manage by the layer and 
app.

> Lastly, do you think can there be cases driver specific actions needs to be
> taken, so should driver provide a callback for removal?

so far, i think the callback should be the same functional for hotplug, 
so the answer is there can be but no needs to taken other specific 
actions, just let it handler by bus layer but not driver.