[dpdk-dev,v2,2/3] l2fwd: keep alive sample application

  Modification of l2fwd to demonstrate keep-alive functionality.

Signed-off-by: Remy Horton <remy.horton@intel.com>
---
 examples/l2fwd/Makefile |   2 +-
 examples/l2fwd/main.c   | 125 ++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 118 insertions(+), 9 deletions(-)
  

> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Remy Horton
> Sent: Wednesday, September 30, 2015 10:05 AM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH v2 2/3] l2fwd: keep alive sample application
> 
> Modification of l2fwd to demonstrate keep-alive functionality.
> 
> Signed-off-by: Remy Horton <remy.horton@intel.com>
> ---
>  examples/l2fwd/Makefile |   2 +-
>  examples/l2fwd/main.c   | 125
> ++++++++++++++++++++++++++++++++++++++++++++----
>  2 files changed, 118 insertions(+), 9 deletions(-)
> 
> diff --git a/examples/l2fwd/Makefile b/examples/l2fwd/Makefile index
> 78feeeb..8647174 100644
> --- a/examples/l2fwd/Makefile
> +++ b/examples/l2fwd/Makefile
> @@ -39,7 +39,7 @@ RTE_TARGET ?= x86_64-native-linuxapp-gcc  include
> $(RTE_SDK)/mk/rte.vars.mk
> 
>  # binary name
> -APP = l2fwd
> +APP = l2fwd-keepalive
> 
Hi Remy,
Looks great overall, just a couple of comments
For backward compatibility the rename could be a nightmare? What do you think?

