> -----Original Message-----
> From: Wang, Zhihong
> Sent: Tuesday, December 29, 2015 1:21 AM
> To: dev at dpdk.org
> Cc: Ananyev, Konstantin; stephen at networkplumber.org; Qiu, Michael; Wang,
> Zhihong
> Subject: [PATCH v3 3/3] examples/l3fwd: Handle SIGINT and SIGTERM in l3fwd
>
> Handle SIGINT and SIGTERM in l3fwd.
>
> Signed-off-by: Zhihong Wang <zhihong.wang at intel.com>
> Acked-by: Michael Qiu <michael.qiu at intel.com>
> ---
> examples/l3fwd/main.c | 129
> +++++++++++++++++++++++++++++++++++++++++---------
> 1 file changed, 107 insertions(+), 22 deletions(-)
>
> 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 <stdarg.h>
> #include <errno.h>
> #include <getopt.h>
> +#include <signal.h>
> +#include <unistd.h>
> +#include <stdbool.h>
>
> #include <rte_common.h>
> #include <rte_vect.h>
> @@ -75,6 +78,10 @@
> #include <cmdline_parse.h>
> #include <cmdline_parse_etheraddr.h>
>
> +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);
> }
>
> - while (1) {
> -
> + while (!force_quit) {
> cur_tsc = rte_rdtsc();
>
> /*
> @@ -2516,8 +2522,12 @@ check_all_ports_link_status(uint8_t port_num, uint32_t
> port_mask)
> printf("\nChecking link status");
> fflush(stdout);
> for (count = 0; count <= MAX_CHECK_TIME; count++) {
> + if (force_quit)
> + return;
> all_ports_up = 1;
> for (portid = 0; portid < port_num; portid++) {
> + if (force_quit)
> + return;
> if ((port_mask & (1 << portid)) == 0)
> continue;
> memset(&link, 0, sizeof(link));
> @@ -2559,6 +2569,76 @@ check_all_ports_link_status(uint8_t port_num, uint32_t
> port_mask)
> }
> }
>
> +static uint8_t
> +start_ports(void)
> +{
> + unsigned portid, nb_ports, avail_ports;
> + int ret;
> +
> + nb_ports = rte_eth_dev_count();
> + avail_ports = 0;
> + for (portid = 0; portid < nb_ports; portid++) {
> + if ((enabled_port_mask & (1 << portid)) == 0)
> + continue;
> + avail_ports++;
> + port_started = true;
Why do you need it at each iteration?
> + printf("Starting port %d...", portid);
> + ret = rte_eth_dev_start(portid);
> + if (ret < 0)
> + rte_exit(EXIT_FAILURE,
> + "rte_eth_dev_start: err=%d, port=%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 = rte_eth_dev_count();
> + for (portid = 0; portid < nb_ports; portid++) {
> + if ((enabled_port_mask & (1 << portid)) == 0)
> + continue;
> + printf("Stopping port %d...", portid);
> + rte_eth_dev_stop(portid);
> + rte_eth_dev_close(portid);
> + printf(" Done\n");
> + }
> + port_started = false;
> +}
> +
> +static void
> +signal_handler(int signum)
> +{
> + if (signum == SIGINT || signum == SIGTERM) {
> + printf("\nSignal %d received, preparing to exit...\n",
> + signum);
> + if (port_started) {
> + printf("Ports started already...\n");
> + signo_quit = signum;
> + force_quit = true;
> + } else {
Hmm, and what if signal_handler() would be executed not in the context of
master lcore?
Then there could be a raise condition, and you could end up here, while master
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 = signum;
force_quit = 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 = false;
> + force_quit = false;
> + signal(SIGINT, signal_handler);
> + signal(SIGTERM, signal_handler);
>
> /* init EAL */
> ret = rte_eal_init(argc, argv);
> @@ -2711,34 +2797,33 @@ main(int argc, char **argv)
> printf("\n");
>
> /* start ports */
> - for (portid = 0; portid < nb_ports; portid++) {
> - if ((enabled_port_mask & (1 << portid)) == 0) {
> - continue;
> - }
> - /* Start device */
> - ret = rte_eth_dev_start(portid);
> - if (ret < 0)
> - rte_exit(EXIT_FAILURE, "rte_eth_dev_start: err=%d,
> port=%d\n",
> - ret, portid);
> + avail_ports = start_ports();
>
> - /*
> - * 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");
> }
>
> check_all_ports_link_status((uint8_t)nb_ports, enabled_port_mask);
>
> + ret = 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 = -1;
> + break;
> + }
> }
>
> - 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
