>
> +static int
> +eth_dev_event_callback_register(portid_t port_id)
> +{
> + int diag;
> + char *device_name;
> +
> + /* if port id equal -1, unregister all device event callbacks */
> + if (port_id == (portid_t)HOT_PLUG_FOR_ALL_DEVICE) {
> + device_name = NULL;
> + } else {
> + device_name = strdup(rte_eth_devices[port_id].device->name);
> + if (!device_name) {
> + free(device_name);
If device_name is NULL, no memory allocated, why free?
> + return -1;
> + }
> + }
> + /* register the dev_event callback */
> + diag = rte_dev_event_callback_register(device_name,
> + eth_dev_event_callback, (void *)(intptr_t)port_id);
> + if (diag) {
> + printf("Failed to setup dev_event callback\n");
> + return -1;
> + }
> +
> + free(device_name);
> + return 0;
> +}
> +
> +
> +static int
> +eth_dev_event_callback_unregister(portid_t port_id)
> +{
> + int diag;
> + char *device_name;
> +
> + /* if port id equal -1, unregister all device event callbacks */
> + if (port_id == (portid_t)HOT_PLUG_FOR_ALL_DEVICE) {
> + device_name = NULL;
> + } else {
> + device_name = strdup(rte_eth_devices[port_id].device->name);
> + if (!device_name) {
> + free(device_name);
Same as above.
> + return -1;
> + }
> + }
> +
> + /* unregister the dev_event callback */
> + diag = rte_dev_event_callback_unregister(device_name,
> + eth_dev_event_callback, (void *)(intptr_t)port_id);
> + if (diag) {
> + printf("Failed to setup dev_event callback\n");
> + return -1;
> + }
> +
> + free(device_name);
> + return 0;
> +}
> +
> void
> attach_port(char *identifier)
> {
> @@ -1869,6 +1941,8 @@ attach_port(char *identifier)
> if (rte_eth_dev_attach(identifier, &pi))
> return;
>
> + eth_dev_event_callback_register(pi);
> +
> socket_id = (unsigned)rte_eth_dev_socket_id(pi);
> /* if socket_id is invalid, set to 0 */
> if (check_socket_id(socket_id) < 0)
> @@ -1880,6 +1954,12 @@ attach_port(char *identifier)
>
> ports[pi].port_status = RTE_PORT_STOPPED;
>
> + if (hot_plug) {
> + hotplug_list_add(rte_eth_devices[pi].device,
> + rte_eth_devices[pi].data->kdrv);
> + eth_dev_event_callback_register(pi);
> + }
> +
> printf("Port %d is attached. Now total ports is %d\n", pi, nb_ports);
> printf("Done\n");
> }
> @@ -1906,6 +1986,12 @@ detach_port(portid_t port_id)
>
> nb_ports = rte_eth_dev_count();
>
> + if (hot_plug) {
> + hotplug_list_add(rte_eth_devices[port_id].device,
> + rte_eth_devices[port_id].data->kdrv);
> + eth_dev_event_callback_register(port_id);
> + }
> +
> printf("Port '%s' is detached. Now total ports is %d\n",
> name, nb_ports);
> printf("Done\n");
> @@ -1916,6 +2002,7 @@ void
> pmd_test_exit(void)
> {
> portid_t pt_id;
> + int ret;
>
> if (test_done == 0)
> stop_packet_forwarding();
> @@ -1929,6 +2016,19 @@ pmd_test_exit(void)
> close_port(pt_id);
> }
> }
> +
Need to check if hot_plug is enabled?
> + ret = rte_dev_event_monitor_stop();
> + if (ret) {
> + RTE_LOG(ERR, EAL, "fail to stop device event monitor.");
> + return;
> + }
> +
> + ret = eth_dev_event_callback_unregister(HOT_PLUG_FOR_ALL_DEVICE);
> + if (ret) {
> + RTE_LOG(ERR, EAL, "fail to unregister all event callbacks.");
> + return;
> + }
<...>
> +static void
> +add_dev_event_callback(void *arg)
> +{
> + char *dev_name = (char *)arg;
> +
> + rte_eal_alarm_cancel(add_dev_event_callback, arg);
> +
> + if (!in_hotplug_list(dev_name))
Remove "!" in the check
> + return;
> +
> + RTE_LOG(ERR, EAL, "add device: %s\n", dev_name);
It is not ERR, please make the log aligned with remove device.
> + attach_port(dev_name);
> +}
> +
<...>
> +
> +/* This function is used by the interrupt thread */
> +static int
> +eth_dev_event_callback(char *device_name, enum rte_dev_event_type type,
> + void *arg)
> +{
> + static const char * const event_desc[] = {
> + [RTE_DEV_EVENT_ADD] = "add",
> + [RTE_DEV_EVENT_REMOVE] = "remove",
> + };
> + char *dev_name = malloc(strlen(device_name) + 1);
> +
> + strcpy(dev_name, device_name);
Why not use strdup as above?
> + if (type >= RTE_DEV_EVENT_MAX) {
> + fprintf(stderr, "%s called upon invalid event %d\n",
> + __func__, type);
> + fflush(stderr);
> + } else if (event_print_mask & (UINT32_C(1) << type)) {
> + printf("%s event\n",
> + event_desc[type]);
> + fflush(stdout);
> + }
> +
> + switch (type) {
> + case RTE_DEV_EVENT_REMOVE:
> + if (rte_eal_alarm_set(100000,
> + rmv_dev_event_callback, arg))
> + fprintf(stderr,
> + "Could not set up deferred device removal\n");
> + break;
> + case RTE_DEV_EVENT_ADD:
> + if (rte_eal_alarm_set(100000,
> + add_dev_event_callback, dev_name))
> + fprintf(stderr,
> + "Could not set up deferred device add\n");
> + break;
> + default:
> + break;
> + }
> + return 0;
Always 0, even alarm set fails?
Thanks
Jingjing
