[dpdk-dev] [PATCH 0/3] use a common eal device event for hot-unplug

[dpdk-dev] [PATCH 0/3] use a common eal device event for hot-unplug

[Jeff Guo]

This patch set aim to use a common eal device event mechanism to manage
devices for hot-unplug whatever device types, so the previous ethdev
remove event will be deleted and the device detach will be remove from
app into eal framework.

Jeff Guo (3):
  eal: add --dev-hotplug option
  ethdev: remove ethdev rmv interrupt
  testpmd: remove device detach into eal

 app/test-pmd/parameters.c                  |  2 --
 app/test-pmd/testpmd.c                     | 41 +++---------------------------
 drivers/bus/pci/pci_common.c               |  7 +++++
 drivers/net/failsafe/failsafe_ether.c      | 12 ++++-----
 drivers/net/failsafe/failsafe_ops.c        |  3 +--
 drivers/net/failsafe/failsafe_private.h    |  6 ++---
 drivers/net/mlx4/mlx4_intr.c               |  1 -
 drivers/net/mlx5/mlx5_ethdev.c             |  7 ++---
 lib/librte_eal/bsdapp/eal/eal.c            | 16 ++++++++++++
 lib/librte_eal/common/eal_common_options.c |  5 ++++
 lib/librte_eal/common/eal_internal_cfg.h   |  1 +
 lib/librte_eal/common/eal_options.h        |  4 ++-
 lib/librte_eal/linuxapp/eal/eal.c          | 16 ++++++++++++
 lib/librte_eal/linuxapp/eal/eal_dev.c      | 16 +++++++++++-
 lib/librte_ethdev/rte_ethdev.h             |  1 -
 15 files changed, 79 insertions(+), 59 deletions(-)

-- 
2.7.4

[dpdk-dev] [PATCH 1/3] eal: add --dev-hotplug option

[Jeff Guo]

This command-line option will enable hotplug event detecting and enable
hotplug handling for device hotplug.

Signed-off-by: Jeff Guo <jia.guo@intel.com>
---
 lib/librte_eal/bsdapp/eal/eal.c            | 16 ++++++++++++++++
 lib/librte_eal/common/eal_common_options.c |  5 +++++
 lib/librte_eal/common/eal_internal_cfg.h   |  1 +
 lib/librte_eal/common/eal_options.h        |  4 +++-
 lib/librte_eal/linuxapp/eal/eal.c          | 16 ++++++++++++++++
 5 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index b8152a7..b9c29e4 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -699,6 +699,22 @@ rte_eal_init(int argc, char **argv)
 #endif
 	}
 
+	if (internal_config.dev_hotplug) {
+		ret = rte_dev_hotplug_handle_enable();
+		if (ret) {
+			RTE_LOG(ERR, EAL,
+				"fail to enable hotplug handling.");
+			return -1;
+		}
+
+		ret = rte_dev_event_monitor_start();
+		if (ret) {
+			RTE_LOG(ERR, EAL,
+				"fail to start device event monitoring.");
+			return -1;
+		}
+	}
+
 	rte_srand(rte_rdtsc());
 
 	/* in secondary processes, memory init may allocate additional fbarrays
diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index e31eca5..70cb374 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -79,6 +79,7 @@ eal_long_options[] = {
 	{OPT_VMWARE_TSC_MAP,    0, NULL, OPT_VMWARE_TSC_MAP_NUM   },
 	{OPT_LEGACY_MEM,        0, NULL, OPT_LEGACY_MEM_NUM       },
 	{OPT_SINGLE_FILE_SEGMENTS, 0, NULL, OPT_SINGLE_FILE_SEGMENTS_NUM},
+	{OPT_DEV_HOTPLUG,       0, NULL, OPT_DEV_HOTPLUG_NUM      },
 	{0,                     0, NULL, 0                        }
 };
 
@@ -1309,6 +1310,9 @@ eal_parse_common_option(int opt, const char *optarg,
 			return -1;
 		}
 		break;
+	case OPT_DEV_HOTPLUG_NUM:
+		conf->dev_hotplug = 1;
+		break;
 
 	/* don't know what to do, leave this to caller */
 	default:
