Re: [dpdk-dev] [PATCH 1/2] app/testpmd: support the heavywight mode GRO

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 8/10/2017 3:50 AM, Jiayu Hu wrote:
> The GRO library provides two reassembly modes: lightweight mode and
> heavyweight mode. This patch is to support the heavyweight mode in
> csum forwarding engine.
> 
> With the command "gro (heavymode|lightmode) (on|off) <port id>", users
> can select the lightweight mode or the heavyweight mode to use. With
> the command "gro flush interval <num>", users can set the interval of
> flushing GROed packets from the reassembly tables for the heavyweight
> mode.
> 
> Signed-off-by: Jiayu Hu <jiayu.hu@intel.com>
> ---
>  app/test-pmd/cmdline.c  | 79 ++++++++++++++++++++++++++++++++++++++++++++++---
>  app/test-pmd/config.c   | 33 +++++++++++++--------
>  app/test-pmd/csumonly.c | 29 ++++++++++++++----
>  app/test-pmd/testpmd.c  | 17 +++++++++++
>  app/test-pmd/testpmd.h  | 12 +++++++-
>  5 files changed, 147 insertions(+), 23 deletions(-)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index cd8c358..3224ce1 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -423,7 +423,7 @@ static void cmd_help_long_parsed(void *parsed_result,
>  			"tso show (portid)"
>  			"    Display the status of TCP Segmentation Offload.\n\n"
>  
> -			"gro (on|off) (port_id)"
> +			"gro (heavymode|lightmode) (on|off) (port_id)\n"

Not specific / limited to gro, but I have a few testpmd usability questions:

1) to update some settings, there are two root level commands already:
a) set ... (like: "set fwd mac", "set promisc #P on")
b) port config ... (like: "port config 	#P speed auto duplex auto")

I don't know what it difference between above two, but thinking as "port
config" is to configure ports and "set" is to set rest of testpmd config
makes sense to me. (but "set vf .." and "set port .."  doesn't fit to
this statement)

Instead of adding "gro" root level command, why not add this under "port
config", like: "port config #P gro on" ? (or "set port #P gro on")


2) Should each configuration set have a corresponding show command?

How a user can see the current gro setting?
Do we need a "show port gro" ?


3) Where to place #P in the command:
There are inconsistencies about where to put it, like:
"port config #P l2-tunnel enable"
"port config mtu #P value"
"port #P rxq #Q start"

or

"show port info #P"
"show port #P rss-hash ip4 key"

It can be good to define a place for it for consistency.


4) Using "port" keyword in some commands before port_id:
Like: "set link-down port #P", "reset port #P mirror-rule value"

Mostly we don't have it:
"show txq info #P #Q", "set bonding mode #M #P", "set stat_qmap rx #P
#Q", "vlan set strip on #P"

It can be good to define one or other.

And I guess having need to use "port" keyword can be an indication that
command should move under "port" command:
"reset port #P mirror-rule #id" can be "port reset #P mirror-rule #id"


Thanks,
ferruh