From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Ananyev, Konstantin" Subject: Re: [PATCH v3 3/3] examples/l3fwd: Handle SIGINT and SIGTERM in l3fwd Date: Tue, 29 Dec 2015 13:34:19 +0000 Message-ID: <2601191342CEEE43887BDE71AB97725836ADDED8@irsmsx105.ger.corp.intel.com> References: <1451011032-83106-1-git-send-email-zhihong.wang@intel.com> <1451352032-105576-1-git-send-email-zhihong.wang@intel.com> <1451352032-105576-4-git-send-email-zhihong.wang@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable To: "Wang, Zhihong" , "dev@dpdk.org" Return-path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 3CDBC8E97 for ; Tue, 29 Dec 2015 14:34:37 +0100 (CET) In-Reply-To: <1451352032-105576-4-git-send-email-zhihong.wang@intel.com> Content-Language: en-US List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" > -----Original Message----- > From: Wang, Zhihong > Sent: Tuesday, December 29, 2015 1:21 AM > To: dev@dpdk.org > Cc: Ananyev, Konstantin; stephen@networkplumber.org; Qiu, Michael; Wang, = Zhihong > Subject: [PATCH v3 3/3] examples/l3fwd: Handle SIGINT and SIGTERM in l3fw= d >=20 > Handle SIGINT and SIGTERM in l3fwd. >=20 > Signed-off-by: Zhihong Wang > Acked-by: Michael Qiu > --- > examples/l3fwd/main.c | 129 +++++++++++++++++++++++++++++++++++++++++---= ------ > 1 file changed, 107 insertions(+), 22 deletions(-) >=20 > diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c > index 5b0c2dd..c766cf5 100644 > --- a/examples/l3fwd/main.c > +++ b/examples/l3fwd/main.c > @@ -41,6 +41,9 @@ > #include > #include > #include > +#include > +#include > +#include >=20 > #include > #include > @@ -75,6 +78,10 @@ > #include > #include >=20 > +static volatile bool port_started; > +static volatile bool force_quit; > +static volatile int signo_quit; > + > #define APP_LOOKUP_EXACT_MATCH 0 > #define APP_LOOKUP_LPM 1 > #define DO_RFC_1812_CHECKS > @@ -1553,8 +1560,7 @@ main_loop(__attribute__((unused)) void *dummy) > portid, queueid); > } >=20 > - while (1) { > - > + while (!force_quit) { > cur_tsc =3D rte_rdtsc(); >=20 > /* > @@ -2516,8 +2522,12 @@ check_all_ports_link_status(uint8_t port_num, uint= 32_t port_mask) > printf("\nChecking link status"); > fflush(stdout); > for (count =3D 0; count <=3D MAX_CHECK_TIME; count++) { > + if (force_quit) > + return; > all_ports_up =3D 1; > for (portid =3D 0; portid < port_num; portid++) { > + if (force_quit) > + return; > if ((port_mask & (1 << portid)) =3D=3D 0) > continue; > memset(&link, 0, sizeof(link)); > @@ -2559,6 +2569,76 @@ check_all_ports_link_status(uint8_t port_num, uint= 32_t port_mask) > } > } >=20 > +static uint8_t > +start_ports(void) > +{ > + unsigned portid, nb_ports, avail_ports; > + int ret; > + > + nb_ports =3D rte_eth_dev_count(); > + avail_ports =3D 0; > + for (portid =3D 0; portid < nb_ports; portid++) { > + if ((enabled_port_mask & (1 << portid)) =3D=3D 0) > + continue; > + avail_ports++; > + port_started =3D true; Why do you need it at each iteration? > + printf("Starting port %d...", portid); > + ret =3D rte_eth_dev_start(portid); > + if (ret < 0) > + rte_exit(EXIT_FAILURE, > + "rte_eth_dev_start: err=3D%d, port=3D%d\n", > + ret, portid); > + /* > + * If enabled, put device in promiscuous mode. > + * This allows IO forwarding mode to forward packets > + * to itself through 2 cross-connected ports of the > + * target machine. > + */ > + if (promiscuous_on) > + rte_eth_promiscuous_enable(portid); > + printf(" Done\n"); > + } > + > + return avail_ports; > +} > + > +static void > +stop_ports(void) > +{ > + unsigned portid, nb_ports; > + > + nb_ports =3D rte_eth_dev_count(); > + for (portid =3D 0; portid < nb_ports; portid++) { > + if ((enabled_port_mask & (1 << portid)) =3D=3D 0) > + continue; > + printf("Stopping port %d...", portid); > + rte_eth_dev_stop(portid); > + rte_eth_dev_close(portid); > + printf(" Done\n"); > + } > + port_started =3D false; > +} > + > +static void > +signal_handler(int signum) > +{ > + if (signum =3D=3D SIGINT || signum =3D=3D SIGTERM) { > + printf("\nSignal %d received, preparing to exit...\n", > + signum); > + if (port_started) { > + printf("Ports started already...\n"); > + signo_quit =3D signum; > + force_quit =3D true; > + } else { Hmm, and what if signal_handler() would be executed not in the context of m= aster lcore? Then there could be a raise condition, and you could end up here, while mas= ter lcore would be in the middle of start_ports()->rte_eth_dev_start(). Probably not a big deal, but why do you need this if (port_started) {...} = else {...} at all? Why not just: signal_handler(int signum) { signo_quit =3D signum; force_quit =3D true; } ? Konstantin > + printf("Ports not started yet...\n"); > + printf("Bye...\n"); > + /* exit with the expected status */ > + signal(signum, SIG_DFL); > + kill(getpid(), signum); > + } > + } > +} > + > int > main(int argc, char **argv) > { > @@ -2571,6 +2651,12 @@ main(int argc, char **argv) > unsigned lcore_id; > uint32_t n_tx_queue, nb_lcores; > uint8_t portid, nb_rx_queue, queue, socketid; > + uint8_t avail_ports; > + > + port_started =3D false; > + force_quit =3D false; > + signal(SIGINT, signal_handler); > + signal(SIGTERM, signal_handler); >=20 > /* init EAL */ > ret =3D rte_eal_init(argc, argv); > @@ -2711,34 +2797,33 @@ main(int argc, char **argv) > printf("\n"); >=20 > /* start ports */ > - for (portid =3D 0; portid < nb_ports; portid++) { > - if ((enabled_port_mask & (1 << portid)) =3D=3D 0) { > - continue; > - } > - /* Start device */ > - ret =3D rte_eth_dev_start(portid); > - if (ret < 0) > - rte_exit(EXIT_FAILURE, "rte_eth_dev_start: err=3D%d, port=3D%d\n", > - ret, portid); > + avail_ports =3D start_ports(); >=20 > - /* > - * If enabled, put device in promiscuous mode. > - * This allows IO forwarding mode to forward packets > - * to itself through 2 cross-connected ports of the > - * target machine. > - */ > - if (promiscuous_on) > - rte_eth_promiscuous_enable(portid); > + if (!avail_ports) { > + rte_exit(EXIT_FAILURE, > + "All ports are disabled, please check portmask...\n"); > } >=20 > check_all_ports_link_status((uint8_t)nb_ports, enabled_port_mask); >=20 > + ret =3D 0; > /* launch per-lcore init on every lcore */ > rte_eal_mp_remote_launch(main_loop, NULL, CALL_MASTER); > RTE_LCORE_FOREACH_SLAVE(lcore_id) { > - if (rte_eal_wait_lcore(lcore_id) < 0) > - return -1; > + if (rte_eal_wait_lcore(lcore_id) < 0) { > + ret =3D -1; > + break; > + } > } >=20 > - return 0; > + /* stop ports */ > + stop_ports(); > + printf("Bye...\n"); > + /* exit with the expected status */ > + if (force_quit) { > + signal(signo_quit, SIG_DFL); > + kill(getpid(), signo_quit); > + } > + > + return ret; > } > -- > 2.5.0