@@ -1476,6 +1480,7 @@ eal_common_usage(void)
 	       "  -h, --help          This help\n"
 	       "  --"OPT_IN_MEMORY"   Operate entirely in memory. This will\n"
 	       "                      disable secondary process support\n"
+	       "  --"OPT_DEV_HOTPLUG" Enable device hotplug\n"
 	       "\nEAL options for DEBUG use only:\n"
 	       "  --"OPT_HUGE_UNLINK"       Unlink hugepage files after init\n"
 	       "  --"OPT_NO_HUGE"           Use malloc instead of hugetlbfs\n"
diff --git a/lib/librte_eal/common/eal_internal_cfg.h b/lib/librte_eal/common/eal_internal_cfg.h
index 737f17e..d160ee3 100644
--- a/lib/librte_eal/common/eal_internal_cfg.h
+++ b/lib/librte_eal/common/eal_internal_cfg.h
@@ -73,6 +73,7 @@ struct internal_config {
 	enum rte_iova_mode iova_mode ;    /**< Set IOVA mode on this system  */
 	volatile unsigned int init_complete;
 	/**< indicates whether EAL has completed initialization */
+	volatile unsigned dev_hotplug;    /**< true to enable device hotplug */
 };
 extern struct internal_config internal_config; /**< Global EAL configuration. */
 
diff --git a/lib/librte_eal/common/eal_options.h b/lib/librte_eal/common/eal_options.h
index 5271f94..4d8a12e 100644
--- a/lib/librte_eal/common/eal_options.h
+++ b/lib/librte_eal/common/eal_options.h
@@ -65,7 +65,9 @@ enum {
 	OPT_SINGLE_FILE_SEGMENTS_NUM,
 #define OPT_IOVA_MODE          "iova-mode"
 	OPT_IOVA_MODE_NUM,
-	OPT_LONG_MAX_NUM
+	OPT_LONG_MAX_NUM,
+#define OPT_DEV_HOTPLUG	      "dev-hotplug"
+	OPT_DEV_HOTPLUG_NUM,
 };
 
 extern const char eal_short_options[];
diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index 361744d..8de7401 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -960,6 +960,22 @@ rte_eal_init(int argc, char **argv)
 #endif
 	}
 
