[dpdk-dev] [PATCH] mk: add support for gdb debug info generation

[dpdk-dev] [PATCH] mk: add support for gdb debug info generation

[Cyril Chemparathy]

From: Cyril Chemparathy <cchemparathy@tilera.com>

It is often useful to build with debug enabled, we add a config
(CONFIG_RTE_TOOLCHAIN_DEBUG) to do so.

Note: This patch does not include corresponding changes for ICC.  The
author pleads abject ignorance in this regard, and welcomes
recommendations. :-)

Change-Id: I499e591e1b7d71df751fd40d1fdcbe6975eeeb27
Signed-off-by: Cyril Chemparathy <cchemparathy@ezchip.com>
---
 mk/toolchain/gcc/rte.vars.mk | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/mk/toolchain/gcc/rte.vars.mk b/mk/toolchain/gcc/rte.vars.mk
index 0f51c66..22c4c1f 100644
--- a/mk/toolchain/gcc/rte.vars.mk
+++ b/mk/toolchain/gcc/rte.vars.mk
@@ -71,6 +71,11 @@ ifeq (,$(findstring -O0,$(EXTRA_CFLAGS)))
 endif
 endif
 
+ifeq ($(CONFIG_RTE_TOOLCHAIN_DEBUG),y)
+TOOLCHAIN_CFLAGS += -g -ggdb
+TOOLCHAIN_LDFLAGS += -g -ggdb
+endif
+
 WERROR_FLAGS := -W -Wall -Werror -Wstrict-prototypes -Wmissing-prototypes
 WERROR_FLAGS += -Wmissing-declarations -Wold-style-definition -Wpointer-arith
 WERROR_FLAGS += -Wcast-align -Wnested-externs -Wcast-qual
-- 
2.1.2

[dpdk-dev] [PATCH] examples/l2fwd: Add forward count limit.

[Cyril Chemparathy]

This commit adds a forward count argument, which is used to terminate
the test once a certain number of packets have been forwarded.

Change-Id: Ia3e7ff5d41c3e947509b0653d53271b882fc04de
Signed-off-by: Cyril Chemparathy <cchemparathy@ezchip.com>
---
 examples/l2fwd/main.c | 46 ++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 44 insertions(+), 2 deletions(-)

diff --git a/examples/l2fwd/main.c b/examples/l2fwd/main.c
index 720fd5a..20c1dd2 100644
--- a/examples/l2fwd/main.c
+++ b/examples/l2fwd/main.c
@@ -137,7 +137,9 @@ 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 MAX_TIMER_PERIOD 86400 /* 1 day max */
+#define MAX_FORWARD_COUNT 1000000000 /* a cool billion :) */
 static int64_t timer_period = 10 * TIMER_MILLISECOND * 1000; /* default period is 10 seconds */
+static int64_t forward_count;
 
 /* Print out statistics on packets dropped */
 static void
@@ -262,6 +264,7 @@ l2fwd_main_loop(void)
 	unsigned i, j, portid, nb_rx;
 	struct lcore_queue_conf *qconf;
 	const uint64_t drain_tsc = (rte_get_tsc_hz() + US_PER_S - 1) / US_PER_S * BURST_TX_DRAIN_US;
+	uint64_t total_packets_tx;
 
 	prev_tsc = 0;
 	timer_tsc = 0;
@@ -321,6 +324,17 @@ l2fwd_main_loop(void)
 			}
 
 			prev_tsc = cur_tsc;
+
+			if (forward_count > 0) {
+				total_packets_tx = 0;
+				for (portid = 0; portid < RTE_MAX_ETHPORTS; portid++) {
+					/* skip disabled ports */
+					if ((l2fwd_enabled_port_mask & (1 << portid)) != 0)
+						total_packets_tx += port_statistics[portid].tx;
+				}
+				if (total_packets_tx >= forward_count)
+					break;
+			}
 		}
 
 		/*
@@ -357,7 +371,8 @@ 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"
-		   "  -T PERIOD: statistics will be refreshed each PERIOD seconds (0 to disable, 10 default, 86400 maximum)\n",
+	       "  -T PERIOD: statistics will be refreshed each PERIOD seconds (0 to disable, 10 default, 86400 maximum)\n"
+	       "  -C COUNT: exit after transmitting COUNT packets\n",
 	       prgname);
 }
 
@@ -412,6 +427,22 @@ l2fwd_parse_timer_period(const char *q_arg)
 	return n;
 }
 
+static int
+l2fwd_parse_forward_count(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_FORWARD_COUNT)
+		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 +457,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:C:",
 				  lgopts, &option_index)) != EOF) {
 
 		switch (opt) {
@@ -460,6 +491,16 @@ l2fwd_parse_args(int argc, char **argv)
 			}
 			break;
 
+		/* forward count */
+		case 'C':
+			forward_count = l2fwd_parse_forward_count(optarg);
+			if (forward_count < 0) {
+				printf("invalid forward count\n");
+				l2fwd_usage(prgname);
+				return -1;
+			}
+			break;
+
 		/* long options */
 		case 0:
 			l2fwd_usage(prgname);
