[dpdk-dev] [PATCH v5 2/6] net/mlx5: support basic flow items and actions

[Ferruh Yigit]

On 1/4/2017 6:42 PM, Adrien Mazarguil wrote:
> Hi Ferruh,
> 
> On Wed, Jan 04, 2017 at 05:49:46PM +0000, Ferruh Yigit wrote:
>> Hi Nelio,
>>
>> A quick question.
> 
> I'll reply since it's related to the API.
> 
>> On 12/29/2016 3:15 PM, Nelio Laranjeiro wrote:
>>> Introduce initial software for rte_flow rules.
>>>
>>> VLAN, VXLAN are still not supported.
>>>
>>> Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro at 6wind.com>
>>> Acked-by: Adrien Mazarguil <adrien.mazarguil at 6wind.com>
>>
>> <...>
>>
>>> +static int
>>> +priv_flow_validate(struct priv *priv,
>>> +		   const struct rte_flow_attr *attr,
>>> +		   const struct rte_flow_item items[],
>>> +		   const struct rte_flow_action actions[],
>>> +		   struct rte_flow_error *error,
>>> +		   struct mlx5_flow *flow)
>>> +{
>>> +	const struct mlx5_flow_items *cur_item = mlx5_flow_items;
>>
>> <...>
>>
>>> +	for (; items->type != RTE_FLOW_ITEM_TYPE_END; ++items) {
>> <...>
>>> +	}
>>> +	for (; actions->type != RTE_FLOW_ACTION_TYPE_END; ++actions) {
>> <...>
>>> +	}
>>
>> Is it guarantied in somewhere that items or actions terminated with
>> TYPE_END?
> 
> Yes, since it's now the only way to terminate items/actions lists [1][2].
> There used to be a "max" value in the original draft but it seemed redundant
> and proved annoying to use, and was therefore dropped.
> 
> END items/actions behave like a NUL terminator for C strings. They are
> likewise defined with value 0 for convenience.

At least it is good idea to set END values to 0, but still if user not
set it, most probably this will crash the app.

Although most probably this kind of error will be detected easily in
development phase, still it would be nice to return an error instead of
crashing when user provide wrong input.

> 
>> And these fields are direct inputs from user.
>> Is there a way to verify user provided values are with TYPE_END terminated?
> 
> No, applications must check for its presence (they normally add it
> themselves) before feeding these lists to PMDs. I think that's safe enough.
> 
> Note the testpmd flow command does not allow entering a flow rule without
> "end" tokens in both lists, there is no way around this restriction.
> 
> [1] http://dpdk.org/doc/guides/prog_guide/rte_flow.html#matching-pattern
> [2] http://dpdk.org/doc/guides/prog_guide/rte_flow.html#actions
>