+	if (internal_config.dev_hotplug) {
+		ret = rte_dev_hotplug_handle_enable();
+		if (ret) {
+			RTE_LOG(ERR, EAL,
+				"fail to enable hotplug handling.");
+			return -1;
+		}
+
+		ret = rte_dev_event_monitor_start();
+		if (ret) {
+			RTE_LOG(ERR, EAL,
+				"fail to start device event monitoring.");
+			return -1;
+		}
+	}
+
 	rte_srand(rte_rdtsc());
 
 	if (rte_eal_log_init(logid, internal_config.syslog_facility) < 0) {
-- 
2.7.4

[dpdk-dev] [PATCH 2/3] ethdev: remove ethdev rmv interrupt

[Jeff Guo]

Since eal device event had been introduced, and application could monitor
eal device event and accordingly handle hot-unplug for device, so the
ethdev rmv event could be replaced by eal device event. This patch aim to
abandon ethdev rmv interrupt, its every usages in pmd and testpmd will be
removed, while use a common way to detect device hotplug.

Signed-off-by: Jeff Guo <jia.guo@intel.com>
---
 app/test-pmd/parameters.c               |  2 --
 app/test-pmd/testpmd.c                  | 40 +++------------------------------
 drivers/net/failsafe/failsafe_ether.c   | 12 +++++-----
 drivers/net/failsafe/failsafe_ops.c     |  3 +--
 drivers/net/failsafe/failsafe_private.h |  6 ++---
 drivers/net/mlx4/mlx4_intr.c            |  1 -
 drivers/net/mlx5/mlx5_ethdev.c          |  7 +++---
 lib/librte_ethdev/rte_ethdev.h          |  1 -
 8 files changed, 16 insertions(+), 56 deletions(-)

diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index 38b4197..7819b30 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -529,8 +529,6 @@ parse_event_printing_config(const char *optarg, int enable)
 		mask = UINT32_C(1) << RTE_ETH_EVENT_IPSEC;
 	else if (!strcmp(optarg, "macsec"))
 		mask = UINT32_C(1) << RTE_ETH_EVENT_MACSEC;
-	else if (!strcmp(optarg, "intr_rmv"))
-		mask = UINT32_C(1) << RTE_ETH_EVENT_INTR_RMV;
 	else if (!strcmp(optarg, "dev_probed"))
 		mask = UINT32_C(1) << RTE_ETH_EVENT_NEW;
 	else if (!strcmp(optarg, "dev_released"))
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 4c75587..bd44b21 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -357,7 +357,6 @@ static const char * const eth_event_desc[] = {
 	[RTE_ETH_EVENT_VF_MBOX] = "VF mbox",
 	[RTE_ETH_EVENT_IPSEC] = "IPsec",
 	[RTE_ETH_EVENT_MACSEC] = "MACsec",
-	[RTE_ETH_EVENT_INTR_RMV] = "device removal",
 	[RTE_ETH_EVENT_NEW] = "device probed",
 	[RTE_ETH_EVENT_DESTROY] = "device released",
 	[RTE_ETH_EVENT_MAX] = NULL,
@@ -372,8 +371,8 @@ uint32_t event_print_mask = (UINT32_C(1) << RTE_ETH_EVENT_UNKNOWN) |
 			    (UINT32_C(1) << RTE_ETH_EVENT_QUEUE_STATE) |
 			    (UINT32_C(1) << RTE_ETH_EVENT_INTR_RESET) |
 			    (UINT32_C(1) << RTE_ETH_EVENT_IPSEC) |
-			    (UINT32_C(1) << RTE_ETH_EVENT_MACSEC) |
-			    (UINT32_C(1) << RTE_ETH_EVENT_INTR_RMV);
+			    (UINT32_C(1) << RTE_ETH_EVENT_MACSEC);
+
 /*
  * Decide if all memory are locked for performance.
  */
@@ -2567,13 +2566,6 @@ eth_event_callback(portid_t port_id, enum rte_eth_event_type type, void *param,
 		ports[port_id].need_setup = 1;
 		ports[port_id].port_status = RTE_PORT_HANDLING;
 		break;
-	case RTE_ETH_EVENT_INTR_RMV:
-		if (port_id_is_invalid(port_id, DISABLED_WARN))
-			break;
-		if (rte_eal_alarm_set(100000,
-				rmv_port_callback, (void *)(intptr_t)port_id))
-			fprintf(stderr, "Could not set up deferred device removal\n");
-		break;
 	default:
 		break;
 	}
@@ -2626,19 +2618,7 @@ dev_event_callback(const char *device_name, enum rte_dev_event_type type,
 				device_name);
 			return;
 		}
-		/*
-		 * Because the user's callback is invoked in eal interrupt
-		 * callback, the interrupt callback need to be finished before
-		 * it can be unregistered when detaching device. So finish
-		 * callback soon and use a deferred removal to detach device
-		 * is need. It is a workaround, once the device detaching be
-		 * moved into the eal in the future, the deferred removal could
-		 * be deleted.
-		 */
-		if (rte_eal_alarm_set(100000,
-				rmv_port_callback, (void *)(intptr_t)port_id))
-			RTE_LOG(ERR, EAL,
-				"Could not set up deferred device removal\n");
+		rmv_port_callback((void *)(intptr_t)port_id);
 		break;
 	case RTE_DEV_EVENT_ADD:
 		RTE_LOG(ERR, EAL, "The device: %s has been added!\n",
@@ -3170,20 +3150,6 @@ main(int argc, char** argv)
 	init_config();
 
 	if (hot_plug) {
-		ret = rte_dev_hotplug_handle_enable();
-		if (ret) {
-			RTE_LOG(ERR, EAL,
-				"fail to enable hotplug handling.");
-			return -1;
-		}
-
-		ret = rte_dev_event_monitor_start();
-		if (ret) {
-			RTE_LOG(ERR, EAL,
-				"fail to start device event monitoring.");
-			return -1;
-		}
-
 		ret = rte_dev_event_callback_register(NULL,
 			dev_event_callback, NULL);
 		if (ret) {
diff --git a/drivers/net/failsafe/failsafe_ether.c b/drivers/net/failsafe/failsafe_ether.c
index 1783165..e3ddbfa 100644
--- a/drivers/net/failsafe/failsafe_ether.c
+++ b/drivers/net/failsafe/failsafe_ether.c
@@ -345,8 +345,7 @@ failsafe_eth_dev_unregister_callbacks(struct sub_device *sdev)
 	if (sdev == NULL)
 		return;
 	if (sdev->rmv_callback) {
-		ret = rte_eth_dev_callback_unregister(PORT_ID(sdev),
-						RTE_ETH_EVENT_INTR_RMV,
+		ret = rte_dev_event_callback_unregister(sdev->dev->name,
 						failsafe_eth_rmv_event_callback,
 						sdev);
 		if (ret)
@@ -559,10 +558,10 @@ failsafe_stats_increment(struct rte_eth_stats *to, struct rte_eth_stats *from)
 	}
 }
 
-int
-failsafe_eth_rmv_event_callback(uint16_t port_id __rte_unused,
-				enum rte_eth_event_type event __rte_unused,
-				void *cb_arg, void *out __rte_unused)
+void
+failsafe_eth_rmv_event_callback(const char *device_name __rte_unused,
+				enum rte_dev_event_type event __rte_unused,
+				void *cb_arg)
 {
 	struct sub_device *sdev = cb_arg;
 
@@ -577,7 +576,6 @@ failsafe_eth_rmv_event_callback(uint16_t port_id __rte_unused,
 	 */
 	sdev->remove = 1;
 	fs_unlock(sdev->fs_dev, 0);
-	return 0;
 }
 
 int
diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c
index 7f8bcd4..7868f42 100644
--- a/drivers/net/failsafe/failsafe_ops.c
+++ b/drivers/net/failsafe/failsafe_ops.c
@@ -140,8 +140,7 @@ fs_dev_configure(struct rte_eth_dev *dev)
 			return ret;
 		}
 		if (rmv_interrupt && sdev->rmv_callback == 0) {
-			ret = rte_eth_dev_callback_register(PORT_ID(sdev),
-					RTE_ETH_EVENT_INTR_RMV,
+			ret = rte_dev_event_callback_register(sdev->dev->name,
 					failsafe_eth_rmv_event_callback,
 					sdev);
 			if (ret)
diff --git a/drivers/net/failsafe/failsafe_private.h b/drivers/net/failsafe/failsafe_private.h
index 7e31896..163bb98 100644
--- a/drivers/net/failsafe/failsafe_private.h
+++ b/drivers/net/failsafe/failsafe_private.h
@@ -224,9 +224,9 @@ void failsafe_eth_dev_unregister_callbacks(struct sub_device *sdev);
 void failsafe_dev_remove(struct rte_eth_dev *dev);
 void failsafe_stats_increment(struct rte_eth_stats *to,
 				struct rte_eth_stats *from);
-int failsafe_eth_rmv_event_callback(uint16_t port_id,
-				    enum rte_eth_event_type type,
-				    void *arg, void *out);
+void failsafe_eth_rmv_event_callback(const char *device_name,
+				     enum rte_dev_event_type event,
+				     void *cb_arg);
 int failsafe_eth_lsc_event_callback(uint16_t port_id,
 				    enum rte_eth_event_type event,
 				    void *cb_arg, void *out);
diff --git a/drivers/net/mlx4/mlx4_intr.c b/drivers/net/mlx4/mlx4_intr.c
index eeb982a..401cc84 100644
--- a/drivers/net/mlx4/mlx4_intr.c
+++ b/drivers/net/mlx4/mlx4_intr.c
@@ -180,7 +180,6 @@ mlx4_interrupt_handler(struct priv *priv)
 	enum { LSC, RMV, };
 	static const enum rte_eth_event_type type[] = {
 		[LSC] = RTE_ETH_EVENT_INTR_LSC,
-		[RMV] = RTE_ETH_EVENT_INTR_RMV,
 	};
 	uint32_t caught[RTE_DIM(type)] = { 0 };
 	struct ibv_async_event event;
diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
index d178ed6..7d1194f 100644
--- a/drivers/net/mlx5/mlx5_ethdev.c
+++ b/drivers/net/mlx5/mlx5_ethdev.c
@@ -1033,7 +1033,7 @@ mlx5_dev_status_handler(struct rte_eth_dev *dev)
 			ret |= (1 << RTE_ETH_EVENT_INTR_LSC);
 		else if (event.event_type == IBV_EVENT_DEVICE_FATAL &&
 			dev->data->dev_conf.intr_conf.rmv == 1)
-			ret |= (1 << RTE_ETH_EVENT_INTR_RMV);
+			ret |= (1 << RTE_DEV_EVENT_REMOVE);
 		else
 			DRV_LOG(DEBUG,
 				"port %u event type %d on not handled",
@@ -1060,8 +1060,9 @@ mlx5_dev_interrupt_handler(void *cb_arg)
 	events = mlx5_dev_status_handler(dev);
 	if (events & (1 << RTE_ETH_EVENT_INTR_LSC))
 		_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC, NULL);
-	if (events & (1 << RTE_ETH_EVENT_INTR_RMV))
-		_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_RMV, NULL);
+	if (events & (1 << RTE_DEV_EVENT_REMOVE))
+		rte_dev_event_callback_process(dev->device->name,
+					       RTE_DEV_EVENT_REMOVE);
 }
 
 /**
diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index 1960f3a..6d54743 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -2619,7 +2619,6 @@ enum rte_eth_event_type {
 			/**< reset interrupt event, sent to VF on PF reset */
 	RTE_ETH_EVENT_VF_MBOX,  /**< message from the VF received by PF */
 	RTE_ETH_EVENT_MACSEC,   /**< MACsec offload related event */
-	RTE_ETH_EVENT_INTR_RMV, /**< device removal event */
 	RTE_ETH_EVENT_NEW,      /**< port is probed */
 	RTE_ETH_EVENT_DESTROY,  /**< port is released */
 	RTE_ETH_EVENT_IPSEC,    /**< IPsec offload related event */
-- 
2.7.4

[dpdk-dev] [PATCH 3/3] testpmd: remove device detach into eal

[Jeff Guo]

Assume that eal detect the removal event and handle hot-unplug, so device
detach should be located in eal framework, the sequence should be eal
detect and handle hot-unplug event at first, then eal notify app to finish
pre-detach prepare, such as stop forwarding and clean ports, finally device
be detached safely in eal. This patch aim to remove the invoke of device
detach from app callback to eal device event handler.

Signed-off-by: Jeff Guo <jia.guo@intel.com>
---
 app/test-pmd/testpmd.c                |  1 -
 drivers/bus/pci/pci_common.c          |  7 +++++++
 lib/librte_eal/linuxapp/eal/eal_dev.c | 16 +++++++++++++++-
 3 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index bd44b21..f15035c 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2538,7 +2538,6 @@ rmv_port_callback(void *arg)
 	stop_port(port_id);
 	no_link_check = org_no_link_check;
 	close_port(port_id);
-	detach_port_device(port_id);
 	if (need_to_start)
 		start_packet_forwarding(0);
 }
diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c
index 6276e5d..565f8cf 100644
--- a/drivers/bus/pci/pci_common.c
+++ b/drivers/bus/pci/pci_common.c
@@ -465,6 +465,13 @@ pci_hot_unplug_handler(struct rte_device *dev)
 		 */
 		rte_dev_event_callback_process(dev->name,
 					       RTE_DEV_EVENT_REMOVE);
+		ret = rte_dev_remove(dev);
+		if (ret) {
+			RTE_LOG(ERR, EAL, "Failed to remove device %s\n",
+				dev->name);
+			return -1;
+		}
+		RTE_LOG(INFO, EAL, "Success to remove device %s\n", dev->name);
 		break;
 #endif
 	case RTE_KDRV_IGB_UIO:
diff --git a/lib/librte_eal/linuxapp/eal/eal_dev.c b/lib/librte_eal/linuxapp/eal/eal_dev.c
index 2830c86..f4a75f8 100644
--- a/lib/librte_eal/linuxapp/eal/eal_dev.c
+++ b/lib/librte_eal/linuxapp/eal/eal_dev.c
@@ -275,8 +275,22 @@ dev_uev_handler(__rte_unused void *param)
 					"for device (%s)\n", dev->name);
 			}
 			rte_spinlock_unlock(&failure_handle_lock);
+
+			rte_dev_event_callback_process(uevent.devname,
+						       uevent.type);
+			ret = rte_dev_remove(dev);
+			if (ret) {
+				RTE_LOG(ERR, EAL,
+					"Failed to remove device %s\n",
+					dev->name);
+				return;
+			}
+			RTE_LOG(INFO, EAL, "Success to remove device %s\n",
+				dev->name);
+		} else {
+			rte_dev_event_callback_process(uevent.devname,
+						       uevent.type);
 		}
