On Thu, 24 Dec 2015 21:37:11 -0500
Zhihong Wang <zhihong.wang at intel.com> wrote:
> Handle SIGINT and SIGTERM in l2fwd.
>
> Signed-off-by: Zhihong Wang <zhihong.wang at 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.
