[dpdk-dev,V18,4/4] app/testpmd: enable device hotplug monitoring

Message ID 1522751639-2315-5-git-send-email-jia.guo@intel.com (mailing list archive)
State Superseded, archived
Headers

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation fail Compilation issues

Commit Message

Guo, Jia April 3, 2018, 10:33 a.m. UTC
  Use testpmd for example, to show how an application use device event
APIs to monitor the hotplug events, including both hot removal event
and hot insertion event.

The process is that, testpmd first enable hotplug by below commands,

E.g. ./build/app/testpmd -c 0x3 --n 4 -- -i --hot-plug

then testpmd start the device event monitor by call the new API
(rte_dev_event_monitor_start) and register the user's callback by call
the API (rte_dev_event_callback_register), when device being hotplug
insertion or hotplug removal, the device event monitor detects the event
and call user's callbacks, user could process the event in the callback
accordingly.

This patch only shows the event monitoring, device attach/detach would
not be involved here, will add from other hotplug patch set.

Signed-off-by: Jeff Guo <jia.guo@intel.com>
---
v18->v17:
remove hotplug policy and detach/attach process from testpmd, let it
focus on the device event monitoring which the patch set introduced.
---
 app/test-pmd/parameters.c             |   5 +-
 app/test-pmd/testpmd.c                | 112 +++++++++++++++++++++++++++++++++-
 app/test-pmd/testpmd.h                |   2 +
 doc/guides/testpmd_app_ug/run_app.rst |   4 ++
 4 files changed, 121 insertions(+), 2 deletions(-)
  

Comments

Jianfeng Tan April 4, 2018, 3:22 a.m. UTC | #1
> -----Original Message-----
> From: Guo, Jia
> Sent: Tuesday, April 3, 2018 6:34 PM
> To: stephen@networkplumber.org; Richardson, Bruce; Yigit, Ferruh;
> Ananyev, Konstantin; gaetan.rivet@6wind.com; Wu, Jingjing;
> thomas@monjalon.net; motih@mellanox.com; Van Haaren, Harry; Tan,
> Jianfeng
> Cc: jblunck@infradead.org; shreyansh.jain@nxp.com; dev@dpdk.org; Guo,
> Jia; Zhang, Helin
> Subject: [PATCH V18 4/4] app/testpmd: enable device hotplug monitoring
> 
> Use testpmd for example, to show how an application use device event

s/use/uses

> APIs to monitor the hotplug events, including both hot removal event
> and hot insertion event.
> 
> The process is that, testpmd first enable hotplug by below commands,
> 
> E.g. ./build/app/testpmd -c 0x3 --n 4 -- -i --hot-plug
> 
> then testpmd start the device event monitor by call the new API

s/start/starts
s/call/calling

> (rte_dev_event_monitor_start) and register the user's callback by call
> the API (rte_dev_event_callback_register), when device being hotplug
> insertion or hotplug removal, the device event monitor detects the event
> and call user's callbacks, user could process the event in the callback
> accordingly.
> 
> This patch only shows the event monitoring, device attach/detach would
> not be involved here, will add from other hotplug patch set.
> 
> Signed-off-by: Jeff Guo <jia.guo@intel.com>

Some typos and a trivial suggestion. Feel free to carry my

Reviewed-by: Jianfeng Tan <jianfeng.tan@intel.com>

in the next version.

> ---
> v18->v17:
> remove hotplug policy and detach/attach process from testpmd, let it
> focus on the device event monitoring which the patch set introduced.
> ---
>  app/test-pmd/parameters.c             |   5 +-
>  app/test-pmd/testpmd.c                | 112
> +++++++++++++++++++++++++++++++++-
>  app/test-pmd/testpmd.h                |   2 +
>  doc/guides/testpmd_app_ug/run_app.rst |   4 ++
>  4 files changed, 121 insertions(+), 2 deletions(-)
> 
> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
> index 97d22b8..558cd40 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: enable 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..2faeb90 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,8 @@ 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
> @@ -391,6 +394,12 @@ 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(void);
> +static int eth_dev_event_callback_unregister(void);
> +
> 
>  /*
>   * Check if all the ports are started.
> @@ -1853,6 +1862,39 @@ reset_port(portid_t pid)
>  	printf("Done\n");
>  }
> 
> +static int
> +eth_dev_event_callback_register(void)
> +{
> +	int diag;
> +
> +	/* register the device event callback */
> +	diag = rte_dev_event_callback_register(NULL,
> +		eth_dev_event_callback, NULL);
> +	if (diag) {
> +		printf("Failed to setup dev_event callback\n");
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
> +
> +static int
> +eth_dev_event_callback_unregister(void)
> +{
> +	int diag;
> +
> +	/* unregister the device event callback */
> +	diag = rte_dev_event_callback_unregister(NULL,
> +		eth_dev_event_callback, NULL);
> +	if (diag) {
> +		printf("Failed to setup dev_event callback\n");
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
>  void
>  attach_port(char *identifier)
>  {
> @@ -1916,6 +1958,7 @@ void
>  pmd_test_exit(void)
>  {
>  	portid_t pt_id;
> +	int ret;
> 
>  	if (test_done == 0)
>  		stop_packet_forwarding();
> @@ -1929,6 +1972,18 @@ pmd_test_exit(void)
>  			close_port(pt_id);
>  		}
>  	}
> +
> +	if (hot_plug) {
> +		ret = rte_dev_event_monitor_stop();
> +		if (ret)
> +			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");
>  }
> 
> @@ -2059,6 +2114,48 @@ 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 int
> +eth_dev_event_callback(char *device_name, enum rte_dev_event_type
> type,
> +			     __rte_unused void *arg)
> +{
> +	int ret = 0;

From here

> +	static const char * const event_desc[] = {
> +		[RTE_DEV_EVENT_ADD] = "add",
> +		[RTE_DEV_EVENT_REMOVE] = "remove",
> +	};
> +
> +	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);
> +	}

to here, these check are not necessary.

> +
> +	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;
> +	}
> +	return ret;
> +}
> +
>  static int
>  set_tx_queue_stats_mapping_registers(portid_t port_id, struct rte_port
> *port)
>  {
> @@ -2474,8 +2571,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 +2641,18 @@ 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;
> +		}
> +		eth_dev_event_callback_register();
> +
> +	}
> +
>  	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..8fde68d 100644
> --- a/app/test-pmd/testpmd.h
> +++ b/app/test-pmd/testpmd.h
> @@ -319,6 +319,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
> diff --git a/doc/guides/testpmd_app_ug/run_app.rst
> b/doc/guides/testpmd_app_ug/run_app.rst
> index 1fd5395..d0ced36 100644
> --- a/doc/guides/testpmd_app_ug/run_app.rst
> +++ b/doc/guides/testpmd_app_ug/run_app.rst
> @@ -479,3 +479,7 @@ The commandline options are:
> 
>      Set the hexadecimal bitmask of TX queue offloads.
>      The default value is 0.
> +
> +*   ``--hot-plug``
> +
> +    Enable device event monitor machenism for hotplug.