-		rte_dev_event_callback_process(uevent.devname, uevent.type);
 	}
 
 	return;
-- 
2.7.4

Re: [dpdk-dev] [PATCH 1/3] eal: add --dev-hotplug option

[David Marchand]

On Fri, Dec 14, 2018 at 8:41 AM Jeff Guo <jia.guo@intel.com> wrote:

> This command-line option will enable hotplug event detecting and enable
> hotplug handling for device hotplug.
>
> Signed-off-by: Jeff Guo <jia.guo@intel.com>
>

Is there a reason why we would want this disabled by default and enabled
via option ?


diff --git a/lib/librte_eal/common/eal_options.h
> b/lib/librte_eal/common/eal_options.h
> index 5271f94..4d8a12e 100644
> --- a/lib/librte_eal/common/eal_options.h
> +++ b/lib/librte_eal/common/eal_options.h
> @@ -65,7 +65,9 @@ enum {
>         OPT_SINGLE_FILE_SEGMENTS_NUM,
>  #define OPT_IOVA_MODE          "iova-mode"
>         OPT_IOVA_MODE_NUM,
> -       OPT_LONG_MAX_NUM
> +       OPT_LONG_MAX_NUM,
> +#define OPT_DEV_HOTPLUG              "dev-hotplug"
> +       OPT_DEV_HOTPLUG_NUM,
>  };
>
>  extern const char eal_short_options[];
>

