diff mbox series

[v2,2/2] app/testpmd: add support for integrity item

Message ID 20210411173414.12568-3-getelson@nvidia.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series add packet integrity checks |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/travis-robot success travis build: passed
ci/github-robot success github build: passed
ci/iol-abi-testing success Testing PASS
ci/iol-testing success Testing PASS
ci/intel-Testing success Testing PASS

Commit Message

Gregory Etelson April 11, 2021, 5:34 p.m. UTC 
  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 +++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

Comments

Ferruh Yigit April 12, 2021, 5:49 p.m. UTC | #1 
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?


>   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",
>
Ori Kam April 13, 2021, 7:53 a.m. UTC | #2 
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,
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.

> 
> >   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",
> >
Ferruh Yigit April 13, 2021, 8:14 a.m. UTC | #3 
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",
>>>
>
Ori Kam April 13, 2021, 11:36 a.m. UTC | #4 
Hi

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> item
> 
> 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'.
> 
I mean something like for example new set,
create_shared_action..
I mean new total new commands.
Does it make sense?

> > 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.
> 
I fully agree with you, the question is how to do it,
since in some cases it is jut one line of code,
and in other cases it can be much more complex for example raw_encap,
the new Conntrack action.
I think we should think how we improve the examples in the rte_flow context,

> >>
> >>>    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",
> >>>
> >
diff mbox series

Patch

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",