s/machenism/mechanism

> --
> 2.7.4
  
Matan Azrad April 4, 2018, 4:31 p.m. UTC | #2
Hi all

What do you think about adding the "--hotplug" parameter as a new EAL command line parameter?

From: Tan, Jianfeng, Wednesday, April 4, 2018 6:23 AM
> > -----Original Message-----
> > From: Guo, Jia
> > Sent: Tuesday, April 3, 2018 6:34 PM
> > To: stephen@networkplumber.org; Richardson, Bruce; Yigit, Ferruh;
> > Ananyev, Konstantin; gaetan.rivet@6wind.com; Wu, Jingjing;
> > thomas@monjalon.net; motih@mellanox.com; Van Haaren, Harry; Tan,
> > Jianfeng
> > Cc: jblunck@infradead.org; shreyansh.jain@nxp.com; dev@dpdk.org; Guo,
> > Jia; Zhang, Helin
> > Subject: [PATCH V18 4/4] app/testpmd: enable device hotplug monitoring
> >
> > Use testpmd for example, to show how an application use device event
> 
> s/use/uses
> 
> > APIs to monitor the hotplug events, including both hot removal event
> > and hot insertion event.
> >
> > The process is that, testpmd first enable hotplug by below commands,
> >
> > E.g. ./build/app/testpmd -c 0x3 --n 4 -- -i --hot-plug
> >
> > then testpmd start the device event monitor by call the new API
> 
> s/start/starts
> s/call/calling
> 
> > (rte_dev_event_monitor_start) and register the user's callback by call
> > the API (rte_dev_event_callback_register), when device being hotplug
> > insertion or hotplug removal, the device event monitor detects the
> > event and call user's callbacks, user could process the event in the
> > callback accordingly.
> >
> > This patch only shows the event monitoring, device attach/detach would
> > not be involved here, will add from other hotplug patch set.
> >
> > Signed-off-by: Jeff Guo <jia.guo@intel.com>
> 
> Some typos and a trivial suggestion. Feel free to carry my
> 
> Reviewed-by: Jianfeng Tan <jianfeng.tan@intel.com>
> 
> in the next version.
> 
> > ---
> > v18->v17:
> > remove hotplug policy and detach/attach process from testpmd, let it
> > focus on the device event monitoring which the patch set introduced.
> > ---
> >  app/test-pmd/parameters.c             |   5 +-
> >  app/test-pmd/testpmd.c                | 112
> > +++++++++++++++++++++++++++++++++-
> >  app/test-pmd/testpmd.h                |   2 +
> >  doc/guides/testpmd_app_ug/run_app.rst |   4 ++
> >  4 files changed, 121 insertions(+), 2 deletions(-)
> >
> > diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
> > index 97d22b8..558cd40 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: enable 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..2faeb90 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,8 @@ 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 @@ -391,6 +394,12 @@ 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(void);
> > +static int eth_dev_event_callback_unregister(void);
> > +
> >
> >  /*
> >   * Check if all the ports are started.
> > @@ -1853,6 +1862,39 @@ reset_port(portid_t pid)
> >  	printf("Done\n");
> >  }
> >
> > +static int
> > +eth_dev_event_callback_register(void)
> > +{
> > +	int diag;
> > +
> > +	/* register the device event callback */
> > +	diag = rte_dev_event_callback_register(NULL,
> > +		eth_dev_event_callback, NULL);
> > +	if (diag) {
> > +		printf("Failed to setup dev_event callback\n");
> > +		return -1;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +
> > +static int
> > +eth_dev_event_callback_unregister(void)
> > +{
> > +	int diag;
> > +
> > +	/* unregister the device event callback */
> > +	diag = rte_dev_event_callback_unregister(NULL,
> > +		eth_dev_event_callback, NULL);
> > +	if (diag) {
> > +		printf("Failed to setup dev_event callback\n");
> > +		return -1;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  void
> >  attach_port(char *identifier)
> >  {
> > @@ -1916,6 +1958,7 @@ void
> >  pmd_test_exit(void)
> >  {
> >  	portid_t pt_id;
> > +	int ret;
> >
> >  	if (test_done == 0)
> >  		stop_packet_forwarding();
> > @@ -1929,6 +1972,18 @@ pmd_test_exit(void)
> >  			close_port(pt_id);
> >  		}
> >  	}
> > +
> > +	if (hot_plug) {
> > +		ret = rte_dev_event_monitor_stop();
> > +		if (ret)
> > +			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");
> >  }
> >
> > @@ -2059,6 +2114,48 @@ 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 int
> > +eth_dev_event_callback(char *device_name, enum rte_dev_event_type
> > type,
> > +			     __rte_unused void *arg)
> > +{
> > +	int ret = 0;
> 
> From here
> 
> > +	static const char * const event_desc[] = {
> > +		[RTE_DEV_EVENT_ADD] = "add",
> > +		[RTE_DEV_EVENT_REMOVE] = "remove",
> > +	};
> > +
> > +	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);
> > +	}
> 
> to here, these check are not necessary.
> 
> > +
> > +	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;
> > +	}
> > +	return ret;
> > +}
> > +
> >  static int
> >  set_tx_queue_stats_mapping_registers(portid_t port_id, struct
> > rte_port
> > *port)
> >  {
> > @@ -2474,8 +2571,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 +2641,18 @@ 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;
> > +		}
> > +		eth_dev_event_callback_register();
> > +
> > +	}
> > +
> >  	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..8fde68d 100644
> > --- a/app/test-pmd/testpmd.h
> > +++ b/app/test-pmd/testpmd.h
> > @@ -319,6 +319,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
> > diff --git a/doc/guides/testpmd_app_ug/run_app.rst
> > b/doc/guides/testpmd_app_ug/run_app.rst
> > index 1fd5395..d0ced36 100644
> > --- a/doc/guides/testpmd_app_ug/run_app.rst
> > +++ b/doc/guides/testpmd_app_ug/run_app.rst
> > @@ -479,3 +479,7 @@ The commandline options are:
> >
> >      Set the hexadecimal bitmask of TX queue offloads.
> >      The default value is 0.
> > +
> > +*   ``--hot-plug``
> > +
> > +    Enable device event monitor machenism for hotplug.
> 
> s/machenism/mechanism
> 
> > --
> > 2.7.4
  