OPT_LONG_MAX_NUM is supposed to be the last enum.

-- 
David Marchand

Re: [dpdk-dev] [PATCH 1/3] eal: add --dev-hotplug option

[Jeff Guo]

hi, david

On 12/17/2018 6:15 PM, David Marchand wrote:
>
> On Fri, Dec 14, 2018 at 8:41 AM Jeff Guo <jia.guo@intel.com 
> <mailto:jia.guo@intel.com>> wrote:
>
>     This command-line option will enable hotplug event detecting and
>     enable
>     hotplug handling for device hotplug.
>
>     Signed-off-by: Jeff Guo <jia.guo@intel.com <mailto:jia.guo@intel.com>>
>
>
> Is there a reason why we would want this disabled by default and 
> enabled via option ?
>
>

Before i can give you an answer, let's see what will bring on if enable it.

When enable the hotplug will means that it will bring a new netlink 
socket communication

and a sigbus detecting and specific processing. So if user not want to 
add this work load, just

let it to be optional. Do you agree with that? If not please show what 
is your concern. Thanks.


>     diff --git a/lib/librte_eal/common/eal_options.h
>     b/lib/librte_eal/common/eal_options.h
>     index 5271f94..4d8a12e 100644
>     --- a/lib/librte_eal/common/eal_options.h
>     +++ b/lib/librte_eal/common/eal_options.h
>     @@ -65,7 +65,9 @@ enum {
>             OPT_SINGLE_FILE_SEGMENTS_NUM,
>      #define OPT_IOVA_MODE          "iova-mode"
>             OPT_IOVA_MODE_NUM,
>     -       OPT_LONG_MAX_NUM
>     +       OPT_LONG_MAX_NUM,
>     +#define OPT_DEV_HOTPLUG              "dev-hotplug"
>     +       OPT_DEV_HOTPLUG_NUM,
>      };
>
>      extern const char eal_short_options[];
>
>
> OPT_LONG_MAX_NUM is supposed to be the last enum.
>
> -- 
> David Marchand