@@ -702,6 +743,7 @@ main(int argc, char **argv)
 		if (rte_eal_wait_lcore(lcore_id) < 0)
 			return -1;
 	}
+	print_stats();
 
 	return 0;
 }
-- 
2.1.2

Re: [dpdk-dev] [PATCH] examples/l2fwd: Add forward count limit.

[Thomas Monjalon]

NACK

2015-06-19 14:29, Cyril Chemparathy:
> This commit adds a forward count argument, which is used to terminate
> the test once a certain number of packets have been forwarded.

It is not a test but an example application.
This patch doesn't demonstrate a DPDK feature.
If you need to improve a test tool, please prefer testpmd.

Re: [dpdk-dev] [PATCH] mk: add support for gdb debug info generation

[Gonzalez Monroy, Sergio]

On 19/06/2015 22:29, Cyril Chemparathy wrote:
> From: Cyril Chemparathy <cchemparathy@tilera.com>
>
> It is often useful to build with debug enabled, we add a config
> (CONFIG_RTE_TOOLCHAIN_DEBUG) to do so.
>
> Note: This patch does not include corresponding changes for ICC.  The
> author pleads abject ignorance in this regard, and welcomes
> recommendations. :-)
>
> Change-Id: I499e591e1b7d71df751fd40d1fdcbe6975eeeb27
> Signed-off-by: Cyril Chemparathy <cchemparathy@ezchip.com>
> ---
>   mk/toolchain/gcc/rte.vars.mk | 5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/mk/toolchain/gcc/rte.vars.mk b/mk/toolchain/gcc/rte.vars.mk
> index 0f51c66..22c4c1f 100644
> --- a/mk/toolchain/gcc/rte.vars.mk
> +++ b/mk/toolchain/gcc/rte.vars.mk
> @@ -71,6 +71,11 @@ ifeq (,$(findstring -O0,$(EXTRA_CFLAGS)))
>   endif
>   endif
>   
> +ifeq ($(CONFIG_RTE_TOOLCHAIN_DEBUG),y)
> +TOOLCHAIN_CFLAGS += -g -ggdb
> +TOOLCHAIN_LDFLAGS += -g -ggdb
> +endif
> +
>   WERROR_FLAGS := -W -Wall -Werror -Wstrict-prototypes -Wmissing-prototypes
>   WERROR_FLAGS += -Wmissing-declarations -Wold-style-definition -Wpointer-arith
>   WERROR_FLAGS += -Wcast-align -Wnested-externs -Wcast-qual
I don't think you need to modify the makefiles and introduce a new 
compile time option for this.
The same result can be easily achieved by setting EXTRA_CFLAGS in the 
command line. ie:
     $ make install T=x86_64-native-linuxapp-gcc EXTRA_CFLAGS='-g -ggdb'

Sergio

Re: [dpdk-dev] [PATCH] mk: add support for gdb debug info generation

[Simon Kågström]

On 2015-06-22 09:44, Gonzalez Monroy, Sergio wrote:
> On 19/06/2015 22:29, Cyril Chemparathy wrote:
>> From: Cyril Chemparathy <cchemparathy@tilera.com>
>>
>> It is often useful to build with debug enabled, we add a config
>> (CONFIG_RTE_TOOLCHAIN_DEBUG) to do so.
>>
>>   +ifeq ($(CONFIG_RTE_TOOLCHAIN_DEBUG),y)
>> +TOOLCHAIN_CFLAGS += -g -ggdb
>> +TOOLCHAIN_LDFLAGS += -g -ggdb
>> +endif

> I don't think you need to modify the makefiles and introduce a new
> compile time option for this.
> The same result can be easily achieved by setting EXTRA_CFLAGS in the
> command line. ie:
>     $ make install T=x86_64-native-linuxapp-gcc EXTRA_CFLAGS='-g -ggdb'

Why isn't -g standard though? The binaries should/will anyhow be
stripped when used for production - but debugging information should be
useful when analysing crashes.

// Simon

Re: [dpdk-dev] [PATCH] mk: add support for gdb debug info generation

[Gonzalez Monroy, Sergio]

