[dpdk-dev] [PATCH V16 4/4] app/testpmd: enable device hotplug monitoring
Tan, Jianfeng
jianfeng.tan at intel.com
Wed Mar 28 18:41:58 CEST 2018
On 3/26/2018 7:20 PM, Jeff Guo wrote:
> Use testpmd for example, to show an application how to use device event
> mechanism to monitor the hotplug event, involve both hot removal event
involve -> including
> and the hot insertion event.
>
> The process is that, testpmd first enable hotplug monitoring and register
> the user's callback, when device being hotplug insertion or hotplug
> removal, the eal monitor the event and call user's callbacks, the
> application according their hot plug policy to detach or attach the device
> from the bus.
You are not exactly describing what's done in this example. From what I
see, you only implement hot-unplug. For hot-plug, we will need to
register callback with dev_name of NULL.
And without the other patch series, this only works without starting
datapath.
>
> Signed-off-by: Jeff Guo <jia.guo at intel.com>
> ---
> 1.modify log and patch description.
> ---
> app/test-pmd/parameters.c | 5 +-
> app/test-pmd/testpmd.c | 195 +++++++++++++++++++++++++++++++++++++++++++++-
> app/test-pmd/testpmd.h | 11 +++
> 3 files changed, 209 insertions(+), 2 deletions(-)
>
> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
> index 97d22b8..825d602 100644
> --- a/app/test-pmd/parameters.c
> +++ b/app/test-pmd/parameters.c
> @@ -186,6 +186,7 @@ usage(char* progname)
> printf(" --flow-isolate-all: "
> "requests flow API isolated mode on all ports at initialization time.\n");
> printf(" --tx-offloads=0xXXXXXXXX: hexadecimal bitmask of TX queue offloads\n");
> + printf(" --hot-plug: enalbe hot plug for device.\n");
> }
>
> #ifdef RTE_LIBRTE_CMDLINE
> @@ -621,6 +622,7 @@ launch_args_parse(int argc, char** argv)
> { "print-event", 1, 0, 0 },
> { "mask-event", 1, 0, 0 },
> { "tx-offloads", 1, 0, 0 },
> + { "hot-plug", 0, 0, 0 },
> { 0, 0, 0, 0 },
> };
>
> @@ -1102,7 +1104,8 @@ launch_args_parse(int argc, char** argv)
> rte_exit(EXIT_FAILURE,
> "invalid mask-event argument\n");
> }
> -
> + if (!strcmp(lgopts[opt_idx].name, "hot-plug"))
> + hot_plug = 1;
> break;
> case 'h':
> usage(argv[0]);
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index 4c0e258..bb1ac8f 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -12,6 +12,7 @@
> #include <sys/mman.h>
> #include <sys/types.h>
> #include <errno.h>
> +#include <stdbool.h>
>
> #include <sys/queue.h>
> #include <sys/stat.h>
> @@ -284,6 +285,9 @@ uint8_t lsc_interrupt = 1; /* enabled by default */
> */
> uint8_t rmv_interrupt = 1; /* enabled by default */
>
> +
> +uint8_t hot_plug = 0; /**< hotplug disabled by default. */
> +
> /*
> * Display or mask ether events
> * Default to all events except VF_MBOX
> @@ -384,6 +388,8 @@ uint8_t bitrate_enabled;
> struct gro_status gro_ports[RTE_MAX_ETHPORTS];
> uint8_t gro_flush_cycles = GRO_DEFAULT_FLUSH_CYCLES;
>
> +static struct hotplug_request_list hp_list;
> +
> /* Forward function declarations */
> static void map_port_queue_stats_mapping_registers(portid_t pi,
> struct rte_port *port);
> @@ -391,6 +397,14 @@ 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 int eth_dev_event_callback(char *device_name,
> + enum rte_dev_event_type type,
> + void *param);
> +static int eth_dev_event_callback_register(portid_t port_id);
> +static bool in_hotplug_list(const char *dev_name);
> +
> +static int hotplug_list_add(struct rte_device *device,
> + enum rte_kernel_driver device_kdrv);
>
> /*
> * Check if all the ports are started.
> @@ -1853,6 +1867,27 @@ reset_port(portid_t pid)
> printf("Done\n");
> }
>
> +static int
> +eth_dev_event_callback_register(portid_t port_id)
> +{
> + int diag;
> + char device_name[128];
> +
> + snprintf(device_name, sizeof(device_name),
> + "%s", rte_eth_devices[port_id].device->name);
> +
> + /* register the dev_event callback */
> +
> + diag = rte_dev_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;
> + }
> +
> + return 0;
> +}
> +
> void
> attach_port(char *identifier)
> {
> @@ -1869,6 +1904,8 @@ attach_port(char *identifier)
> if (rte_eth_dev_attach(identifier, &pi))
> return;
>
> + eth_dev_event_callback_register(pi);
What's the difference with below one?
> +
> 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 +1917,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 +1949,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");
> @@ -1929,6 +1978,9 @@ pmd_test_exit(void)
> close_port(pt_id);
> }
> }
> +
> + rte_dev_event_monitor_stop();
> +
> printf("\nBye...\n");
> }
>
> @@ -2013,6 +2065,49 @@ rmv_event_callback(void *arg)
> dev->device->name);
> }
>
> +static void
> +rmv_dev_event_callback(void *arg)
> +{
> + char name[RTE_ETH_NAME_MAX_LEN];
> + uint8_t port_id = (intptr_t)arg;
> +
> + rte_eal_alarm_cancel(rmv_dev_event_callback, arg);
> +
> + RTE_ETH_VALID_PORTID_OR_RET(port_id);
> + printf("removing port id:%u\n", port_id);
> +
> + if (!in_hotplug_list(rte_eth_devices[port_id].device->name))
> + return;
> +
> + stop_packet_forwarding();
> +
> + stop_port(port_id);
> + close_port(port_id);
> + if (rte_eth_dev_detach(port_id, name)) {
> + RTE_LOG(ERR, USER1, "Failed to detach port '%s'\n", name);
> + return;
> + }
> +
> + nb_ports = rte_eth_dev_count();
> +
> + printf("Port '%s' is detached. Now total ports is %d\n",
> + name, nb_ports);
> +}
> +
> +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))
> + return;
> +
> + RTE_LOG(ERR, EAL, "add device: %s\n", dev_name);
> + attach_port(dev_name);
> +}
> +
> /* This function is used by the interrupt thread */
> static int
> eth_event_callback(portid_t port_id, enum rte_eth_event_type type, void *param,
> @@ -2059,6 +2154,86 @@ eth_event_callback(portid_t port_id, enum rte_eth_event_type type, void *param,
> return 0;
> }
>
> +static bool
> +in_hotplug_list(const char *dev_name)
> +{
> + struct hotplug_request *hp_request = NULL;
> +
> + TAILQ_FOREACH(hp_request, &hp_list, next) {
> + if (!strcmp(hp_request->dev_name, dev_name))
> + break;
> + }
> +
> + if (hp_request)
> + return true;
> +
> + return false;
> +}
> +
> +static int
> +hotplug_list_add(struct rte_device *device, enum rte_kernel_driver device_kdrv)
> +{
> + struct hotplug_request *hp_request;
> +
> + hp_request = rte_zmalloc("hoplug request",
> + sizeof(*hp_request), 0);
> + if (hp_request == NULL) {
> + fprintf(stderr, "%s can not alloc memory\n",
> + __func__);
> + return -ENOMEM;
> + }
> +
> + hp_request->dev_name = device->name;
> + hp_request->dev_kdrv = device_kdrv;
> +
> + TAILQ_INSERT_TAIL(&hp_list, hp_request, next);
> +
> + return 0;
> +}
> +
> +/* 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_UNKNOWN] = "Unknown",
> + [RTE_DEV_EVENT_ADD] = "add",
> + [RTE_DEV_EVENT_REMOVE] = "remove",
> + };
> + char *dev_name = malloc(strlen(device_name) + 1);
> +
> + strcpy(dev_name, device_name);
strdup is easier?
> +
> + 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))
Why not rmv_dev_event_callback directly?
Besides, the name of rmv_dev_event_callback is really confusing. It's
not a dev event callback.
> + 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");
Ditto.
> + break;
> + default:
> + break;
> + }
> + return 0;
> +}
> +
> static int
> set_tx_queue_stats_mapping_registers(portid_t port_id, struct rte_port *port)
> {
> @@ -2474,8 +2649,9 @@ signal_handler(int signum)
> int
> main(int argc, char** argv)
> {
> - int diag;
> + int diag;
> portid_t port_id;
> + int ret;
>
> signal(SIGINT, signal_handler);
> signal(SIGTERM, signal_handler);
> @@ -2543,6 +2719,23 @@ main(int argc, char** argv)
> nb_rxq, nb_txq);
>
> init_config();
> +
> + if (hot_plug) {
> + /* enable hot plug monitoring */
> + ret = rte_dev_event_monitor_start();
> + if (ret) {
> + rte_errno = EINVAL;
> + return -1;
> + }
> + if (TAILQ_EMPTY(&hp_list))
> + TAILQ_INIT(&hp_list);
> + RTE_ETH_FOREACH_DEV(port_id) {
> + hotplug_list_add(rte_eth_devices[port_id].device,
> + rte_eth_devices[port_id].data->kdrv);
> + eth_dev_event_callback_register(port_id);
> + }
Why not monitoring all devices with dev_name of NULL? It makes things
much easier. And we can also monitor the hot-plug event.
> + }
> +
> if (start_port(RTE_PORT_ALL) != 0)
> rte_exit(EXIT_FAILURE, "Start ports failed\n");
>
> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
> index 153abea..c619e11 100644
> --- a/app/test-pmd/testpmd.h
> +++ b/app/test-pmd/testpmd.h
> @@ -63,6 +63,15 @@ typedef uint16_t streamid_t;
> #define TM_MODE 0
> #endif
>
> +struct hotplug_request {
> + TAILQ_ENTRY(hotplug_request) next; /**< Callbacks list */
> + const char *dev_name; /* request device name */
> + enum rte_kernel_driver dev_kdrv; /* kernel driver binded */
> +};
> +
> +/** @internal Structure to keep track of registered callbacks */
> +TAILQ_HEAD(hotplug_request_list, hotplug_request);
> +
> enum {
> PORT_TOPOLOGY_PAIRED,
> PORT_TOPOLOGY_CHAINED,
> @@ -319,6 +328,8 @@ extern volatile int test_done; /* stop packet forwarding when set to 1. */
> extern uint8_t lsc_interrupt; /**< disabled by "--no-lsc-interrupt" parameter */
> extern uint8_t rmv_interrupt; /**< disabled by "--no-rmv-interrupt" parameter */
> extern uint32_t event_print_mask;
> +extern uint8_t hot_plug; /**< enable by "--hot-plug" parameter */
> +
> /**< set by "--print-event xxxx" and "--mask-event xxxx parameters */
>
> #ifdef RTE_LIBRTE_IXGBE_BYPASS
More information about the dev
mailing list