Re: [dpdk-dev] [PATCH 1/3] eal: add --dev-hotplug option

[David Marchand]

Hello Jeff,

On Sat, Dec 29, 2018 at 5:06 AM Jeff Guo <jia.guo@intel.com> wrote:

> On 12/17/2018 6:15 PM, David Marchand wrote:
>
>
> On Fri, Dec 14, 2018 at 8:41 AM Jeff Guo <jia.guo@intel.com> wrote:
>
>> This command-line option will enable hotplug event detecting and enable
>> hotplug handling for device hotplug.
>>
>> Signed-off-by: Jeff Guo <jia.guo@intel.com>
>>
>
> Is there a reason why we would want this disabled by default and enabled
> via option ?
>
>
> Before i can give you an answer, let's see what will bring on if enable
> it.
>
> When enable the hotplug will means that it will bring a new netlink socket
> communication
>
> and a sigbus detecting and specific processing. So if user not want to add
> this work load, just
>
> let it to be optional. Do you agree with that? If not please show what is
> your concern. Thanks.
>

If the user does nothing about the sigbus signal handling but the eal
signal handler was not registered, the dpdk app will end up being
terminated by the kernel.
If the user wants to do its own things and don't want the eal to mess with
it... I am under the impression that he can disable the eal sigbus handler
by calling rte_dev_hotplug_handle_disable().
The netlink stuff is handled in the interrupt thread, no impact on the
processing threads and no additional thread afaics.


