Re: [PATCH v4] app/testpmd: enable cli for programmable action

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

On 10/7/2023 11:47 AM, Qi Zhang wrote:
> Parsing command line for rte_flow_action_prog.
> 
> Syntax:
> 
> "prog name <name> [arguments <arg_name_0> <arg_value_0> \
>  <arg_name_1> <arg_value1> ... end]"
> 

Can you please put full rte flow command in the commit log? Like what is
the 'pattern' for above command?


> Use parse_string0 to parse name string.
> Use parse_hex to parse hex string.
> Use struct action_prog_data to store parsed result.
> 
> Example:
> 
> Action with 2 arguments:
> 
> "prog name action0 arguments field0 03FF field1 55AA end"
> 
> Action without argument:
> 
> "prog name action1"
> 
> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
>

Is there an existing driver implementation, checking it helps to
understand feature implementation?


> ---
> 
> v4:
> - be more generous on the max size of name and value.
> 
> v3:
> - refine struct action_prog_data
> - enlarge the max size
> 
> v2:
> - fix title
> - minor coding style refine.
> 
>  app/test-pmd/cmdline_flow.c | 232 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 232 insertions(+)
> 

Hi Qi,

Can you please update documentation too,
`doc/guides/testpmd_app_ug/testpmd_funcs.rst`, `Flow rules management`
section.


> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> index 21828c144c..ae5556e704 100644
> --- a/app/test-pmd/cmdline_flow.c
> +++ b/app/test-pmd/cmdline_flow.c
> @@ -719,6 +719,13 @@ enum index {
>  	ACTION_IPV6_EXT_PUSH,
>  	ACTION_IPV6_EXT_PUSH_INDEX,
>  	ACTION_IPV6_EXT_PUSH_INDEX_VALUE,
> +	ACTION_PROG,
> +	ACTION_PROG_NAME,
> +	ACTION_PROG_NAME_STRING,
> +	ACTION_PROG_ARGUMENTS,
> +	ACTION_PROG_ARG_NAME,
> +	ACTION_PROG_ARG_VALUE,
> +	ACTION_PROG_ARG_END,
>  };
>  
>  /** Maximum size for pattern in struct rte_flow_item_raw. */
> @@ -749,6 +756,23 @@ struct action_rss_data {
>  	uint16_t queue[ACTION_RSS_QUEUE_NUM];
>  };
>  
> +#define ACTION_PROG_NAME_SIZE_MAX 256
> +#define ACTION_PROG_ARG_NUM_MAX 16
> +#define ACTION_PROG_ARG_VALUE_SIZE_MAX 64
> +
> +/** Storage for struct rte_flow_action_prog including external data. */
> +struct action_prog_data {
> +	struct rte_flow_action_prog conf;
> +	struct {
> +		char name[ACTION_PROG_NAME_SIZE_MAX];
> +		struct rte_flow_action_prog_argument args[ACTION_PROG_ARG_NUM_MAX];
> +		struct {
> +			char names[ACTION_PROG_NAME_SIZE_MAX];
> +			uint8_t value[ACTION_PROG_ARG_VALUE_SIZE_MAX];
> +		} arg_data[ACTION_PROG_ARG_NUM_MAX];
> +	} data;
> +};
> +
>  /** Maximum data size in struct rte_flow_action_raw_encap. */
>  #define ACTION_RAW_ENCAP_MAX_DATA 512
>  #define RAW_ENCAP_CONFS_MAX_NUM 8
> @@ -2169,6 +2193,7 @@ static const enum index next_action[] = {
>  	ACTION_QUOTA_QU,
>  	ACTION_IPV6_EXT_REMOVE,
>  	ACTION_IPV6_EXT_PUSH,
> +	ACTION_PROG,
>  	ZERO,
>  };
>  
> @@ -2510,6 +2535,13 @@ static const enum index action_represented_port[] = {
>  	ZERO,
>  };
>  
> +static const enum index action_prog[] = {
> +	ACTION_PROG_NAME,
> +	ACTION_PROG_ARGUMENTS,
> +	ACTION_NEXT,
> +	ZERO,
> +};
> +
>  static int parse_set_raw_encap_decap(struct context *, const struct token *,
>  				     const char *, unsigned int,
>  				     void *, unsigned int);
> @@ -2786,6 +2818,18 @@ static int
>  parse_qu_mode_name(struct context *ctx, const struct token *token,
>  		   const char *str, unsigned int len, void *buf,
>  		   unsigned int size);
> +static int
> +parse_vc_action_prog(struct context *, const struct token *,
> +		     const char *, unsigned int, void *,
> +		     unsigned int);
> +static int
> +parse_vc_action_prog_arg_name(struct context *, const struct token *,
> +			      const char *, unsigned int, void *,
> +			      unsigned int);
> +static int
> +parse_vc_action_prog_arg_value(struct context *, const struct token *,
> +			       const char *, unsigned int, void *,
> +			       unsigned int);
>  static int comp_none(struct context *, const struct token *,
>  		     unsigned int, char *, unsigned int);
>  static int comp_boolean(struct context *, const struct token *,
> @@ -7518,6 +7562,48 @@ static const struct token token_list[] = {
>  		.args = ARGS(ARGS_ENTRY(struct rte_flow_item_tx_queue,
>  					tx_queue)),
>  	},
> +	[ACTION_PROG] = {
> +		.name = "prog",
> +		.help = "match a programmable action",
> +		.priv = PRIV_ACTION(PROG, sizeof(struct action_prog_data)),
> +		.next = NEXT(action_prog),
> +		.call = parse_vc_action_prog,
> +	},
> +	[ACTION_PROG_NAME] = {
> +		.name = "name",
> +		.help = "programble action name",
>

Can you please remind me again what was the 'name' filed of "struct
rte_flow_action_prog" was for?


> +		.next = NEXT(action_prog, NEXT_ENTRY(ACTION_PROG_NAME_STRING)),
> +		.args = ARGS(ARGS_ENTRY(struct action_prog_data, data.name)),
> +	},
> +	[ACTION_PROG_NAME_STRING] = {
> +		.name = "{string}",
> +		.type = "STRING",
> +		.help = "programmable action name string",
> +		.call = parse_string0,
> +	},
> +	[ACTION_PROG_ARGUMENTS] = {
> +		.name = "arguments",
> +		.help = "programmable action name",
> +		.next = NEXT(action_prog, NEXT_ENTRY(ACTION_PROG_ARG_NAME)),
> +		.call = parse_vc_conf,
> +	},
> +	[ACTION_PROG_ARG_NAME] = {
> +		.name = "{string}",
> +		.help = "programmable action argument name",
> +		.next = NEXT(NEXT_ENTRY(ACTION_PROG_ARG_VALUE)),
> +		.call = parse_vc_action_prog_arg_name,
> +	},
> +	[ACTION_PROG_ARG_VALUE] = {
> +		.name = "{hex}",
> +		.help = "programmable action argument value",
> +		.next = NEXT(NEXT_ENTRY(ACTION_PROG_ARG_END, ACTION_PROG_ARG_NAME)),
> +		.call = parse_vc_action_prog_arg_value,
> +	},
> +	[ACTION_PROG_ARG_END] = {
> +		.name = "end",
> +		.help = "end of the programmable action arguments",
> +	},
> +
>

Does this means two 'end' required if multiple args provided, like:
prog name "name" arguments field0 03FF field1 1 end / end

I am aware there is variable length of key/value, and need a marker to
stop, but this end specific for action is not used, I wonder if that is
because there is a better way to do it, @Ori may comment better.