[dpdk-dev] [PATCH 0/2] ethdev: add support for raw flow type for flow director

[dpdk-dev] [PATCH 0/2] ethdev: add support for raw flow type for flow director

[Kirill Rybalchenko]

For complex packets use raw flow type with pre-constructed packet buffer
instead of creating a packet internally in PMD.

Kirill Rybalchenko (2):
  ethdev: add support for raw flow type for flow director
  net/i40e: add support for raw flow type for flow director

 drivers/net/i40e/i40e_fdir.c    | 27 +++++++++++++++++++--------
 lib/librte_ether/rte_eth_ctrl.h | 10 ++++++++++
 2 files changed, 29 insertions(+), 8 deletions(-)

-- 
2.5.5

[dpdk-dev] [PATCH 1/2] ethdev: add support for raw flow type for flow director

[Kirill Rybalchenko]

Add new structure rte_eth_raw_flow to the union rte_eth_fdir_flow
to support filter for raw flow type

Signed-off-by: Kirill Rybalchenko <kirill.rybalchenko@intel.com>
---
 lib/librte_ether/rte_eth_ctrl.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/lib/librte_ether/rte_eth_ctrl.h b/lib/librte_ether/rte_eth_ctrl.h
index 8386904..22d9640 100644
--- a/lib/librte_ether/rte_eth_ctrl.h
+++ b/lib/librte_ether/rte_eth_ctrl.h
@@ -525,6 +525,15 @@ struct rte_eth_tunnel_flow {
 };
 
 /**
+ * A structure used to define the input for raw flow
+ */
+struct rte_eth_raw_flow {
+	uint16_t flow; /**< flow type. */
+	void *packet; /**< pre-constructed packet buffer. */
+	uint16_t length; /**< buffer length. */
+};
+
+/**
  * An union contains the inputs for all types of flow
  * Items in flows need to be in big endian
  */
@@ -540,6 +549,7 @@ union rte_eth_fdir_flow {
 	struct rte_eth_ipv6_flow   ipv6_flow;
 	struct rte_eth_mac_vlan_flow mac_vlan_flow;
 	struct rte_eth_tunnel_flow   tunnel_flow;
+	struct rte_eth_raw_flow    raw_flow;
 };
 
 /**
-- 
2.5.5

Re: [dpdk-dev] [PATCH 1/2] ethdev: add support for raw flow type for flow director

[Radu Nicolau]

On 8/24/2017 4:30 PM, Kirill Rybalchenko wrote:
> Add new structure rte_eth_raw_flow to the union rte_eth_fdir_flow
> to support filter for raw flow type
>
> Signed-off-by: Kirill Rybalchenko <kirill.rybalchenko@intel.com>
> ---
>   lib/librte_ether/rte_eth_ctrl.h | 10 ++++++++++
>   1 file changed, 10 insertions(+)
>
> diff --git a/lib/librte_ether/rte_eth_ctrl.h b/lib/librte_ether/rte_eth_ctrl.h
> index 8386904..22d9640 100644
> --- a/lib/librte_ether/rte_eth_ctrl.h
> +++ b/lib/librte_ether/rte_eth_ctrl.h
> @@ -525,6 +525,15 @@ struct rte_eth_tunnel_flow {
>   };
>   
>   /**
> + * A structure used to define the input for raw flow
> + */
> +struct rte_eth_raw_flow {
> +	uint16_t flow; /**< flow type. */
> +	void *packet; /**< pre-constructed packet buffer. */
> +	uint16_t length; /**< buffer length. */
> +};
Nitpicking on comments alignment.
> +
> +/**
>    * An union contains the inputs for all types of flow
>    * Items in flows need to be in big endian
>    */
> @@ -540,6 +549,7 @@ union rte_eth_fdir_flow {
>   	struct rte_eth_ipv6_flow   ipv6_flow;
>   	struct rte_eth_mac_vlan_flow mac_vlan_flow;
>   	struct rte_eth_tunnel_flow   tunnel_flow;
> +	struct rte_eth_raw_flow    raw_flow;
>   };
>   
>   /**

[dpdk-dev] [PATCH 2/2] net/i40e: add support for raw flow type for flow director

[Kirill Rybalchenko]

When addidng flow director filter for raw flow type, instead
of constructing packet use buffer with pre-constructed packet.

Signed-off-by: Kirill Rybalchenko <kirill.rybalchenko@intel.com>
---
 drivers/net/i40e/i40e_fdir.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c
index 8013add..0558914 100644
--- a/drivers/net/i40e/i40e_fdir.c
+++ b/drivers/net/i40e/i40e_fdir.c
@@ -1093,6 +1093,7 @@ i40e_add_del_fdir_filter(struct rte_eth_dev *dev,
 	struct i40e_fdir_filter *fdir_filter, *node;
 	struct i40e_fdir_filter check_filter; /* Check if the filter exists */
 	int ret = 0;
