DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [RFC] New CLI for DPDK
@ 2017-03-10 15:25 Wiles, Keith
  2017-03-10 16:06 ` Stephen Hemminger
  2017-03-23 16:13 ` Wiles, Keith
  0 siblings, 2 replies; 11+ messages in thread
From: Wiles, Keith @ 2017-03-10 15:25 UTC (permalink / raw)
  To: DPDK

I would like to request for comments on a new CLI design and get any feedback. I have attached the cli.rst text, which is still a work in progress for you review.

I have also ported the CLI to a version of Pktgen on the ‘dev’ branch of the repo in DPDK.org.

http://dpdk.org/browse/apps/pktgen-dpdk/refs/?h=dev

I would like to submit the CLI library to be used in DPDK, if that seems reasonable to everyone. I need more testing of the API and Pktgen, but I feel it has a simpler design, easier to understand and hopefully make it easier for developers to add commands.

As an example I quickly converted over testpmd from CMDLINE to CLI (I just add a -I option to select CLI instead) and reduced the test-pmd/cmdline.c file from 12.6K lines to about 4.5K lines. I did not fully test the code, but the ones I did test seem to work.

I do not expect DPDK to convert to the new CLI only if it makes sense and I am not suggesting to replace CMDLINE library.

If you play with the new CLI in pktgen and see any problems or want to suggest new features or changes please let me know.

Comments on the cli.rst text is also welcome, but the cli.rst is not complete. I think this file needs to be broken into two one to explain the example and another to explain CLI internals.

-------

..  BSD LICENSE
   Copyright(c) 2017 Intel Corporation. All rights reserved.
   All rights reserved.

   Redistribution and use in source and binary forms, with or without
   modification, are permitted provided that the following conditions
   are met:

   * Redistributions of source code must retain the above copyright
   notice, this list of conditions and the following disclaimer.
   * Redistributions in binary form must reproduce the above copyright
   notice, this list of conditions and the following disclaimer in
   the documentation and/or other materials provided with the
   distribution.
   * Neither the name of Intel Corporation nor the names of its
   contributors may be used to endorse or promote products derived
   from this software without specific prior written permission.

   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

CLI Sample Application
===============================

CLI stands for "Command Line Interface".

This chapter describes the CLI sample application that is part of the
Data Plane Development Kit (DPDK). The CLI is a workalike replacement for
cmdline library in DPDK and has a simpler interface and programming model.

The primary goal of CLI is to allow the developer to create commands quickly
and with very little compile or runtime configuration. Using standard Unix*
like constructs which are very familar to the developer. Allowing the developer
to construct a set of commands for development or deployment of the application.

The CLI design uses a directory like design instead of a single level command
line interface. Allowing the developer to use a directory style solution to
controlling a DPDK application. The directory style design is nothing new, but
it does have some advantages.

Overview
--------

The CLI sample application is a simple application that demonstrates the
use of the command line interface in the DPDK. This application is a
readline-like interface that can be used to control a DPDK application.

One of the big advantages of CLI over Cmdline is it is dynamic, which means
nodes or items can be added and removed on the fly. Which allows adding
new directories, file or commands as needed or removing these items at runtime.
The CLI has no global modifiable variable as the one global pointer is a
thread based variable. Which allows the developer to have multiple CLI
commands per thread if needed.

Another big advantage is the calling of the backend function to support a
command is very familar to developers as it is basically just a argc/argv
style command and the developer gets the complete command line.

One other big advantage is the use of MAP structures, to help identify commands
quickly plus allowing the developer to define new versions of commands and
be able to identify these new versions using a simple identifier value. Look at
the sample application to see a simple usage.

Another advantage of CLI is how simple it is to add new directroies, files and
commands for user development. The basic concept is for the developer to use
standard Unix like designs. To add a command a developer needs to add an entry
to the cli_tree_t structure and create a function using the following prototype:

	.. code-block:: console

	int user_cmd(int argc, char **argv);

The argc/argv is exactly like the standard usage in a Unix* system, which allows
for using getopt() and other standard functions. The Cmdline structures and
text conversions were defined at compile time in most cases. In CLI the routine
is passed the argc/argv information to convert these options as needed. The cli
variable being a thread Local Storage (TLS) all user routines a CLI routine only
need to access the thread variable to eliminate needing a global variable to
reference the specific CLI instance and passing the value in the API.

The user can also set environment variables using the **env** command. These
variables are also parsed in the command line a direct substitution is done.

The CLI system also has support for simple files along with alias like commands.
These alias commands are fixed strings which are executed instead of a function
provided by the developer. If the user has more arguments these are appended
to the alias string and processed as if typed on the command line.

.. note::

   The CLI library was designed to be used in production code and the Cmdline
   was not validated to the same standard as other DPDK libraries. The goal
   is to provide a production CLI design.

The CLI sample application supports some of the features of the Cmdline
library such as, completion, cut/paste and some other special bindings that
make configuration and debug faster and easier.

The CLI desin uses some very simple VT100 control strings for displaying data
and accepting input from the user. Some of the control strings are used to
clear the screen or line and position the cursor on a VT100 compatible terminal.
The CLI screen code also supports basic color and many other VT100 commands.

The application also shows how the CLI application can be extended to handle
a list of commands and user input.

The example presents a simple command prompt "DPKD-cli:/>" similar to a Unix*
shell command along with a directory like file system.

Some of the basic commands contained under /sbin directory are:

*   ls: list the current or provided directory files/commands.

*   cd: Change directory command.

*   pwd: print out the current working directory.

*   history: List the current command line history if enabled.

*   more: A simple command to page contents of files.

*   help: display a the help screen.

*   quit: exit the CLI application, also **Ctrl-x** will exit as well.

*   mkdir: add a directory to the current directory.

*   delay: wait for a given number of microseconds.

*   sleep: wait for a given number of seconds.

*   rm: remove a directory, file or command. Removing a file will delete the data.

*   cls: clear the screen and redisplay the prompt.

*   version: Display the current DPDK version being used.

*   path: display the current search path for executable commands.

*   cmap: Display the current system core and socket information.

*   hugepages: Display the current hugepage information.

*   sizes: a collection system structure and buffer sizes for debugging.

*   copyright: a file containing DPDK copyright information.

*   env: a command show/set/modify the environment variables.

Some example commands under /bin directory are:

*   ll: an alias command to display long ls listing **ls -l**

*   h: alias command for **history**

*   hello: a simple Hello World! command.

*   show: has a number of commands using the map feature.

Under the /data directory is:

*   pci: a simple example file for displaying the **lspci** command in CLI.

.. note::

   To terminate the application, use **Ctrl-x** or the command **quit**.

Auto completion
---------------

CLI does support auto completion at the file or directory level, meaning the
arguments to commands are not expanded as was done in Cmdline code. The CLI
auto completion works similar to the standard Unix* system by expanding
commands and directory paths. In normal Unix* like commands the user needs to
execute the command asking for the help information and CLI uses this method.

Special command features
------------------------

Using the '!' followed by a number from the history list of commands you can
execute that command again. Using the UP/Down arrows the user can quickly find
and execute or modify a previous command in history.

