[dpdk-dev] [PATCH v6 14/18] net/ixgbe: parse L2 tunnel filter

[Ferruh Yigit]

On 1/18/2017 1:59 AM, Zhao1, Wei wrote:
> Hi, Ferruh
> 
>> -----Original Message-----
>> From: Yigit, Ferruh
>> Sent: Tuesday, January 17, 2017 6:03 PM
>> To: Zhao1, Wei <wei.zhao1 at intel.com>; Adrien Mazarguil
>> <adrien.mazarguil at 6wind.com>
>> Cc: dev at dpdk.org; Lu, Wenzhuo <wenzhuo.lu at intel.com>
>> Subject: Re: [dpdk-dev] [PATCH v6 14/18] net/ixgbe: parse L2 tunnel filter
>>
>> On 1/17/2017 9:27 AM, Zhao1, Wei wrote:
>>> Hi, Ferruh
>>>
>>>> -----Original Message-----
>>>> From: Yigit, Ferruh
>>>> Sent: Tuesday, January 17, 2017 12:39 AM
>>>> To: Adrien Mazarguil <adrien.mazarguil at 6wind.com>; Zhao1, Wei
>>>> <wei.zhao1 at intel.com>
>>>> Cc: dev at dpdk.org; Lu, Wenzhuo <wenzhuo.lu at intel.com>
>>>> Subject: Re: [dpdk-dev] [PATCH v6 14/18] net/ixgbe: parse L2 tunnel
>>>> filter
>>>>
>>>> On 1/16/2017 1:03 PM, Adrien Mazarguil wrote:
>>>>> Hi,
>>>>>
>>>>> On Fri, Jan 13, 2017 at 04:13:08PM +0800, Wei Zhao wrote:
>>>>>> check if the rule is a L2 tunnel rule, and get the L2 tunnel info.
>>>>>>
>>>>>> Signed-off-by: Wei Zhao <wei.zhao1 at intel.com>
>>>>>> Signed-off-by: Wenzhuo Lu <wenzhuo.lu at intel.com>
>>>>>> ---
>>>>>>  drivers/net/ixgbe/ixgbe_ethdev.c |   3 +-
>>>>>>  drivers/net/ixgbe/ixgbe_flow.c   | 216
>>>> +++++++++++++++++++++++++++++++++++++++
>>>>>>  lib/librte_ether/rte_flow.h      |  48 +++++++++
>>>>>>  3 files changed, 266 insertions(+), 1 deletion(-)
>>>>> [...]
>>>>>> diff --git a/lib/librte_ether/rte_flow.h
>>>>>> b/lib/librte_ether/rte_flow.h index 98084ac..7142479 100644
>>>>>> --- a/lib/librte_ether/rte_flow.h
>>>>>> +++ b/lib/librte_ether/rte_flow.h
>>>>>> @@ -268,6 +268,20 @@ enum rte_flow_item_type {
>>>>>>  	 * See struct rte_flow_item_vxlan.
>>>>>>  	 */
>>>>>>  	RTE_FLOW_ITEM_TYPE_VXLAN,
>>>>>> +
>>>>>> +	/**
>>>>>> +	 * Matches a E_TAG header.
>>>>>> +	 *
>>>>>> +	 * See struct rte_flow_item_e_tag.
>>>>>> +	 */
>>>>>> +	RTE_FLOW_ITEM_TYPE_E_TAG,
>>>>>> +
>>>>>> +	/**
>>>>>> +	 * Matches a NVGRE header.
>>>>>> +	 *
>>>>>> +	 * See struct rte_flow_item_nvgre.
>>>>>> +	 */
>>>>>> +	RTE_FLOW_ITEM_TYPE_NVGRE,
>>>>>>  };
>>>>>>
>>>>>>  /**
>>>>>> @@ -454,6 +468,40 @@ struct rte_flow_item_vxlan {  };
>>>>>>
>>>>>>  /**
>>>>>> + * RTE_FLOW_ITEM_TYPE_E_TAG.
>>>>>> + *
>>>>>> + * Matches a E-tag header.
>>>>>> + */
>>>>>> +struct rte_flow_item_e_tag {
>>>>>> +	uint16_t tpid; /**< Tag protocol identifier (0x893F). */
>>>>>> +	/** E-Tag control information (E-TCI). */
>>>>>> +	/**< E-PCP (3b), E-DEI (1b), ingress E-CID base (12b). */
>>>>>> +	uint16_t epcp_edei_in_ecid_b;
>>>>>> +	/**< Reserved (2b), GRP (2b), E-CID base (12b). */
>>>>>> +	uint16_t rsvd_grp_ecid_b;
>>>>>> +	uint8_t in_ecid_e; /**< Ingress E-CID ext. */
>>>>>> +	uint8_t ecid_e; /**< E-CID ext. */ };
>>>>>> +
>>>>>> +/**
>>>>>> + * RTE_FLOW_ITEM_TYPE_NVGRE.
>>>>>> + *
>>>>>> + * Matches a NVGRE header.
>>>>>> + */
>>>>>> +struct rte_flow_item_nvgre {
>>>>>> +     /**
>>>>>> +      * Checksum (1b), undefined (1b), key bit (1b), sequence number
>> (1b),
>>>>>> +      * reserved 0 (9b), version (3b).
>>>>>> +      *
>>>>>> +      * \c_k_s_rsvd0_ver must have value 0x2000 according to RFC 7637.
>>>>>> +      */
>>>>>> +	uint16_t c_k_s_rsvd0_ver;
>>>>>> +	uint16_t protocol; /**< Protocol type (0x6558). */
>>>>>> +	uint8_t tni[3]; /**< Virtual subnet ID. */
>>>>>> +	uint8_t flow_id; /**< Flow ID. */ };
>>>>>> +
>>>>>> +/**
>>>>>>   * Matching pattern item definition.
>>>>>>   *
>>>>>>   * A pattern is formed by stacking items starting from the lowest
>>>>>> protocol
>>>>>> --
>>>>>> 2.5.5
>>>>>>
>>>>>
>>>>> OK for these definitions, however API documentation
>>>>> (doc/guides/prog_guide/rte_flow.rst) must be kept up to date, and it
>>>>> would be great if testpmd support for these new items was added
>>>>> simultaneously (changes in app/test-pmd/cmdline.c,
>>>>> app/test-pmd/cmdline_flow.c and
>>>> doc/guides/testpmd_app_ug/testpmd_funcs.rst).
>>>>>
>>>>> How about putting all rte_flow changes (API & testpmd) in their own
>>>>> separate patch?
>>>>
>>>> I thought it can be more useful to have library and its user updated
>>>> in same patch, gives more context. But missed rte_flow documentation ...
>>>>
>>>>>
>>>>> You could use VLAN PCP/DEI/VID definitions as an example to expose
>>>>> partial bit-fields (e.g. epcp_edei_in_ecid_b) in testpmd, see:
>>>>>
>>>>>  1419fd5a6c9f ("app/testpmd: add protocol fields to flow command")
>>>>>
>>>>> Now if re-spinning this series yet again is too much work, you can
>>>>> go ahead with this commit as long as you do not forget to submit the
>>>>> rest later, thanks.
>>>>>
>>>>
>>>> Is following todo list complete:
>>>> 1- update rte_flow document, doc/guides/prog_guide/rte_flow.rst,
>>>> document two new item types: E_TAG & NVGRE.
>>>>
>>>> 2- Add testpmd sample implementation and documentation.
>>>>
>>>
>>> I am sorry for miss rte_flow.rst document update, I DON'T know there is
>> such a new file of rte_flow.rst.
>>> And also these two types of E_TAG & NVGRE are added into code after
>>> rte_flow patch, So testpmd  implementation do not support for these type.
>>> Now, we have been work on task of 17.05.
>>> How about finish (1) first as one patch, then after busy work of 17.05  to
>> add (2) as another patch?
>>> OR, if these two work merge in to patch set,   I will may be begin to do after
>> the 17.05  task finish?
>>> Which one is OK for you?
>>
>> Changes can be in two separate patches, and if possible, can this get priority
>> against 17.05 task?
>> This is for 17.02 feature and not completely finished, missing for last touches..
>>
>> Thanks,
>> ferruh
>>
>>>
>>> Thank you.
>>>>
>>>> Hi Wei,
>>>>
>>>> Would you mind working on a patch to cover above items?
>>>>
>>>> Thanks,
>>>> ferruh
>>>
> 
> Add testpmd implementation example is not in our plan from the begin of the task generic filter API by us,
> Because all this part is not covered by us(intel) when task allocation.

Right, sample testpmd implementation is not directly related to the
adding rte_flow support to the driver.

But for this, a rte_flow library update required, and sample testpmd
implementation is part of rte_flow update.

> BUT if this component  is need by community, I will report to my leader and add it into our next work plan and try to finish it ASAP.

Yes please, I believe it is needed.

Thanks,
ferruh

> 
> Thank you.
>