+	uint16_t flow_type = filter->input.flow_type;
 
 	if (dev->data->dev_conf.fdir_conf.mode != RTE_FDIR_MODE_PERFECT) {
 		PMD_DRV_LOG(ERR, "FDIR is not enabled, please"
@@ -1100,7 +1101,7 @@ i40e_add_del_fdir_filter(struct rte_eth_dev *dev,
 		return -ENOTSUP;
 	}
 
-	if (!I40E_VALID_FLOW(filter->input.flow_type)) {
+	if (flow_type != RTE_ETH_FLOW_RAW && !I40E_VALID_FLOW(flow_type)) {
 		PMD_DRV_LOG(ERR, "invalid flow_type input.");
 		return -EINVAL;
 	}
@@ -1132,20 +1133,30 @@ i40e_add_del_fdir_filter(struct rte_eth_dev *dev,
 
 	memset(pkt, 0, I40E_FDIR_PKT_LEN);
 
-	ret = i40e_fdir_construct_pkt(pf, &filter->input, pkt);
-	if (ret < 0) {
-		PMD_DRV_LOG(ERR, "construct packet for fdir fails.");
-		return ret;
+	if (flow_type == RTE_ETH_FLOW_RAW) {
+		if (filter->input.flow.raw_flow.length > I40E_FDIR_PKT_LEN ||
+		    !filter->input.flow.raw_flow.packet ||
+		    !I40E_VALID_FLOW(filter->input.flow.raw_flow.flow)) {
+			PMD_DRV_LOG(ERR, "Invalid raw flow filter parameters!");
+		}
+		memcpy(pkt, filter->input.flow.raw_flow.packet,
+		       filter->input.flow.raw_flow.length);
+		flow_type = filter->input.flow.raw_flow.flow;
+	} else {
+		ret = i40e_fdir_construct_pkt(pf, &filter->input, pkt);
+		if (ret < 0) {
+			PMD_DRV_LOG(ERR, "construct packet for fdir fails.");
+			return ret;
+		}
 	}
 
 	if (hw->mac.type == I40E_MAC_X722) {
 		/* get translated pctype value in fd pctype register */
 		pctype = (enum i40e_filter_pctype)i40e_read_rx_ctl(
 			hw, I40E_GLQF_FD_PCTYPES(
-			(int)i40e_flowtype_to_pctype(
-			filter->input.flow_type)));
+			(int)i40e_flowtype_to_pctype(flow_type)));
 	} else
-		pctype = i40e_flowtype_to_pctype(filter->input.flow_type);
+		pctype = i40e_flowtype_to_pctype(flow_type);
 
 	ret = i40e_fdir_filter_programming(pf, pctype, filter, add);
 	if (ret < 0) {
-- 
2.5.5

Re: [dpdk-dev] [PATCH 2/2] net/i40e: add support for raw flow type for flow director

[Radu Nicolau]

On 8/24/2017 4:30 PM, Kirill Rybalchenko wrote:
> When addidng flow director filter for raw flow type, instead
> of constructing packet use buffer with pre-constructed packet.
>
> Signed-off-by: Kirill Rybalchenko <kirill.rybalchenko@intel.com>
> ---
>   drivers/net/i40e/i40e_fdir.c | 27 +++++++++++++++++++--------
>   1 file changed, 19 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c
> index 8013add..0558914 100644
> --- a/drivers/net/i40e/i40e_fdir.c
> +++ b/drivers/net/i40e/i40e_fdir.c
> @@ -1093,6 +1093,7 @@ i40e_add_del_fdir_filter(struct rte_eth_dev *dev,
>   	struct i40e_fdir_filter *fdir_filter, *node;
>   	struct i40e_fdir_filter check_filter; /* Check if the filter exists */
>   	int ret = 0;
> +	uint16_t flow_type = filter->input.flow_type;
>   
>   	if (dev->data->dev_conf.fdir_conf.mode != RTE_FDIR_MODE_PERFECT) {
>   		PMD_DRV_LOG(ERR, "FDIR is not enabled, please"
> @@ -1100,7 +1101,7 @@ i40e_add_del_fdir_filter(struct rte_eth_dev *dev,
>   		return -ENOTSUP;
>   	}
>   
> -	if (!I40E_VALID_FLOW(filter->input.flow_type)) {
> +	if (flow_type != RTE_ETH_FLOW_RAW && !I40E_VALID_FLOW(flow_type)) {
Why not expand I40E_VALID_FLOW to check for RTE_ETH_FLOW_RAW as well?
>   		PMD_DRV_LOG(ERR, "invalid flow_type input.");
>   		return -EINVAL;
>   	}
> @@ -1132,20 +1133,30 @@ i40e_add_del_fdir_filter(struct rte_eth_dev *dev,
>   
>   	memset(pkt, 0, I40E_FDIR_PKT_LEN);
>   
> -	ret = i40e_fdir_construct_pkt(pf, &filter->input, pkt);
> -	if (ret < 0) {
> -		PMD_DRV_LOG(ERR, "construct packet for fdir fails.");
> -		return ret;
> +	if (flow_type == RTE_ETH_FLOW_RAW) {
> +		if (filter->input.flow.raw_flow.length > I40E_FDIR_PKT_LEN ||
> +		    !filter->input.flow.raw_flow.packet ||
> +		    !I40E_VALID_FLOW(filter->input.flow.raw_flow.flow)) {
> +			PMD_DRV_LOG(ERR, "Invalid raw flow filter parameters!");
> +		}
> +		memcpy(pkt, filter->input.flow.raw_flow.packet,
> +		       filter->input.flow.raw_flow.length);
> +		flow_type = filter->input.flow.raw_flow.flow;
> +	} else {
> +		ret = i40e_fdir_construct_pkt(pf, &filter->input, pkt);
> +		if (ret < 0) {
> +			PMD_DRV_LOG(ERR, "construct packet for fdir fails.");
> +			return ret;
> +		}
>   	}
>   
>   	if (hw->mac.type == I40E_MAC_X722) {
>   		/* get translated pctype value in fd pctype register */
>   		pctype = (enum i40e_filter_pctype)i40e_read_rx_ctl(
>   			hw, I40E_GLQF_FD_PCTYPES(
> -			(int)i40e_flowtype_to_pctype(
> -			filter->input.flow_type)));
> +			(int)i40e_flowtype_to_pctype(flow_type)));
>   	} else
> -		pctype = i40e_flowtype_to_pctype(filter->input.flow_type);
> +		pctype = i40e_flowtype_to_pctype(flow_type);
>   
>   	ret = i40e_fdir_filter_programming(pf, pctype, filter, add);
>   	if (ret < 0) {

[dpdk-dev] [PATCH v2 0/2] ethdev: add support for raw flow type for flow director

[Kirill Rybalchenko]

For complex packets use raw flow type with pre-constructed packet buffer
instead of creating a packet internally in PMD.

v2:
Fixed code style, comment added

Kirill Rybalchenko (2):
  ethdev: add support for raw flow type for flow director
  net/i40e: add support for raw flow type for flow director

 drivers/net/i40e/i40e_fdir.c    | 31 +++++++++++++++++++++++--------
 lib/librte_ether/rte_eth_ctrl.h | 10 ++++++++++
 2 files changed, 33 insertions(+), 8 deletions(-)

-- 
2.5.5

[dpdk-dev] [PATCH v2 1/2] ethdev: add support for raw flow type for flow director

[Kirill Rybalchenko]

Add new structure rte_eth_raw_flow to the union rte_eth_fdir_flow
to support filter for raw flow type

Signed-off-by: Kirill Rybalchenko <kirill.rybalchenko@intel.com>
---
 lib/librte_ether/rte_eth_ctrl.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/lib/librte_ether/rte_eth_ctrl.h b/lib/librte_ether/rte_eth_ctrl.h
index 8386904..213b408 100644
--- a/lib/librte_ether/rte_eth_ctrl.h
+++ b/lib/librte_ether/rte_eth_ctrl.h
@@ -525,6 +525,15 @@ struct rte_eth_tunnel_flow {
 };
 
 /**
+ * A structure used to define the input for raw flow
+ */
+struct rte_eth_raw_flow {
+	uint16_t flow;   /**< flow type. */
+	void *packet;    /**< pre-constructed packet buffer. */
+	uint16_t length; /**< buffer length. */
+};
+
+/**
  * An union contains the inputs for all types of flow
  * Items in flows need to be in big endian
  */
@@ -540,6 +549,7 @@ union rte_eth_fdir_flow {
 	struct rte_eth_ipv6_flow   ipv6_flow;
 	struct rte_eth_mac_vlan_flow mac_vlan_flow;
 	struct rte_eth_tunnel_flow   tunnel_flow;
+	struct rte_eth_raw_flow    raw_flow;
 };
 
 /**
-- 
2.5.5

[dpdk-dev] [PATCH v2 2/2] net/i40e: add support for raw flow type for flow director

[Kirill Rybalchenko]

When addidng flow director filter for raw flow type, instead
of constructing packet use buffer with pre-constructed packet.

Signed-off-by: Kirill Rybalchenko <kirill.rybalchenko@intel.com>
---
 drivers/net/i40e/i40e_fdir.c | 31 +++++++++++++++++++++++--------
 1 file changed, 23 insertions(+), 8 deletions(-)

diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c
index 84c0a1f..d609bd5 100644
--- a/drivers/net/i40e/i40e_fdir.c
+++ b/drivers/net/i40e/i40e_fdir.c
@@ -1093,6 +1093,7 @@ i40e_add_del_fdir_filter(struct rte_eth_dev *dev,
 	struct i40e_fdir_filter *fdir_filter, *node;
 	struct i40e_fdir_filter check_filter; /* Check if the filter exists */
 	int ret = 0;
+	uint16_t flow_type = filter->input.flow_type;
 
 	if (dev->data->dev_conf.fdir_conf.mode != RTE_FDIR_MODE_PERFECT) {
 		PMD_DRV_LOG(ERR, "FDIR is not enabled, please"
@@ -1100,7 +1101,11 @@ i40e_add_del_fdir_filter(struct rte_eth_dev *dev,
 		return -ENOTSUP;
 	}
 
-	if (!I40E_VALID_FLOW(filter->input.flow_type)) {
+	/*
+	 * This check against statically defined flow types will be removed
+	 * once we implement dynamic mappings of flow types to pctypes
+	 */
+	if (flow_type != RTE_ETH_FLOW_RAW && !I40E_VALID_FLOW(flow_type)) {
 		PMD_DRV_LOG(ERR, "invalid flow_type input.");
 		return -EINVAL;
 	}
@@ -1132,20 +1137,30 @@ i40e_add_del_fdir_filter(struct rte_eth_dev *dev,
 
 	memset(pkt, 0, I40E_FDIR_PKT_LEN);
 
-	ret = i40e_fdir_construct_pkt(pf, &filter->input, pkt);
-	if (ret < 0) {
-		PMD_DRV_LOG(ERR, "construct packet for fdir fails.");
-		return ret;
+	if (flow_type == RTE_ETH_FLOW_RAW) {
+		if (filter->input.flow.raw_flow.length > I40E_FDIR_PKT_LEN ||
+		    !filter->input.flow.raw_flow.packet ||
+		    !I40E_VALID_FLOW(filter->input.flow.raw_flow.flow)) {
+			PMD_DRV_LOG(ERR, "Invalid raw flow filter parameters!");
+		}
+		memcpy(pkt, filter->input.flow.raw_flow.packet,
+		       filter->input.flow.raw_flow.length);
+		flow_type = filter->input.flow.raw_flow.flow;
+	} else {
+		ret = i40e_fdir_construct_pkt(pf, &filter->input, pkt);
+		if (ret < 0) {
+			PMD_DRV_LOG(ERR, "construct packet for fdir fails.");
+			return ret;
+		}
 	}
 
 	if (hw->mac.type == I40E_MAC_X722) {
 		/* get translated pctype value in fd pctype register */
 		pctype = (enum i40e_filter_pctype)i40e_read_rx_ctl(
 			hw, I40E_GLQF_FD_PCTYPES(
-			(int)i40e_flowtype_to_pctype(
-			filter->input.flow_type)));
+			(int)i40e_flowtype_to_pctype(flow_type)));
 	} else
-		pctype = i40e_flowtype_to_pctype(filter->input.flow_type);
+		pctype = i40e_flowtype_to_pctype(flow_type);
 
 	ret = i40e_fdir_filter_programming(pf, pctype, filter, add);
 	if (ret < 0) {
-- 
2.5.5

Re: [dpdk-dev] [PATCH v2 0/2] ethdev: add support for raw flow type for flow director

[Ferruh Yigit]

On 9/20/2017 9:42 AM, Kirill Rybalchenko wrote:
> For complex packets use raw flow type with pre-constructed packet buffer
> instead of creating a packet internally in PMD.
> 
> v2:
> Fixed code style, comment added
> 
> Kirill Rybalchenko (2):
>   ethdev: add support for raw flow type for flow director
>   net/i40e: add support for raw flow type for flow director

Series Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

Re: [dpdk-dev] [PATCH v2 0/2] ethdev: add support for raw flow type for flow director

[Thomas Monjalon]

03/10/2017 21:02, Ferruh Yigit:
> On 9/20/2017 9:42 AM, Kirill Rybalchenko wrote:
> > For complex packets use raw flow type with pre-constructed packet buffer
> > instead of creating a packet internally in PMD.

Sorry for not catching this series before.

As it has been said several times on this mailing list,
the flow director API is deprecated.
I think everybody here knows that it is going to be replaced
by rte_flow.

That's why it does not make sense to extend flow director.
We are not going to update PMDs to support a new type
of legacy flow director.
Please focus your efforts on rte_flow.


> > Kirill Rybalchenko (2):
> >   ethdev: add support for raw flow type for flow director
> >   net/i40e: add support for raw flow type for flow director
> 
> Series Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

Re: [dpdk-dev] [PATCH v2 0/2] ethdev: add support for raw flow type for flow director

[Ferruh Yigit]

On 10/4/2017 5:57 PM, Thomas Monjalon wrote:
> 03/10/2017 21:02, Ferruh Yigit:
>> On 9/20/2017 9:42 AM, Kirill Rybalchenko wrote:
>>> For complex packets use raw flow type with pre-constructed packet buffer
>>> instead of creating a packet internally in PMD.
> 
> Sorry for not catching this series before.
> 
> As it has been said several times on this mailing list,
> the flow director API is deprecated.
> I think everybody here knows that it is going to be replaced
> by rte_flow.
> 
> That's why it does not make sense to extend flow director.
> We are not going to update PMDs to support a new type
> of legacy flow director.
> Please focus your efforts on rte_flow.

As far as I can see this is not to to extend flow director. But driver
uses this struct and adding a new feature into driver requires update in
this struct.

I guess idea was for new filter functionalities PMD should use rte_flow,
that is the new shiny method we have, I see the point here. But I don't
see the point of making these structs under use completely immutable.

> 
> 
>>> Kirill Rybalchenko (2):
>>>   ethdev: add support for raw flow type for flow director
>>>   net/i40e: add support for raw flow type for flow director
>>
>> Series Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 

Re: [dpdk-dev] [PATCH v2 0/2] ethdev: add support for raw flow type for flow director

[Thomas Monjalon]

04/10/2017 19:44, Ferruh Yigit:
> On 10/4/2017 5:57 PM, Thomas Monjalon wrote:
> > 03/10/2017 21:02, Ferruh Yigit:
> >> On 9/20/2017 9:42 AM, Kirill Rybalchenko wrote:
> >>> For complex packets use raw flow type with pre-constructed packet buffer
> >>> instead of creating a packet internally in PMD.
> > 
> > Sorry for not catching this series before.
> > 
> > As it has been said several times on this mailing list,
> > the flow director API is deprecated.
> > I think everybody here knows that it is going to be replaced
> > by rte_flow.
> > 
> > That's why it does not make sense to extend flow director.
> > We are not going to update PMDs to support a new type
> > of legacy flow director.
> > Please focus your efforts on rte_flow.
> 
> As far as I can see this is not to to extend flow director. But driver
> uses this struct and adding a new feature into driver requires update in
> this struct.
> 
> I guess idea was for new filter functionalities PMD should use rte_flow,
> that is the new shiny method we have, I see the point here. But I don't
> see the point of making these structs under use completely immutable.

I don't know what is RTE_ETH_FLOW_RAW.
Let's start by explaining it, how it is used by users,
and why this struct is needed.
Thanks

Re: [dpdk-dev] [PATCH v2 0/2] ethdev: add support for raw flow type for flow director

[Ferruh Yigit]

On 10/4/2017 6:56 PM, Thomas Monjalon wrote:
> 04/10/2017 19:44, Ferruh Yigit:
>> On 10/4/2017 5:57 PM, Thomas Monjalon wrote:
>>> 03/10/2017 21:02, Ferruh Yigit:
>>>> On 9/20/2017 9:42 AM, Kirill Rybalchenko wrote:
>>>>> For complex packets use raw flow type with pre-constructed packet buffer
>>>>> instead of creating a packet internally in PMD.
>>>
>>> Sorry for not catching this series before.
>>>
>>> As it has been said several times on this mailing list,
>>> the flow director API is deprecated.
>>> I think everybody here knows that it is going to be replaced
>>> by rte_flow.
>>>
>>> That's why it does not make sense to extend flow director.
>>> We are not going to update PMDs to support a new type
>>> of legacy flow director.
>>> Please focus your efforts on rte_flow.
>>
>> As far as I can see this is not to to extend flow director. But driver
>> uses this struct and adding a new feature into driver requires update in
>> this struct.
>>
>> I guess idea was for new filter functionalities PMD should use rte_flow,
>> that is the new shiny method we have, I see the point here. But I don't
>> see the point of making these structs under use completely immutable.
> 
> I don't know what is RTE_ETH_FLOW_RAW.
> Let's start by explaining it, how it is used by users,
> and why this struct is needed.

Let me answer as much as I get from patches, if something is missing or
wrong Kirill needs to correct it.

Driver (i40e) works with static pre-defined flow and pctypes. But new
feature DDP lets loading custom profiles and work with custom and
perhaps not yet defined flow and pctypes. So there are a few other
patches to make driver more dynamic.

For this case I40E_VALID_FLOW() fails with custom flow types, and
RTE_ETH_FLOW_RAW used as kind of (void *) to be able to work with new
dynamic types.

> Thanks
> 

Re: [dpdk-dev] [PATCH v2 0/2] ethdev: add support for raw flow type for flow director

[Thomas Monjalon]

04/10/2017 21:47, Ferruh Yigit:
> On 10/4/2017 6:56 PM, Thomas Monjalon wrote:
> > 04/10/2017 19:44, Ferruh Yigit:
> >> On 10/4/2017 5:57 PM, Thomas Monjalon wrote:
> >>> 03/10/2017 21:02, Ferruh Yigit:
> >>>> On 9/20/2017 9:42 AM, Kirill Rybalchenko wrote:
> >>>>> For complex packets use raw flow type with pre-constructed packet buffer
> >>>>> instead of creating a packet internally in PMD.
> >>>
> >>> Sorry for not catching this series before.
> >>>
> >>> As it has been said several times on this mailing list,
> >>> the flow director API is deprecated.
> >>> I think everybody here knows that it is going to be replaced
> >>> by rte_flow.
> >>>
> >>> That's why it does not make sense to extend flow director.
> >>> We are not going to update PMDs to support a new type
> >>> of legacy flow director.
> >>> Please focus your efforts on rte_flow.
> >>
> >> As far as I can see this is not to to extend flow director. But driver
> >> uses this struct and adding a new feature into driver requires update in
> >> this struct.
> >>
> >> I guess idea was for new filter functionalities PMD should use rte_flow,
> >> that is the new shiny method we have, I see the point here. But I don't
> >> see the point of making these structs under use completely immutable.
> > 
> > I don't know what is RTE_ETH_FLOW_RAW.
> > Let's start by explaining it, how it is used by users,
> > and why this struct is needed.
> 
> Let me answer as much as I get from patches, if something is missing or
> wrong Kirill needs to correct it.
> 
> Driver (i40e) works with static pre-defined flow and pctypes. But new
> feature DDP lets loading custom profiles and work with custom and
> perhaps not yet defined flow and pctypes. So there are a few other
> patches to make driver more dynamic.
> 
> For this case I40E_VALID_FLOW() fails with custom flow types, and
> RTE_ETH_FLOW_RAW used as kind of (void *) to be able to work with new
> dynamic types.

So the application could use a new flow type?
If an application has to be modified for this, it can use rte_flow.
Again, no new feature in legacy flow director.

Re: [dpdk-dev] [PATCH v2 0/2] ethdev: add support for raw flow type for flow director

[Rybalchenko, Kirill]

Hi Thomas,

As Ferruh rightly said, this feature does not affect any
other drivers except i40e.
It was implemented as a temporary measure to accelerate
adoption of new protocols in DPDK.
It does not eliminate importance of rte_flow.
As you can see, we've already added GTP as a part of rte_flow.
(see Beilei Xing patch http://dpdk.org/ml/archives/dev/2017-October/077483.html)

Regards,
Kirill.

> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Wednesday 4 October 2017 20:47
> To: Thomas Monjalon <thomas@monjalon.net>; Rybalchenko, Kirill
> <kirill.rybalchenko@intel.com>
> Cc: dev@dpdk.org; Chilikin, Andrey <andrey.chilikin@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v2 0/2] ethdev: add support for raw flow
> type for flow director
> 
> On 10/4/2017 6:56 PM, Thomas Monjalon wrote:
> > 04/10/2017 19:44, Ferruh Yigit:
> >> On 10/4/2017 5:57 PM, Thomas Monjalon wrote:
> >>> 03/10/2017 21:02, Ferruh Yigit:
> >>>> On 9/20/2017 9:42 AM, Kirill Rybalchenko wrote:
> >>>>> For complex packets use raw flow type with pre-constructed packet
> >>>>> buffer instead of creating a packet internally in PMD.
> >>>
> >>> Sorry for not catching this series before.
> >>>
> >>> As it has been said several times on this mailing list, the flow
> >>> director API is deprecated.
> >>> I think everybody here knows that it is going to be replaced by
> >>> rte_flow.
> >>>
> >>> That's why it does not make sense to extend flow director.
> >>> We are not going to update PMDs to support a new type of legacy flow
> >>> director.
> >>> Please focus your efforts on rte_flow.
> >>
> >> As far as I can see this is not to to extend flow director. But
> >> driver uses this struct and adding a new feature into driver requires
> >> update in this struct.
> >>
> >> I guess idea was for new filter functionalities PMD should use
> >> rte_flow, that is the new shiny method we have, I see the point here.
> >> But I don't see the point of making these structs under use completely
> immutable.
> >
> > I don't know what is RTE_ETH_FLOW_RAW.
> > Let's start by explaining it, how it is used by users, and why this
> > struct is needed.
> 
> Let me answer as much as I get from patches, if something is missing or
> wrong Kirill needs to correct it.
> 
> Driver (i40e) works with static pre-defined flow and pctypes. But new
> feature DDP lets loading custom profiles and work with custom and perhaps
> not yet defined flow and pctypes. So there are a few other patches to make
> driver more dynamic.
> 
> For this case I40E_VALID_FLOW() fails with custom flow types, and
> RTE_ETH_FLOW_RAW used as kind of (void *) to be able to work with new
> dynamic types.
> 
> > Thanks
> >

[dpdk-dev] [PATCH v3 0/4] ethdev: add support for raw flow type for flow director

[Kirill Rybalchenko]

For complex packets use raw flow type with pre-constructed packet buffer
instead of creating a packet internally in PMD.

v2:
Fixed code style, comment added

v3:
Merged with dunamic pctype to flow type mapping patch.
Added raw fode to flow_director_filter command in testpmd.

Kirill Rybalchenko (4):
  ethdev: add support for raw flow type for flow director
  net/i40e: add support for raw flow type for flow director
  app/testpmd: add raw flow type to flow director
  doc: add description of raw flow type in flow director in testpmd

 app/test-pmd/cmdline.c                      | 69 +++++++++++++++++++++++++++--
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  4 ++
 drivers/net/i40e/i40e_fdir.c                | 25 ++++++++---
 lib/librte_ether/rte_eth_ctrl.h             | 10 +++++
 4 files changed, 100 insertions(+), 8 deletions(-)

-- 
2.5.5

[dpdk-dev] [PATCH v3 1/4] ethdev: add support for raw flow type for flow director

[Kirill Rybalchenko]

Add new structure rte_eth_raw_flow to the union rte_eth_fdir_flow
to support filter for raw flow type.

Signed-off-by: Kirill Rybalchenko <kirill.rybalchenko@intel.com>
---
 lib/librte_ether/rte_eth_ctrl.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/lib/librte_ether/rte_eth_ctrl.h b/lib/librte_ether/rte_eth_ctrl.h
index 8386904..213b408 100644
--- a/lib/librte_ether/rte_eth_ctrl.h
+++ b/lib/librte_ether/rte_eth_ctrl.h
@@ -525,6 +525,15 @@ struct rte_eth_tunnel_flow {
 };
 
 /**
+ * A structure used to define the input for raw flow
+ */
+struct rte_eth_raw_flow {
+	uint16_t flow;   /**< flow type. */
+	void *packet;    /**< pre-constructed packet buffer. */
+	uint16_t length; /**< buffer length. */
+};
+
+/**
  * An union contains the inputs for all types of flow
  * Items in flows need to be in big endian
  */
@@ -540,6 +549,7 @@ union rte_eth_fdir_flow {
 	struct rte_eth_ipv6_flow   ipv6_flow;
 	struct rte_eth_mac_vlan_flow mac_vlan_flow;
 	struct rte_eth_tunnel_flow   tunnel_flow;
+	struct rte_eth_raw_flow    raw_flow;
 };
 
 /**
-- 
2.5.5

[dpdk-dev] [PATCH v3 2/4] net/i40e: add support for raw flow type for flow director

[Kirill Rybalchenko]

When addidng flow director filter for raw flow type, instead
of constructing packet use buffer with pre-constructed packet.

v3:
Merge with dynamic pctype to flow type mapping patch.

Signed-off-by: Kirill Rybalchenko <kirill.rybalchenko@intel.com>
---
 drivers/net/i40e/i40e_fdir.c | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c
index 268ada0..8eca9fe 100644
--- a/drivers/net/i40e/i40e_fdir.c
+++ b/drivers/net/i40e/i40e_fdir.c
@@ -1099,7 +1099,12 @@ i40e_add_del_fdir_filter(struct rte_eth_dev *dev,
 		return -ENOTSUP;
 	}
 
-	pctype = i40e_flowtype_to_pctype(pf->adapter, filter->input.flow_type);
+	if (filter->input.flow_type == RTE_ETH_FLOW_RAW)
+		pctype = i40e_flowtype_to_pctype(pf->adapter,
+					filter->input.flow.raw_flow.flow);
+	else
+		pctype = i40e_flowtype_to_pctype(pf->adapter,
+					filter->input.flow_type);
 	if (pctype == I40E_FILTER_PCTYPE_INVALID) {
 		PMD_DRV_LOG(ERR, "invalid flow_type input.");
 		return -EINVAL;
@@ -1132,10 +1137,20 @@ i40e_add_del_fdir_filter(struct rte_eth_dev *dev,
 
 	memset(pkt, 0, I40E_FDIR_PKT_LEN);
 
-	ret = i40e_fdir_construct_pkt(pf, &filter->input, pkt);
-	if (ret < 0) {
-		PMD_DRV_LOG(ERR, "construct packet for fdir fails.");
-		return ret;
+	if (filter->input.flow_type == RTE_ETH_FLOW_RAW) {
+		if (filter->input.flow.raw_flow.length > I40E_FDIR_PKT_LEN ||
+		    !filter->input.flow.raw_flow.packet) {
+			PMD_DRV_LOG(ERR, "Invalid raw flow filter parameters!");
+			return -EINVAL;
+		}
+		memcpy(pkt, filter->input.flow.raw_flow.packet,
+		       filter->input.flow.raw_flow.length);
+	} else {
+		ret = i40e_fdir_construct_pkt(pf, &filter->input, pkt);
+		if (ret < 0) {
+			PMD_DRV_LOG(ERR, "construct packet for fdir fails.");
+			return ret;
+		}
 	}
 
 	if (hw->mac.type == I40E_MAC_X722) {
-- 
2.5.5

[dpdk-dev] [PATCH v3 3/4] app/testpmd: add raw flow type to flow director

[Kirill Rybalchenko]

v3:
Add raw flow type support to flow_director_filter command

Signed-off-by: Kirill Rybalchenko <kirill.rybalchenko@intel.com>
---
 app/test-pmd/cmdline.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 66 insertions(+), 3 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index cdde5a1..b08c1b4 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -843,6 +843,12 @@ static void cmd_help_long_parsed(void *parsed_result,
 			" queue (queue_id) fd_id (fd_id_value)\n"
 			"    Add/Del a Tunnel flow director filter.\n\n"
 
+			"flow_director_filter (port_id) mode raw (add|del|update)"
+			" flow (flow_id) flexbytes (flexbytes_value) (drop|fwd)"
+			" pf|vf(vf_id) queue (queue_id) fd_id (fd_id_value)"
+			" packet (packet file name)\n"
+			"    Add/Del a raw type flow director filter.\n\n"
+
 			"flush_flow_director (port_id)\n"
 			"    Flush all flow director entries of a device.\n\n"
 
@@ -8904,6 +8910,7 @@ struct cmd_flow_director_result {
 	cmdline_fixed_string_t mode_value;
 	cmdline_fixed_string_t ops;
 	cmdline_fixed_string_t flow;
+	uint16_t flow_id;
 	cmdline_fixed_string_t flow_type;
 	cmdline_fixed_string_t ether;
 	uint16_t ether_type;
@@ -8937,6 +8944,8 @@ struct cmd_flow_director_result {
 	cmdline_fixed_string_t tunnel_type;
 	cmdline_fixed_string_t tunnel_id;
 	uint32_t tunnel_id_value;
+	cmdline_fixed_string_t packet;
+	char filepath[];
 };
 
 static inline int
@@ -9063,6 +9072,8 @@ cmd_flow_director_filter_parsed(void *parsed_result,
 	struct rte_eth_fdir_filter entry;
 	uint8_t flexbytes[RTE_ETH_FDIR_MAX_FLEXLEN];
 	char *end;
+	uint8_t *raw_packet_buff;
+	uint32_t raw_packet_size;
 	unsigned long vf_id;
 	int ret = 0;
 
@@ -9086,11 +9097,14 @@ cmd_flow_director_filter_parsed(void *parsed_result,
 			return;
 		}
 	} else {
-		if (strcmp(res->mode_value, "IP")) {
-			printf("Please set mode to IP.\n");
+		if (!strcmp(res->mode_value, "raw")) {
+			entry.input.flow_type = RTE_ETH_FLOW_RAW;
+		} else if (!strcmp(res->mode_value, "IP")) {
+			entry.input.flow_type = str2flowtype(res->flow_type);
+		} else {
+			printf("Please set mode to IP or raw.\n");
 			return;
 		}
-		entry.input.flow_type = str2flowtype(res->flow_type);
 	}
 
 	ret = parse_flexbytes(res->flexbytes_value,
@@ -9172,6 +9186,15 @@ cmd_flow_director_filter_parsed(void *parsed_result,
 		entry.input.flow.l2_flow.ether_type =
 			rte_cpu_to_be_16(res->ether_type);
 		break;
+	case RTE_ETH_FLOW_RAW:
+		entry.input.flow.raw_flow.flow = res->flow_id;
+		raw_packet_buff = open_ddp_package_file(res->filepath,
+							&raw_packet_size);
+		if (!raw_packet_buff)
+			return;
+		entry.input.flow.raw_flow.packet = raw_packet_buff;
+		entry.input.flow.raw_flow.length = (uint16_t)raw_packet_size;
+		break;
 	default:
 		break;
 	}
@@ -9261,6 +9284,9 @@ cmdline_parse_token_string_t cmd_flow_director_flow_type =
 	TOKEN_STRING_INITIALIZER(struct cmd_flow_director_result,
 		flow_type, "ipv4-other#ipv4-frag#ipv4-tcp#ipv4-udp#ipv4-sctp#"
 		"ipv6-other#ipv6-frag#ipv6-tcp#ipv6-udp#ipv6-sctp#l2_payload");
+cmdline_parse_token_num_t cmd_flow_director_flow_id =
+	TOKEN_NUM_INITIALIZER(struct cmd_flow_director_result,
+			      flow_id, UINT16);
 cmdline_parse_token_string_t cmd_flow_director_ether =
 	TOKEN_STRING_INITIALIZER(struct cmd_flow_director_result,
 				 ether, "ether");
@@ -9352,6 +9378,9 @@ cmdline_parse_token_string_t cmd_flow_director_mode_mac_vlan =
 cmdline_parse_token_string_t cmd_flow_director_mode_tunnel =
 	TOKEN_STRING_INITIALIZER(struct cmd_flow_director_result,
 				 mode_value, "Tunnel");
+cmdline_parse_token_string_t cmd_flow_director_mode_raw =
+	TOKEN_STRING_INITIALIZER(struct cmd_flow_director_result,
+				 mode_value, "raw");
 cmdline_parse_token_string_t cmd_flow_director_mac =
 	TOKEN_STRING_INITIALIZER(struct cmd_flow_director_result,
 				 mac, "mac");
@@ -9370,6 +9399,12 @@ cmdline_parse_token_string_t cmd_flow_director_tunnel_id =
 cmdline_parse_token_num_t cmd_flow_director_tunnel_id_value =
 	TOKEN_NUM_INITIALIZER(struct cmd_flow_director_result,
 			      tunnel_id_value, UINT32);
+cmdline_parse_token_string_t cmd_flow_director_packet =
+	TOKEN_STRING_INITIALIZER(struct cmd_flow_director_result,
+				 packet, "packet");
+cmdline_parse_token_string_t cmd_flow_director_filepath =
+	TOKEN_STRING_INITIALIZER(struct cmd_flow_director_result,
+				 filepath, NULL);
 
 cmdline_parse_inst_t cmd_add_del_ip_flow_director = {
 	.f = cmd_flow_director_filter_parsed,
@@ -9573,6 +9608,33 @@ cmdline_parse_inst_t cmd_add_del_tunnel_flow_director = {
 	},
 };
 
+cmdline_parse_inst_t cmd_add_del_raw_flow_director = {
+	.f = cmd_flow_director_filter_parsed,
+	.data = NULL,
+	.help_str = "flow_director_filter ... : Add or delete a raw flow "
+		"director entry on NIC",
+	.tokens = {
+		(void *)&cmd_flow_director_filter,
+		(void *)&cmd_flow_director_port_id,
+		(void *)&cmd_flow_director_mode,
+		(void *)&cmd_flow_director_mode_raw,
+		(void *)&cmd_flow_director_ops,
+		(void *)&cmd_flow_director_flow,
+		(void *)&cmd_flow_director_flow_id,
+		(void *)&cmd_flow_director_flexbytes,
+		(void *)&cmd_flow_director_flexbytes_value,
+		(void *)&cmd_flow_director_drop,
+		(void *)&cmd_flow_director_pf_vf,
+		(void *)&cmd_flow_director_queue,
+		(void *)&cmd_flow_director_queue_id,
+		(void *)&cmd_flow_director_fd_id,
+		(void *)&cmd_flow_director_fd_id_value,
+		(void *)&cmd_flow_director_packet,
+		(void *)&cmd_flow_director_filepath,
+		NULL,
+	},
+};
+
 struct cmd_flush_flow_director_result {
 	cmdline_fixed_string_t flush_flow_director;
 	uint8_t port_id;
@@ -14755,6 +14817,7 @@ cmdline_parse_ctx_t main_ctx[] = {
 	(cmdline_parse_inst_t *)&cmd_add_del_l2_flow_director,
 	(cmdline_parse_inst_t *)&cmd_add_del_mac_vlan_flow_director,
 	(cmdline_parse_inst_t *)&cmd_add_del_tunnel_flow_director,
+	(cmdline_parse_inst_t *)&cmd_add_del_raw_flow_director,
 	(cmdline_parse_inst_t *)&cmd_flush_flow_director,
 	(cmdline_parse_inst_t *)&cmd_set_flow_director_ip_mask,
 	(cmdline_parse_inst_t *)&cmd_set_flow_director_mac_vlan_mask,
-- 
2.5.5

[dpdk-dev] [PATCH v3 4/4] doc: add description of raw flow type in flow director in testpmd

[Kirill Rybalchenko]

v3:
Add description of raw flow type mode for flow_director_filter
command in testpmd.

Signed-off-by: Kirill Rybalchenko <kirill.rybalchenko@intel.com>
---
 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index aeef3e1..434b4b5 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -2193,6 +2193,10 @@ Different NICs may have different capabilities, command show port fdir (port_id)
                         flexbytes (flexbytes_value) (drop|fwd) \
                         queue (queue_id) fd_id (fd_id_value)
 
+   flow_director_filter (port_id) mode raw (add|del|update) \
+                        flow (flow_id) flexbytes (flexbytes_value) (drop|fwd) \
+                        pf|vf(vf_id) queue (queue_id) fd_id (fd_id_value)
+
 For example, to add an ipv4-udp flow type filter::
 
    testpmd> flow_director_filter 0 mode IP add flow ipv4-udp src 2.2.2.3 32 \
-- 
2.5.5

[dpdk-dev] [PATCH v4 0/4] add support for raw flow type for flow director

[Kirill Rybalchenko]

For complex packets use raw flow type with pre-constructed packet buffer
instead of creating a packet internally in PMD.

v2:
Fix code style, add comments

v3:
Merge with dunamic pctype to flow type mapping patch.
Add raw flow type mode to flow_director_filter command in testpmd.

v4:
Remove unnecessary parameters from flow_director_filter command
in testpmd for raw flow type mode. 
Modify testpmd documentation accordingly.

Kirill Rybalchenko (4):
  ethdev: add support for raw flow type for flow director
  net/i40e: add support for raw flow type for flow director
  app/testpmd: add raw flow type to flow director
  doc: add description of raw mode in flow director in testpmd

 app/test-pmd/cmdline.c                      | 99 ++++++++++++++++++++++-------
 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 18 ++++--
 drivers/net/i40e/i40e_fdir.c                | 25 ++++++--
 lib/librte_ether/rte_eth_ctrl.h             | 10 +++
 4 files changed, 120 insertions(+), 32 deletions(-)

-- 
2.5.5

[dpdk-dev] [PATCH v4 1/4] ethdev: add support for raw flow type for flow director

[Kirill Rybalchenko]

Add new structure rte_eth_raw_flow to the union rte_eth_fdir_flow
to support filter for raw flow type.

Signed-off-by: Kirill Rybalchenko <kirill.rybalchenko@intel.com>
---
 lib/librte_ether/rte_eth_ctrl.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/lib/librte_ether/rte_eth_ctrl.h b/lib/librte_ether/rte_eth_ctrl.h
index 8386904..213b408 100644
--- a/lib/librte_ether/rte_eth_ctrl.h
+++ b/lib/librte_ether/rte_eth_ctrl.h
@@ -525,6 +525,15 @@ struct rte_eth_tunnel_flow {
 };
 
 /**
+ * A structure used to define the input for raw flow
+ */
+struct rte_eth_raw_flow {
+	uint16_t flow;   /**< flow type. */
+	void *packet;    /**< pre-constructed packet buffer. */
+	uint16_t length; /**< buffer length. */
+};
+
+/**
  * An union contains the inputs for all types of flow
  * Items in flows need to be in big endian
  */
@@ -540,6 +549,7 @@ union rte_eth_fdir_flow {
 	struct rte_eth_ipv6_flow   ipv6_flow;
 	struct rte_eth_mac_vlan_flow mac_vlan_flow;
 	struct rte_eth_tunnel_flow   tunnel_flow;
+	struct rte_eth_raw_flow    raw_flow;
 };
 
 /**
-- 
2.5.5

[dpdk-dev] [PATCH v4 2/4] net/i40e: add support for raw flow type for flow director

[Kirill Rybalchenko]

When addidng flow director filter for raw flow type, instead
of constructing packet use buffer with pre-constructed packet.

v3:
Merge with dynamic pctype to flow type mapping patch.

Signed-off-by: Kirill Rybalchenko <kirill.rybalchenko@intel.com>
---
 drivers/net/i40e/i40e_fdir.c | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c
index 491c643..4b6a64f 100644
--- a/drivers/net/i40e/i40e_fdir.c
+++ b/drivers/net/i40e/i40e_fdir.c
@@ -1456,7 +1456,17 @@ i40e_add_del_fdir_filter(struct rte_eth_dev *dev,
 		return -ENOTSUP;
 	}
 
-	pctype = i40e_flowtype_to_pctype(pf->adapter, filter->input.flow_type);
+	if (filter->input.flow_type == RTE_ETH_FLOW_RAW) {
+		if (filter->input.flow.raw_flow.length > I40E_FDIR_PKT_LEN ||
+		    !filter->input.flow.raw_flow.packet) {
+			PMD_DRV_LOG(ERR, "Invalid raw flow filter parameters!");
+			return -EINVAL;
+		}
+		pctype = i40e_flowtype_to_pctype(pf->adapter,
+						 filter->input.flow.raw_flow.flow);
+	} else
+		pctype = i40e_flowtype_to_pctype(pf->adapter,
+						 filter->input.flow_type);
 	if (pctype == I40E_FILTER_PCTYPE_INVALID) {
 		PMD_DRV_LOG(ERR, "invalid flow_type input.");
 		return -EINVAL;
@@ -1473,10 +1483,15 @@ i40e_add_del_fdir_filter(struct rte_eth_dev *dev,
 
 	memset(pkt, 0, I40E_FDIR_PKT_LEN);
 
-	ret = i40e_fdir_construct_pkt(pf, &filter->input, pkt);
-	if (ret < 0) {
-		PMD_DRV_LOG(ERR, "construct packet for fdir fails.");
-		return ret;
+	if (filter->input.flow_type == RTE_ETH_FLOW_RAW) {
+		memcpy(pkt, filter->input.flow.raw_flow.packet,
+		       filter->input.flow.raw_flow.length);
+	} else {
+		ret = i40e_fdir_construct_pkt(pf, &filter->input, pkt);
+		if (ret < 0) {
+			PMD_DRV_LOG(ERR, "construct packet for fdir fails.");
+			return ret;
+		}
 	}
 
 	if (hw->mac.type == I40E_MAC_X722) {
-- 
2.5.5

[dpdk-dev] [PATCH v4 3/4] app/testpmd: add raw flow type to flow director

[Kirill Rybalchenko]

v3:
Add raw flow type support to flow_director_filter command

v4:
Remove unnecessary flexbytes parameter for raw flow type flow
director.
Add possibility for arbitrary flow type for set_fdir_input_set
and set_hash_global_config commands.

Signed-off-by: Kirill Rybalchenko <kirill.rybalchenko@intel.com>
---
 app/test-pmd/cmdline.c | 99 +++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 77 insertions(+), 22 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index a72679d..0512be0 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -862,6 +862,11 @@ static void cmd_help_long_parsed(void *parsed_result,
 			" queue (queue_id) fd_id (fd_id_value)\n"
 			"    Add/Del a Tunnel flow director filter.\n\n"
 
+			"flow_director_filter (port_id) mode raw (add|del|update)"
+			" flow (flow_id) (drop|fwd) pf|vf(vf_id) queue (queue_id)"
+			" fd_id (fd_id_value) packet (packet file name)\n"
+			"    Add/Del a raw type flow director filter.\n\n"
+
 			"flush_flow_director (port_id)\n"
 			"    Flush all flow director entries of a device.\n\n"
 
@@ -902,8 +907,8 @@ static void cmd_help_long_parsed(void *parsed_result,
 
 			"set_hash_global_config (port_id) (toeplitz|simple_xor|default)"
 			" (ipv4|ipv4-frag|ipv4-tcp|ipv4-udp|ipv4-sctp|ipv4-other|ipv6|"
-			"ipv6-frag|ipv6-tcp|ipv6-udp|ipv6-sctp|ipv6-other|l2_payload)"
-			" (enable|disable)\n"
+			"ipv6-frag|ipv6-tcp|ipv6-udp|ipv6-sctp|ipv6-other|l2_payload|"
+			"<flowtype_id>) (enable|disable)\n"
 			"    Set the global configurations of hash filters.\n\n"
 
 			"set_hash_input_set (port_id) (ipv4|ipv4-frag|"
@@ -921,7 +926,8 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"set_fdir_input_set (port_id) "
 			"(ipv4-frag|ipv4-tcp|ipv4-udp|ipv4-sctp|ipv4-other|"
 			"ipv6-frag|ipv6-tcp|ipv6-udp|ipv6-sctp|ipv6-other|"
-			"l2_payload) (ivlan|ethertype|src-ipv4|dst-ipv4|src-ipv6|"
+			"l2_payload|<flowtype_id>) "
+			"(ivlan|ethertype|src-ipv4|dst-ipv4|src-ipv6|"
 			"dst-ipv6|ipv4-tos|ipv4-proto|ipv4-ttl|ipv6-tc|"
 			"ipv6-next-header|ipv6-hop-limits|udp-src-port|"
 			"udp-dst-port|tcp-src-port|tcp-dst-port|"
@@ -9154,6 +9160,8 @@ struct cmd_flow_director_result {
 	cmdline_fixed_string_t tunnel_type;
 	cmdline_fixed_string_t tunnel_id;
 	uint32_t tunnel_id_value;
+	cmdline_fixed_string_t packet;
+	char filepath[];
 };
 
 static inline int
@@ -9280,6 +9288,8 @@ cmd_flow_director_filter_parsed(void *parsed_result,
 	struct rte_eth_fdir_filter entry;
 	uint8_t flexbytes[RTE_ETH_FDIR_MAX_FLEXLEN];
 	char *end;
+	uint8_t *raw_packet_buff;
+	uint32_t raw_packet_size;
 	unsigned long vf_id;
 	int ret = 0;
 
@@ -9303,19 +9313,26 @@ cmd_flow_director_filter_parsed(void *parsed_result,
 			return;
 		}
 	} else {
-		if (strcmp(res->mode_value, "IP")) {
-			printf("Please set mode to IP.\n");
+		if (!strcmp(res->mode_value, "raw")) {
+			entry.input.flow_type = RTE_ETH_FLOW_RAW;
+			entry.input.flow.raw_flow.flow =
+						str2flowtype(res->flow_type);
+		} else if (!strcmp(res->mode_value, "IP")) {
+			entry.input.flow_type = str2flowtype(res->flow_type);
+		} else {
+			printf("Please set mode to IP or raw.\n");
 			return;
 		}
-		entry.input.flow_type = str2flowtype(res->flow_type);
 	}
 
-	ret = parse_flexbytes(res->flexbytes_value,
-					flexbytes,
-					RTE_ETH_FDIR_MAX_FLEXLEN);
-	if (ret < 0) {
-		printf("error: Cannot parse flexbytes input.\n");
-		return;
+	if (entry.input.flow_type != RTE_ETH_FLOW_RAW) {
+		ret = parse_flexbytes(res->flexbytes_value,
+						flexbytes,
+						RTE_ETH_FDIR_MAX_FLEXLEN);
+		if (ret < 0) {
+			printf("error: Cannot parse flexbytes input.\n");
+			return;
+		}
 	}
 
 	switch (entry.input.flow_type) {
@@ -9389,6 +9406,14 @@ cmd_flow_director_filter_parsed(void *parsed_result,
 		entry.input.flow.l2_flow.ether_type =
 			rte_cpu_to_be_16(res->ether_type);
 		break;
+	case RTE_ETH_FLOW_RAW:
+		raw_packet_buff = open_ddp_package_file(res->filepath,
+							&raw_packet_size);
+		if (!raw_packet_buff)
+			return;
+		entry.input.flow.raw_flow.packet = raw_packet_buff;
+		entry.input.flow.raw_flow.length = (uint32_t)raw_packet_size;
+		break;
 	default:
 		break;
 	}
@@ -9476,8 +9501,7 @@ cmdline_parse_token_string_t cmd_flow_director_flow =
 				 flow, "flow");
 cmdline_parse_token_string_t cmd_flow_director_flow_type =
 	TOKEN_STRING_INITIALIZER(struct cmd_flow_director_result,
-		flow_type, "ipv4-other#ipv4-frag#ipv4-tcp#ipv4-udp#ipv4-sctp#"
-		"ipv6-other#ipv6-frag#ipv6-tcp#ipv6-udp#ipv6-sctp#l2_payload");
+		flow_type, NULL);
 cmdline_parse_token_string_t cmd_flow_director_ether =
 	TOKEN_STRING_INITIALIZER(struct cmd_flow_director_result,
 				 ether, "ether");
@@ -9569,6 +9593,9 @@ cmdline_parse_token_string_t cmd_flow_director_mode_mac_vlan =
 cmdline_parse_token_string_t cmd_flow_director_mode_tunnel =
 	TOKEN_STRING_INITIALIZER(struct cmd_flow_director_result,
 				 mode_value, "Tunnel");
+cmdline_parse_token_string_t cmd_flow_director_mode_raw =
+	TOKEN_STRING_INITIALIZER(struct cmd_flow_director_result,
+				 mode_value, "raw");
 cmdline_parse_token_string_t cmd_flow_director_mac =
 	TOKEN_STRING_INITIALIZER(struct cmd_flow_director_result,
 				 mac, "mac");
@@ -9587,6 +9614,12 @@ cmdline_parse_token_string_t cmd_flow_director_tunnel_id =
 cmdline_parse_token_num_t cmd_flow_director_tunnel_id_value =
 	TOKEN_NUM_INITIALIZER(struct cmd_flow_director_result,
 			      tunnel_id_value, UINT32);
+cmdline_parse_token_string_t cmd_flow_director_packet =
+	TOKEN_STRING_INITIALIZER(struct cmd_flow_director_result,
+				 packet, "packet");
+cmdline_parse_token_string_t cmd_flow_director_filepath =
+	TOKEN_STRING_INITIALIZER(struct cmd_flow_director_result,
+				 filepath, NULL);
 
 cmdline_parse_inst_t cmd_add_del_ip_flow_director = {
 	.f = cmd_flow_director_filter_parsed,
@@ -9790,6 +9823,31 @@ cmdline_parse_inst_t cmd_add_del_tunnel_flow_director = {
 	},
 };
 
+cmdline_parse_inst_t cmd_add_del_raw_flow_director = {
+	.f = cmd_flow_director_filter_parsed,
+	.data = NULL,
+	.help_str = "flow_director_filter ... : Add or delete a raw flow "
+		"director entry on NIC",
+	.tokens = {
+		(void *)&cmd_flow_director_filter,
+		(void *)&cmd_flow_director_port_id,
+		(void *)&cmd_flow_director_mode,
+		(void *)&cmd_flow_director_mode_raw,
+		(void *)&cmd_flow_director_ops,
+		(void *)&cmd_flow_director_flow,
+		(void *)&cmd_flow_director_flow_type,
+		(void *)&cmd_flow_director_drop,
+		(void *)&cmd_flow_director_pf_vf,
+		(void *)&cmd_flow_director_queue,
+		(void *)&cmd_flow_director_queue_id,
+		(void *)&cmd_flow_director_fd_id,
+		(void *)&cmd_flow_director_fd_id_value,
+		(void *)&cmd_flow_director_packet,
+		(void *)&cmd_flow_director_filepath,
+		NULL,
+	},
+};
+
 struct cmd_flush_flow_director_result {
 	cmdline_fixed_string_t flush_flow_director;
 	uint8_t port_id;
@@ -10589,9 +10647,7 @@ cmdline_parse_token_string_t cmd_set_hash_global_config_hash_func =
 		hash_func, "toeplitz#simple_xor#default");
 cmdline_parse_token_string_t cmd_set_hash_global_config_flow_type =
 	TOKEN_STRING_INITIALIZER(struct cmd_set_hash_global_config_result,
-		flow_type,
-		"ipv4#ipv4-frag#ipv4-tcp#ipv4-udp#ipv4-sctp#ipv4-other#ipv6#"
-		"ipv6-frag#ipv6-tcp#ipv6-udp#ipv6-sctp#ipv6-other#l2_payload");
+		flow_type, NULL);
 cmdline_parse_token_string_t cmd_set_hash_global_config_enable =
 	TOKEN_STRING_INITIALIZER(struct cmd_set_hash_global_config_result,
 		enable, "enable#disable");
@@ -10603,7 +10659,7 @@ cmdline_parse_inst_t cmd_set_hash_global_config = {
 		"toeplitz|simple_xor|default "
 		"ipv4|ipv4-frag|ipv4-tcp|ipv4-udp|ipv4-sctp|ipv4-other|"
 		"ipv6|ipv6-frag|ipv6-tcp|ipv6-udp|ipv6-sctp|ipv6-other|"
-		"l2_payload enable|disable",
+		"l2_payload|<flow_id> enable|disable",
 	.tokens = {
 		(void *)&cmd_set_hash_global_config_all,
 		(void *)&cmd_set_hash_global_config_port_id,
@@ -10775,9 +10831,7 @@ cmdline_parse_token_num_t cmd_set_fdir_input_set_port_id =
 	port_id, UINT8);
 cmdline_parse_token_string_t cmd_set_fdir_input_set_flow_type =
 	TOKEN_STRING_INITIALIZER(struct cmd_set_fdir_input_set_result,
-	flow_type,
-	"ipv4-frag#ipv4-tcp#ipv4-udp#ipv4-sctp#ipv4-other#"
-	"ipv6-frag#ipv6-tcp#ipv6-udp#ipv6-sctp#ipv6-other#l2_payload");
+	flow_type, NULL);
 cmdline_parse_token_string_t cmd_set_fdir_input_set_field =
 	TOKEN_STRING_INITIALIZER(struct cmd_set_fdir_input_set_result,
 	inset_field,
@@ -10795,7 +10849,7 @@ cmdline_parse_inst_t cmd_set_fdir_input_set = {
 	.data = NULL,
 	.help_str = "set_fdir_input_set <port_id> "
 	"ipv4-frag|ipv4-tcp|ipv4-udp|ipv4-sctp|ipv4-other|"
-	"ipv6-frag|ipv6-tcp|ipv6-udp|ipv6-sctp|ipv6-other|l2_payload "
+	"ipv6-frag|ipv6-tcp|ipv6-udp|ipv6-sctp|ipv6-other|l2_payload|<flow_id> "
 	"ivlan|ethertype|src-ipv4|dst-ipv4|src-ipv6|dst-ipv6|"
 	"ipv4-tos|ipv4-proto|ipv4-ttl|ipv6-tc|ipv6-next-header|"
 	"ipv6-hop-limits|udp-src-port|udp-dst-port|"
@@ -15055,6 +15109,7 @@ cmdline_parse_ctx_t main_ctx[] = {
 	(cmdline_parse_inst_t *)&cmd_add_del_l2_flow_director,
 	(cmdline_parse_inst_t *)&cmd_add_del_mac_vlan_flow_director,
 	(cmdline_parse_inst_t *)&cmd_add_del_tunnel_flow_director,
+	(cmdline_parse_inst_t *)&cmd_add_del_raw_flow_director,
 	(cmdline_parse_inst_t *)&cmd_flush_flow_director,
 	(cmdline_parse_inst_t *)&cmd_set_flow_director_ip_mask,
 	(cmdline_parse_inst_t *)&cmd_set_flow_director_mac_vlan_mask,
-- 
2.5.5

[dpdk-dev] [PATCH v4 4/4] doc: add description of raw mode in flow director in testpmd

[Kirill Rybalchenko]

v3:
Add description of raw flow type mode for flow_director_filter
command in testpmd.

v4:
Fix documentation of flow_director_filter command in testpmd
according to modified parameters.

Signed-off-by: Kirill Rybalchenko <kirill.rybalchenko@intel.com>
---
 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index eb3cc66..d2ffce1 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -2259,6 +2259,10 @@ Different NICs may have different capabilities, command show port fdir (port_id)
                         flexbytes (flexbytes_value) (drop|fwd) \
                         queue (queue_id) fd_id (fd_id_value)
 
+   flow_director_filter (port_id) mode raw (add|del|update) flow (flow_id) \
+                        (drop|fwd) pf|vf(vf_id) queue (queue_id) \
+                        fd_id (fd_id_value) packet (packet file name)
+
 For example, to add an ipv4-udp flow type filter::
 
    testpmd> flow_director_filter 0 mode IP add flow ipv4-udp src 2.2.2.3 32 \
@@ -2271,6 +2275,10 @@ For example, add an ipv4-other flow type filter::
              dst 2.2.2.5 tos 2 proto 20 ttl 40 vlan 0x1 \
              flexbytes (0x88,0x48) fwd pf queue 1 fd_id 1
 
+Note that for raw flow type mode the source and destination fields in the
+raw packet buffer need to be presented in a reversed order with respect
+to the expected received packets.
+
 flush_flow_director
 ~~~~~~~~~~~~~~~~~~~
 
@@ -2373,7 +2381,7 @@ Set the global configurations of hash filters::
 
    set_hash_global_config (port_id) (toeplitz|simple_xor|default) \
    (ipv4|ipv4-frag|ipv4-tcp|ipv4-udp|ipv4-sctp|ipv4-other|ipv6|ipv6-frag| \
-   ipv6-tcp|ipv6-udp|ipv6-sctp|ipv6-other|l2_payload) \
+   ipv6-tcp|ipv6-udp|ipv6-sctp|ipv6-other|l2_payload|<flow_id>) \
    (enable|disable)
 
 For example, to enable simple_xor for flow type of ipv6 on port 2::
@@ -2387,8 +2395,8 @@ Set the input set for hash::
 
    set_hash_input_set (port_id) (ipv4-frag|ipv4-tcp|ipv4-udp|ipv4-sctp| \
    ipv4-other|ipv6-frag|ipv6-tcp|ipv6-udp|ipv6-sctp|ipv6-other| \
-   l2_payload) (ovlan|ivlan|src-ipv4|dst-ipv4|src-ipv6|dst-ipv6|ipv4-tos| \
-   ipv4-proto|ipv6-tc|ipv6-next-header|udp-src-port|udp-dst-port| \
+   l2_payload|<flow_id>) (ovlan|ivlan|src-ipv4|dst-ipv4|src-ipv6|dst-ipv6| \
+   ipv4-tos|ipv4-proto|ipv6-tc|ipv6-next-header|udp-src-port|udp-dst-port| \
    tcp-src-port|tcp-dst-port|sctp-src-port|sctp-dst-port|sctp-veri-tag| \
    udp-key|gre-key|fld-1st|fld-2nd|fld-3rd|fld-4th|fld-5th|fld-6th|fld-7th| \
    fld-8th|none) (select|add)
@@ -2407,8 +2415,8 @@ Set the input set for flow director::
 
    set_fdir_input_set (port_id) (ipv4-frag|ipv4-tcp|ipv4-udp|ipv4-sctp| \
    ipv4-other|ipv6|ipv6-frag|ipv6-tcp|ipv6-udp|ipv6-sctp|ipv6-other| \
-   l2_payload) (ivlan|ethertype|src-ipv4|dst-ipv4|src-ipv6|dst-ipv6|ipv4-tos| \
-   ipv4-proto|ipv4-ttl|ipv6-tc|ipv6-next-header|ipv6-hop-limits| \
+   l2_payload|<flow_id>) (ivlan|ethertype|src-ipv4|dst-ipv4|src-ipv6|dst-ipv6| \
+   ipv4-tos|ipv4-proto|ipv4-ttl|ipv6-tc|ipv6-next-header|ipv6-hop-limits| \
    tudp-src-port|udp-dst-port|cp-src-port|tcp-dst-port|sctp-src-port| \
    sctp-dst-port|sctp-veri-tag|none) (select|add)
 
-- 
2.5.5

[dpdk-dev] [PATCH v5 0/4] add support for raw flow type for flow director

[Kirill Rybalchenko]

For complex packets use raw flow type with pre-constructed packet buffer
instead of creating a packet internally in PMD.

v2:
Fix code style, add comments

v3:
Merge with dunamic pctype to flow type mapping patch.
Add raw flow type mode to flow_director_filter command in testpmd.

v4:
Remove unnecessary parameters from flow_director_filter command
in testpmd for raw flow type mode. 
Modify testpmd documentation accordingly.

v5:
Fix code style warning.

Kirill Rybalchenko (4):
  ethdev: add support for raw flow type for flow director
  net/i40e: add support for raw flow type for flow      director
  app/testpmd: add raw flow type to flow director
  doc: add description of raw mode in flow director in      testpmd

 app/test-pmd/cmdline.c                      | 99 ++++++++++++++++++++++-------
 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 18 ++++--
 drivers/net/i40e/i40e_fdir.c                | 26 ++++++--
 lib/librte_ether/rte_eth_ctrl.h             | 10 +++
 4 files changed, 121 insertions(+), 32 deletions(-)

-- 
2.5.5

[dpdk-dev] [PATCH v5 1/4] ethdev: add support for raw flow type for flow director

[Kirill Rybalchenko]

Add new structure rte_eth_raw_flow to the union rte_eth_fdir_flow
to support filter for raw flow type.

Signed-off-by: Kirill Rybalchenko <kirill.rybalchenko@intel.com>
---
 lib/librte_ether/rte_eth_ctrl.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/lib/librte_ether/rte_eth_ctrl.h b/lib/librte_ether/rte_eth_ctrl.h
index 8386904..213b408 100644
--- a/lib/librte_ether/rte_eth_ctrl.h
+++ b/lib/librte_ether/rte_eth_ctrl.h
@@ -525,6 +525,15 @@ struct rte_eth_tunnel_flow {
 };
 
 /**
+ * A structure used to define the input for raw flow
+ */
+struct rte_eth_raw_flow {
+	uint16_t flow;   /**< flow type. */
+	void *packet;    /**< pre-constructed packet buffer. */
+	uint16_t length; /**< buffer length. */
+};
+
+/**
  * An union contains the inputs for all types of flow
  * Items in flows need to be in big endian
  */
@@ -540,6 +549,7 @@ union rte_eth_fdir_flow {
 	struct rte_eth_ipv6_flow   ipv6_flow;
 	struct rte_eth_mac_vlan_flow mac_vlan_flow;
 	struct rte_eth_tunnel_flow   tunnel_flow;
+	struct rte_eth_raw_flow    raw_flow;
 };
 
 /**
-- 
2.5.5

Re: [dpdk-dev] [PATCH v5 1/4] ethdev: add support for raw flow type for flow director

[Thomas Monjalon]

Hi,

10/10/2017 22:28, Kirill Rybalchenko:
> Add new structure rte_eth_raw_flow to the union rte_eth_fdir_flow
> to support filter for raw flow type.
> 
> Signed-off-by: Kirill Rybalchenko <kirill.rybalchenko@intel.com>

This description does not explain why you add this new flow director type.
It seems you are allowing a new feature to filter custom protocols.

As I replied on v2, you are implementing your new feature with
a deprecated API (there is a deprecation notice without any deadline).
It is dangerous because this new use case will be settled on top
of a fragile foundation. And because of these new users, it will be
harder to drop this API as announced.
It is also dangerous because you are not trying to implement your
feature with the new rte_flow API. So we cannot be sure that it
will fit for every use cases.
If rte_flow is not good enough, we must improve it.

This is my suggestion:
1/ Implement this interesting feature with rte_flow.
2/ Switch every other use cases to rte_flow.
3/ Let's agree on a date to drop the legacy flow director API.

So this is a NACK.
Please let's move forward.

Re: [dpdk-dev] [PATCH v5 1/4] ethdev: add support for raw flow type for flow director

[Rybalchenko, Kirill]

Hi Thomas,

The reason this feature is needed is to be able to program custom flow types using a template packet rather than building up a C struct to define the protocol. This means that users don't have to work on the DPDK internals to support new flow types that they may be using, but can instead add them dynamically as part of their application. There are also several customers who are looking for this feature as part of the 17.11 LTS release.

This patchset has been out since August and these comments are very late, with the first objections last week, which we tried to answer. This short notice doesn't allow us a reasonable amount of time to take them into account.

However, to address your primary concern, we can implement this using a i40e private API, so that we are not tying users to FDIR APIs and thus not blocking the removal of the APIs in time.

Ideally we would like to use rte_flow but it is based around the idea of describing packet headers which is significantly different from the proposed method using template packets. Longer term it may be possible to support this in rte_flow, we could propose this for discussion in the next release, and if there is community interest/agreement we can add it.

We will rework this, in the short term, as a private API, as suggested above, and then propose an rte_flow API in the longer term. Let us know if you have any concerns about this.

Regards,
Kirill.

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Wednesday 11 October 2017 23:27
> To: Rybalchenko, Kirill <kirill.rybalchenko@intel.com>
> Cc: dev@dpdk.org; Chilikin, Andrey <andrey.chilikin@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Yigit, Ferruh
> <ferruh.yigit@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v5 1/4] ethdev: add support for raw flow
> type for flow director
> 
> Hi,
> 
> 10/10/2017 22:28, Kirill Rybalchenko:
> > Add new structure rte_eth_raw_flow to the union rte_eth_fdir_flow to
> > support filter for raw flow type.
> >
> > Signed-off-by: Kirill Rybalchenko <kirill.rybalchenko@intel.com>
> 
> This description does not explain why you add this new flow director type.
> It seems you are allowing a new feature to filter custom protocols.
> 
> As I replied on v2, you are implementing your new feature with a deprecated
> API (there is a deprecation notice without any deadline).
> It is dangerous because this new use case will be settled on top of a fragile
> foundation. And because of these new users, it will be harder to drop this
> API as announced.
> It is also dangerous because you are not trying to implement your feature
> with the new rte_flow API. So we cannot be sure that it will fit for every use
> cases.
> If rte_flow is not good enough, we must improve it.
> 
> This is my suggestion:
> 1/ Implement this interesting feature with rte_flow.
> 2/ Switch every other use cases to rte_flow.
> 3/ Let's agree on a date to drop the legacy flow director API.
> 
> So this is a NACK.
> Please let's move forward.

Re: [dpdk-dev] [PATCH v5 1/4] ethdev: add support for raw flow type for flow director

[Thomas Monjalon]

OK, a private API is a good short term approach.


12/10/2017 13:41, Rybalchenko, Kirill:
> Hi Thomas,
> 
> The reason this feature is needed is to be able to program custom flow types using a template packet rather than building up a C struct to define the protocol. This means that users don't have to work on the DPDK internals to support new flow types that they may be using, but can instead add them dynamically as part of their application. There are also several customers who are looking for this feature as part of the 17.11 LTS release.
> 
> This patchset has been out since August and these comments are very late, with the first objections last week, which we tried to answer. This short notice doesn't allow us a reasonable amount of time to take them into account.
> 
> However, to address your primary concern, we can implement this using a i40e private API, so that we are not tying users to FDIR APIs and thus not blocking the removal of the APIs in time.
> 
> Ideally we would like to use rte_flow but it is based around the idea of describing packet headers which is significantly different from the proposed method using template packets. Longer term it may be possible to support this in rte_flow, we could propose this for discussion in the next release, and if there is community interest/agreement we can add it.
> 
> We will rework this, in the short term, as a private API, as suggested above, and then propose an rte_flow API in the longer term. Let us know if you have any concerns about this.
> 
> Regards,
> Kirill.
> 
> > -----Original Message-----
> > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > Sent: Wednesday 11 October 2017 23:27
> > To: Rybalchenko, Kirill <kirill.rybalchenko@intel.com>
> > Cc: dev@dpdk.org; Chilikin, Andrey <andrey.chilikin@intel.com>; Xing, Beilei
> > <beilei.xing@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Yigit, Ferruh
> > <ferruh.yigit@intel.com>
> > Subject: Re: [dpdk-dev] [PATCH v5 1/4] ethdev: add support for raw flow
> > type for flow director
> > 
> > Hi,
> > 
> > 10/10/2017 22:28, Kirill Rybalchenko:
> > > Add new structure rte_eth_raw_flow to the union rte_eth_fdir_flow to
> > > support filter for raw flow type.
> > >
> > > Signed-off-by: Kirill Rybalchenko <kirill.rybalchenko@intel.com>
> > 
> > This description does not explain why you add this new flow director type.
> > It seems you are allowing a new feature to filter custom protocols.
> > 
> > As I replied on v2, you are implementing your new feature with a deprecated
> > API (there is a deprecation notice without any deadline).
> > It is dangerous because this new use case will be settled on top of a fragile
> > foundation. And because of these new users, it will be harder to drop this
> > API as announced.
> > It is also dangerous because you are not trying to implement your feature
> > with the new rte_flow API. So we cannot be sure that it will fit for every use
> > cases.
> > If rte_flow is not good enough, we must improve it.
> > 
> > This is my suggestion:
> > 1/ Implement this interesting feature with rte_flow.
> > 2/ Switch every other use cases to rte_flow.
> > 3/ Let's agree on a date to drop the legacy flow director API.
> > 
> > So this is a NACK.
> > Please let's move forward.

Re: [dpdk-dev] [PATCH v5 1/4] ethdev: add support for raw flow type for flow director

[Adrien Mazarguil]

Hi Kirill,

On Thu, Oct 12, 2017 at 11:41:50AM +0000, Rybalchenko, Kirill wrote:
> Hi Thomas,
> 
> The reason this feature is needed is to be able to program custom flow types using a template packet rather than building up a C struct to define the protocol. This means that users don't have to work on the DPDK internals to support new flow types that they may be using, but can instead add them dynamically as part of their application. There are also several customers who are looking for this feature as part of the 17.11 LTS release.
> 
> This patchset has been out since August and these comments are very late, with the first objections last week, which we tried to answer. This short notice doesn't allow us a reasonable amount of time to take them into account.
> 
> However, to address your primary concern, we can implement this using a i40e private API, so that we are not tying users to FDIR APIs and thus not blocking the removal of the APIs in time.
> 
> Ideally we would like to use rte_flow but it is based around the idea of describing packet headers which is significantly different from the proposed method using template packets. Longer term it may be possible to support this in rte_flow, we could propose this for discussion in the next release, and if there is community interest/agreement we can add it.
> 
> We will rework this, in the short term, as a private API, as suggested above, and then propose an rte_flow API in the longer term. Let us know if you have any concerns about this.

I am not trying to push for its integration through rte_flow at this stage
and I don't mind the chosen PMD-specific approach, I'm just curious about
the reasons that made it hard to implement as a RTE_FLOW_ITEM_RAW thing?
(e.g. a rule with a pattern that only contains one or several such items)

Please have a look at the rte_flow_item_raw structure in rte_flow.h, tell me
what's missing in there and I'll take it into account during the next
overhaul. Thanks in advance for your feedback.

-- 
Adrien Mazarguil
6WIND

[dpdk-dev] [PATCH v5 2/4] net/i40e: add support for raw flow type for flow director

[Kirill Rybalchenko]

When addidng flow director filter for raw flow type, instead
of constructing packet use buffer with pre-constructed packet.

v3:
Merge with dynamic pctype to flow type mapping patch.

v5:
Fix code style warning.

Signed-off-by: Kirill Rybalchenko <kirill.rybalchenko@intel.com>
---
 drivers/net/i40e/i40e_fdir.c | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c
index 491c643..ae5cf9a 100644
--- a/drivers/net/i40e/i40e_fdir.c
+++ b/drivers/net/i40e/i40e_fdir.c
@@ -1456,7 +1456,18 @@ i40e_add_del_fdir_filter(struct rte_eth_dev *dev,
 		return -ENOTSUP;
 	}
 
-	pctype = i40e_flowtype_to_pctype(pf->adapter, filter->input.flow_type);
+	if (filter->input.flow_type == RTE_ETH_FLOW_RAW) {
+		if (filter->input.flow.raw_flow.length > I40E_FDIR_PKT_LEN ||
+		    !filter->input.flow.raw_flow.packet) {
+			PMD_DRV_LOG(ERR, "Invalid raw flow filter parameters!");
+			return -EINVAL;
+		}
+		pctype = i40e_flowtype_to_pctype(pf->adapter,
+						 filter->input.flow.raw_flow.flow);
+	} else {
+		pctype = i40e_flowtype_to_pctype(pf->adapter,
+						 filter->input.flow_type);
+	}
 	if (pctype == I40E_FILTER_PCTYPE_INVALID) {
 		PMD_DRV_LOG(ERR, "invalid flow_type input.");
 		return -EINVAL;
@@ -1473,10 +1484,15 @@ i40e_add_del_fdir_filter(struct rte_eth_dev *dev,
 
 	memset(pkt, 0, I40E_FDIR_PKT_LEN);
 
-	ret = i40e_fdir_construct_pkt(pf, &filter->input, pkt);
-	if (ret < 0) {
-		PMD_DRV_LOG(ERR, "construct packet for fdir fails.");
-		return ret;
+	if (filter->input.flow_type == RTE_ETH_FLOW_RAW) {
+		memcpy(pkt, filter->input.flow.raw_flow.packet,
+		       filter->input.flow.raw_flow.length);
+	} else {
+		ret = i40e_fdir_construct_pkt(pf, &filter->input, pkt);
+		if (ret < 0) {
+			PMD_DRV_LOG(ERR, "construct packet for fdir fails.");
+			return ret;
+		}
 	}
 
 	if (hw->mac.type == I40E_MAC_X722) {
-- 
2.5.5

[dpdk-dev] [PATCH v5 3/4] app/testpmd: add raw flow type to flow director

[Kirill Rybalchenko]

v3:
Add raw flow type support to flow_director_filter command

v4:
Remove unnecessary flexbytes parameter for raw flow type flow
director.
Add possibility for arbitrary flow type for set_fdir_input_set
and set_hash_global_config commands.

Signed-off-by: Kirill Rybalchenko <kirill.rybalchenko@intel.com>
---
 app/test-pmd/cmdline.c | 99 +++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 77 insertions(+), 22 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index a72679d..0512be0 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -862,6 +862,11 @@ static void cmd_help_long_parsed(void *parsed_result,
 			" queue (queue_id) fd_id (fd_id_value)\n"
 			"    Add/Del a Tunnel flow director filter.\n\n"
 
+			"flow_director_filter (port_id) mode raw (add|del|update)"
+			" flow (flow_id) (drop|fwd) pf|vf(vf_id) queue (queue_id)"
+			" fd_id (fd_id_value) packet (packet file name)\n"
+			"    Add/Del a raw type flow director filter.\n\n"
+
 			"flush_flow_director (port_id)\n"
 			"    Flush all flow director entries of a device.\n\n"
 
@@ -902,8 +907,8 @@ static void cmd_help_long_parsed(void *parsed_result,
 
 			"set_hash_global_config (port_id) (toeplitz|simple_xor|default)"
 			" (ipv4|ipv4-frag|ipv4-tcp|ipv4-udp|ipv4-sctp|ipv4-other|ipv6|"
-			"ipv6-frag|ipv6-tcp|ipv6-udp|ipv6-sctp|ipv6-other|l2_payload)"
-			" (enable|disable)\n"
+			"ipv6-frag|ipv6-tcp|ipv6-udp|ipv6-sctp|ipv6-other|l2_payload|"
+			"<flowtype_id>) (enable|disable)\n"
 			"    Set the global configurations of hash filters.\n\n"
 
 			"set_hash_input_set (port_id) (ipv4|ipv4-frag|"
@@ -921,7 +926,8 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"set_fdir_input_set (port_id) "
 			"(ipv4-frag|ipv4-tcp|ipv4-udp|ipv4-sctp|ipv4-other|"
 			"ipv6-frag|ipv6-tcp|ipv6-udp|ipv6-sctp|ipv6-other|"
-			"l2_payload) (ivlan|ethertype|src-ipv4|dst-ipv4|src-ipv6|"
+			"l2_payload|<flowtype_id>) "
+			"(ivlan|ethertype|src-ipv4|dst-ipv4|src-ipv6|"
 			"dst-ipv6|ipv4-tos|ipv4-proto|ipv4-ttl|ipv6-tc|"
 			"ipv6-next-header|ipv6-hop-limits|udp-src-port|"
 			"udp-dst-port|tcp-src-port|tcp-dst-port|"
@@ -9154,6 +9160,8 @@ struct cmd_flow_director_result {
 	cmdline_fixed_string_t tunnel_type;
 	cmdline_fixed_string_t tunnel_id;
 	uint32_t tunnel_id_value;
+	cmdline_fixed_string_t packet;
+	char filepath[];
 };
 
 static inline int
@@ -9280,6 +9288,8 @@ cmd_flow_director_filter_parsed(void *parsed_result,
 	struct rte_eth_fdir_filter entry;
 	uint8_t flexbytes[RTE_ETH_FDIR_MAX_FLEXLEN];
 	char *end;
+	uint8_t *raw_packet_buff;
+	uint32_t raw_packet_size;
 	unsigned long vf_id;
 	int ret = 0;
 
@@ -9303,19 +9313,26 @@ cmd_flow_director_filter_parsed(void *parsed_result,
 			return;
 		}
 	} else {
-		if (strcmp(res->mode_value, "IP")) {
-			printf("Please set mode to IP.\n");
+		if (!strcmp(res->mode_value, "raw")) {
+			entry.input.flow_type = RTE_ETH_FLOW_RAW;
+			entry.input.flow.raw_flow.flow =
+						str2flowtype(res->flow_type);
+		} else if (!strcmp(res->mode_value, "IP")) {
+			entry.input.flow_type = str2flowtype(res->flow_type);
+		} else {
+			printf("Please set mode to IP or raw.\n");
 			return;
 		}
-		entry.input.flow_type = str2flowtype(res->flow_type);
 	}
 
-	ret = parse_flexbytes(res->flexbytes_value,
-					flexbytes,
-					RTE_ETH_FDIR_MAX_FLEXLEN);
-	if (ret < 0) {
-		printf("error: Cannot parse flexbytes input.\n");
-		return;
+	if (entry.input.flow_type != RTE_ETH_FLOW_RAW) {
+		ret = parse_flexbytes(res->flexbytes_value,
+						flexbytes,
+						RTE_ETH_FDIR_MAX_FLEXLEN);
+		if (ret < 0) {
+			printf("error: Cannot parse flexbytes input.\n");
+			return;
+		}
 	}
 
 	switch (entry.input.flow_type) {
@@ -9389,6 +9406,14 @@ cmd_flow_director_filter_parsed(void *parsed_result,
 		entry.input.flow.l2_flow.ether_type =
 			rte_cpu_to_be_16(res->ether_type);
 		break;
+	case RTE_ETH_FLOW_RAW:
+		raw_packet_buff = open_ddp_package_file(res->filepath,
+							&raw_packet_size);
+		if (!raw_packet_buff)
+			return;
+		entry.input.flow.raw_flow.packet = raw_packet_buff;
+		entry.input.flow.raw_flow.length = (uint32_t)raw_packet_size;
+		break;
 	default:
 		break;
 	}
@@ -9476,8 +9501,7 @@ cmdline_parse_token_string_t cmd_flow_director_flow =
 				 flow, "flow");
 cmdline_parse_token_string_t cmd_flow_director_flow_type =
 	TOKEN_STRING_INITIALIZER(struct cmd_flow_director_result,
-		flow_type, "ipv4-other#ipv4-frag#ipv4-tcp#ipv4-udp#ipv4-sctp#"
-		"ipv6-other#ipv6-frag#ipv6-tcp#ipv6-udp#ipv6-sctp#l2_payload");
+		flow_type, NULL);
 cmdline_parse_token_string_t cmd_flow_director_ether =
 	TOKEN_STRING_INITIALIZER(struct cmd_flow_director_result,
 				 ether, "ether");
@@ -9569,6 +9593,9 @@ cmdline_parse_token_string_t cmd_flow_director_mode_mac_vlan =
 cmdline_parse_token_string_t cmd_flow_director_mode_tunnel =
 	TOKEN_STRING_INITIALIZER(struct cmd_flow_director_result,
 				 mode_value, "Tunnel");
+cmdline_parse_token_string_t cmd_flow_director_mode_raw =
+	TOKEN_STRING_INITIALIZER(struct cmd_flow_director_result,
+				 mode_value, "raw");
 cmdline_parse_token_string_t cmd_flow_director_mac =
 	TOKEN_STRING_INITIALIZER(struct cmd_flow_director_result,
 				 mac, "mac");
@@ -9587,6 +9614,12 @@ cmdline_parse_token_string_t cmd_flow_director_tunnel_id =
 cmdline_parse_token_num_t cmd_flow_director_tunnel_id_value =
 	TOKEN_NUM_INITIALIZER(struct cmd_flow_director_result,
 			      tunnel_id_value, UINT32);
+cmdline_parse_token_string_t cmd_flow_director_packet =
+	TOKEN_STRING_INITIALIZER(struct cmd_flow_director_result,
+				 packet, "packet");
+cmdline_parse_token_string_t cmd_flow_director_filepath =
+	TOKEN_STRING_INITIALIZER(struct cmd_flow_director_result,
+				 filepath, NULL);
 
 cmdline_parse_inst_t cmd_add_del_ip_flow_director = {
 	.f = cmd_flow_director_filter_parsed,
@@ -9790,6 +9823,31 @@ cmdline_parse_inst_t cmd_add_del_tunnel_flow_director = {
 	},
 };
 
+cmdline_parse_inst_t cmd_add_del_raw_flow_director = {
+	.f = cmd_flow_director_filter_parsed,
+	.data = NULL,
+	.help_str = "flow_director_filter ... : Add or delete a raw flow "
+		"director entry on NIC",
+	.tokens = {
+		(void *)&cmd_flow_director_filter,
+		(void *)&cmd_flow_director_port_id,
+		(void *)&cmd_flow_director_mode,
+		(void *)&cmd_flow_director_mode_raw,
+		(void *)&cmd_flow_director_ops,
+		(void *)&cmd_flow_director_flow,
+		(void *)&cmd_flow_director_flow_type,
+		(void *)&cmd_flow_director_drop,
+		(void *)&cmd_flow_director_pf_vf,
+		(void *)&cmd_flow_director_queue,
+		(void *)&cmd_flow_director_queue_id,
+		(void *)&cmd_flow_director_fd_id,
+		(void *)&cmd_flow_director_fd_id_value,
+		(void *)&cmd_flow_director_packet,
+		(void *)&cmd_flow_director_filepath,
+		NULL,
+	},
+};
+
 struct cmd_flush_flow_director_result {
 	cmdline_fixed_string_t flush_flow_director;
 	uint8_t port_id;
@@ -10589,9 +10647,7 @@ cmdline_parse_token_string_t cmd_set_hash_global_config_hash_func =
 		hash_func, "toeplitz#simple_xor#default");
 cmdline_parse_token_string_t cmd_set_hash_global_config_flow_type =
 	TOKEN_STRING_INITIALIZER(struct cmd_set_hash_global_config_result,
-		flow_type,
-		"ipv4#ipv4-frag#ipv4-tcp#ipv4-udp#ipv4-sctp#ipv4-other#ipv6#"
-		"ipv6-frag#ipv6-tcp#ipv6-udp#ipv6-sctp#ipv6-other#l2_payload");
+		flow_type, NULL);
 cmdline_parse_token_string_t cmd_set_hash_global_config_enable =
 	TOKEN_STRING_INITIALIZER(struct cmd_set_hash_global_config_result,
 		enable, "enable#disable");
@@ -10603,7 +10659,7 @@ cmdline_parse_inst_t cmd_set_hash_global_config = {
 		"toeplitz|simple_xor|default "
 		"ipv4|ipv4-frag|ipv4-tcp|ipv4-udp|ipv4-sctp|ipv4-other|"
 		"ipv6|ipv6-frag|ipv6-tcp|ipv6-udp|ipv6-sctp|ipv6-other|"
-		"l2_payload enable|disable",
+		"l2_payload|<flow_id> enable|disable",
 	.tokens = {
 		(void *)&cmd_set_hash_global_config_all,
 		(void *)&cmd_set_hash_global_config_port_id,
@@ -10775,9 +10831,7 @@ cmdline_parse_token_num_t cmd_set_fdir_input_set_port_id =
 	port_id, UINT8);
 cmdline_parse_token_string_t cmd_set_fdir_input_set_flow_type =
 	TOKEN_STRING_INITIALIZER(struct cmd_set_fdir_input_set_result,
-	flow_type,
-	"ipv4-frag#ipv4-tcp#ipv4-udp#ipv4-sctp#ipv4-other#"
-	"ipv6-frag#ipv6-tcp#ipv6-udp#ipv6-sctp#ipv6-other#l2_payload");
+	flow_type, NULL);
 cmdline_parse_token_string_t cmd_set_fdir_input_set_field =
 	TOKEN_STRING_INITIALIZER(struct cmd_set_fdir_input_set_result,
 	inset_field,
@@ -10795,7 +10849,7 @@ cmdline_parse_inst_t cmd_set_fdir_input_set = {
 	.data = NULL,
 	.help_str = "set_fdir_input_set <port_id> "
 	"ipv4-frag|ipv4-tcp|ipv4-udp|ipv4-sctp|ipv4-other|"
-	"ipv6-frag|ipv6-tcp|ipv6-udp|ipv6-sctp|ipv6-other|l2_payload "
+	"ipv6-frag|ipv6-tcp|ipv6-udp|ipv6-sctp|ipv6-other|l2_payload|<flow_id> "
 	"ivlan|ethertype|src-ipv4|dst-ipv4|src-ipv6|dst-ipv6|"
 	"ipv4-tos|ipv4-proto|ipv4-ttl|ipv6-tc|ipv6-next-header|"
 	"ipv6-hop-limits|udp-src-port|udp-dst-port|"
@@ -15055,6 +15109,7 @@ cmdline_parse_ctx_t main_ctx[] = {
 	(cmdline_parse_inst_t *)&cmd_add_del_l2_flow_director,
 	(cmdline_parse_inst_t *)&cmd_add_del_mac_vlan_flow_director,
 	(cmdline_parse_inst_t *)&cmd_add_del_tunnel_flow_director,
+	(cmdline_parse_inst_t *)&cmd_add_del_raw_flow_director,
 	(cmdline_parse_inst_t *)&cmd_flush_flow_director,
 	(cmdline_parse_inst_t *)&cmd_set_flow_director_ip_mask,
 	(cmdline_parse_inst_t *)&cmd_set_flow_director_mac_vlan_mask,
-- 
2.5.5

Re: [dpdk-dev] [PATCH v5 3/4] app/testpmd: add raw flow type to flow director

[Wu, Jingjing]

>  		break;
> +	case RTE_ETH_FLOW_RAW:
> +		raw_packet_buff = open_ddp_package_file(res->filepath,
> +							&raw_packet_size);

If you think open_ddp_package_file can be used for common binary file open,
could you rename it and then use it? Because it is not ddp file, right?
The buffer is allocated in open_ddp_package_file, after you use it, you need
to free it to avoid memory leak, or call close_ddp_package_file.

Even the ethdev patch is been rejected, and you may go to a private API way.
I think the comments are also worth for the raw package read.
Please pay attention in later version.

Thanks
Jingjing

[dpdk-dev] [PATCH v5 4/4] doc: add description of raw mode in flow director in testpmd

[Kirill Rybalchenko]

v3:
Add description of raw flow type mode for flow_director_filter
command in testpmd.

v4:
Fix documentation of flow_director_filter command in testpmd
according to modified parameters.

Signed-off-by: Kirill Rybalchenko <kirill.rybalchenko@intel.com>
---
 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 7f37f91..7165707 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -2521,6 +2521,10 @@ Different NICs may have different capabilities, command show port fdir (port_id)
                         flexbytes (flexbytes_value) (drop|fwd) \
                         queue (queue_id) fd_id (fd_id_value)
 
+   flow_director_filter (port_id) mode raw (add|del|update) flow (flow_id) \
+                        (drop|fwd) pf|vf(vf_id) queue (queue_id) \
+                        fd_id (fd_id_value) packet (packet file name)
+
 For example, to add an ipv4-udp flow type filter::
 
    testpmd> flow_director_filter 0 mode IP add flow ipv4-udp src 2.2.2.3 32 \
@@ -2533,6 +2537,10 @@ For example, add an ipv4-other flow type filter::
              dst 2.2.2.5 tos 2 proto 20 ttl 40 vlan 0x1 \
              flexbytes (0x88,0x48) fwd pf queue 1 fd_id 1
 
+Note that for raw flow type mode the source and destination fields in the
+raw packet buffer need to be presented in a reversed order with respect
+to the expected received packets.
+
 flush_flow_director
 ~~~~~~~~~~~~~~~~~~~
 
@@ -2635,7 +2643,7 @@ Set the global configurations of hash filters::
 
    set_hash_global_config (port_id) (toeplitz|simple_xor|default) \
    (ipv4|ipv4-frag|ipv4-tcp|ipv4-udp|ipv4-sctp|ipv4-other|ipv6|ipv6-frag| \
-   ipv6-tcp|ipv6-udp|ipv6-sctp|ipv6-other|l2_payload) \
+   ipv6-tcp|ipv6-udp|ipv6-sctp|ipv6-other|l2_payload|<flow_id>) \
    (enable|disable)
 
 For example, to enable simple_xor for flow type of ipv6 on port 2::
@@ -2649,8 +2657,8 @@ Set the input set for hash::
 
    set_hash_input_set (port_id) (ipv4-frag|ipv4-tcp|ipv4-udp|ipv4-sctp| \
    ipv4-other|ipv6-frag|ipv6-tcp|ipv6-udp|ipv6-sctp|ipv6-other| \
-   l2_payload) (ovlan|ivlan|src-ipv4|dst-ipv4|src-ipv6|dst-ipv6|ipv4-tos| \
-   ipv4-proto|ipv6-tc|ipv6-next-header|udp-src-port|udp-dst-port| \
+   l2_payload|<flow_id>) (ovlan|ivlan|src-ipv4|dst-ipv4|src-ipv6|dst-ipv6| \
+   ipv4-tos|ipv4-proto|ipv6-tc|ipv6-next-header|udp-src-port|udp-dst-port| \
    tcp-src-port|tcp-dst-port|sctp-src-port|sctp-dst-port|sctp-veri-tag| \
    udp-key|gre-key|fld-1st|fld-2nd|fld-3rd|fld-4th|fld-5th|fld-6th|fld-7th| \
    fld-8th|none) (select|add)
@@ -2669,8 +2677,8 @@ Set the input set for flow director::
 
    set_fdir_input_set (port_id) (ipv4-frag|ipv4-tcp|ipv4-udp|ipv4-sctp| \
    ipv4-other|ipv6|ipv6-frag|ipv6-tcp|ipv6-udp|ipv6-sctp|ipv6-other| \
-   l2_payload) (ivlan|ethertype|src-ipv4|dst-ipv4|src-ipv6|dst-ipv6|ipv4-tos| \
-   ipv4-proto|ipv4-ttl|ipv6-tc|ipv6-next-header|ipv6-hop-limits| \
+   l2_payload|<flow_id>) (ivlan|ethertype|src-ipv4|dst-ipv4|src-ipv6|dst-ipv6| \
+   ipv4-tos|ipv4-proto|ipv4-ttl|ipv6-tc|ipv6-next-header|ipv6-hop-limits| \
    tudp-src-port|udp-dst-port|cp-src-port|tcp-dst-port|sctp-src-port| \
    sctp-dst-port|sctp-veri-tag|none) (select|add)
 
-- 
2.5.5