Guo, Jia April 5, 2018, 8:32 a.m. UTC | #3
About hot plug in dpdk, We already have proactive way to add/remove devices
through APIs (rte_eal_hotplug_add/remove), and also have fail-safe driver
to offload the fail-safe work from the app user. But there are still lack
of a general mechanism to monitor hotplug event for all driver, now the
hotplug interrupt event is diversity between each device and driver, such
as mlx4, pci driver and others.

Use the hot removal event for example, pci drivers not all exposure the
remove interrupt, so in order to make user to easy use the hot plug
feature for pci driver, something must be done to detect the remove event
at the kernel level and offer a new line of interrupt to the user land.

Base on the uevent of kobject mechanism in kernel, we could use it to
benefit for monitoring the hot plug status of the device which not only
uio/vfio of pci bus devices, but also other, such as cpu/usb/pci-express bus devices.

The idea is comming as bellow.

a.The uevent message form FD monitoring like below.
remove@/devices/pci0000:80/0000:80:02.2/0000:82:00.0/0000:83:03.0/0000:84:00.2/uio/uio2
ACTION=remove
DEVPATH=/devices/pci0000:80/0000:80:02.2/0000:82:00.0/0000:83:03.0/0000:84:00.2/uio/uio2
SUBSYSTEM=uio
MAJOR=243
MINOR=2
DEVNAME=uio2
SEQNUM=11366

b.add device event monitor framework:
add several general api to enable uevent monitoring.

c.show example how to use uevent monitor
enable uevent monitoring in testpmd to show device event monitor machenism usage.

TODO: failure handler mechanism for hot plug and driver auto bind for hot insertion.
that would let the next hot plug patch set to cover.

patchset history:
v19->v18:
fix some typo and misunderstanding part

v18->v17:
1.add feature announcement in release document, fix bsp compile issue.
2.refine socket configuration.
3.remove hotplug policy and detach/attach process from testpmd, let it
focus on the device event monitoring which the patch set introduced.

v17->v16:
1.add related part of the interrupt handle type adding.
2.add new API into map, fix typo issue, add (void*)-1 value for unregister all callback
3.add new file into meson.build, modify coding sytle and add print info, delete unused part.
4.unregister all user's callback when stop event monitor

v16->v15:
1.remove some linux related code out of eal common layer
2.fix some uneasy readble issue.

v15->v14:
1.use exist eal interrupt epoll to replace of rte service usage for monitor thread,
2.add new device event handle type in eal interrupt.
3.remove the uevent type check and any policy from eal,
let it check and management in user's callback.
4.add "--hot-plug" configure parameter in testpmd to switch the hotplug feature.

v14->v13:
1.add __rte_experimental on function defind and fix bsd build issue

v13->v12:
1.fix some logic issue and null check issue
2.fix monitor stop func issue

v12->v11:
1.identify null param in callback for monitor all devices uevent

