Discussion:
[dpdk-dev] [PATCH 0/2] Enable eal event hotplug for i40e
(too old to reply)
Jeff Guo
2018-07-05 10:39:43 UTC
Permalink
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
Jeff Guo
2018-07-05 10:39:44 UTC
Permalink
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 <***@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
Jeff Guo
2018-07-05 10:39:45 UTC
Permalink
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 <***@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
Jeff Guo
2018-07-09 06:56:50 UTC
Permalink
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
Jeff Guo
2018-07-09 06:56:51 UTC
Permalink
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 <***@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
Lu, Wenzhuo
2018-07-09 07:38:01 UTC
Permalink
Hi Jeff,
-----Original Message-----
Sent: Monday, July 9, 2018 2:57 PM
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.
---
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) {
+ 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;
+ RTE_LOG(INFO, EAL, "The device: %s has been added!\n",
+ device_name);
+ break;
+ break;
+ }
+}
I don't get the point. Looks like this's a very common rte code. Why is it put in ixgbe pmd?
Matan Azrad
2018-07-09 07:51:27 UTC
Permalink
Hi

From: Lu, Wenzhuo
Post by Lu, Wenzhuo
Hi Jeff,
-----Original Message-----
Sent: Monday, July 9, 2018 2:57 PM
Van
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.
---
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) {
+ 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;
+ RTE_LOG(INFO, EAL, "The device: %s has been added!\n",
+ device_name);
+ break;
+ 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.
Jeff Guo
2018-07-09 08:57:26 UTC
Permalink
hi, wenzhuo and matan.
Post by Matan Azrad
Hi
From: Lu, Wenzhuo
Post by Lu, Wenzhuo
Hi Jeff,
-----Original Message-----
Sent: Monday, July 9, 2018 2:57 PM
Van
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.
---
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) {
+ 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;
+ RTE_LOG(INFO, EAL, "The device: %s has been added!\n",
+ device_name);
+ break;
+ 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.
Matan Azrad
2018-07-09 09:04:12 UTC
Permalink
Hi

From: Jeff Guo
Post by Jeff Guo
hi, wenzhuo and matan.
Post by Matan Azrad
Hi
From: Lu, Wenzhuo
Post by Lu, Wenzhuo
Hi Jeff,
-----Original Message-----
Sent: Monday, July 9, 2018 2:57 PM
Jingjing
Post by Matan Azrad
Post by Lu, Wenzhuo
Van
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.
---
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
Post by Matan Azrad
Post by Lu, Wenzhuo
*mac_addr)
memcpy(&mac_addr->addr_bytes[3], &random, 3); }
+static void
+eth_dev_event_callback(char *device_name, enum
rte_dev_event_type
Post by Matan Azrad
Post by Lu, Wenzhuo
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) {
+ 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;
+ RTE_LOG(INFO, EAL, "The device: %s has been added!\n",
+ device_name);
+ break;
+ 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.
Post by Matan Azrad
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?
Jeff Guo
2018-07-09 09:54:34 UTC
Permalink
Post by Matan Azrad
Hi
From: Jeff Guo
Post by Jeff Guo
hi, wenzhuo and matan.
Post by Matan Azrad
Hi
From: Lu, Wenzhuo
Post by Lu, Wenzhuo
Hi Jeff,
-----Original Message-----
Sent: Monday, July 9, 2018 2:57 PM
Jingjing
Post by Matan Azrad
Post by Lu, Wenzhuo
Van
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.
---
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
Post by Matan Azrad
Post by Lu, Wenzhuo
*mac_addr)
memcpy(&mac_addr->addr_bytes[3], &random, 3); }
+static void
+eth_dev_event_callback(char *device_name, enum
rte_dev_event_type
Post by Matan Azrad
Post by Lu, Wenzhuo
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) {
+ 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;
+ RTE_LOG(INFO, EAL, "The device: %s has been added!\n",
+ device_name);
+ break;
+ 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.
Post by Matan Azrad
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);
Matan Azrad
2018-07-09 10:01:31 UTC
Permalink
Hi