The user can also execute host level commands if enabled using the '@' prefix
to a command line e.g. @ls or @lspci or ... line is passed to popen or system
function to be executed and the output displayed on the console if any output.
To disable set CONFIG_RTE_CLI_HOST_COMMANDS=n in configuration file.

Compiling the Application
-------------------------

#.  Go to example directory:

   .. code-block:: console

       export RTE_SDK=/path/to/rte_sdk
       cd ${RTE_SDK}/examples/cli

#.  Set the target (a default target is used if not specified). For example:

   .. code-block:: console

       export RTE_TARGET=x86_64-native-linuxapp-gcc

   Refer to the *DPDK Getting Started Guide* for possible RTE_TARGET values.

#.  Build the application:

   .. code-block:: console

       make

Running the Application
-----------------------

To run the application in linuxapp environment, issue the following command:

.. code-block:: console

   $ ./build/cli

.. note::
   The example cli application does not require to be run as superuser
   as it does not startup DPDK by calling rte_eal_init() routine. Which means
   it also does not use DPDK features except for a few routines not requiring
   EAL initialization.

Refer to the *DPDK Getting Started Guide* for general information on running applications
and the Environment Abstraction Layer (EAL) options.

Explanation
-----------

The following sections provide some explanation of the code.

EAL Initialization and cmdline Start
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

The first task is the initialization of the Environment Abstraction Layer (EAL),
if required for the application.