v11->v10:
1:modify some typo and add experimental tag in new file.
2:modify callback register calling.

v10->v9:
1.fix prefix issue.
2.use a common callback lists for all device and all type to replace
add callback parameter into device struct.
3.delete some unuse part.

v9->v8:
split the patch set into small and explicit patch

v8->v7:
1.use rte_service to replace pthread management.
2.fix defind issue and copyright issue
3.fix some lock issue

v7->v6:
1.modify vdev part according to the vdev rework
2.re-define and split the func into common and bus specific code
3.fix some incorrect issue.
4.fix the system hung after send packcet issue.

v6->v5:
1.add hot plug policy, in eal, default handle to prepare hot plug work for
all pci device, then let app to manage to deside which device need to
hot plug.
2.modify to manage event callback in each device.
3.fix some system hung issue when igb_uioome typo error.release.
4.modify the pci part to the bus-pci base on the bus rework.
5.add hot plug policy in app, show example to use hotplug list to manage
to deside which device need to hot plug.

v5->v4:
1.Move uevent monitor epolling from eal interrupt to eal device layer.
2.Redefine the eal device API for common, and distinguish between linux and bsd
3.Add failure handler helper api in bus layer.Add function of find device by name.
4.Replace of individual fd bind with single device, use a common fd to polling all device.
5.Add to register hot insertion monitoring and process, add function to auto bind driver befor user add device
6.Refine some coding style and typos issue
7.add new callback to process hot insertion

v4->v3:
1.move uevent monitor api from eal interrupt to eal device layer.
2.create uevent type and struct in eal device.
3.move uevent handler for each driver to eal layer.
4.add uevent failure handler to process signal fault issue.
5.add example for request and use uevent monitoring in testpmd.

v3->v2:
1.refine some return error
2.refine the string searching logic to avoid memory issue

v2->v1:
1.remove global variables of hotplug_fd, add uevent_fd
in rte_intr_handle to let each pci device self maintain it fd,
to fix dual device fd issue.
2.refine some typo error.

Jeff Guo (4):
  eal: add device event handle in interrupt thread
  eal: add device event monitor framework
  eal/linux: uevent parse and process
  app/testpmd: enable device hotplug monitoring

 app/test-pmd/parameters.c                          |   5 +-
 app/test-pmd/testpmd.c                             | 101 +++++++++-
 app/test-pmd/testpmd.h                             |   2 +
 doc/guides/rel_notes/release_18_05.rst             |   9 +
 doc/guides/testpmd_app_ug/run_app.rst              |   4 +
 lib/librte_eal/bsdapp/eal/Makefile                 |   1 +
 lib/librte_eal/bsdapp/eal/eal_dev.c                |  21 ++
 lib/librte_eal/bsdapp/eal/meson.build              |   1 +
 lib/librte_eal/common/eal_common_dev.c             | 161 ++++++++++++++++
 lib/librte_eal/common/eal_private.h                |  15 ++
 lib/librte_eal/common/include/rte_dev.h            |  94 +++++++++
 lib/librte_eal/common/include/rte_eal_interrupts.h |   1 +
 lib/librte_eal/linuxapp/eal/Makefile               |   1 +
 lib/librte_eal/linuxapp/eal/eal_dev.c              | 214 +++++++++++++++++++++
 lib/librte_eal/linuxapp/eal/eal_interrupts.c       |  11 +-
 lib/librte_eal/linuxapp/eal/meson.build            |   1 +
 lib/librte_eal/rte_eal_version.map                 |  10 +
 test/test/test_interrupts.c                        |  39 +++-
 18 files changed, 686 insertions(+), 5 deletions(-)
 create mode 100644 lib/librte_eal/bsdapp/eal/eal_dev.c
 create mode 100644 lib/librte_eal/linuxapp/eal/eal_dev.c
  