On 22/06/2015 08:56, Simon Kågström wrote:
> On 2015-06-22 09:44, Gonzalez Monroy, Sergio wrote:
>> On 19/06/2015 22:29, Cyril Chemparathy wrote:
>>> From: Cyril Chemparathy <cchemparathy@tilera.com>
>>>
>>> It is often useful to build with debug enabled, we add a config
>>> (CONFIG_RTE_TOOLCHAIN_DEBUG) to do so.
>>>
>>>    +ifeq ($(CONFIG_RTE_TOOLCHAIN_DEBUG),y)
>>> +TOOLCHAIN_CFLAGS += -g -ggdb
>>> +TOOLCHAIN_LDFLAGS += -g -ggdb
>>> +endif
>> I don't think you need to modify the makefiles and introduce a new
>> compile time option for this.
>> The same result can be easily achieved by setting EXTRA_CFLAGS in the
>> command line. ie:
>>      $ make install T=x86_64-native-linuxapp-gcc EXTRA_CFLAGS='-g -ggdb'
> Why isn't -g standard though? The binaries should/will anyhow be
> stripped when used for production - but debugging information should be
> useful when analysing crashes.
I guess you could argue that, to always build with debug info then strip 
it down.
You would need another flag to strip debug info for production, or leave 
it for debugging.

In my opinion is not worth it, but it you feel strongly about it you can 
submit patches and
let the community decide.

Sergio
> // Simon
>

Re: [dpdk-dev] [PATCH] mk: add support for gdb debug info generation

[Thomas Monjalon]

2015-06-23 08:39, Gonzalez Monroy, Sergio:
> On 22/06/2015 08:56, Simon Kågström wrote:
> > On 2015-06-22 09:44, Gonzalez Monroy, Sergio wrote:
> >> On 19/06/2015 22:29, Cyril Chemparathy wrote:
> >>> From: Cyril Chemparathy <cchemparathy@tilera.com>
> >>>
> >>> It is often useful to build with debug enabled, we add a config
> >>> (CONFIG_RTE_TOOLCHAIN_DEBUG) to do so.
> >>>
> >>>    +ifeq ($(CONFIG_RTE_TOOLCHAIN_DEBUG),y)
> >>> +TOOLCHAIN_CFLAGS += -g -ggdb
> >>> +TOOLCHAIN_LDFLAGS += -g -ggdb
> >>> +endif
> >> I don't think you need to modify the makefiles and introduce a new
> >> compile time option for this.
> >> The same result can be easily achieved by setting EXTRA_CFLAGS in the
> >> command line. ie:
> >>      $ make install T=x86_64-native-linuxapp-gcc EXTRA_CFLAGS='-g -ggdb'
> > Why isn't -g standard though? The binaries should/will anyhow be
> > stripped when used for production - but debugging information should be
> > useful when analysing crashes.
> 
> I guess you could argue that, to always build with debug info then strip 
> it down.
> You would need another flag to strip debug info for production, or leave 
> it for debugging.
> 
> In my opinion is not worth it, but it you feel strongly about it you can 
> submit patches and
> let the community decide.

I think stripping is a packaging responsibility.
It would be a good idea to always provide debugging symbols.

Re: [dpdk-dev] [PATCH] mk: add support for gdb debug info generation

[Simon Kågström]

On 2015-06-23 09:47, Thomas Monjalon wrote:
> 2015-06-23 08:39, Gonzalez Monroy, Sergio:
>> I guess you could argue that, to always build with debug info then strip 
>> it down.
>> You would need another flag to strip debug info for production, or leave 
>> it for debugging.
>>
>> In my opinion is not worth it, but it you feel strongly about it you can 
>> submit patches and
>> let the community decide.
> 
> I think stripping is a packaging responsibility.
> It would be a good idea to always provide debugging symbols.

Yes, I think this would be the best way too, and should be pretty much
standard procedure.

DPDK is anyhow just a library - stripping should be up to the
application / packaging to do.

// Simon

Re: [dpdk-dev] [PATCH] mk: add support for gdb debug info generation

[Cyril Chemparathy]

On Mon, 22 Jun 2015 08:44:41 +0100
"Gonzalez Monroy, Sergio" <sergio.gonzalez.monroy@intel.com> wrote:

> I don't think you need to modify the makefiles and introduce a new 
> compile time option for this.
> The same result can be easily achieved by setting EXTRA_CFLAGS in the 
> command line. ie:
>      $ make install T=x86_64-native-linuxapp-gcc EXTRA_CFLAGS='-g
> -ggdb'

Fair enough.  Please ignore this patch then.  Thanks!