From: Jeff Guo
Post by Matan Azrad
Hi
From: Jeff Guo
Post by Jeff Guo
hi, wenzhuo and matan.
Post by Matan Azrad
Hi
From: Lu, Wenzhuo
Post by Lu, Wenzhuo
Hi Jeff,
-----Original Message-----
Sent: Monday, July 9, 2018 2:57 PM
Jingjing
Post by Matan Azrad
Post by Lu, Wenzhuo
Van
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.
Post by Matan Azrad
Post by Jeff Guo
Post by Matan Azrad
Post by Lu, Wenzhuo
This is an example for other driver, that if any driver support
hotplug feature could be use this way to enable hotplug detect.
---
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
Post by Matan Azrad
Post by Lu, Wenzhuo
*mac_addr)
memcpy(&mac_addr->addr_bytes[3], &random, 3); }
+static void
+eth_dev_event_callback(char *device_name, enum
rte_dev_event_type
Post by Matan Azrad
Post by Lu, Wenzhuo
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) {
+ 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)) {
Post by Jeff Guo
Post by Matan Azrad
Post by Lu, Wenzhuo
+
_rte_eth_dev_callback_process(
Post by Matan Azrad
Post by Jeff Guo
Post by Matan Azrad
Post by Lu, Wenzhuo
+
&rte_eth_devices[pid],
Post by Matan Azrad
Post by Jeff Guo
Post by Matan Azrad
Post by Lu, Wenzhuo
+
RTE_ETH_EVENT_INTR_RMV,
Post by Matan Azrad
Post by Jeff Guo
Post by Matan Azrad
Post by Lu, Wenzhuo
NULL);
+ continue;
+ }
+ }
+ }
+ break;
+ RTE_LOG(INFO, EAL, "The device: %s has been
added!\n",
Post by Matan Azrad
Post by Jeff Guo
Post by Matan Azrad
Post by Lu, Wenzhuo
+ device_name);
+ break;
+ 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.
Post by Matan Azrad
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?
Post by Matan Azrad
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?
Jeff Guo
2018-07-09 10:14:26 UTC
Permalink
Post by Matan Azrad
Hi
From: Jeff Guo
Post by Matan Azrad
Hi
From: Jeff Guo
Post by Jeff Guo
hi, wenzhuo and matan.
Post by Matan Azrad
Hi
From: Lu, Wenzhuo
Post by Lu, Wenzhuo
Hi Jeff,
-----Original Message-----
Sent: Monday, July 9, 2018 2:57 PM
Jingjing
Post by Matan Azrad
Post by Lu, Wenzhuo
Van
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.
Post by Matan Azrad
Post by Jeff Guo
Post by Matan Azrad
Post by Lu, Wenzhuo
This is an example for other driver, that if any driver support
hotplug feature could be use this way to enable hotplug detect.
---
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
Post by Matan Azrad
Post by Lu, Wenzhuo
*mac_addr)
memcpy(&mac_addr->addr_bytes[3], &random, 3); }
+static void
+eth_dev_event_callback(char *device_name, enum
rte_dev_event_type
Post by Matan Azrad
Post by Lu, Wenzhuo
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) {
+ 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)) {
Post by Jeff Guo
Post by Matan Azrad
Post by Lu, Wenzhuo
+
_rte_eth_dev_callback_process(
Post by Matan Azrad
Post by Jeff Guo
Post by Matan Azrad
Post by Lu, Wenzhuo
+
&rte_eth_devices[pid],
Post by Matan Azrad
Post by Jeff Guo
Post by Matan Azrad
Post by Lu, Wenzhuo
+
RTE_ETH_EVENT_INTR_RMV,
Post by Matan Azrad
Post by Jeff Guo
Post by Matan Azrad
Post by Lu, Wenzhuo
NULL);
+ continue;
+ }
+ }
+ }
+ break;
+ RTE_LOG(INFO, EAL, "The device: %s has been
added!\n",
Post by Matan Azrad
Post by Jeff Guo
Post by Matan Azrad
Post by Lu, Wenzhuo
+ device_name);
+ break;
+ 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.
Post by Matan Azrad
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?
Post by Matan Azrad
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.
Andrew Rybchenko
2018-07-09 08:13:16 UTC
Permalink
Post by 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.
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
Jeff Guo
2018-07-09 08:46:05 UTC
Permalink
Post by Andrew Rybchenko
Post by 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.
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.
Post by Andrew Rybchenko
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.
Jeff Guo
2018-07-09 06:56:52 UTC
Permalink
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 <***@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
Matan Azrad
2018-07-09 07:47:37 UTC
Permalink
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.
---
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) {
+ 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;
+ RTE_LOG(INFO, EAL, "The device: %s has been added!\n",
+ device_name);
+ break;
+ break;
+ }
+}
+
static int
eth_i40e_dev_init(struct rte_eth_dev *dev, void *init_params
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
Jeff Guo
2018-07-09 08:54:08 UTC
Permalink
hi, matan
Post by 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.
---
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) {
+ 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.
Post by Matan Azrad
+ _rte_eth_dev_callback_process(
+ &rte_eth_devices[pid],
+ RTE_ETH_EVENT_INTR_RMV,
NULL);
+ continue;
+ }
+ }
+ }
+ break;
+ RTE_LOG(INFO, EAL, "The device: %s has been added!\n",
+ device_name);
+ break;
+ break;
+ }
+}
+
static int
eth_i40e_dev_init(struct rte_eth_dev *dev, void *init_params
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
Jeff Guo
2018-07-09 06:56:53 UTC
Permalink
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 <***@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
Lu, Wenzhuo
2018-07-09 07:39:05 UTC
Permalink
Hi,
-----Original Message-----
Sent: Monday, July 9, 2018 2:57 PM
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.
Acked-by: Wenzhuo Lu <***@intel.com>
Andrew Rybchenko
2018-07-09 08:16:23 UTC
Permalink
Post by 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.
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.
Jeff Guo
2018-07-09 08:23:34 UTC
Permalink
Post by Andrew Rybchenko
Post by 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.
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.
Jeff Guo
2018-07-09 11:46:13 UTC
Permalink
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
Jeff Guo
2018-07-09 11:46:14 UTC
Permalink
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 <***@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
Andrew Rybchenko
2018-07-09 13:14:25 UTC
Permalink
Post by 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.
 >
Post by Jeff Guo
---
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.
Post by Jeff Guo
+ 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.
Post by Jeff Guo
+ }
+
+ switch (type) {
+ 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.
Post by Jeff Guo
+ _rte_eth_dev_callback_process(eth_dev,
+ RTE_ETH_EVENT_INTR_RMV,
+ NULL);
+ break;
+ ethdev_log(INFO, "The device: %s has been added!\n",
+ device_name);
+ break;
+ 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);
/**
+ *
+ * Implement a rte eth eal device event callbacks for the specific device.
+ *
+ * Pointer to the name of the rte device.
Is it name of the device which generates the event? If so, it should be
highlighted.
Post by Jeff Guo
+ * Eal device event type.
+ * To pass data back to user application.
+ *
+ * void
+ */
+void __rte_experimental
+rte_eth_dev_event_callback(char *device_name,
+ enum rte_dev_event_type event, void *cb_arg);
+
+/**
* the specific device. It is for DPDK internal user only. User
* application should not call it directly.
Jeff Guo
2018-07-10 07:06:37 UTC
Permalink
hi, andrew
Post by Andrew Rybchenko
Post by 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.
---
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.
Post by Andrew Rybchenko
Post by Jeff Guo
+ 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.
Post by Andrew Rybchenko
Post by Jeff Guo
+ }
+
+ switch (type) {
+ 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.
Post by Andrew Rybchenko
Post by Jeff Guo
+ _rte_eth_dev_callback_process(eth_dev,
+ RTE_ETH_EVENT_INTR_RMV,
+ NULL);
+ break;
+ ethdev_log(INFO, "The device: %s has been added!\n",
+ device_name);
+ break;
+ 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);
/**
+ *
+ * Implement a rte eth eal device event callbacks for the specific device.
+ *
+ * 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.
Post by Andrew Rybchenko
Post by Jeff Guo
+ * Eal device event type.
+ * To pass data back to user application.
+ *
+ * void
+ */
+void __rte_experimental
+rte_eth_dev_event_callback(char *device_name,
+ enum rte_dev_event_type event, void *cb_arg);
+
+/**
* the specific device. It is for DPDK internal user only. User
* application should not call it directly.
Zhang, Qi Z
2018-07-10 09:10:46 UTC
Permalink
-----Original Message-----
From: Guo, Jia
Sent: Monday, July 9, 2018 7:46 PM
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.
---
<...>
/**
+ *
+ * Implement a rte eth eal device event callbacks for the specific device.
+ *
+ * Pointer to the name of the rte device.
+ * Eal device event type.
+ * To pass data back to user application.
+ *
+ * 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
+
+/**
* the specific device. It is for DPDK internal user only. User
* application should not call it directly.
--
2.7.4
Jeff Guo
2018-07-09 11:46:15 UTC
Permalink
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 <***@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
Lu, Wenzhuo
2018-07-10 08:19:32 UTC
Permalink
Hi,
-----Original Message-----
From: Guo, Jia
Sent: Monday, July 9, 2018 7:46 PM
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.
Acked-by: Wenzhuo Lu <***@intel.com>
Jeff Guo
2018-07-09 11:46:16 UTC
Permalink
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 <***@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
Jeff Guo
2018-07-09 11:46:17 UTC
Permalink
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 <***@intel.com>
Acked-by: 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
Jeff Guo
2018-07-10 12:51:34 UTC
Permalink
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
Jeff Guo
2018-07-10 12:51:35 UTC
Permalink
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 <***@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
Zhang, Qi Z
2018-07-10 13:24:21 UTC
Permalink
-----Original Message-----
From: Guo, Jia
Sent: Tuesday, July 10, 2018 8:52 PM
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.
Reviewed-by: Qi Zhang <***@intel.com>
Andrew Rybchenko
2018-07-11 08:49:30 UTC
Permalink
Post by 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.
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.
Post by Jeff Guo
---
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) {
+ 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.
Post by Jeff Guo
+ 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?
Post by Jeff Guo
+ return;
+
+ if (!strcmp(device_name, eth_dev->device->name))
+ _rte_eth_dev_callback_process(eth_dev,
+ RTE_ETH_EVENT_INTR_RMV,
+ NULL);
+ break;
+ ethdev_log(INFO, "The device: %s has been added!\n",
+ device_name);
+ break;
+ 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.
Post by Jeff Guo
+
+ 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)
<...>
Jeff Guo
2018-07-11 11:17:03 UTC
Permalink
Post by Andrew Rybchenko
Post by 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.
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.
Post by Andrew Rybchenko
Post by Jeff Guo
---
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) {
+ 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.
Post by Andrew Rybchenko
Post by Jeff Guo
+ 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.
Post by Andrew Rybchenko
Post by Jeff Guo
+ return;
+
+ if (!strcmp(device_name, eth_dev->device->name))
+ _rte_eth_dev_callback_process(eth_dev,
+ RTE_ETH_EVENT_INTR_RMV,
+ NULL);
+ break;
+ ethdev_log(INFO, "The device: %s has been added!\n",
+ device_name);
+ break;
+ 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.
Post by Andrew Rybchenko
Post by Jeff Guo
+
+ 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)
<...>
Jeff Guo
2018-07-10 12:51:36 UTC
Permalink
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 <***@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
Jeff Guo
2018-07-10 12:51:37 UTC
Permalink
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 <***@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
Zhang, Qi Z
2018-07-10 13:26:39 UTC
Permalink
-----Original Message-----
From: Guo, Jia
Sent: Tuesday, July 10, 2018 8:52 PM
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.
Acked-by: Qi Zhang <***@intel.com>
Jeff Guo
2018-07-10 12:51:38 UTC
Permalink
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 <***@intel.com>
Acked-by: 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
Jeff Guo
2018-07-11 11:51:23 UTC
Permalink
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
Jeff Guo
2018-07-11 11:51:24 UTC
Permalink
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 <***@intel.com>
Reviewed-by: Qi 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
Jeff Guo
2018-07-11 11:51:25 UTC
Permalink
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 <***@intel.com>
Acked-by: 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
Ferruh Yigit
2018-08-24 16:22:48 UTC
Permalink
Post by 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.
---
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?
Jeff Guo
2018-09-12 08:47:58 UTC
Permalink
hi, ferruh
Post by Ferruh Yigit
Post by 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.
---
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.
Post by Ferruh Yigit
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.
Post by Ferruh Yigit
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.
Post by Ferruh Yigit
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.

Jeff Guo
2018-07-11 11:51:26 UTC
Permalink
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 <***@intel.com>
Acked-by: Qi 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
Jeff Guo
2018-07-11 11:51:27 UTC
Permalink
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 <***@intel.com>
Acked-by: 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
Jeff Guo
2018-07-11 11:58:28 UTC
Permalink
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
Jeff Guo
2018-07-11 11:58:29 UTC
Permalink
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 <***@intel.com>
Reviewed-by: Qi 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
Jeff Guo
2018-07-11 11:58:30 UTC
Permalink
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 <***@intel.com>
Acked-by: 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
Jeff Guo
2018-07-11 11:58:31 UTC
Permalink
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 <***@intel.com>
Acked-by: Qi 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
Jeff Guo
2018-07-11 11:58:32 UTC
Permalink
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 <***@intel.com>
Acked-by: 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
Jeff Guo
2018-08-17 10:50:12 UTC
Permalink
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
Jeff Guo
2018-08-17 10:50:13 UTC
Permalink
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 <***@intel.com>
Reviewed-by: Qi 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
Jeff Guo
2018-08-17 10:50:15 UTC
Permalink
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 <***@intel.com>
Acked-by: Qi 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
Jeff Guo
2018-08-17 10:50:14 UTC
Permalink
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 <***@intel.com>
Acked-by: 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
Jeff Guo
2018-08-17 10:50:16 UTC
Permalink
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 <***@intel.com>
Acked-by: 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
Continue reading on narkive:
Loading...