Re: [dpdk-dev] [PATCH v2 2/2] app/testpmd: add support for integrity item

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

On 4/13/2021 8:53 AM, Ori Kam wrote:
> Hi Ferruh,
> 
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>>
>> On 4/11/2021 6:34 PM, Gregory Etelson wrote:
>>> From: Ori Kam <orika@nvidia.com>
>>>
>>> The integrity item allows the application to match
>>> on the integrity of a packet.
>>>
>>> use example:
>>> match that packet integrity checks are ok. The checks depend on
>>> packet layers. For example ICMP packet will not check L4 level.
>>> flow create 0 ingress pattern integrity value mask 0x01 value spec 0x01
>>> match that L4 packet is ok - check L2 & L3 & L4 layers:
>>> flow create 0 ingress pattern integrity value mask 0xfe value spec 0xfe
>>>
>>> Signed-off-by: Ori Kam <orika@nvidia.com>
>>> Signed-off-by: Gregory Etelson <getelson@nvidia.com>
>>> ---
>>> v2 add testpmd patch
>>> ---
>>>    app/test-pmd/cmdline_flow.c | 39
>> +++++++++++++++++++++++++++++++++++++
>>
>> Hi Gregory, Ori,
>>
>> Can you add some samples to "testpmd_funcs.rst#flow-rules-management"?
>>
>> I asked in some other thread but did not get any response, what do you think to
>> make 'testpmd_funcs.rst' sample update mandatory when testpmd flow added?
>>
> I fully agree that each new function should be mandatory,

What is new function here, new flow API? That should go to flow API 
documentation, 'rte_flow.rst'.

> The question is do we want that each new item / action (they  use existing function)
> I think it is a bit of overhead but I don't have strong opinion.
> 

Since the documentation is for the testpmd usage sample, I was thinking to add 
sample for each new item & action indeed.
Same of the flow rules not widely used, and it is not always clear how to use 
them, that is why I believe documenting samples can help.

>>
>>>    1 file changed, 39 insertions(+)
>>>
>>> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
>>> index fb7a3a8bd3..b5dec34325 100644
>>> --- a/app/test-pmd/cmdline_flow.c
>>> +++ b/app/test-pmd/cmdline_flow.c
>>> @@ -289,6 +289,9 @@ enum index {
>>>    	ITEM_GENEVE_OPT_TYPE,
>>>    	ITEM_GENEVE_OPT_LENGTH,
>>>    	ITEM_GENEVE_OPT_DATA,
>>> +	ITEM_INTEGRITY,
>>> +	ITEM_INTEGRITY_LEVEL,
>>> +	ITEM_INTEGRITY_VALUE,
>>>
>>>    	/* Validate/create actions. */
>>>    	ACTIONS,
>>> @@ -956,6 +959,7 @@ static const enum index next_item[] = {
>>>    	ITEM_PFCP,
>>>    	ITEM_ECPRI,
>>>    	ITEM_GENEVE_OPT,
>>> +	ITEM_INTEGRITY,
>>>    	END_SET,
>>>    	ZERO,
>>>    };
>>> @@ -1307,6 +1311,19 @@ static const enum index item_geneve_opt[] = {
>>>    	ZERO,
>>>    };
>>>
>>> +static const enum index item_integrity[] = {
>>> +	ITEM_INTEGRITY_LEVEL,
>>> +	ITEM_INTEGRITY_VALUE,
>>> +	ZERO,
>>> +};
>>> +
>>> +static const enum index item_integrity_lv[] = {
>>> +	ITEM_INTEGRITY_LEVEL,
>>> +	ITEM_INTEGRITY_VALUE,
>>> +	ITEM_NEXT,
>>> +	ZERO,
>>> +};
>>> +
>>>    static const enum index next_action[] = {
>>>    	ACTION_END,
>>>    	ACTION_VOID,
>>> @@ -3373,6 +3390,28 @@ static const struct token token_list[] = {
>>>    				(sizeof(struct rte_flow_item_geneve_opt),
>>>    				ITEM_GENEVE_OPT_DATA_SIZE)),
>>>    	},
>>> +	[ITEM_INTEGRITY] = {
>>> +		.name = "integrity",
>>> +		.help = "match packet integrity",
>>> +		.priv = PRIV_ITEM(INTEGRITY,
>>> +				  sizeof(struct rte_flow_item_integrity)),
>>> +		.next = NEXT(item_integrity),
>>> +		.call = parse_vc,
>>> +	},
>>> +	[ITEM_INTEGRITY_LEVEL] = {
>>> +		.name = "level",
>>> +		.help = "integrity level",
>>> +		.next = NEXT(item_integrity_lv, NEXT_ENTRY(UNSIGNED),
>>> +			     item_param),
>>> +		.args = ARGS(ARGS_ENTRY(struct rte_flow_item_integrity,
>> level)),
>>> +	},
>>> +	[ITEM_INTEGRITY_VALUE] = {
>>> +		.name = "value",
>>> +		.help = "integrity value",
>>> +		.next = NEXT(item_integrity_lv, NEXT_ENTRY(UNSIGNED),
>>> +			     item_param),
>>> +		.args = ARGS(ARGS_ENTRY(struct rte_flow_item_integrity,
>> value)),
>>> +	},
>>>    	/* Validate/create actions. */
>>>    	[ACTIONS] = {
>>>    		.name = "actions",
>>>
>