From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Tan, Jianfeng" Subject: Re: [PATCH V18 4/4] app/testpmd: enable device hotplug monitoring Date: Thu, 5 Apr 2018 17:03:40 +0800 Message-ID: References: <1522339205-27698-5-git-send-email-jia.guo@intel.com> <1522751639-2315-1-git-send-email-jia.guo@intel.com> <1522751639-2315-5-git-send-email-jia.guo@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Cc: "jblunck@infradead.org" , "shreyansh.jain@nxp.com" , "dev@dpdk.org" , "Zhang, Helin" To: Matan Azrad , "Guo, Jia" , "stephen@networkplumber.org" , "Richardson, Bruce" , "Yigit, Ferruh" , "Ananyev, Konstantin" , "gaetan.rivet@6wind.com" , "Wu, Jingjing" , Thomas Monjalon , Mordechay Haimovsky , "Van Haaren, Harry" Return-path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 130161CA2C for ; Thu, 5 Apr 2018 11:03:45 +0200 (CEST) In-Reply-To: List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 >> Some typos and a trivial suggestion. Feel free to carry my >> >> Reviewed-by: Jianfeng Tan >> >> 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 >>> #include >>> #include >>> +#include >>> >>> #include >>> #include >>> @@ -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