Guo, Jia April 5, 2018, 8:40 a.m. UTC | #4
On 4/5/2018 12:31 AM, Matan Azrad wrote:
> Hi all
>
> What do you think about adding the "--hotplug" parameter as a new EAL command line parameter?
that just use testpmd for example at this stage, if the total solution 
is accept for all and got agreement for that i think could let it in EAL 
command in the coming patch set..
good suggestion, azrad.
> From: Tan, Jianfeng, Wednesday, April 4, 2018 6:23 AM
>>> -----Original Message-----
>>> From: Guo, Jia
>>> Sent: Tuesday, April 3, 2018 6:34 PM
>>> To: stephen@networkplumber.org; Richardson, Bruce; Yigit, Ferruh;
>>> Ananyev, Konstantin; gaetan.rivet@6wind.com; Wu, Jingjing;
>>> thomas@monjalon.net; motih@mellanox.com; Van Haaren, Harry; Tan,
>>> Jianfeng
>>> Cc: jblunck@infradead.org; shreyansh.jain@nxp.com; dev@dpdk.org; Guo,
>>> Jia; Zhang, Helin
>>> Subject: [PATCH V18 4/4] app/testpmd: enable device hotplug monitoring
>>>
>>> Use testpmd for example, to show how an application use device event
>> s/use/uses
>>
>>> APIs to monitor the hotplug events, including both hot removal event
>>> and hot insertion event.
>>>
>>> The process is that, testpmd first enable hotplug by below commands,
>>>
>>> E.g. ./build/app/testpmd -c 0x3 --n 4 -- -i --hot-plug
>>>
>>> then testpmd start the device event monitor by call the new API
>> s/start/starts
>> s/call/calling
>>
>>> (rte_dev_event_monitor_start) and register the user's callback by call
>>> the API (rte_dev_event_callback_register), when device being hotplug
>>> insertion or hotplug removal, the device event monitor detects the
>>> event and call user's callbacks, user could process the event in the
>>> callback accordingly.
>>>
>>> This patch only shows the event monitoring, device attach/detach would
>>> not be involved here, will add from other hotplug patch set.
>>>
>>> Signed-off-by: Jeff Guo <jia.guo@intel.com>
>> Some typos and a trivial suggestion. Feel free to carry my
>>
>> Reviewed-by: Jianfeng Tan <jianfeng.tan@intel.com>
>>
>> in the next version.
>>
>>> ---
>>> v18->v17:
>>> remove hotplug policy and detach/attach process from testpmd, let it
>>> focus on the device event monitoring which the patch set introduced.
>>> ---
>>>   app/test-pmd/parameters.c             |   5 +-
>>>   app/test-pmd/testpmd.c                | 112
>>> +++++++++++++++++++++++++++++++++-
>>>   app/test-pmd/testpmd.h                |   2 +
>>>   doc/guides/testpmd_app_ug/run_app.rst |   4 ++
>>>   4 files changed, 121 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
>>> index 97d22b8..558cd40 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: enable 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..2faeb90 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,8 @@ 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 @@ -391,6 +394,12 @@ 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(void);
>>> +static int eth_dev_event_callback_unregister(void);
>>> +
>>>
>>>   /*
>>>    * Check if all the ports are started.
>>> @@ -1853,6 +1862,39 @@ reset_port(portid_t pid)
>>>   	printf("Done\n");
>>>   }
>>>
>>> +static int
>>> +eth_dev_event_callback_register(void)
>>> +{
>>> +	int diag;
>>> +
>>> +	/* register the device event callback */
>>> +	diag = rte_dev_event_callback_register(NULL,
>>> +		eth_dev_event_callback, NULL);
>>> +	if (diag) {
>>> +		printf("Failed to setup dev_event callback\n");
>>> +		return -1;
>>> +	}
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +
>>> +static int
>>> +eth_dev_event_callback_unregister(void)
>>> +{
>>> +	int diag;
>>> +
>>> +	/* unregister the device event callback */
>>> +	diag = rte_dev_event_callback_unregister(NULL,
>>> +		eth_dev_event_callback, NULL);
>>> +	if (diag) {
>>> +		printf("Failed to setup dev_event callback\n");
>>> +		return -1;
>>> +	}
>>> +
>>> +	return 0;
>>> +}
>>> +
>>>   void
>>>   attach_port(char *identifier)
>>>   {
>>> @@ -1916,6 +1958,7 @@ void
>>>   pmd_test_exit(void)
>>>   {
>>>   	portid_t pt_id;
>>> +	int ret;
>>>
>>>   	if (test_done == 0)
>>>   		stop_packet_forwarding();
>>> @@ -1929,6 +1972,18 @@ pmd_test_exit(void)
>>>   			close_port(pt_id);
>>>   		}
>>>   	}
>>> +
>>> +	if (hot_plug) {
>>> +		ret = rte_dev_event_monitor_stop();
>>> +		if (ret)
>>> +			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");
>>>   }
>>>
>>> @@ -2059,6 +2114,48 @@ 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 int
>>> +eth_dev_event_callback(char *device_name, enum rte_dev_event_type
>>> type,
>>> +			     __rte_unused void *arg)
>>> +{
>>> +	int ret = 0;
>>  From here
>>
>>> +	static const char * const event_desc[] = {
>>> +		[RTE_DEV_EVENT_ADD] = "add",
>>> +		[RTE_DEV_EVENT_REMOVE] = "remove",
>>> +	};
>>> +
>>> +	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);
>>> +	}
>> to here, these check are not necessary.
>>
>>> +
>>> +	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;
>>> +	}
>>> +	return ret;
>>> +}
>>> +
>>>   static int
>>>   set_tx_queue_stats_mapping_registers(portid_t port_id, struct
>>> rte_port
>>> *port)
>>>   {
>>> @@ -2474,8 +2571,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 +2641,18 @@ 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;
>>> +		}
>>> +		eth_dev_event_callback_register();
>>> +
>>> +	}
>>> +
>>>   	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..8fde68d 100644
>>> --- a/app/test-pmd/testpmd.h
>>> +++ b/app/test-pmd/testpmd.h
>>> @@ -319,6 +319,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
>>> diff --git a/doc/guides/testpmd_app_ug/run_app.rst
>>> b/doc/guides/testpmd_app_ug/run_app.rst
>>> index 1fd5395..d0ced36 100644
>>> --- a/doc/guides/testpmd_app_ug/run_app.rst
>>> +++ b/doc/guides/testpmd_app_ug/run_app.rst
>>> @@ -479,3 +479,7 @@ The commandline options are:
>>>
>>>       Set the hexadecimal bitmask of TX queue offloads.
>>>       The default value is 0.
>>> +
>>> +*   ``--hot-plug``
>>> +
>>> +    Enable device event monitor machenism for hotplug.
>> s/machenism/mechanism
>>
>>> --
>>> 2.7.4
  
