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: step...@networkplumber.org; Richardson, Bruce; Yigit, Ferruh;
> > Ananyev, Konstantin; gaetan.ri...@6wind.com; Wu, Jingjing;
> > tho...@monjalon.net; mo...@mellanox.com; Van Haaren, Harry; Tan,
> > Jianfeng
> > Cc: jblu...@infradead.org; shreyansh.j...@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....@intel.com>
>
> Some typos and a trivial suggestion. Feel free to carry my
>
> Reviewed-by: Jianfeng Tan <jianfeng....@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