-- 
David Marchand

Re: [dpdk-dev] [PATCH 0/3] use a common eal device event for hot-unplug

[Iremonger, Bernard]

Hi Jia

> -----Original Message-----
> From: Guo, Jia
> Sent: Friday, December 14, 2018 7:46 AM
> To: Iremonger, Bernard <bernard.iremonger@intel.com>; Lu, Wenzhuo
> <wenzhuo.lu@intel.com>; shahafs@mellanox.com; thomas@monjalon.net;
> matan@mellanox.com
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; dev@dpdk.org; Guo, Jia
> <jia.guo@intel.com>; stephen@networkplumber.org; gaetan.rivet@6wind.com;
> Zhang, Qi Z <qi.z.zhang@intel.com>; arybchenko@solarflare.com; Richardson,
> Bruce <bruce.richardson@intel.com>; He, Shaopeng <shaopeng.he@intel.com>;
> Burakov, Anatoly <anatoly.burakov@intel.com>
> Subject: [PATCH 0/3] use a common eal device event for hot-unplug
> 
> This patch set aim to use a common eal device event mechanism to manage
> devices for hot-unplug whatever device types, so the previous ethdev remove
> event will be deleted and the device detach will be remove from app into eal
> framework.
> 
> Jeff Guo (3):
>   eal: add --dev-hotplug option
>   ethdev: remove ethdev rmv interrupt
>   testpmd: remove device detach into eal
> 
>  app/test-pmd/parameters.c                  |  2 --
>  app/test-pmd/testpmd.c                     | 41 +++---------------------------
>  drivers/bus/pci/pci_common.c               |  7 +++++
>  drivers/net/failsafe/failsafe_ether.c      | 12 ++++-----
>  drivers/net/failsafe/failsafe_ops.c        |  3 +--
>  drivers/net/failsafe/failsafe_private.h    |  6 ++---
>  drivers/net/mlx4/mlx4_intr.c               |  1 -
>  drivers/net/mlx5/mlx5_ethdev.c             |  7 ++---
>  lib/librte_eal/bsdapp/eal/eal.c            | 16 ++++++++++++
>  lib/librte_eal/common/eal_common_options.c |  5 ++++
>  lib/librte_eal/common/eal_internal_cfg.h   |  1 +
>  lib/librte_eal/common/eal_options.h        |  4 ++-
>  lib/librte_eal/linuxapp/eal/eal.c          | 16 ++++++++++++
>  lib/librte_eal/linuxapp/eal/eal_dev.c      | 16 +++++++++++-
>  lib/librte_ethdev/rte_ethdev.h             |  1 -
>  15 files changed, 79 insertions(+), 59 deletions(-)
> 
> --
> 2.7.4

