[v13,7/7] app/testpmd: use hotplug failure handler
Checks
Commit Message
This patch use testpmd for example, to show how an app smoothly handle
failure when device be hot-unplug. Except that app should enabled the
device event monitor and register the hotplug event’s callback, it also
need enable hotplug handle mechanism before running. Once app detect the
removal event, the hot-unplug callback would be called. It will first stop
the packet forwarding, then stop the port, close the port, and finally
detach the port to clean the device and release the resources.
Signed-off-by: Jeff Guo <jia.guo@intel.com>
---
v13->v12:
delete needless helper in app.
---
app/test-pmd/testpmd.c | 86 +++++++++++++++++++++++---------------------------
1 file changed, 40 insertions(+), 46 deletions(-)
Comments
Hi Jeff,
> -----Original Message-----
> From: Guo, Jia
> Sent: Thursday, October 4, 2018 7:31 AM
> To: stephen@networkplumber.org; Richardson, Bruce
> <bruce.richardson@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Ananyev,
> Konstantin <konstantin.ananyev@intel.com>; gaetan.rivet@6wind.com; Wu,
> Jingjing <jingjing.wu@intel.com>; thomas@monjalon.net;
> motih@mellanox.com; matan@mellanox.com; Van Haaren, Harry
> <harry.van.haaren@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; He,
> Shaopeng <shaopeng.he@intel.com>; Iremonger, Bernard
> <bernard.iremonger@intel.com>; arybchenko@solarflare.com; Lu, Wenzhuo
> <wenzhuo.lu@intel.com>; Burakov, Anatoly <anatoly.burakov@intel.com>;
> jerin.jacob@caviumnetworks.com
> Cc: jblunck@infradead.org; shreyansh.jain@nxp.com; dev@dpdk.org; Guo, Jia
> <jia.guo@intel.com>; Zhang, Helin <helin.zhang@intel.com>
> Subject: [PATCH v13 7/7] app/testpmd: use hotplug failure handler
>
> This patch use testpmd for example, to show how an app smoothly handle
> failure when device be hot-unplug. Except that app should enabled the device
> event monitor and register the hotplug event’s callback, it also need enable
> hotplug handle mechanism before running. Once app detect the removal event,
> the hot-unplug callback would be called. It will first stop the packet forwarding,
> then stop the port, close the port, and finally detach the port to clean the device
> and release the resources.
>
> Signed-off-by: Jeff Guo <jia.guo@intel.com>
> ---
> v13->v12:
> delete needless helper in app.
> ---
> app/test-pmd/testpmd.c | 86 +++++++++++++++++++++++--------------------------
> -
> 1 file changed, 40 insertions(+), 46 deletions(-)
>
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> 001f0e5..f3f8e44 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -434,9 +434,6 @@ static int eth_event_callback(portid_t port_id, 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.
> @@ -1954,39 +1951,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)
> {
> @@ -2093,14 +2057,25 @@ pmd_test_exit(void)
>
> if (hot_plug) {
> ret = rte_dev_event_monitor_stop();
> - if (ret)
> + if (ret) {
> RTE_LOG(ERR, EAL,
> "fail to stop device event monitor.");
> + return;
> + }
>
> - ret = eth_dev_event_callback_unregister();
> - if (ret)
> + ret = rte_dev_event_callback_unregister(NULL,
> + eth_dev_event_callback, NULL);
> + if (ret < 0) {
> + printf("fail to unregister device event callback.\n");
Better to use RTE_LOG() instead of printf().
> + return;
> + }
> +
> + ret = rte_dev_hotplug_handle_disable();
> + if (ret) {
> RTE_LOG(ERR, EAL,
> - "fail to unregister all event callbacks.");
> + "fail to disable hotplug handling.\n");
> + return;
> + }
> }
>
> printf("\nBye...\n");
> @@ -2244,6 +2219,9 @@ static void
> eth_dev_event_callback(char *device_name, enum rte_dev_event_type type,
> __rte_unused void *arg)
> {
> + uint16_t port_id;
> + int ret;
> +
> if (type >= RTE_DEV_EVENT_MAX) {
> fprintf(stderr, "%s called upon invalid event %d\n",
> __func__, type);
> @@ -2254,9 +2232,13 @@ eth_dev_event_callback(char *device_name, enum
> rte_dev_event_type 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.
> - */
> + ret = rte_eth_dev_get_port_by_name(device_name, &port_id);
> + if (ret) {
> + RTE_LOG(ERR, EAL, "can not get port by device %s!\n",
> + device_name);
> + return;
> + }
> + rmv_event_callback((void *)(intptr_t)port_id);
> break;
> case RTE_DEV_EVENT_ADD:
> RTE_LOG(ERR, EAL, "The device: %s has been added!\n", @@ -
> 2779,14 +2761,26 @@ main(int argc, char** argv)
> init_config();
>
> if (hot_plug) {
> - /* enable hot plug monitoring */
> + ret = rte_dev_hotplug_handle_enable();
> + if (ret) {
> + RTE_LOG(ERR, EAL,
> + "fail to enable hotplug handling.");
> + return -1;
> + }
> +
> ret = rte_dev_event_monitor_start();
> if (ret) {
> - rte_errno = EINVAL;
> + RTE_LOG(ERR, EAL,
> + "fail to start device event monitoring.");
> return -1;
> }
> - eth_dev_event_callback_register();
>
> + ret = rte_dev_event_callback_register(NULL,
> + eth_dev_event_callback, NULL);
> + if (ret) {
> + printf("faile to register device event callback\n");
Better to use RTE_LOG() instead of peintf(). Note type in message, "faile" should be "failed"
> + return -1;
> + }
> }
>
> if (start_port(RTE_PORT_ALL) != 0)
> --
> 2.7.4
thanks for your kindly review.
On 10/4/2018 6:31 PM, Iremonger, Bernard wrote:
> Hi Jeff,
>
>> -----Original Message-----
>> From: Guo, Jia
>> Sent: Thursday, October 4, 2018 7:31 AM
>> To: stephen@networkplumber.org; Richardson, Bruce
>> <bruce.richardson@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Ananyev,
>> Konstantin <konstantin.ananyev@intel.com>; gaetan.rivet@6wind.com; Wu,
>> Jingjing <jingjing.wu@intel.com>; thomas@monjalon.net;
>> motih@mellanox.com; matan@mellanox.com; Van Haaren, Harry
>> <harry.van.haaren@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; He,
>> Shaopeng <shaopeng.he@intel.com>; Iremonger, Bernard
>> <bernard.iremonger@intel.com>; arybchenko@solarflare.com; Lu, Wenzhuo
>> <wenzhuo.lu@intel.com>; Burakov, Anatoly <anatoly.burakov@intel.com>;
>> jerin.jacob@caviumnetworks.com
>> Cc: jblunck@infradead.org; shreyansh.jain@nxp.com; dev@dpdk.org; Guo, Jia
>> <jia.guo@intel.com>; Zhang, Helin <helin.zhang@intel.com>
>> Subject: [PATCH v13 7/7] app/testpmd: use hotplug failure handler
>>
>> This patch use testpmd for example, to show how an app smoothly handle
>> failure when device be hot-unplug. Except that app should enabled the device
>> event monitor and register the hotplug event’s callback, it also need enable
>> hotplug handle mechanism before running. Once app detect the removal event,
>> the hot-unplug callback would be called. It will first stop the packet forwarding,
>> then stop the port, close the port, and finally detach the port to clean the device
>> and release the resources.
>>
>> Signed-off-by: Jeff Guo <jia.guo@intel.com>
>> ---
>> v13->v12:
>> delete needless helper in app.
>> ---
>> app/test-pmd/testpmd.c | 86 +++++++++++++++++++++++--------------------------
>> -
>> 1 file changed, 40 insertions(+), 46 deletions(-)
>>
>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
>> 001f0e5..f3f8e44 100644
>> --- a/app/test-pmd/testpmd.c
>> +++ b/app/test-pmd/testpmd.c
>> @@ -434,9 +434,6 @@ static int eth_event_callback(portid_t port_id, 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.
>> @@ -1954,39 +1951,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)
>> {
>> @@ -2093,14 +2057,25 @@ pmd_test_exit(void)
>>
>> if (hot_plug) {
>> ret = rte_dev_event_monitor_stop();
>> - if (ret)
>> + if (ret) {
>> RTE_LOG(ERR, EAL,
>> "fail to stop device event monitor.");
>> + return;
>> + }
>>
>> - ret = eth_dev_event_callback_unregister();
>> - if (ret)
>> + ret = rte_dev_event_callback_unregister(NULL,
>> + eth_dev_event_callback, NULL);
>> + if (ret < 0) {
>> + printf("fail to unregister device event callback.\n");
> Better to use RTE_LOG() instead of printf().
>
>> + return;
>> + }
>> +
>> + ret = rte_dev_hotplug_handle_disable();
>> + if (ret) {
>> RTE_LOG(ERR, EAL,
>> - "fail to unregister all event callbacks.");
>> + "fail to disable hotplug handling.\n");
>> + return;
>> + }
>> }
>>
>> printf("\nBye...\n");
>> @@ -2244,6 +2219,9 @@ static void
>> eth_dev_event_callback(char *device_name, enum rte_dev_event_type type,
>> __rte_unused void *arg)
>> {
>> + uint16_t port_id;
>> + int ret;
>> +
>> if (type >= RTE_DEV_EVENT_MAX) {
>> fprintf(stderr, "%s called upon invalid event %d\n",
>> __func__, type);
>> @@ -2254,9 +2232,13 @@ eth_dev_event_callback(char *device_name, enum
>> rte_dev_event_type 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.
>> - */
>> + ret = rte_eth_dev_get_port_by_name(device_name, &port_id);
>> + if (ret) {
>> + RTE_LOG(ERR, EAL, "can not get port by device %s!\n",
>> + device_name);
>> + return;
>> + }
>> + rmv_event_callback((void *)(intptr_t)port_id);
>> break;
>> case RTE_DEV_EVENT_ADD:
>> RTE_LOG(ERR, EAL, "The device: %s has been added!\n", @@ -
>> 2779,14 +2761,26 @@ main(int argc, char** argv)
>> init_config();
>>
>> if (hot_plug) {
>> - /* enable hot plug monitoring */
>> + ret = rte_dev_hotplug_handle_enable();
>> + if (ret) {
>> + RTE_LOG(ERR, EAL,
>> + "fail to enable hotplug handling.");
>> + return -1;
>> + }
>> +
>> ret = rte_dev_event_monitor_start();
>> if (ret) {
>> - rte_errno = EINVAL;
>> + RTE_LOG(ERR, EAL,
>> + "fail to start device event monitoring.");
>> return -1;
>> }
>> - eth_dev_event_callback_register();
>>
>> + ret = rte_dev_event_callback_register(NULL,
>> + eth_dev_event_callback, NULL);
>> + if (ret) {
>> + printf("faile to register device event callback\n");
> Better to use RTE_LOG() instead of peintf(). Note type in message, "faile" should be "failed"
>
>> + return -1;
>> + }
>> }
>>
>> if (start_port(RTE_PORT_ALL) != 0)
>> --
>> 2.7.4
@@ -434,9 +434,6 @@ static int eth_event_callback(portid_t port_id,
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.
@@ -1954,39 +1951,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)
{
@@ -2093,14 +2057,25 @@ pmd_test_exit(void)
if (hot_plug) {
ret = rte_dev_event_monitor_stop();
- if (ret)
+ if (ret) {
RTE_LOG(ERR, EAL,
"fail to stop device event monitor.");
+ return;
+ }
- ret = eth_dev_event_callback_unregister();
- if (ret)
+ ret = rte_dev_event_callback_unregister(NULL,
+ eth_dev_event_callback, NULL);
+ if (ret < 0) {
+ printf("fail to unregister device event callback.\n");
+ return;
+ }
+
+ ret = rte_dev_hotplug_handle_disable();
+ if (ret) {
RTE_LOG(ERR, EAL,
- "fail to unregister all event callbacks.");
+ "fail to disable hotplug handling.\n");
+ return;
+ }
}
printf("\nBye...\n");
@@ -2244,6 +2219,9 @@ static void
eth_dev_event_callback(char *device_name, enum rte_dev_event_type type,
__rte_unused void *arg)
{
+ uint16_t port_id;
+ int ret;
+
if (type >= RTE_DEV_EVENT_MAX) {
fprintf(stderr, "%s called upon invalid event %d\n",
__func__, type);
@@ -2254,9 +2232,13 @@ eth_dev_event_callback(char *device_name, enum rte_dev_event_type 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.
- */
+ ret = rte_eth_dev_get_port_by_name(device_name, &port_id);
+ if (ret) {
+ RTE_LOG(ERR, EAL, "can not get port by device %s!\n",
+ device_name);
+ return;
+ }
+ rmv_event_callback((void *)(intptr_t)port_id);
break;
case RTE_DEV_EVENT_ADD:
RTE_LOG(ERR, EAL, "The device: %s has been added!\n",
@@ -2779,14 +2761,26 @@ main(int argc, char** argv)
init_config();
if (hot_plug) {
- /* enable hot plug monitoring */
+ ret = rte_dev_hotplug_handle_enable();
+ if (ret) {
+ RTE_LOG(ERR, EAL,
+ "fail to enable hotplug handling.");
+ return -1;
+ }
+
ret = rte_dev_event_monitor_start();
if (ret) {
- rte_errno = EINVAL;
+ RTE_LOG(ERR, EAL,
+ "fail to start device event monitoring.");
return -1;
}
- eth_dev_event_callback_register();
+ ret = rte_dev_event_callback_register(NULL,
+ eth_dev_event_callback, NULL);
+ if (ret) {
+ printf("faile to register device event callback\n");
+ return -1;
+ }
}
if (start_port(RTE_PORT_ALL) != 0)