Hi Jeff,
<snip>
> Subject: [PATCH v12 7/7] testpmd: use hot-unplug failure handle mechanism
./devtools/check-git-log.sh -1
Wrong headline label:
testpmd: use hot-unplug failure handle mechanism
> This patch use testpmd for example, to show how an app smoothly handle
> failure when device be hot-unplug. Except 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....@intel.com>
> ---
> v12->v11:
> no change.
> ---
> app/test-pmd/testpmd.c | 39 +++++++++++++++++++++++++++++++--------
> 1 file changed, 31 insertions(+), 8 deletions(-)
>
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> 001f0e5..bfef483 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -2093,14 +2093,22 @@ 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)
Should there be an RTE_LOG() call here?
> + return;
> +
> + ret = rte_dev_hotplug_handle_disable();
> + if (ret) {
> RTE_LOG(ERR, EAL,
> - "fail to unregister all event callbacks.");
> + "fail to disable hotplug handling.");
> + return;
> + }
> }
>
> printf("\nBye...\n");
> @@ -2244,6 +2252,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 +2265,12 @@ 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) {
> + printf("can not get port by device %s!\n",
> device_name);
It would be better to use an RTE_LOG() call here instead of printf().
> + 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 +2793,23 @@ 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 = eth_dev_event_callback_register();
> + if (ret)
Should there be an RTE_LOG() call here?
> + return -1;
> }
>
> if (start_port(RTE_PORT_ALL) != 0)
> --
> 2.7.4
Regards,
Bernard.