>  # all source are stored in SRCS-y
>  SRCS-y := main.c
> diff --git a/examples/l2fwd/main.c b/examples/l2fwd/main.c index
> 720fd5a..131e5a2 100644
> --- a/examples/l2fwd/main.c
> +++ b/examples/l2fwd/main.c
> @@ -1,7 +1,7 @@
>  /*-
>   *   BSD LICENSE
>   *
> - *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
> + *   Copyright(c) 2010-2015 Intel Corporation. All rights reserved.
>   *   All rights reserved.
>   *
>   *   Redistribution and use in source and binary forms, with or without
> @@ -68,6 +68,8 @@
>  #include <rte_ring.h>
>  #include <rte_mempool.h>
>  #include <rte_mbuf.h>
> +#include <rte_timer.h>
> +#include <rte_keepalive.h>
> 
>  #define RTE_LOGTYPE_L2FWD RTE_LOGTYPE_USER1
> 
> @@ -135,13 +137,18 @@ struct l2fwd_port_statistics {  struct
> l2fwd_port_statistics port_statistics[RTE_MAX_ETHPORTS];
> 
>  /* A tsc-based timer responsible for triggering statistics printout */ -#define
> TIMER_MILLISECOND 2000000ULL /* around 1ms at 2 Ghz */
> +#define TIMER_MILLISECOND 1
>  #define MAX_TIMER_PERIOD 86400 /* 1 day max */  static int64_t
> timer_period = 10 * TIMER_MILLISECOND * 1000; /* default period is 10
> seconds */
> +static int64_t check_period = 5; /* default check cycle is 5ms */
> +
> +/* Keepalive structure */
> +struct rte_keepalive * rte_global_keepalive_info;
> 
>  /* Print out statistics on packets dropped */  static void
> -print_stats(void)
> +print_stats(__attribute__((unused)) struct rte_timer *ptr_timer,
> +	__attribute__((unused)) void *ptr_data)
>  {
>  	uint64_t total_packets_dropped, total_packets_tx, total_packets_rx;
>  	unsigned portid;
> @@ -283,11 +290,23 @@ l2fwd_main_loop(void)
>  			portid);
>  	}
> 
> +	uint64_t tsc_initial = rte_rdtsc();
> +	uint64_t tsc_lifetime = (rand()&0x07) * rte_get_tsc_hz();
> +
>  	while (1) {
> +		/* Keepalive heartbeat */
> +		rte_keepalive_mark_alive(rte_global_keepalive_info);
> 
>  		cur_tsc = rte_rdtsc();
> 
>  		/*
> +		 * Die randomly within 7 secs for demo purposes if
> +		 * keepalive enables
> +		 */

Typo: If keepalive is enabled

> +		if (check_period > 0 && cur_tsc - tsc_initial > tsc_lifetime)
> +			break;
> +
> +		/*
>  		 * TX burst queue drain
>  		 */
>  		diff_tsc = cur_tsc - prev_tsc;
> @@ -313,7 +332,7 @@ l2fwd_main_loop(void)
> 
>  					/* do this only on master core */
>  					if (lcore_id ==
> rte_get_master_lcore()) {
> -						print_stats();
> +						print_stats(NULL, NULL);
>  						/* reset the timer */
>  						timer_tsc = 0;
>  					}
> @@ -357,6 +376,7 @@ l2fwd_usage(const char *prgname)
>  	printf("%s [EAL options] -- -p PORTMASK [-q NQ]\n"
>  	       "  -p PORTMASK: hexadecimal bitmask of ports to configure\n"
>  	       "  -q NQ: number of queue (=ports) per lcore (default is 1)\n"
> +	       "  -K PERIOD: Keepalive check period (5 default; 86400 max)\n"
>  		   "  -T PERIOD: statistics will be refreshed each PERIOD
> seconds (0 to disable, 10 default, 86400 maximum)\n",
>  	       prgname);
>  }
> @@ -412,6 +432,22 @@ l2fwd_parse_timer_period(const char *q_arg)
>  	return n;
>  }
> 
> +static int
> +l2fwd_parse_check_period(const char *q_arg) {
> +	char *end = NULL;
> +	int n;
> +
> +	/* parse number string */
> +	n = strtol(q_arg, &end, 10);
> +	if ((q_arg[0] == '\0') || (end == NULL) || (*end != '\0'))
> +		return -1;
> +	if (n >= MAX_TIMER_PERIOD)
> +		return -1;
> +
> +	return n;
> +}
> +
>  /* Parse the argument given in the command line of the application */  static
> int  l2fwd_parse_args(int argc, char **argv) @@ -426,7 +462,7 @@
> l2fwd_parse_args(int argc, char **argv)
> 
>  	argvopt = argv;
> 
> -	while ((opt = getopt_long(argc, argvopt, "p:q:T:",
> +	while ((opt = getopt_long(argc, argvopt, "p:q:T:K:",
>  				  lgopts, &option_index)) != EOF) {
> 
>  		switch (opt) {
> @@ -460,6 +496,16 @@ l2fwd_parse_args(int argc, char **argv)
>  			}
>  			break;
> 
> +		/* Check period */
> +		case 'K':
> +			check_period = l2fwd_parse_check_period(optarg);
> +			if (check_period < 0) {
> +				printf("invalid check period\n");
> +				l2fwd_usage(prgname);
> +				return -1;
> +			}
> +			break;
> +
>  		/* long options */
>  		case 0:
>  			l2fwd_usage(prgname);
> @@ -534,6 +580,18 @@ check_all_ports_link_status(uint8_t port_num,
> uint32_t port_mask)
>  	}
>  }
> 
> +static void
> +dead_core(__attribute__((unused)) void *ptr_data, const int id_core) {
> +	printf("Dead core %i - restarting..\n", id_core);
> +	if (rte_eal_get_lcore_state(id_core) == FINISHED) {
> +		rte_eal_wait_lcore(id_core);
> +		rte_eal_remote_launch(l2fwd_launch_one_lcore, NULL,
> id_core);
> +	} else {
> +		printf("..false positive!\n");
> +	}
> +}
> +
>  int
>  main(int argc, char **argv)
>  {
> @@ -600,7 +658,7 @@ main(int argc, char **argv)
>  		l2fwd_dst_ports[last_port] = last_port;
>  	}
> 
> -	rx_lcore_id = 0;
> +	rx_lcore_id = 1;
>  	qconf = NULL;
> 
>  	/* Initialize the port/queue configuration of each logical core */ @@ -
> 696,8 +754,59 @@ main(int argc, char **argv)
> 
>  	check_all_ports_link_status(nb_ports, l2fwd_enabled_port_mask);
> 
> -	/* launch per-lcore init on every lcore */
> -	rte_eal_mp_remote_launch(l2fwd_launch_one_lcore, NULL,
> CALL_MASTER);
> +	struct rte_timer hb_timer, stats_timer;
> +
> +	rte_timer_subsystem_init();
> +	rte_timer_init(&stats_timer);
> +
> +	if (check_period > 0) {
> +		rte_global_keepalive_info =
> rte_keepalive_create(&dead_core, NULL);
> +		if( rte_global_keepalive_info == NULL)
> +			rte_exit(EXIT_FAILURE, "init_keep_alive() failed");
> +		rte_timer_init(&hb_timer);
> +		if (rte_timer_reset(&hb_timer,
> +				(check_period * rte_get_timer_hz()) / 1000,
> +				PERIODICAL,
> +				rte_lcore_id(),
> +				(void(*)(struct rte_timer*, void*))
> +				&rte_keepalive_dispatch_pings,
> +				rte_global_keepalive_info
> +				) != 0 )
> +			rte_exit(EXIT_FAILURE, "Keepalive setup failure.\n");
> +	}
> +	if (timer_period > 0) {
> +		if (rte_timer_reset(&stats_timer,
> +				(timer_period * rte_get_timer_hz()) / 1000,
> +				PERIODICAL,
> +				rte_lcore_id(),
> +				&print_stats, NULL
> +				) != 0 )
> +			rte_exit(EXIT_FAILURE, "Stats setup failure.\n");
> +	}
> +	/* launch per-lcore init on every slave lcore */
> +	RTE_LCORE_FOREACH_SLAVE(lcore_id) {
> +		struct lcore_queue_conf *qconf =
> &lcore_queue_conf[lcore_id];
> +
> +		if (qconf->n_rx_port == 0)
> +			RTE_LOG(INFO, L2FWD,
> +				"lcore %u has nothing to do\n",
> +				lcore_id
> +				);
> +		else {
> +			rte_eal_remote_launch(
> +				l2fwd_launch_one_lcore,
> +				NULL,
> +				lcore_id
> +				);
> +
> 	rte_keepalive_register_core(rte_global_keepalive_info,
> +				lcore_id);
> +		}
> +	}
> +	for (;;) {
> +		rte_timer_manage();
> +		rte_delay_ms(5);
> +		}
> +
>  	RTE_LCORE_FOREACH_SLAVE(lcore_id) {
>  		if (rte_eal_wait_lcore(lcore_id) < 0)
>  			return -1;
> --
> 1.9.3
  

On 23/10/2015 15:23, Tahhan, Maryam wrote:

>>   # binary name
>> -APP = l2fwd
>> +APP = l2fwd-keepalive
> Hi Remy,
> Looks great overall, just a couple of comments
> For backward compatibility the rename could be a nightmare? What do you think?

True. Undecided whether to change the Makefile(s) so that both keepalive 
and non-keepalive l2fwd are built, or to simply clone all of l2fwd..


>> +		 * Die randomly within 7 secs for demo purposes if
>> +		 * keepalive enables
>> +		 */
>
> Typo: If keepalive is enabled

Fixed.. :)
  

On Tue, Oct 27, 2015 at 08:48:41AM +0000, Remy Horton wrote:
> 
> 
> On 23/10/2015 15:23, Tahhan, Maryam wrote:
> 
> >>  # binary name
> >>-APP = l2fwd
> >>+APP = l2fwd-keepalive
> >Hi Remy,
> >Looks great overall, just a couple of comments
> >For backward compatibility the rename could be a nightmare? What do you think?
> 
> True. Undecided whether to change the Makefile(s) so that both keepalive and
> non-keepalive l2fwd are built, or to simply clone all of l2fwd..
> 

My opinion is that l2fwd should be kept as simple as possible, so I'd suggest
cloning to provide a separate keepalive example.

/Bruce