This patch set fails to apply to DPDK 19.05.rc1 and probably needs to be rebased.

Regards,

Bernard.

Re: [dpdk-dev] [PATCH 0/3] use a common eal device event for hot-unplug

[Iremonger, Bernard]

Hi Jia

> -----Original Message-----
> From: Guo, Jia
> Sent: Friday, December 14, 2018 7:46 AM
> To: Iremonger, Bernard <bernard.iremonger@intel.com>; Lu, Wenzhuo
> <wenzhuo.lu@intel.com>; shahafs@mellanox.com; thomas@monjalon.net;
> matan@mellanox.com
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; dev@dpdk.org; Guo, Jia
> <jia.guo@intel.com>; stephen@networkplumber.org; gaetan.rivet@6wind.com;
> Zhang, Qi Z <qi.z.zhang@intel.com>; arybchenko@solarflare.com; Richardson,
> Bruce <bruce.richardson@intel.com>; He, Shaopeng <shaopeng.he@intel.com>;
> Burakov, Anatoly <anatoly.burakov@intel.com>
> Subject: [PATCH 0/3] use a common eal device event for hot-unplug
> 
> This patch set aim to use a common eal device event mechanism to manage
> devices for hot-unplug whatever device types, so the previous ethdev remove
> event will be deleted and the device detach will be remove from app into eal
> framework.
> 
> Jeff Guo (3):
>   eal: add --dev-hotplug option
>   ethdev: remove ethdev rmv interrupt
>   testpmd: remove device detach into eal
> 
>  app/test-pmd/parameters.c                  |  2 --
>  app/test-pmd/testpmd.c                     | 41 +++---------------------------
>  drivers/bus/pci/pci_common.c               |  7 +++++
>  drivers/net/failsafe/failsafe_ether.c      | 12 ++++-----
>  drivers/net/failsafe/failsafe_ops.c        |  3 +--
>  drivers/net/failsafe/failsafe_private.h    |  6 ++---
>  drivers/net/mlx4/mlx4_intr.c               |  1 -
>  drivers/net/mlx5/mlx5_ethdev.c             |  7 ++---
>  lib/librte_eal/bsdapp/eal/eal.c            | 16 ++++++++++++
>  lib/librte_eal/common/eal_common_options.c |  5 ++++
>  lib/librte_eal/common/eal_internal_cfg.h   |  1 +
>  lib/librte_eal/common/eal_options.h        |  4 ++-
>  lib/librte_eal/linuxapp/eal/eal.c          | 16 ++++++++++++
>  lib/librte_eal/linuxapp/eal/eal_dev.c      | 16 +++++++++++-
>  lib/librte_ethdev/rte_ethdev.h             |  1 -
>  15 files changed, 79 insertions(+), 59 deletions(-)
> 
> --
> 2.7.4

This patch set fails to apply to DPDK 19.05.rc1 and probably needs to be rebased.

Regards,

Bernard.

Re: [PATCH 0/3] use a common eal device event for hot-unplug

[Stephen Hemminger]

Unifying hotplug and device event support would be good idea
but this patch has gone stale. And supporting it would require
API changes (need to wait for next LTS) and a lot of testing.

Also, hotplug is a complex topic which has a lot of platform
dependencies.  If someone wants to do a more general solution
this patchset is worth looking at. But for now it needs to
be marked rejected in patchwork.