Guo, Jia April 5, 2018, 9:02 a.m. UTC | #5
About hot plug in dpdk, We already have proactive way to add/remove devices
through APIs (rte_eal_hotplug_add/remove), and also have fail-safe driver
to offload the fail-safe work from the app user. But there are still lack
of a general mechanism to monitor hotplug event for all driver, now the
hotplug interrupt event is diversity between each device and driver, such
as mlx4, pci driver and others.

Use the hot removal event for example, pci drivers not all exposure the
remove interrupt, so in order to make user to easy use the hot plug
feature for pci driver, something must be done to detect the remove event
at the kernel level and offer a new line of interrupt to the user land.

Base on the uevent of kobject mechanism in kernel, we could use it to
benefit for monitoring the hot plug status of the device which not only
uio/vfio of pci bus devices, but also other, such as cpu/usb/pci-express bus devices.

The idea is comming as bellow.

a.The uevent message form FD monitoring like below.
remove@/devices/pci0000:80/0000:80:02.2/0000:82:00.0/0000:83:03.0/0000:84:00.2/uio/uio2
ACTION=remove
DEVPATH=/devices/pci0000:80/0000:80:02.2/0000:82:00.0/0000:83:03.0/0000:84:00.2/uio/uio2
SUBSYSTEM=uio
MAJOR=243
MINOR=2
DEVNAME=uio2
SEQNUM=11366

b.add device event monitor framework:
add several general api to enable uevent monitoring.

c.show example how to use uevent monitor
enable uevent monitoring in testpmd to show device event monitor machenism usage.

TODO: failure handler mechanism for hot plug and driver auto bind for hot insertion.
that would let the next hot plug patch set to cover.

patchset history:
v19->v18:
fix some typo and misunderstanding part

v18->v17:
1.add feature announcement in release document, fix bsp compile issue.
2.refine socket configuration.
3.remove hotplug policy and detach/attach process from testpmd, let it
focus on the device event monitoring which the patch set introduced.

v17->v16:
1.add related part of the interrupt handle type adding.
2.add new API into map, fix typo issue, add (void*)-1 value for unregister all callback
3.add new file into meson.build, modify coding sytle and add print info, delete unused part.
4.unregister all user's callback when stop event monitor

v16->v15:
1.remove some linux related code out of eal common layer
2.fix some uneasy readble issue.

v15->v14:
1.use exist eal interrupt epoll to replace of rte service usage for monitor thread,
2.add new device event handle type in eal interrupt.
3.remove the uevent type check and any policy from eal,
let it check and management in user's callback.
4.add "--hot-plug" configure parameter in testpmd to switch the hotplug feature.

v14->v13:
1.add __rte_experimental on function defind and fix bsd build issue

v13->v12:
1.fix some logic issue and null check issue
2.fix monitor stop func issue

v12->v11:
1.identify null param in callback for monitor all devices uevent

v11->v10:
1:modify some typo and add experimental tag in new file.
2:modify callback register calling.

v10->v9:
1.fix prefix issue.
2.use a common callback lists for all device and all type to replace
add callback parameter into device struct.
3.delete some unuse part.

v9->v8:
split the patch set into small and explicit patch

v8->v7:
1.use rte_service to replace pthread management.
2.fix defind issue and copyright issue
3.fix some lock issue

v7->v6:
1.modify vdev part according to the vdev rework
2.re-define and split the func into common and bus specific code
3.fix some incorrect issue.
4.fix the system hung after send packcet issue.

v6->v5:
1.add hot plug policy, in eal, default handle to prepare hot plug work for
all pci device, then let app to manage to deside which device need to
hot plug.
2.modify to manage event callback in each device.
3.fix some system hung issue when igb_uioome typo error.release.
4.modify the pci part to the bus-pci base on the bus rework.
5.add hot plug policy in app, show example to use hotplug list to manage
to deside which device need to hot plug.

v5->v4:
1.Move uevent monitor epolling from eal interrupt to eal device layer.
2.Redefine the eal device API for common, and distinguish between linux and bsd
3.Add failure handler helper api in bus layer.Add function of find device by name.
4.Replace of individual fd bind with single device, use a common fd to polling all device.
5.Add to register hot insertion monitoring and process, add function to auto bind driver befor user add device
6.Refine some coding style and typos issue
7.add new callback to process hot insertion

v4->v3:
1.move uevent monitor api from eal interrupt to eal device layer.
2.create uevent type and struct in eal device.
3.move uevent handler for each driver to eal layer.
4.add uevent failure handler to process signal fault issue.
5.add example for request and use uevent monitoring in testpmd.

v3->v2:
1.refine some return error
2.refine the string searching logic to avoid memory issue

v2->v1:
1.remove global variables of hotplug_fd, add uevent_fd
in rte_intr_handle to let each pci device self maintain it fd,
to fix dual device fd issue.
2.refine some typo error.

