From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <stephen@networkplumber.org>
Received: from mail-pa0-f46.google.com (mail-pa0-f46.google.com
 [209.85.220.46]) by dpdk.org (Postfix) with ESMTP id 812558D93
 for <dev@dpdk.org>; Sun, 27 Dec 2015 22:48:55 +0100 (CET)
Received: by mail-pa0-f46.google.com with SMTP id uo6so78236519pac.1
 for <dev@dpdk.org>; Sun, 27 Dec 2015 13:48:55 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=networkplumber-org.20150623.gappssmtp.com; s=20150623;
 h=date:from:to:cc:subject:message-id:in-reply-to:references
 :mime-version:content-type:content-transfer-encoding;
 bh=e4q8jmZL/7c0u4i6n+IdlzT1uARVI1vHGScf5laKJZk=;
 b=LnttiFH6T7TW58m8CamITPh8/mzRUa1xw+6KqfZbh3lMB/a9lmWh5xgEOxHYcqnMAD
 XW9Wa93qth7bDQdzGi8VsbnUFr1H0poh77r0IUi+ej2+Ca7BYNIXwejbAjI0omaM7Stz
 SKAcm+I1hxyFSUbJyjIhdd1KQeuOy/5d655CcaROBkibAzC/kBypSmI8Ron+ZkAKEMMk
 m9oSysYxgMrkYOxGaHwX6R6JmsQ3DGQ+jQMrlsE3xl8vWB7blZprXHU2oq+AL4Reofm2
 80p5IRP+5BvUAbQmpw7Q2PBOEu++u9mCMGMrntPEu8Qam5h/Dh0GBpJzGdNS6GBrhUJw
 mPpA==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20130820;
 h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to
 :references:mime-version:content-type:content-transfer-encoding;
 bh=e4q8jmZL/7c0u4i6n+IdlzT1uARVI1vHGScf5laKJZk=;
 b=JtI2cAjBCVu3hA7xWTxnvKtqwLIXtl8F66wdBxB9Jn8eqivzKRGeXBeaqdk57+P23e
 pPSmBeekMJ0LBeQlb7dJRQffd/qNpEWLmYN4JaqO1ji77w4qpErJTQjrChk7QQuxIaiP
 GyZBSn7xAU3D0JkLuOnSZZyj7reX6Q1qjb8jilyFjki1qhu53YPX/HwqCsZtqKhYCT1a
 8oM8SJd2YC3zrAMkQ4Q/1b318gHnwYVZ8McHHxkzgvaLA/rNiZ5KwDUTJnL/vmLb+2Mn
 zPHagD8mOHp2hmTUbnXL8Hy4m1Fg00+cxVpEjrKGsZug0XGmyhKflQzeV1jJsxqYAO0W
 m+nA==
X-Gm-Message-State: ALoCoQnidr13n1uAukocPePSdN2fvRkDr6xjug6/zvIXmjMJRB/0mqAqCMThkblGWx+Nc8XCWkSDWXFkXKlnvdEqOjodPbLB1g==
X-Received: by 10.67.3.170 with SMTP id bx10mr40255838pad.34.1451252934818;
 Sun, 27 Dec 2015 13:48:54 -0800 (PST)
Received: from xeon-e3 (static-50-53-82-155.bvtn.or.frontiernet.net.
 [50.53.82.155])
 by smtp.gmail.com with ESMTPSA id 3sm27469640pfn.61.2015.12.27.13.48.54
 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);
 Sun, 27 Dec 2015 13:48:54 -0800 (PST)
Date: Sun, 27 Dec 2015 13:49:04 -0800
From: Stephen Hemminger <stephen@networkplumber.org>
To: Zhihong Wang <zhihong.wang@intel.com>
Message-ID: <20151227134904.3fd9ecce@xeon-e3>
In-Reply-To: <1451011032-83106-3-git-send-email-zhihong.wang@intel.com>
References: <1451011032-83106-1-git-send-email-zhihong.wang@intel.com>
 <1451011032-83106-3-git-send-email-zhihong.wang@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v2 2/3] examples/l2fwd: Handle SIGINT and
 SIGTERM in l2fwd
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Sun, 27 Dec 2015 21:48:55 -0000

On Thu, 24 Dec 2015 21:37:11 -0500
Zhihong Wang <zhihong.wang@intel.com> wrote:

> Handle SIGINT and SIGTERM in l2fwd.
> 
> Signed-off-by: Zhihong Wang <zhihong.wang@intel.com>
> ---
>  examples/l2fwd/main.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 60 insertions(+)
> 
> diff --git a/examples/l2fwd/main.c b/examples/l2fwd/main.c
> index 720fd5a..75899dd 100644
> --- a/examples/l2fwd/main.c
> +++ b/examples/l2fwd/main.c
> @@ -44,6 +44,8 @@
>  #include <ctype.h>
>  #include <errno.h>
>  #include <getopt.h>
> +#include <signal.h>
> +#include <unistd.h>
>  
>  #include <rte_common.h>
>  #include <rte_log.h>
> @@ -69,6 +71,9 @@
>  #include <rte_mempool.h>
>  #include <rte_mbuf.h>
>  
> +static int force_quit = -1;
> +static int signo_quit = -1;

These need to be volatile otherwise you risk compiler optimizing
away your checks.

Also, don't use -1/0 just use 0/1 for boolean or better yet
the definition in <stdbool.h> of bool and true/false.
That way the code can read much nicer.

>  #define RTE_LOGTYPE_L2FWD RTE_LOGTYPE_USER1
>  
>  #define NB_MBUF   8192
> @@ -284,6 +289,8 @@ l2fwd_main_loop(void)
>  	}
>  
>  	while (1) {
> +		if (unlikely(force_quit != 0))
> +			break;

Please maske this a proper while loop instead.

        while (!force_quit) {

>  
>  		cur_tsc = rte_rdtsc();
>  
> @@ -534,6 +541,45 @@ check_all_ports_link_status(uint8_t port_num, uint32_t port_mask)
>  	}
>  }
>  
> +static void
> +stop_ports(void)
> +{
> +	unsigned portid, nb_ports;
> +
> +	nb_ports = rte_eth_dev_count();
> +	for (portid = 0; portid < nb_ports; portid++) {
> +		if ((l2fwd_enabled_port_mask & (1 << portid)) == 0) {
> +			continue;
> +		}

No need for {} here.

> +		printf("Stopping port %d...", portid);
> +		rte_eth_dev_stop(portid);
> +		rte_eth_dev_close(portid);
> +		printf(" Done\n");
> +	}
> +}
> +
> +static void
> +signal_handler(__rte_unused int signum)
> +{
> +	if (signum == SIGINT || signum == SIGTERM) {

signum is used, dont give __rte_unused attribute.

>  
>  	/* launch per-lcore init on every lcore */
> +	force_quit = 0;

What is gained by having tri-value here. Just initialize it as false.


>  	rte_eal_mp_remote_launch(l2fwd_launch_one_lcore, NULL, CALL_MASTER);
>  	RTE_LCORE_FOREACH_SLAVE(lcore_id) {
>  		if (rte_eal_wait_lcore(lcore_id) < 0)
>  			return -1;
>  	}
>  
> +	printf("Stopping forwarding... Done\n");
> +	/* stop ports */
> +	stop_ports();
> +	printf("Bye...\n");
> +	/* inform if there's a caller */
> +	if (force_quit != 0) {
> +		signal(signo_quit, SIG_DFL);
> +		kill(getpid(), signo_quit);

The kill should not be needed.

It would be good if examples cleaned up allocations, that way they
could be used with valgrind for validation of drivers, etc.