.. code-block:: c

   int
   main(int argc, char **argv)
   {
       if (cli_create_with_tree(init_tree) ==0) {
           cli_start(NULL, 0); /* NULL is some init message done only once */
                               /* 0 means do not use color themes */
           cli_destroy();
       }

The cli_start() function returns when the user types **Ctrl-x** or uses the
quit command in this case, the application exits. The cli_create() call takes
four arguments and each has a default value if not provided. The API used here
is the cli_create_with_tree(), which uses defaults for three of the arguments.

.. code-block:: c

   /**
   * Create the CLI engine
   *
   * @param prompt_func
   *   Function pointer to call for displaying the prompt.
   * @param tree_func
   *   The user supplied function to init the tree or can be NULL. If NULL then
   *   a default tree is initialized with basic commands.
   * @param nb_entries
   *   Total number of commands, files, aliases and directories. If 0 then use
   *   the default number of nodes. If -1 then unlimited number of nodes.
   * @param nb_hist
   *   The number of lines to keep in history. If zero then turn off history.
   *   If the value is CLI_DEFAULT_HISTORY use CLI_DEFAULT_HIST_LINES
   * @return
   *   0 on success or -1
   */
   int cli_create(cli_prompt_t prompt_func, cli_tree_t tree_func,
                       int nb_entries, uint32_t nb_hist);

The cli_create_with_tree() has only one argument which is the structure to use
in order to setup the initial directory structure. Also the wrapper function
int cli_create_with_defaults(void) can be used as well.

Consult the cli.h header file for the default values. Also the alias node is a
special alias file to allow for aliasing a command to another command.

The tree init routine is defined like:

.. code-block:: c

	static struct cli_tree my_tree[] = {
	    c_dir("/data"),
	    c_file("pci", pci_file, "display lspci information"),
	    c_dir("/bin"),
	    c_cmd("hello", hello_cmd, "Hello-World!!"),
	    c_alias("h", "history", "display history commands"),
	    c_alias("ll", "ls -l", "long directory listing alias"),
	    c_end()
	};

	static int
	init_tree(void)
	{
	    /*
	     * Root is created already and using system default cmds and dirs, the
	     * developer is not required to use the system default cmds/dirs.
	     */
	    if (cli_default_tree_init())
	        return -1;

		/* Using NULL here to start at root directory */
	    if (cli_add_tree(NULL, my_tree))
	        return -1;

		return cli_add_bin_path("/bin");
	}


The above structure is used to create the tree structure at initialization
time. The struct cli_tree or cli_tree_t typedef can be used to setup a new
directory tree or agument the default tree.

The elements are using a set of macros c_dir, c_file, c_cmd, c_alias and c_end.
These macros help fill out the cli_tree_t structure for the given type of item.

The developer can create his own tree structure with any commands that are
needed and/or call the cli_default_tree_init() routine to get the default
structure of commands. If the developer does not wish to call the default
CLI routine, then he must call the cli_create_root() function first before
adding other nodes. Other nodes can be added and removed at anytime.

CLI Map command support
~~~~~~~~~~~~~~~~~~~~~~~

The CLI command has two types of support to handle arguments normal argc/argv
and the map system. As shown above the developer creates a directory tree and
attaches a function to a command. The function takes the CLI pointer plus the
argc/argv arguments and the developer can just parse the arguments to decode
the command arguments. Sometimes you have multiple commands or different versions
of a command being handled by a single routine, this is were the map support
comes into play.

The map support defines a set of struct cli_map map[]; to help detect the
correct command from the user. In the list of cli_map structures a single
structure contains two items a developer defined index value and a command
strings. The index value is used on the function to identify the specific type
of command found in the list. The string is a special printf like string to
help identify the command typed by the user. One of the first things todo in
the command routine is to call the cli_mapping() function passing in the CLI
pointer and the argc/argv values.The two method can be used at the same time.

The cli_mapping() command matches up the special format string with the values
in the argc/argv array and returns the developer supplied index value or really
the pointer the struct cli_map instance.

Now the developer can use the cli_map.index value in a switch() statement to
locate the command the user typed or if not found a return of -1.

Example:

.. code-block:: c

	static int
	hello_cmd(int argc, char **argv)
	{
	    int i, opt;

	    optind = 1;
	    while((opt = getopt(argc, argv, "?")) != -1) {
	        switch(opt) {
	            case '?': cli_usage(); return 0;
	            default:
	                break;
	        }
	    }

	    cli_printf("Hello command said: Hello World!! ");
	    for(i = 1; i < argc; i++)
	        cli_printf("%s ", argv[i]);
	    cli_printf("\n");

	    return 0;
	}

	static int
	pci_file(struct cli_node *node, char *buff, int len, uint32_t opt)
	{
		if (is_file_open(opt)) {
			FILE *f;

			if (node->file_data && (node->fflags & CLI_FREE_DATA))
				free(node->file_data);

	        node->file_data = malloc(32 * 1024);
			if (!node->file_data)
				return -1;
	        node->file_size = 32 * 1024;
	        node->fflags = CLI_DATA_RDONLY | CLI_FREE_DATA;

			f = popen("lspci", "r");
			if (!f)
				return -1;

			node->file_size = fread(node->file_data, 1, node->file_size, f);

			pclose(f);
	        return 0;
	    }
	    return cli_file_handler(node, buff, len, opt);
	}

	static struct cli_map show_map[] = {
		{ 10, "show %P" },
		{ 20, "show %P mac %m" },
		{ 30, "show %P vlan %d mac %m" },
		{ 40, "show %P %|vlan|mac" },
		{ -1, NULL }
	};

	static const char *show_help[] = {
		"show <portlist>",
		"show <portlist> mac <ether_addr>",
		"show <portlist> vlan <vlanid> mac <ether_addr>",
		"show <portlist> [vlan|mac]",
		NULL
	};

	CLI_INFO(Show, show_map, show_help);

	static int
	show_cmd(int argc, char **argv)
	{
		struct cli_map *m;
		uint32_t portlist;
		struct ether_addr mac;

		m = cli_mapping(Show_info.map, argc, argv);
		if (!m)
			return -1;

		switch(m->index) {
			case 10:
				rte_parse_portlist(argv[1], &portlist);
				cli_printf("   Show Portlist: %08x\n", portlist);
				break;
			case 20:
				rte_parse_portlist(argv[1], &portlist);
				rte_ether_aton(argv[3], &mac);
				cli_printf("   Show Portlist: %08x, MAC: %02x:%02x:%02x:%02x:%02x:%02x\n",
						   portlist,
						   mac.addr_bytes[0],
						   mac.addr_bytes[1],
						   mac.addr_bytes[2],
						   mac.addr_bytes[3],
						   mac.addr_bytes[4],
						   mac.addr_bytes[5]);
				break;
			case 30:
				rte_parse_portlist(argv[1], &portlist);
				rte_ether_aton(argv[5], &mac);
				cli_printf("   Show Portlist: %08x vlan %d MAC: %02x:%02x:%02x:%02x:%02x:%02x\n",
						   portlist,
						   atoi(argv[3]),
						   mac.addr_bytes[0],
						   mac.addr_bytes[1],
						   mac.addr_bytes[2],
						   mac.addr_bytes[3],
						   mac.addr_bytes[4],
						   mac.addr_bytes[5]);
				break;
			case 40:
				rte_parse_portlist(argv[1], &portlist);
				rte_ether_aton("1234:4567:8901", &mac);
				cli_printf("   Show Portlist: %08x %s: ",
						   portlist, argv[2]);
				if (argv[2][0] == 'm')
					cli_printf("%02x:%02x:%02x:%02x:%02x:%02x\n",
						   mac.addr_bytes[0],
						   mac.addr_bytes[1],
						   mac.addr_bytes[2],
						   mac.addr_bytes[3],
						   mac.addr_bytes[4],
						   mac.addr_bytes[5]);
				else
					cli_printf("%d\n", 101);
				break;
			default:
				cli_help_show(Show_info.help);
				return -1;
		}
		return 0;
	}

	static struct cli_tree my_tree[] = {
		c_dir("/data"),
	    c_file("pci",	pci_file, 	"display lspci information"),
	    c_dir("/bin"),
	    c_cmd("show",	show_cmd, 	"show mapping options"),
	    c_cmd("hello",	hello_cmd, 	"Hello-World!!"),
	    c_alias("h", 	"history", 	"display history commands"),
	    c_alias("ll", 	"ls -l", 	"long directory listing alias"),
	    c_end()
	};

Here is the cli_tree for this example, note it has a lot more commands. The show_cmd
or show command is located a number of lines down. This cli_tree creates in the
/bin directory a number of commands, which one is the show command. The
show command has four different formats if you look at the show_map[].

The user types one of these commands and cli_mapping() attempts to locate the
correct entry in the list. You will also notice another structure called pcap_help,
which is an array of strings giving a cleaner and longer help description of
each of the commands.

These two structure show_map/show_help can be included into another structure
called cli_info. The cli_info structure contains pointers to each of the structures
and a group name string. The CLI_INFO() macro is used to help create the
cli_info structure called Show_info. Note the first argument in the macro is
the name of the group and is used to create the Show_info structure name. The
second and third arguments to the macro are the map and help structures.

The reason for the cli_info structure called Show_info is to be used with the
cli_map_help_all(struct cli_info **info); routine to use to display
help information for autocomplete and the help command. The help_data[] is used
in the help command to dump out the help strings. In this array of structure
pointers I have included a number of cli_info structures not shown in this doc
as an example. The only structure required is the struct cli_map show_map[]. The
Show_info help_date[] structure and const char **show_help are not required.

The following is from Pktgen source code to show add more help to the global
help for the system.

.. code-block:: c

	static struct cli_info *help_data[] = {
		&Title_info,
		&Page_info,
		&Enable_info,
		&Set_info,
		&Range_info,
		&Seq_info,
		&PCAP_info,
		&Start_info,
		&Debug_info,
		&Misc_info,
		&Theme_info,
		&Status_info,
		NULL
	};

Understanding the CLI system
~~~~~~~~~~~~~~~~~~~~~~~~~~~~

The command line interface is defined as a fake directory tree with executables,
directorys and files. The user uses shell like standard commands to move about
the directory and execute commands. The CLI is not a powerful as the
Bash shell, but has a number of similar concepts.

Our fake directory tree has a '/' or root directory which is created when
cli_create() is called along with the default sbin directory. The user starts out
at the root directory '/' and is allowed to cd to other directories, which could
contain more executables, aliases or directories. The max number of directory
levels is limited to the number of nodes given at startup.

The default directory tree starts out as just root (/) and a sbin directory.
Also it contains a file called copyright in root, which can be displayed
using the default 'more copyright' command.

A number of default commands are predefined in the /sbin directory and are
defined above. Other bin directories can be added to the system if needed,
but a limit of CLI_MAX_BINS is defined in the cli.h header file.

The CLI structure is created at run time adding directories, commands and
aliases as needed, which is different from the cmdline interface in DPDK today.

The basic concept for a command is similar to a standard Linux executable,
meaning the command when executed it is passed the command line in a argc/argv
format to be parsed by the function. The function is attached to a command file
in the directory tree and is executed when the user types the name of the
function along with it arguments. Some examples of the default commands can be
seen in the lib/librte_cli/cli_cmds.c file.



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [RFC] New CLI for DPDK
  2017-03-10 15:25 [dpdk-dev] [RFC] New CLI for DPDK Wiles, Keith
@ 2017-03-10 16:06 ` Stephen Hemminger
  2017-03-10 16:22   ` Wiles, Keith
  2017-03-23 16:13 ` Wiles, Keith
  1 sibling, 1 reply; 11+ messages in thread
From: Stephen Hemminger @ 2017-03-10 16:06 UTC (permalink / raw)
  To: Wiles, Keith; +Cc: DPDK

On Fri, 10 Mar 2017 15:25:31 +0000
"Wiles, Keith" <keith.wiles@intel.com> wrote:

> I would like to request for comments on a new CLI design and get any feedback. I have attached the cli.rst text, which is still a work in progress for you review.
> 
> I have also ported the CLI to a version of Pktgen on the ‘dev’ branch of the repo in DPDK.org.
> 
> http://dpdk.org/browse/apps/pktgen-dpdk/refs/?h=dev
> 
> I would like to submit the CLI library to be used in DPDK, if that seems reasonable to everyone. I need more testing of the API and Pktgen, but I feel it has a simpler design, easier to understand and hopefully make it easier for developers to add commands.
> 
> As an example I quickly converted over testpmd from CMDLINE to CLI (I just add a -I option to select CLI instead) and reduced the test-pmd/cmdline.c file from 12.6K lines to about 4.5K lines. I did not fully test the code, but the ones I did test seem to work.
> 
> I do not expect DPDK to convert to the new CLI only if it makes sense and I am not suggesting to replace CMDLINE library.
> 
> If you play with the new CLI in pktgen and see any problems or want to suggest new features or changes please let me know.
> 
> Comments on the cli.rst text is also welcome, but the cli.rst is not complete. I think this file needs to be broken into two one to explain the example and another to explain CLI internals.
> 

It would be great if all DPDK examples used a similar architecture. And having a common
infrastructure would help.

But not sure it needs to be special. Why should this be DPDK specific?
What you are building really ends up being an application
framework at some point. Surely, there are lots of others already in open source.

Heck even VPP has its own CLI inside.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [RFC] New CLI for DPDK
  2017-03-10 16:06 ` Stephen Hemminger
@ 2017-03-10 16:22   ` Wiles, Keith
  2017-03-10 16:41     ` Stephen Hemminger
  0 siblings, 1 reply; 11+ messages in thread
From: Wiles, Keith @ 2017-03-10 16:22 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: DPDK


> On Mar 10, 2017, at 10:06 AM, Stephen Hemminger <stephen@networkplumber.org> wrote:
> 
> On Fri, 10 Mar 2017 15:25:31 +0000
> "Wiles, Keith" <keith.wiles@intel.com> wrote:
> 
>> I would like to request for comments on a new CLI design and get any feedback. I have attached the cli.rst text, which is still a work in progress for you review.
>> 
>> I have also ported the CLI to a version of Pktgen on the ‘dev’ branch of the repo in DPDK.org.
>> 
>> http://dpdk.org/browse/apps/pktgen-dpdk/refs/?h=dev
>> 
>> I would like to submit the CLI library to be used in DPDK, if that seems reasonable to everyone. I need more testing of the API and Pktgen, but I feel it has a simpler design, easier to understand and hopefully make it easier for developers to add commands.
>> 
>> As an example I quickly converted over testpmd from CMDLINE to CLI (I just add a -I option to select CLI instead) and reduced the test-pmd/cmdline.c file from 12.6K lines to about 4.5K lines. I did not fully test the code, but the ones I did test seem to work.
>> 
>> I do not expect DPDK to convert to the new CLI only if it makes sense and I am not suggesting to replace CMDLINE library.
>> 
>> If you play with the new CLI in pktgen and see any problems or want to suggest new features or changes please let me know.
>> 
>> Comments on the cli.rst text is also welcome, but the cli.rst is not complete. I think this file needs to be broken into two one to explain the example and another to explain CLI internals.
>> 
> 
> It would be great if all DPDK examples used a similar architecture. And having a common
> infrastructure would help.
> 
> But not sure it needs to be special. Why should this be DPDK specific?
> What you are building really ends up being an application
> framework at some point. Surely, there are lots of others already in open source.
> 
> Heck even VPP has its own CLI inside.

I have been looking for one for years and never found one that met my needs of easy to use and easy to understand. If you can find a better one then please let me know.

This code does use some DPDK APIs but they can be removed to make it standalone and the first version I did was standalone. Some of the ones I found were similar to cmdline and some took it a step farther by trying to do everything one would ever need in a CLI. Those are way too big and difficult to use, then you have the ones that are barely a step above readline or just writing you own. The cmdline interface falls closer to the trying to do everything for you, by converting strings into values with structures/macros difficult to understand at a glance. IMHO this one is simple and easy to understand.

But in truth the cmdline interface in DPDK is difficult to use and to write code for, takes way to many lines of code to make a simple command. The current Cmdline is  also not dynamic, which makes it difficult to add features on the fly.

All of the commands are at the same level and using a directory structure allows the developer to use what directory path he takes to denote a context for the command. As the example of converting test-pmd to use CLI the number of lines dropped from 12.6K to 4.5K lines. The cmdline code is also not consider to be production quality (from the docs) and I would like to fix that problem for DPDK.

Regards,
Keith


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [RFC] New CLI for DPDK
  2017-03-10 16:22   ` Wiles, Keith
@ 2017-03-10 16:41     ` Stephen Hemminger
  2017-03-10 17:04       ` Wiles, Keith
  0 siblings, 1 reply; 11+ messages in thread
From: Stephen Hemminger @ 2017-03-10 16:41 UTC (permalink / raw)
  To: Wiles, Keith; +Cc: DPDK

On Fri, 10 Mar 2017 16:22:49 +0000
"Wiles, Keith" <keith.wiles@intel.com> wrote:

> > On Mar 10, 2017, at 10:06 AM, Stephen Hemminger <stephen@networkplumber.org> wrote:
> > 
> > On Fri, 10 Mar 2017 15:25:31 +0000
> > "Wiles, Keith" <keith.wiles@intel.com> wrote:
> >   
> >> I would like to request for comments on a new CLI design and get any feedback. I have attached the cli.rst text, which is still a work in progress for you review.
> >> 
> >> I have also ported the CLI to a version of Pktgen on the ‘dev’ branch of the repo in DPDK.org.
> >> 
> >> http://dpdk.org/browse/apps/pktgen-dpdk/refs/?h=dev
> >> 
> >> I would like to submit the CLI library to be used in DPDK, if that seems reasonable to everyone. I need more testing of the API and Pktgen, but I feel it has a simpler design, easier to understand and hopefully make it easier for developers to add commands.
> >> 
> >> As an example I quickly converted over testpmd from CMDLINE to CLI (I just add a -I option to select CLI instead) and reduced the test-pmd/cmdline.c file from 12.6K lines to about 4.5K lines. I did not fully test the code, but the ones I did test seem to work.
> >> 
> >> I do not expect DPDK to convert to the new CLI only if it makes sense and I am not suggesting to replace CMDLINE library.
> >> 
> >> If you play with the new CLI in pktgen and see any problems or want to suggest new features or changes please let me know.
> >> 
> >> Comments on the cli.rst text is also welcome, but the cli.rst is not complete. I think this file needs to be broken into two one to explain the example and another to explain CLI internals.
> >>   
> > 
> > It would be great if all DPDK examples used a similar architecture. And having a common
> > infrastructure would help.
> > 
> > But not sure it needs to be special. Why should this be DPDK specific?
> > What you are building really ends up being an application
> > framework at some point. Surely, there are lots of others already in open source.
> > 
> > Heck even VPP has its own CLI inside.  
> 
> I have been looking for one for years and never found one that met my needs of easy to use and easy to understand. If you can find a better one then please let me know.
> 
> This code does use some DPDK APIs but they can be removed to make it standalone and the first version I did was standalone. Some of the ones I found were similar to cmdline and some took it a step farther by trying to do everything one would ever need in a CLI. Those are way too big and difficult to use, then you have the ones that are barely a step above readline or just writing you own. The cmdline interface falls closer to the trying to do everything for you, by converting strings into values with structures/macros difficult to understand at a glance. IMHO this one is simple and easy to understand.
> 
> But in truth the cmdline interface in DPDK is difficult to use and to write code for, takes way to many lines of code to make a simple command. The current Cmdline is  also not dynamic, which makes it difficult to add features on the fly.
> 
> All of the commands are at the same level and using a directory structure allows the developer to use what directory path he takes to denote a context for the command. As the example of converting test-pmd to use CLI the number of lines dropped from 12.6K to 4.5K lines. The cmdline code is also not consider to be production quality (from the docs) and I would like to fix that problem for DPDK.
> 

If you look beyond the simple C model there are a lot.
 TCL
 http://wanderinghorse.net/computing/shellish/eshell.html

Though these may not match what is needed.  Agree that the current cmdline() method is not suitable.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [RFC] New CLI for DPDK
  2017-03-10 16:41     ` Stephen Hemminger
@ 2017-03-10 17:04       ` Wiles, Keith
  0 siblings, 0 replies; 11+ messages in thread
From: Wiles, Keith @ 2017-03-10 17:04 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: DPDK


> On Mar 10, 2017, at 10:41 AM, Stephen Hemminger <stephen@networkplumber.org> wrote:
> 
> On Fri, 10 Mar 2017 16:22:49 +0000
> "Wiles, Keith" <keith.wiles@intel.com> wrote:
> 
>>> On Mar 10, 2017, at 10:06 AM, Stephen Hemminger <stephen@networkplumber.org> wrote:
>>> 
>>> On Fri, 10 Mar 2017 15:25:31 +0000
>>> "Wiles, Keith" <keith.wiles@intel.com> wrote:
>>> 
>>>> I would like to request for comments on a new CLI design and get any feedback. I have attached the cli.rst text, which is still a work in progress for you review.
>>>> 
>>>> I have also ported the CLI to a version of Pktgen on the ‘dev’ branch of the repo in DPDK.org.
>>>> 
>>>> http://dpdk.org/browse/apps/pktgen-dpdk/refs/?h=dev
>>>> 
>>>> I would like to submit the CLI library to be used in DPDK, if that seems reasonable to everyone. I need more testing of the API and Pktgen, but I feel it has a simpler design, easier to understand and hopefully make it easier for developers to add commands.
>>>> 
>>>> As an example I quickly converted over testpmd from CMDLINE to CLI (I just add a -I option to select CLI instead) and reduced the test-pmd/cmdline.c file from 12.6K lines to about 4.5K lines. I did not fully test the code, but the ones I did test seem to work.
>>>> 
>>>> I do not expect DPDK to convert to the new CLI only if it makes sense and I am not suggesting to replace CMDLINE library.
>>>> 
>>>> If you play with the new CLI in pktgen and see any problems or want to suggest new features or changes please let me know.
>>>> 
>>>> Comments on the cli.rst text is also welcome, but the cli.rst is not complete. I think this file needs to be broken into two one to explain the example and another to explain CLI internals.
>>>> 
>>> 
>>> It would be great if all DPDK examples used a similar architecture. And having a common
>>> infrastructure would help.
>>> 
>>> But not sure it needs to be special. Why should this be DPDK specific?
>>> What you are building really ends up being an application
>>> framework at some point. Surely, there are lots of others already in open source.
>>> 
>>> Heck even VPP has its own CLI inside.  
>> 
>> I have been looking for one for years and never found one that met my needs of easy to use and easy to understand. If you can find a better one then please let me know.
>> 
>> This code does use some DPDK APIs but they can be removed to make it standalone and the first version I did was standalone. Some of the ones I found were similar to cmdline and some took it a step farther by trying to do everything one would ever need in a CLI. Those are way too big and difficult to use, then you have the ones that are barely a step above readline or just writing you own. The cmdline interface falls closer to the trying to do everything for you, by converting strings into values with structures/macros difficult to understand at a glance. IMHO this one is simple and easy to understand.
>> 
>> But in truth the cmdline interface in DPDK is difficult to use and to write code for, takes way to many lines of code to make a simple command. The current Cmdline is  also not dynamic, which makes it difficult to add features on the fly.
>> 
>> All of the commands are at the same level and using a directory structure allows the developer to use what directory path he takes to denote a context for the command. As the example of converting test-pmd to use CLI the number of lines dropped from 12.6K to 4.5K lines. The cmdline code is also not consider to be production quality (from the docs) and I would like to fix that problem for DPDK.
>> 
> 
> If you look beyond the simple C model there are a lot.
> TCL
> http://wanderinghorse.net/computing/shellish/eshell.html

I have looked at this one I believe and TCL is pretty big of a code base and not a fan of TCL in the first place :-)

In VxWorks it used Tcl, which is good language to quickly write code, but not very friendly on a small memory machine in the case of embedded devices.

This one is written in C++, which I am not again not a fan of C++ for these type of product. Also not a big C++ coder could be the other reason :-)

> 
> Though these may not match what is needed.  Agree that the current cmdline() method is not suitable.

Regards,
Keith


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [RFC] New CLI for DPDK
  2017-03-10 15:25 [dpdk-dev] [RFC] New CLI for DPDK Wiles, Keith
  2017-03-10 16:06 ` Stephen Hemminger
@ 2017-03-23 16:13 ` Wiles, Keith
  2017-03-24 13:09   ` Olivier Matz
  1 sibling, 1 reply; 11+ messages in thread
From: Wiles, Keith @ 2017-03-23 16:13 UTC (permalink / raw)
  To: DPDK


> On Mar 10, 2017, at 9:25 AM, Wiles, Keith <keith.wiles@intel.com> wrote:
> 
> I would like to request for comments on a new CLI design and get any feedback. I have attached the cli.rst text, which is still a work in progress for you review.
> 
> I have also ported the CLI to a version of Pktgen on the ‘dev’ branch of the repo in DPDK.org.
> 
> http://dpdk.org/browse/apps/pktgen-dpdk/refs/?h=dev
> 
> I would like to submit the CLI library to be used in DPDK, if that seems reasonable to everyone. I need more testing of the API and Pktgen, but I feel it has a simpler design, easier to understand and hopefully make it easier for developers to add commands.
> 
> As an example I quickly converted over testpmd from CMDLINE to CLI (I just add a -I option to select CLI instead) and reduced the test-pmd/cmdline.c file from 12.6K lines to about 4.5K lines. I did not fully test the code, but the ones I did test seem to work.
> 
> I do not expect DPDK to convert to the new CLI only if it makes sense and I am not suggesting to replace CMDLINE library.
> 
> If you play with the new CLI in pktgen and see any problems or want to suggest new features or changes please let me know.
> 
> Comments on the cli.rst text is also welcome, but the cli.rst is not complete. I think this file needs to be broken into two one to explain the example and another to explain CLI internals.

Any more comments on the CLI code for DPDK.

Should I submit a patch for DPDK or would it be better to push this into its own repo on DPDK?

Regards,
Keith


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [RFC] New CLI for DPDK
  2017-03-23 16:13 ` Wiles, Keith
@ 2017-03-24 13:09   ` Olivier Matz
  2017-03-24 14:48     ` Wiles, Keith
  0 siblings, 1 reply; 11+ messages in thread
From: Olivier Matz @ 2017-03-24 13:09 UTC (permalink / raw)
  To: Wiles, Keith; +Cc: DPDK

Hi Keith,

On Thu, 23 Mar 2017 16:13:21 +0000, "Wiles, Keith" <keith.wiles@intel.com> wrote:
> > On Mar 10, 2017, at 9:25 AM, Wiles, Keith <keith.wiles@intel.com> wrote:
> > 
> > I would like to request for comments on a new CLI design and get any feedback. I have attached the cli.rst text, which is still a work in progress for you review.
> > 
> > I have also ported the CLI to a version of Pktgen on the ‘dev’ branch of the repo in DPDK.org.
> > 
> > http://dpdk.org/browse/apps/pktgen-dpdk/refs/?h=dev
> > 
> > I would like to submit the CLI library to be used in DPDK, if that seems reasonable to everyone. I need more testing of the API and Pktgen, but I feel it has a simpler design, easier to understand and hopefully make it easier for developers to add commands.
> > 
> > As an example I quickly converted over testpmd from CMDLINE to CLI (I just add a -I option to select CLI instead) and reduced the test-pmd/cmdline.c file from 12.6K lines to about 4.5K lines. I did not fully test the code, but the ones I did test seem to work.
> > 
> > I do not expect DPDK to convert to the new CLI only if it makes sense and I am not suggesting to replace CMDLINE library.
> > 
> > If you play with the new CLI in pktgen and see any problems or want to suggest new features or changes please let me know.
> > 
> > Comments on the cli.rst text is also welcome, but the cli.rst is not complete. I think this file needs to be broken into two one to explain the example and another to explain CLI internals.  
> 
> Any more comments on the CLI code for DPDK.
> 
> Should I submit a patch for DPDK or would it be better to push this into its own repo on DPDK?
> 

I agree that there is a large possibility of improvements in librte_cmdline.

Just for reference, I think this design was not that bad at the time it
was introduced:
- declaring commands as static variables makes sense when running on
  baremetal without malloc library
- implementing a readline-like part was also needed because not available
  on baremetal
- masking structures to the user was harder due to the lack of malloc
- having a cli is really helpful for a program like testpmd (both for
  a user or for an automatic test program)

Few efforts were made to enhance this library because it's not the heart
of dpdk.

The current status is acceptable to me: this library is mostly used
internally in dpdk for test apps, and application developers are free
to use a better one if they want.

But adding another cli lib in dpdk does not make sense to me. I think
the proper direction would be to remove the cli from dpdk and use a
good cli library that is available in distros. But for that:
- we need to find one
- we need to update all examples/tests that use the cmdline, and
  that will be a lot of work
- we need to enhance dpdk framework to better manage deps with
  external libraries

So, in my opinion, the CLI you are proposing should be hosted on another
repo.

Regards,
Olivier

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [RFC] New CLI for DPDK
  2017-03-24 13:09   ` Olivier Matz
@ 2017-03-24 14:48     ` Wiles, Keith
  2017-03-28 10:06       ` Thomas Monjalon
  0 siblings, 1 reply; 11+ messages in thread
From: Wiles, Keith @ 2017-03-24 14:48 UTC (permalink / raw)
  To: Olivier Matz; +Cc: DPDK


> On Mar 24, 2017, at 8:09 AM, Olivier Matz <olivier.matz@6wind.com> wrote:
> 
> Hi Keith,
> 
> On Thu, 23 Mar 2017 16:13:21 +0000, "Wiles, Keith" <keith.wiles@intel.com> wrote:
>>> On Mar 10, 2017, at 9:25 AM, Wiles, Keith <keith.wiles@intel.com> wrote:
>>> 
>>> I would like to request for comments on a new CLI design and get any feedback. I have attached the cli.rst text, which is still a work in progress for you review.
>>> 
>>> I have also ported the CLI to a version of Pktgen on the ‘dev’ branch of the repo in DPDK.org.
>>> 
>>> http://dpdk.org/browse/apps/pktgen-dpdk/refs/?h=dev
>>> 
>>> I would like to submit the CLI library to be used in DPDK, if that seems reasonable to everyone. I need more testing of the API and Pktgen, but I feel it has a simpler design, easier to understand and hopefully make it easier for developers to add commands.
>>> 
>>> As an example I quickly converted over testpmd from CMDLINE to CLI (I just add a -I option to select CLI instead) and reduced the test-pmd/cmdline.c file from 12.6K lines to about 4.5K lines. I did not fully test the code, but the ones I did test seem to work.
>>> 
>>> I do not expect DPDK to convert to the new CLI only if it makes sense and I am not suggesting to replace CMDLINE library.
>>> 
>>> If you play with the new CLI in pktgen and see any problems or want to suggest new features or changes please let me know.
>>> 
>>> Comments on the cli.rst text is also welcome, but the cli.rst is not complete. I think this file needs to be broken into two one to explain the example and another to explain CLI internals.  
>> 
>> Any more comments on the CLI code for DPDK.
>> 
>> Should I submit a patch for DPDK or would it be better to push this into its own repo on DPDK?
>> 
> 
> I agree that there is a large possibility of improvements in librte_cmdline.
> 
> Just for reference, I think this design was not that bad at the time it
> was introduced:
> - declaring commands as static variables makes sense when running on
>  baremetal without malloc library
> - implementing a readline-like part was also needed because not available
>  on baremetal
> - masking structures to the user was harder due to the lack of malloc
> - having a cli is really helpful for a program like testpmd (both for
>  a user or for an automatic test program)

I agree, but we do need to use a better version.

> 
> Few efforts were made to enhance this library because it's not the heart
> of dpdk.

True CLI is not the heart of DPDK, but adding a better supported CLI could be a reasonable addition to DPDK for developers to create a real produce instead of cmdline, which is stated was not a CLI for a real product.

> 
> The current status is acceptable to me: this library is mostly used
> internally in dpdk for test apps, and application developers are free
> to use a better one if they want.
> 
> But adding another cli lib in dpdk does not make sense to me. I think
> the proper direction would be to remove the cli from dpdk and use a
> good cli library that is available in distros. But for that:
> - we need to find one
> - we need to update all examples/tests that use the cmdline, and
>  that will be a lot of work
> - we need to enhance dpdk framework to better manage deps with
>  external libraries

Replacing applications with a new CLI is background work and we do not need to toss out cmdline until all have been converted, which could take a while. We can move cmdline out of DPDK, but the problem is most of the current applications require cmdline and the developer would be required to pull DPDK and cmdline. I would suggest we have both in DPDK and slowly convert the apps to use the new CLI.

If we split up DPDK repo into smaller repos and use something like the ‘repo’ command with a catalog file then it would be mostly transparent to the developer we have multiple repos. Until that day it does seem more reasonable to have CLI in DPDK repo, but I am willing to have it in a different repo for now.

> 
> So, in my opinion, the CLI you are proposing should be hosted on another
> repo.

I will contact Thomas to setup a new repo for the CLI.

> 
> Regards,
> Olivier

Regards,
Keith


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [RFC] New CLI for DPDK
  2017-03-24 14:48     ` Wiles, Keith
@ 2017-03-28 10:06       ` Thomas Monjalon
  2017-03-28 13:19         ` Wiles, Keith
  0 siblings, 1 reply; 11+ messages in thread
From: Thomas Monjalon @ 2017-03-28 10:06 UTC (permalink / raw)
  To: Wiles, Keith; +Cc: dev, Olivier Matz, techboard

2017-03-24 14:48, Wiles, Keith:
> 
> > On Mar 24, 2017, at 8:09 AM, Olivier Matz <olivier.matz@6wind.com> wrote:
> > 
> > Hi Keith,
> > 
> > On Thu, 23 Mar 2017 16:13:21 +0000, "Wiles, Keith" <keith.wiles@intel.com> wrote:
> >>> On Mar 10, 2017, at 9:25 AM, Wiles, Keith <keith.wiles@intel.com> wrote:
> >>> 
> >>> I would like to request for comments on a new CLI design and get any feedback. I have attached the cli.rst text, which is still a work in progress for you review.
> >>> 
> >>> I have also ported the CLI to a version of Pktgen on the ‘dev’ branch of the repo in DPDK.org.
> >>> 
> >>> http://dpdk.org/browse/apps/pktgen-dpdk/refs/?h=dev
> >>> 
> >>> I would like to submit the CLI library to be used in DPDK, if that seems reasonable to everyone. I need more testing of the API and Pktgen, but I feel it has a simpler design, easier to understand and hopefully make it easier for developers to add commands.
> >>> 
> >>> As an example I quickly converted over testpmd from CMDLINE to CLI (I just add a -I option to select CLI instead) and reduced the test-pmd/cmdline.c file from 12.6K lines to about 4.5K lines. I did not fully test the code, but the ones I did test seem to work.
> >>> 
> >>> I do not expect DPDK to convert to the new CLI only if it makes sense and I am not suggesting to replace CMDLINE library.
> >>> 
> >>> If you play with the new CLI in pktgen and see any problems or want to suggest new features or changes please let me know.
> >>> 
> >>> Comments on the cli.rst text is also welcome, but the cli.rst is not complete. I think this file needs to be broken into two one to explain the example and another to explain CLI internals.  
> >> 
> >> Any more comments on the CLI code for DPDK.
> >> 
> >> Should I submit a patch for DPDK or would it be better to push this into its own repo on DPDK?
> >> 
> > 
> > I agree that there is a large possibility of improvements in librte_cmdline.
> > 
> > Just for reference, I think this design was not that bad at the time it
> > was introduced:
> > - declaring commands as static variables makes sense when running on
> >  baremetal without malloc library
> > - implementing a readline-like part was also needed because not available
> >  on baremetal
> > - masking structures to the user was harder due to the lack of malloc
> > - having a cli is really helpful for a program like testpmd (both for
> >  a user or for an automatic test program)
> 
> I agree, but we do need to use a better version.
> 
> > 
> > Few efforts were made to enhance this library because it's not the heart
> > of dpdk.
> 
> True CLI is not the heart of DPDK, but adding a better supported CLI could be a reasonable addition to DPDK for developers to create a real produce instead of cmdline, which is stated was not a CLI for a real product.

We can ask to the techboard whether CLI is in DPDK scope.
My position: it is neither in the scope of the repo nor in the scope of dpdk.org.

> > The current status is acceptable to me: this library is mostly used
> > internally in dpdk for test apps, and application developers are free
> > to use a better one if they want.
> > 
> > But adding another cli lib in dpdk does not make sense to me. I think
> > the proper direction would be to remove the cli from dpdk and use a
> > good cli library that is available in distros. But for that:
> > - we need to find one
> > - we need to update all examples/tests that use the cmdline, and
> >  that will be a lot of work
> > - we need to enhance dpdk framework to better manage deps with
> >  external libraries

I agree we could convert testpmd and examples to a new CLI lib.
Then we could remove librte_cmdline from DPDK.

> Replacing applications with a new CLI is background work and we do not need to toss out cmdline until all have been converted, which could take a while. We can move cmdline out of DPDK, but the problem is most of the current applications require cmdline and the developer would be required to pull DPDK and cmdline. I would suggest we have both in DPDK and slowly convert the apps to use the new CLI.
> 
> If we split up DPDK repo into smaller repos and use something like the ‘repo’ command with a catalog file then it would be mostly transparent to the developer we have multiple repos. Until that day it does seem more reasonable to have CLI in DPDK repo, but I am willing to have it in a different repo for now.

It is reasonnable for an application to have some dependencies.
It is commonly managed in three ways:
- detect the lib on the system
- download the lib as part of the build process
- ask user to download it

> > So, in my opinion, the CLI you are proposing should be hosted on another
> > repo.
> 
> I will contact Thomas to setup a new repo for the CLI.

We had a private discussion.
I have tried to put here my conclusions.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [RFC] New CLI for DPDK
  2017-03-28 10:06       ` Thomas Monjalon
@ 2017-03-28 13:19         ` Wiles, Keith
  2017-03-28 13:30           ` Thomas Monjalon
  0 siblings, 1 reply; 11+ messages in thread
From: Wiles, Keith @ 2017-03-28 13:19 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev, Olivier Matz, techboard


> On Mar 28, 2017, at 5:06 AM, Thomas Monjalon <thomas.monjalon@6wind.com> wrote:
> 
> 2017-03-24 14:48, Wiles, Keith:
>> 
>>> On Mar 24, 2017, at 8:09 AM, Olivier Matz <olivier.matz@6wind.com> wrote:
>>> 
>>> Hi Keith,
>>> 
>>> On Thu, 23 Mar 2017 16:13:21 +0000, "Wiles, Keith" <keith.wiles@intel.com> wrote:
>>>>> On Mar 10, 2017, at 9:25 AM, Wiles, Keith <keith.wiles@intel.com> wrote:
>>>>> 
>>>>> I would like to request for comments on a new CLI design and get any feedback. I have attached the cli.rst text, which is still a work in progress for you review.
>>>>> 
>>>>> I have also ported the CLI to a version of Pktgen on the ‘dev’ branch of the repo in DPDK.org.
>>>>> 
>>>>> http://dpdk.org/browse/apps/pktgen-dpdk/refs/?h=dev
>>>>> 
>>>>> I would like to submit the CLI library to be used in DPDK, if that seems reasonable to everyone. I need more testing of the API and Pktgen, but I feel it has a simpler design, easier to understand and hopefully make it easier for developers to add commands.
>>>>> 
>>>>> As an example I quickly converted over testpmd from CMDLINE to CLI (I just add a -I option to select CLI instead) and reduced the test-pmd/cmdline.c file from 12.6K lines to about 4.5K lines. I did not fully test the code, but the ones I did test seem to work.
>>>>> 
>>>>> I do not expect DPDK to convert to the new CLI only if it makes sense and I am not suggesting to replace CMDLINE library.
>>>>> 
>>>>> If you play with the new CLI in pktgen and see any problems or want to suggest new features or changes please let me know.
>>>>> 
>>>>> Comments on the cli.rst text is also welcome, but the cli.rst is not complete. I think this file needs to be broken into two one to explain the example and another to explain CLI internals.  
>>>> 
>>>> Any more comments on the CLI code for DPDK.
>>>> 
>>>> Should I submit a patch for DPDK or would it be better to push this into its own repo on DPDK?
>>>> 
>>> 
>>> I agree that there is a large possibility of improvements in librte_cmdline.
>>> 
>>> Just for reference, I think this design was not that bad at the time it
>>> was introduced:
>>> - declaring commands as static variables makes sense when running on
>>> baremetal without malloc library
>>> - implementing a readline-like part was also needed because not available
>>> on baremetal
>>> - masking structures to the user was harder due to the lack of malloc
>>> - having a cli is really helpful for a program like testpmd (both for
>>> a user or for an automatic test program)
>> 
>> I agree, but we do need to use a better version.
>> 
>>> 
>>> Few efforts were made to enhance this library because it's not the heart
>>> of dpdk.
>> 
>> True CLI is not the heart of DPDK, but adding a better supported CLI could be a reasonable addition to DPDK for developers to create a real produce instead of cmdline, which is stated was not a CLI for a real product.
> 
> We can ask to the techboard whether CLI is in DPDK scope.
> My position: it is neither in the scope of the repo nor in the scope of dpdk.org.

The goal is to replace the cmdline with CLI or at least provide a much better supported and easier solution for DPDK developers.

The CLI was written just for DPDK and uses DPDK APIs only, so to me it is related to DPDK like any other tool or library we have in DPDK that is not really providing packet transport. We have a number of these in DPDK today and having a library repo on DPDK.org is the very reasonable.

Also I thought we decided to add the repo to DPDK.org and the only reason we are revisiting if CLI should be on DPDK.org is how we support external libs of this type.

Thomas started out not liking the name of librte_cli and wanted some other name. I suggested ‘cli’, but I wanted to keep the librte_cli name as I wanted to be able to pull the librte_cli into the DPDK/lib directory to make it simpler to integrate into DPDK for the developer. Building library or applications external to DPDK has a few issues and not completely supported well in DPDK.

I would like to clone DPDK then clone librte_cli into the DPDK/lib directory allowing librte_cli to be built as a internal library. Then add the config option to the common_base and DPDK/lib/Makefile to allow the library to be built. This allow the developer to find the include and library easily, which then allows us to start looking at updating the current apps to use CLI.

I could provide a patch to DPDK to help integrate CLI into DPDK without modifying DPDK today for the developer to be able to use CLI as a standard internal library. My goal is to provide a cleaner solution for DPDK developers and a DPDK support CLI. 

I am now a bit confused as to why Thomas has objected here and requested to be push to the TechBoard.

> 
>>> The current status is acceptable to me: this library is mostly used
>>> internally in dpdk for test apps, and application developers are free
>>> to use a better one if they want.
>>> 
>>> But adding another cli lib in dpdk does not make sense to me. I think
>>> the proper direction would be to remove the cli from dpdk and use a
>>> good cli library that is available in distros. But for that:
>>> - we need to find one
>>> - we need to update all examples/tests that use the cmdline, and
>>> that will be a lot of work
>>> - we need to enhance dpdk framework to better manage deps with
>>> external libraries
> 
> I agree we could convert testpmd and examples to a new CLI lib.
> Then we could remove librte_cmdline from DPDK.
> 
>> Replacing applications with a new CLI is background work and we do not need to toss out cmdline until all have been converted, which could take a while. We can move cmdline out of DPDK, but the problem is most of the current applications require cmdline and the developer would be required to pull DPDK and cmdline. I would suggest we have both in DPDK and slowly convert the apps to use the new CLI.
>> 
>> If we split up DPDK repo into smaller repos and use something like the ‘repo’ command with a catalog file then it would be mostly transparent to the developer we have multiple repos. Until that day it does seem more reasonable to have CLI in DPDK repo, but I am willing to have it in a different repo for now.
> 
> It is reasonnable for an application to have some dependencies.
> It is commonly managed in three ways:
> - detect the lib on the system
> - download the lib as part of the build process
> - ask user to download it

In this case the developer clones the repo into the DPDK/lib directory and enable it to be built via enabling the config option or some other method (Patch). Then build DPDK as normal to allow his applications to detect and include the CLI code.

> 
>>> So, in my opinion, the CLI you are proposing should be hosted on another
>>> repo.
>> 
>> I will contact Thomas to setup a new repo for the CLI.
> 
> We had a private discussion.
> I have tried to put here my conclusions.

Regards,
Keith


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [RFC] New CLI for DPDK
  2017-03-28 13:19         ` Wiles, Keith
@ 2017-03-28 13:30           ` Thomas Monjalon
  0 siblings, 0 replies; 11+ messages in thread
From: Thomas Monjalon @ 2017-03-28 13:30 UTC (permalink / raw)
  To: Wiles, Keith; +Cc: dev, Olivier Matz, techboard

2017-03-28 13:19, Wiles, Keith:
> > On Mar 28, 2017, at 5:06 AM, Thomas Monjalon <thomas.monjalon@6wind.com> wrote:
> > 2017-03-24 14:48, Wiles, Keith:
> >> True CLI is not the heart of DPDK, but adding a better supported CLI could be a reasonable addition to DPDK for developers to create a real produce instead of cmdline, which is stated was not a CLI for a real product.
> > 
> > We can ask to the techboard whether CLI is in DPDK scope.
> > My position: it is neither in the scope of the repo nor in the scope of dpdk.org.
> 
> The goal is to replace the cmdline with CLI or at least provide a much better supported and easier solution for DPDK developers.
> 
> The CLI was written just for DPDK and uses DPDK APIs only, so to me it is related to DPDK like any other tool or library we have in DPDK that is not really providing packet transport. We have a number of these in DPDK today and having a library repo on DPDK.org is the very reasonable.
> 
> Also I thought we decided to add the repo to DPDK.org and the only reason we are revisiting if CLI should be on DPDK.org is how we support external libs of this type.
> 
> Thomas started out not liking the name of librte_cli and wanted some other name. I suggested ‘cli’, but I wanted to keep the librte_cli name as I wanted to be able to pull the librte_cli into the DPDK/lib directory to make it simpler to integrate into DPDK for the developer. Building library or applications external to DPDK has a few issues and not completely supported well in DPDK.
> 
> I would like to clone DPDK then clone librte_cli into the DPDK/lib directory allowing librte_cli to be built as a internal library. Then add the config option to the common_base and DPDK/lib/Makefile to allow the library to be built. This allow the developer to find the include and library easily, which then allows us to start looking at updating the current apps to use CLI.
> 
> I could provide a patch to DPDK to help integrate CLI into DPDK without modifying DPDK today for the developer to be able to use CLI as a standard internal library. My goal is to provide a cleaner solution for DPDK developers and a DPDK support CLI.
> 
> I am now a bit confused as to why Thomas has objected here and requested to be push to the TechBoard.

I am sorry, I have realized in the discussion that it is not my call anymore
to decide which repo can be added on dpdk.org.
I have to apply a Technical Board decision which must comply with the (upcoming)
Governing Board directives.

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2017-03-28 13:30 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-10 15:25 [dpdk-dev] [RFC] New CLI for DPDK Wiles, Keith
2017-03-10 16:06 ` Stephen Hemminger
2017-03-10 16:22   ` Wiles, Keith
2017-03-10 16:41     ` Stephen Hemminger
2017-03-10 17:04       ` Wiles, Keith
2017-03-23 16:13 ` Wiles, Keith
2017-03-24 13:09   ` Olivier Matz
2017-03-24 14:48     ` Wiles, Keith
2017-03-28 10:06       ` Thomas Monjalon
2017-03-28 13:19         ` Wiles, Keith
2017-03-28 13:30           ` Thomas Monjalon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).