Jeff Guo (4):
  eal: add device event handle in interrupt thread
  eal: add device event monitor framework
  eal/linux: uevent parse and process
  app/testpmd: enable device hotplug monitoring

 app/test-pmd/parameters.c                          |   5 +-
 app/test-pmd/testpmd.c                             | 101 +++++++++-
 app/test-pmd/testpmd.h                             |   2 +
 doc/guides/rel_notes/release_18_05.rst             |   9 +
 doc/guides/testpmd_app_ug/run_app.rst              |   4 +
 lib/librte_eal/bsdapp/eal/Makefile                 |   1 +
 lib/librte_eal/bsdapp/eal/eal_dev.c                |  21 ++
 lib/librte_eal/bsdapp/eal/meson.build              |   1 +
 lib/librte_eal/common/eal_common_dev.c             | 161 ++++++++++++++++
 lib/librte_eal/common/eal_private.h                |  15 ++
 lib/librte_eal/common/include/rte_dev.h            |  94 +++++++++
 lib/librte_eal/common/include/rte_eal_interrupts.h |   1 +
 lib/librte_eal/linuxapp/eal/Makefile               |   1 +
 lib/librte_eal/linuxapp/eal/eal_dev.c              | 214 +++++++++++++++++++++
 lib/librte_eal/linuxapp/eal/eal_interrupts.c       |  11 +-
 lib/librte_eal/linuxapp/eal/meson.build            |   1 +
 lib/librte_eal/rte_eal_version.map                 |  10 +
 test/test/test_interrupts.c                        |  39 +++-
 18 files changed, 686 insertions(+), 5 deletions(-)
 create mode 100644 lib/librte_eal/bsdapp/eal/eal_dev.c
 create mode 100644 lib/librte_eal/linuxapp/eal/eal_dev.c
  
Jianfeng Tan April 5, 2018, 9:03 a.m. UTC | #6
On 4/5/2018 12:31 AM, Matan Azrad wrote:
> Hi all
>
> What do you think about adding the "--hotplug" parameter as a new EAL command line parameter?

+1

Thanks,
Jianfeng

>
> From: Tan, Jianfeng, Wednesday, April 4, 2018 6:23 AM
>>> -----Original Message-----
>>> From: Guo, Jia
>>> Sent: Tuesday, April 3, 2018 6:34 PM
>>> To: stephen@networkplumber.org; Richardson, Bruce; Yigit, Ferruh;
>>> Ananyev, Konstantin; gaetan.rivet@6wind.com; Wu, Jingjing;
>>> thomas@monjalon.net; motih@mellanox.com; Van Haaren, Harry; Tan,
>>> Jianfeng
>>> Cc: jblunck@infradead.org; shreyansh.jain@nxp.com; dev@dpdk.org; Guo,
>>> Jia; Zhang, Helin
>>> Subject: [PATCH V18 4/4] app/testpmd: enable device hotplug monitoring
>>>
>>> Use testpmd for example, to show how an application use device event
>> s/use/uses
>>
>>> APIs to monitor the hotplug events, including both hot removal event
>>> and hot insertion event.
>>>
>>> The process is that, testpmd first enable hotplug by below commands,
>>>
>>> E.g. ./build/app/testpmd -c 0x3 --n 4 -- -i --hot-plug
>>>
>>> then testpmd start the device event monitor by call the new API
>> s/start/starts
>> s/call/calling
>>
>>> (rte_dev_event_monitor_start) and register the user's callback by call
>>> the API (rte_dev_event_callback_register), when device being hotplug
>>> insertion or hotplug removal, the device event monitor detects the
>>> event and call user's callbacks, user could process the event in the
>>> callback accordingly.
>>>
>>> This patch only shows the event monitoring, device attach/detach would
>>> not be involved here, will add from other hotplug patch set.
>>>
>>> Signed-off-by: Jeff Guo <jia.guo@intel.com>
>> Some typos and a trivial suggestion. Feel free to carry my
>>
>> Reviewed-by: Jianfeng Tan <jianfeng.tan@intel.com>
>>
>> in the next version.
>>
>>> ---
>>> v18->v17:
>>> remove hotplug policy and detach/attach process from testpmd, let it
>>> focus on the device event monitoring which the patch set introduced.
>>> ---
>>>   app/test-pmd/parameters.c             |   5 +-
>>>   app/test-pmd/testpmd.c                | 112
>>> +++++++++++++++++++++++++++++++++-
>>>   app/test-pmd/testpmd.h                |   2 +
>>>   doc/guides/testpmd_app_ug/run_app.rst |   4 ++
>>>   4 files changed, 121 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
>>> index 97d22b8..558cd40 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: enable 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..2faeb90 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,8 @@ 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 @@ -391,6 +394,12 @@ 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(void);
>>> +static int eth_dev_event_callback_unregister(void);
>>> +
>>>
>>>   /*
>>>    * Check if all the ports are started.
>>> @@ -1853,6 +1862,39 @@ reset_port(portid_t pid)
>>>   	printf("Done\n");
>>>   }
>>>
>>> +static int
>>> +eth_dev_event_callback_register(void)
>>> +{
>>> +	int diag;
>>> +
>>> +	/* register the device event callback */
>>> +	diag = rte_dev_event_callback_register(NULL,
>>> +		eth_dev_event_callback, NULL);
>>> +	if (diag) {
>>> +		printf("Failed to setup dev_event callback\n");
>>> +		return -1;
>>> +	}
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +
>>> +static int
>>> +eth_dev_event_callback_unregister(void)
>>> +{
>>> +	int diag;
>>> +
>>> +	/* unregister the device event callback */
>>> +	diag = rte_dev_event_callback_unregister(NULL,
>>> +		eth_dev_event_callback, NULL);
>>> +	if (diag) {
>>> +		printf("Failed to setup dev_event callback\n");
>>> +		return -1;
>>> +	}
>>> +
>>> +	return 0;
>>> +}
>>> +
>>>   void
>>>   attach_port(char *identifier)
>>>   {
>>> @@ -1916,6 +1958,7 @@ void
>>>   pmd_test_exit(void)
>>>   {
>>>   	portid_t pt_id;
>>> +	int ret;
>>>
>>>   	if (test_done == 0)
>>>   		stop_packet_forwarding();
>>> @@ -1929,6 +1972,18 @@ pmd_test_exit(void)
>>>   			close_port(pt_id);
>>>   		}
>>>   	}
>>> +
>>> +	if (hot_plug) {
>>> +		ret = rte_dev_event_monitor_stop();
>>> +		if (ret)
>>> +			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");
>>>   }
>>>
>>> @@ -2059,6 +2114,48 @@ 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 int
>>> +eth_dev_event_callback(char *device_name, enum rte_dev_event_type
>>> type,
>>> +			     __rte_unused void *arg)
>>> +{
>>> +	int ret = 0;
>>  From here
>>
>>> +	static const char * const event_desc[] = {
>>> +		[RTE_DEV_EVENT_ADD] = "add",
>>> +		[RTE_DEV_EVENT_REMOVE] = "remove",
>>> +	};
>>> +
>>> +	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);
>>> +	}
>> to here, these check are not necessary.
>>
>>> +
>>> +	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;
>>> +	}
>>> +	return ret;
>>> +}
>>> +
>>>   static int
>>>   set_tx_queue_stats_mapping_registers(portid_t port_id, struct
>>> rte_port
>>> *port)
>>>   {
>>> @@ -2474,8 +2571,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 +2641,18 @@ 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;
>>> +		}
>>> +		eth_dev_event_callback_register();
>>> +
>>> +	}
>>> +
>>>   	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..8fde68d 100644
>>> --- a/app/test-pmd/testpmd.h
>>> +++ b/app/test-pmd/testpmd.h
>>> @@ -319,6 +319,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
>>> diff --git a/doc/guides/testpmd_app_ug/run_app.rst
>>> b/doc/guides/testpmd_app_ug/run_app.rst
>>> index 1fd5395..d0ced36 100644
>>> --- a/doc/guides/testpmd_app_ug/run_app.rst
>>> +++ b/doc/guides/testpmd_app_ug/run_app.rst
>>> @@ -479,3 +479,7 @@ The commandline options are:
>>>
>>>       Set the hexadecimal bitmask of TX queue offloads.
>>>       The default value is 0.
>>> +
>>> +*   ``--hot-plug``
>>> +
>>> +    Enable device event monitor machenism for hotplug.
>> s/machenism/mechanism
>>
>>> --
>>> 2.7.4
  

