[dpdk-dev] [PKTGEN] fixing weird termio issues that complicate debugging

[Panu Matilainen]

On 01/20/2016 08:32 AM, Matthew Hall wrote:
> Hello,
>
> Since the pktgen code is reindented I am finding time to read through it
> and experiment and see if I can get it working.
>
> I have issues with the init process of pktgen. It is difficult to debug
> it because the init code does a lot of very scary stuff to the terminal
> control / TTY device at inconvenient times in an inconvenient order, and
> in the process damages the debug output and damages the screen of your
> GDB without doing weird things to run GDB on a different TTY.
>
> Of course I am willing to contribute patches and not just complain, but
> first I need some help to follow what is going on.
>
> Here is the problematic call-flow with some explanation what went wrong
> trying it on some community machines outside of its original environment:
>
> 1) it calls printf("\n%s %s\n", wr_copyright_msg(), wr_powered_by());
> which dumps tons of weird boilerplate of licenses, copyrights, code
> creator, etc.
>
> It is open source and everybody that matters already knows who coded it,
> so is this stuff really that important? This gets in the way when you
> are trying to work on it and I just have to comment it out.
>
> 2) it calls wr_scrn_setw and tinkers with the windows size very early in
> the init which can make your terminal weird
>
> 3) it calls rte_eal_init which produces a lot of nice debug output,
> which is fine
>
> 4) it calls pktgen_init_screen, which calls wr_scrn_init, which calls
> wr_scrn_erase which destroys the valuable debug output just created in
> (c) which is a bad thing
>
> 5) it calls wr_print_copyright and dumps more boilerplate I am not sure
> is needed
>
> 6) it logs some helpful messages about the port / descriptor settings
> which is fine
>
> 7) it calls the pktgen_config_ports function which can crash in ways you
> need the destroyed debug output to fix.
>
> For example in my case that function crashes here:
>
>          if (pktgen.nb_ports == 0)
>                  pktgen_log_panic("*** Did not find any ports to use ***");
>
> 8) Later it makes a logo and a splash screen (wr_log, wr_splash_screen).
> Is this stuff really needed? This is a ton of output for just starting
> up some test program.
>
> To fix this debug problem I propose some changes which I am happy to
> help develop:
>
> 1) decide what of this output we really need here and greatly simplify
> how much gets printed out
>
> 2) move wr_scrn_setw right before pktgen_init_screen and after
> rte_eal_init to prevent damaging that output
>
> 3) consider how wr_scrn_init is called in pktgen_init_screen, because it
> calls wr_scrn_erase which damages output
>
> 4) I think that pktgen_config_ports should be called before all this
> weird screen init stuff, so that if it fails you can actually see what
> happened there.
>
> One other random topic... on the long lines of code it looks like there
> are some gigantic tab-indents pushing things off to the right still. One
> example, maybe there are others or another setting which is needed to
> fix all of these:
>
>                  info->seq_pkt = (pkt_seq_t *)rte_zmalloc_socket(buff,
> (sizeof(pkt_seq_t) * NUM_TOTAL_PKTS),
>
>                                           RTE_CACHE_LINE_SIZE,
> rte_socket_id());
>
> Thoughts?

Just that I'm in violent agreement about the splash screens and all.
Unfortunately the license explicitly forbids removal of the copyright 
messages (http://dpdk.org/browse/apps/pktgen-dpdk/tree/LICENSE#n18):

--
# 4) The screens displayed by the application must contain the copyright 
notice as defined
# above and can not be removed without specific prior written permission.
--

Keith, any chance you could work out the details with Wind River to get 
the ridiculous startup messages straightened out? I dont think anybody 
would mind a line or two "copyright by..." kind of printf() in there if 
that's what it takes, but the current screen after screen after screen 
copyrights and advertisements are obnoxious to the point of driving 
potential users away.

	- Panu -

> Matthew Hall