Patch

diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index 97d22b8..558cd40 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: enable 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..2faeb90 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,8 @@  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
@@ -391,6 +394,12 @@  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(void);
+static int eth_dev_event_callback_unregister(void);
+
 
 /*
  * Check if all the ports are started.
@@ -1853,6 +1862,39 @@  reset_port(portid_t pid)
 	printf("Done\n");
 }
 
+static int
+eth_dev_event_callback_register(void)
+{
+	int diag;
+
+	/* register the device event callback */
+	diag = rte_dev_event_callback_register(NULL,
+		eth_dev_event_callback, NULL);
+	if (diag) {
+		printf("Failed to setup dev_event callback\n");
+		return -1;
+	}
+
+	return 0;
+}
+
+
+static int
+eth_dev_event_callback_unregister(void)
+{
+	int diag;
+
+	/* unregister the device event callback */
+	diag = rte_dev_event_callback_unregister(NULL,
+		eth_dev_event_callback, NULL);
+	if (diag) {
+		printf("Failed to setup dev_event callback\n");
+		return -1;
+	}
+
+	return 0;
+}
+
 void
 attach_port(char *identifier)
 {
@@ -1916,6 +1958,7 @@  void
 pmd_test_exit(void)
 {
 	portid_t pt_id;
+	int ret;
 
 	if (test_done == 0)
 		stop_packet_forwarding();
@@ -1929,6 +1972,18 @@  pmd_test_exit(void)
 			close_port(pt_id);
 		}
 	}
+
+	if (hot_plug) {
+		ret = rte_dev_event_monitor_stop();
+		if (ret)
+			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");
 }
 
@@ -2059,6 +2114,48 @@  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 int
+eth_dev_event_callback(char *device_name, enum rte_dev_event_type type,
+			     __rte_unused void *arg)
+{
+	int ret = 0;
+	static const char * const event_desc[] = {
+		[RTE_DEV_EVENT_ADD] = "add",
+		[RTE_DEV_EVENT_REMOVE] = "remove",
+	};
+
+	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:
+		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;
+	}
+	return ret;
+}
+
 static int
 set_tx_queue_stats_mapping_registers(portid_t port_id, struct rte_port *port)
 {
@@ -2474,8 +2571,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 +2641,18 @@  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;
+		}
+		eth_dev_event_callback_register();
+
+	}
+
 	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..8fde68d 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -319,6 +319,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
diff --git a/doc/guides/testpmd_app_ug/run_app.rst b/doc/guides/testpmd_app_ug/run_app.rst
index 1fd5395..d0ced36 100644
--- a/doc/guides/testpmd_app_ug/run_app.rst
+++ b/doc/guides/testpmd_app_ug/run_app.rst
@@ -479,3 +479,7 @@  The commandline options are:
 
     Set the hexadecimal bitmask of TX queue offloads.
     The default value is 0.
+
+*   ``--hot-plug``
+
+    Enable device event monitor machenism for hotplug.