[dpdk-dev,10/24] ethdev: parse ethertype filter

Message ID 1480679625-4157-11-git-send-email-beilei.xing@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
checkpatch/checkpatch warning coding style issues

Commit Message

Xing, Beilei Dec. 2, 2016, 11:53 a.m. UTC
  Check if the rule is a ethertype rule, and get the ethertype
info BTW.

Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
Signed-off-by: Beilei Xing <beilei.xing@intel.com>
---
 lib/librte_ether/rte_flow.c        | 136 +++++++++++++++++++++++++++++++++++++
 lib/librte_ether/rte_flow_driver.h |  34 ++++++++++
 2 files changed, 170 insertions(+)
  

Comments

Ferruh Yigit Dec. 20, 2016, 6:12 p.m. UTC | #1
On 12/2/2016 11:53 AM, Beilei Xing wrote:
> Check if the rule is a ethertype rule, and get the ethertype
> info BTW.
> 
> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> Signed-off-by: Beilei Xing <beilei.xing@intel.com>
> ---

CC: Adrien Mazarguil <adrien.mazarguil@6wind.com>

>  lib/librte_ether/rte_flow.c        | 136 +++++++++++++++++++++++++++++++++++++
>  lib/librte_ether/rte_flow_driver.h |  34 ++++++++++

<...>

> diff --git a/lib/librte_ether/rte_flow_driver.h b/lib/librte_ether/rte_flow_driver.h
> index a88c621..2760c74 100644
> --- a/lib/librte_ether/rte_flow_driver.h
> +++ b/lib/librte_ether/rte_flow_driver.h
> @@ -170,6 +170,40 @@ rte_flow_error_set(struct rte_flow_error *error,
>  const struct rte_flow_ops *
>  rte_flow_ops_get(uint8_t port_id, struct rte_flow_error *error);
>  
> +int cons_parse_ethertype_filter(const struct rte_flow_attr *attr,
> +			    const struct rte_flow_item *pattern,
> +			    const struct rte_flow_action *actions,
> +			    struct rte_eth_ethertype_filter *filter,
> +			    struct rte_flow_error *error);

Although this is helper function, it may be good if it follows the
rte_follow namespace.

> +
> +#define PATTERN_SKIP_VOID(filter, filter_struct, error_type)		\
> +	do {								\
> +		if (!pattern) {						\
> +			memset(filter, 0, sizeof(filter_struct));	\
> +			error->type = error_type;                       \
> +			return -EINVAL;					\
> +		}							\
> +		item = pattern + i;					\

I believe macros that relies on variables that not passed as argument is
not good idea.

> +		while (item->type == RTE_FLOW_ITEM_TYPE_VOID) {		\
> +			i++;						\
> +			item = pattern + i;				\
> +		}							\
> +	} while (0)
> +
> +#define ACTION_SKIP_VOID(filter, filter_struct, error_type)		\
> +	do {								\
> +		if (!actions) {						\
> +			memset(filter, 0, sizeof(filter_struct));	\
> +			error->type = error_type;			\
> +			return -EINVAL;					\
> +		}							\
> +		act = actions + i;					\
> +		while (act->type == RTE_FLOW_ACTION_TYPE_VOID) {	\
> +			i++;						\
> +			act = actions + i;				\
> +		}							\
> +	} while (0)

Are these macros generic enough for all rte_flow consumers?

What do you think separate this patch, and use these after applied,
meanwhile keeping function and MACROS PMD internal?

> +
>  #ifdef __cplusplus
>  }
>  #endif
>
  
Xing, Beilei Dec. 21, 2016, 3:54 a.m. UTC | #2
Hi Ferruh,

> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Wednesday, December 21, 2016 2:12 AM
> To: Xing, Beilei <beilei.xing@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; Zhang, Helin <helin.zhang@intel.com>
> Cc: dev@dpdk.org; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Adrien Mazarguil
> <adrien.mazarguil@6wind.com>
> Subject: Re: [dpdk-dev] [PATCH 10/24] ethdev: parse ethertype filter
> 
> On 12/2/2016 11:53 AM, Beilei Xing wrote:
> > Check if the rule is a ethertype rule, and get the ethertype info BTW.
> >
> > Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> > Signed-off-by: Beilei Xing <beilei.xing@intel.com>
> > ---
> 
> CC: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> 
> >  lib/librte_ether/rte_flow.c        | 136
> +++++++++++++++++++++++++++++++++++++
> >  lib/librte_ether/rte_flow_driver.h |  34 ++++++++++
> 
> <...>
> 
> > diff --git a/lib/librte_ether/rte_flow_driver.h
> > b/lib/librte_ether/rte_flow_driver.h
> > index a88c621..2760c74 100644
> > --- a/lib/librte_ether/rte_flow_driver.h
> > +++ b/lib/librte_ether/rte_flow_driver.h
> > @@ -170,6 +170,40 @@ rte_flow_error_set(struct rte_flow_error *error,
> > const struct rte_flow_ops *  rte_flow_ops_get(uint8_t port_id, struct
> > rte_flow_error *error);
> >
> > +int cons_parse_ethertype_filter(const struct rte_flow_attr *attr,
> > +			    const struct rte_flow_item *pattern,
> > +			    const struct rte_flow_action *actions,
> > +			    struct rte_eth_ethertype_filter *filter,
> > +			    struct rte_flow_error *error);
> 
> Although this is helper function, it may be good if it follows the rte_follow
> namespace.

OK, I will rename it in the next version, thanks very much.

> 
> > +
> > +#define PATTERN_SKIP_VOID(filter, filter_struct, error_type)
> 	\
> > +	do {								\
> > +		if (!pattern) {						\
> > +			memset(filter, 0, sizeof(filter_struct));	\
> > +			error->type = error_type;                       \
> > +			return -EINVAL;
> 	\
> > +		}							\
> > +		item = pattern + i;					\
> 
> I believe macros that relies on variables that not passed as argument is not
> good idea.

Yes, I'm reworking the macros, and it will be changed in v2.

> 
> > +		while (item->type == RTE_FLOW_ITEM_TYPE_VOID) {
> 	\
> > +			i++;						\
> > +			item = pattern + i;				\
> > +		}							\
> > +	} while (0)
> > +
> > +#define ACTION_SKIP_VOID(filter, filter_struct, error_type)
> 	\
> > +	do {								\
> > +		if (!actions) {						\
> > +			memset(filter, 0, sizeof(filter_struct));	\
> > +			error->type = error_type;			\
> > +			return -EINVAL;
> 	\
> > +		}							\
> > +		act = actions + i;					\
> > +		while (act->type == RTE_FLOW_ACTION_TYPE_VOID) {	\
> > +			i++;						\
> > +			act = actions + i;				\
> > +		}							\
> > +	} while (0)
> 
> Are these macros generic enough for all rte_flow consumers?
> 
> What do you think separate this patch, and use these after applied,
> meanwhile keeping function and MACROS PMD internal?

The main purpose of the macros is to reduce the code in PMD, otherwise there'll be many such codes to get the next non-void item in all parse functions, including the parse_ethertype_filter function in rte_flow.c. But actually I'm not very sure if it's generic enough for all consumers, although I think it's general at present:) 
Thanks for your advice, I'll move the macros to PMD currently, then there'll be no macros used in parse_ethertype_filter function, and optimize it after applied.
BTW, I plan to send out V2 patch set in this week.

Best Regards,
Beilei

> 
> > +
> >  #ifdef __cplusplus
> >  }
> >  #endif
> >
  
Adrien Mazarguil Dec. 23, 2016, 8:43 a.m. UTC | #3
Hi all,

On Wed, Dec 21, 2016 at 03:54:50AM +0000, Xing, Beilei wrote:
> Hi Ferruh,
> 
> > -----Original Message-----
> > From: Yigit, Ferruh
> > Sent: Wednesday, December 21, 2016 2:12 AM
> > To: Xing, Beilei <beilei.xing@intel.com>; Wu, Jingjing
> > <jingjing.wu@intel.com>; Zhang, Helin <helin.zhang@intel.com>
> > Cc: dev@dpdk.org; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Adrien Mazarguil
> > <adrien.mazarguil@6wind.com>
> > Subject: Re: [dpdk-dev] [PATCH 10/24] ethdev: parse ethertype filter
> > 
> > On 12/2/2016 11:53 AM, Beilei Xing wrote:
> > > Check if the rule is a ethertype rule, and get the ethertype info BTW.
> > >
> > > Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> > > Signed-off-by: Beilei Xing <beilei.xing@intel.com>
> > > ---
> > 
> > CC: Adrien Mazarguil <adrien.mazarguil@6wind.com>

Thanks again for CC'ing me.

> > >  lib/librte_ether/rte_flow.c        | 136
> > +++++++++++++++++++++++++++++++++++++
> > >  lib/librte_ether/rte_flow_driver.h |  34 ++++++++++
> > 
> > <...>
> > 
> > > diff --git a/lib/librte_ether/rte_flow_driver.h
> > > b/lib/librte_ether/rte_flow_driver.h
> > > index a88c621..2760c74 100644
> > > --- a/lib/librte_ether/rte_flow_driver.h
> > > +++ b/lib/librte_ether/rte_flow_driver.h
> > > @@ -170,6 +170,40 @@ rte_flow_error_set(struct rte_flow_error *error,
> > > const struct rte_flow_ops *  rte_flow_ops_get(uint8_t port_id, struct
> > > rte_flow_error *error);
> > >
> > > +int cons_parse_ethertype_filter(const struct rte_flow_attr *attr,
> > > +			    const struct rte_flow_item *pattern,
> > > +			    const struct rte_flow_action *actions,
> > > +			    struct rte_eth_ethertype_filter *filter,
> > > +			    struct rte_flow_error *error);
> > 
> > Although this is helper function, it may be good if it follows the rte_follow
> > namespace.
> 
> OK, I will rename it in the next version, thanks very much.

Agreed, all public symbols exposed by headers must be prefixed with
rte_flow.

Now I'm not so sure about the need to convert a rte_flow rule to a
rte_eth_ethertype_filter. This definition basically makes rte_flow depend on
rte_eth_ctrl.h (related #include is missing by the way).

I understand that both ixgbe and i40e would benefit from it, and considering
rte_flow_driver.h is free from ABI versioning I guess it's acceptable, but
remember we'll gradually remove existing filter types so we should avoid new
dependencies on them. Just keep in mind this will be temporary.

Please add full documentation as well in Doxygen style like for existing
symbols. We have to maintain this API properly documented.

> > > +
> > > +#define PATTERN_SKIP_VOID(filter, filter_struct, error_type)
> > 	\
> > > +	do {								\
> > > +		if (!pattern) {						\
> > > +			memset(filter, 0, sizeof(filter_struct));	\
> > > +			error->type = error_type;                       \
> > > +			return -EINVAL;
> > 	\
> > > +		}							\
> > > +		item = pattern + i;					\
> > 
> > I believe macros that relies on variables that not passed as argument is not
> > good idea.
> 
> Yes, I'm reworking the macros, and it will be changed in v2.
> 
> > 
> > > +		while (item->type == RTE_FLOW_ITEM_TYPE_VOID) {
> > 	\
> > > +			i++;						\
> > > +			item = pattern + i;				\
> > > +		}							\
> > > +	} while (0)
> > > +
> > > +#define ACTION_SKIP_VOID(filter, filter_struct, error_type)
> > 	\
> > > +	do {								\
> > > +		if (!actions) {						\
> > > +			memset(filter, 0, sizeof(filter_struct));	\
> > > +			error->type = error_type;			\
> > > +			return -EINVAL;
> > 	\
> > > +		}							\
> > > +		act = actions + i;					\
> > > +		while (act->type == RTE_FLOW_ACTION_TYPE_VOID) {	\
> > > +			i++;						\
> > > +			act = actions + i;				\
> > > +		}							\
> > > +	} while (0)
> > 
> > Are these macros generic enough for all rte_flow consumers?
> > 
> > What do you think separate this patch, and use these after applied,
> > meanwhile keeping function and MACROS PMD internal?
> 
> The main purpose of the macros is to reduce the code in PMD, otherwise there'll be many such codes to get the next non-void item in all parse functions, including the parse_ethertype_filter function in rte_flow.c. But actually I'm not very sure if it's generic enough for all consumers, although I think it's general at present:) 

I'll concede skipping VOIDs can be tedious depending on the parser
implementation, but I do not think these macros need to be exposed
either. PMDs can duplicate some code such as this.

I think ixgbe and i40e share a fair amount of code already, and factoring it
should be part of larger task to create a common Intel-specific library
instead.

> Thanks for your advice, I'll move the macros to PMD currently, then there'll be no macros used in parse_ethertype_filter function, and optimize it after applied.
> 
> BTW, I plan to send out V2 patch set in this week.
> 
> Best Regards,
> Beilei
> 
> > 
> > > +
> > >  #ifdef __cplusplus
> > >  }
> > >  #endif
> > >
>
  
Xing, Beilei Dec. 27, 2016, 6:36 a.m. UTC | #4
> -----Original Message-----
> From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com]
> Sent: Friday, December 23, 2016 4:43 PM
> To: Xing, Beilei <beilei.xing@intel.com>
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; Zhang, Helin <helin.zhang@intel.com>;
> dev@dpdk.org; Lu, Wenzhuo <wenzhuo.lu@intel.com>
> Subject: Re: [dpdk-dev] [PATCH 10/24] ethdev: parse ethertype filter
> 
> Hi all,
> 
> On Wed, Dec 21, 2016 at 03:54:50AM +0000, Xing, Beilei wrote:
> > Hi Ferruh,
> >
> > > -----Original Message-----
> > > From: Yigit, Ferruh
> > > Sent: Wednesday, December 21, 2016 2:12 AM
> > > To: Xing, Beilei <beilei.xing@intel.com>; Wu, Jingjing
> > > <jingjing.wu@intel.com>; Zhang, Helin <helin.zhang@intel.com>
> > > Cc: dev@dpdk.org; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Adrien
> > > Mazarguil <adrien.mazarguil@6wind.com>
> > > Subject: Re: [dpdk-dev] [PATCH 10/24] ethdev: parse ethertype filter
> > >
> > > On 12/2/2016 11:53 AM, Beilei Xing wrote:
> > > > Check if the rule is a ethertype rule, and get the ethertype info BTW.
> > > >
> > > > Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> > > > Signed-off-by: Beilei Xing <beilei.xing@intel.com>
> > > > ---
> > >
> > > CC: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> 
> Thanks again for CC'ing me.
> 
> > > >  lib/librte_ether/rte_flow.c        | 136
> > > +++++++++++++++++++++++++++++++++++++
> > > >  lib/librte_ether/rte_flow_driver.h |  34 ++++++++++
> > >
> > > <...>
> > >
> > > > diff --git a/lib/librte_ether/rte_flow_driver.h
> > > > b/lib/librte_ether/rte_flow_driver.h
> > > > index a88c621..2760c74 100644
> > > > --- a/lib/librte_ether/rte_flow_driver.h
> > > > +++ b/lib/librte_ether/rte_flow_driver.h
> > > > @@ -170,6 +170,40 @@ rte_flow_error_set(struct rte_flow_error
> > > > *error, const struct rte_flow_ops *  rte_flow_ops_get(uint8_t
> > > > port_id, struct rte_flow_error *error);
> > > >
> > > > +int cons_parse_ethertype_filter(const struct rte_flow_attr *attr,
> > > > +			    const struct rte_flow_item *pattern,
> > > > +			    const struct rte_flow_action *actions,
> > > > +			    struct rte_eth_ethertype_filter *filter,
> > > > +			    struct rte_flow_error *error);
> > >
> > > Although this is helper function, it may be good if it follows the
> > > rte_follow namespace.
> >
> > OK, I will rename it in the next version, thanks very much.
> 
> Agreed, all public symbols exposed by headers must be prefixed with
> rte_flow.
> 
> Now I'm not so sure about the need to convert a rte_flow rule to a
> rte_eth_ethertype_filter. This definition basically makes rte_flow depend on
> rte_eth_ctrl.h (related #include is missing by the way).
> 

Since the whole implementation of parse function is modified, there'll be no common rte_eth_ethertype_filter here temporarily.

> I understand that both ixgbe and i40e would benefit from it, and considering
> rte_flow_driver.h is free from ABI versioning I guess it's acceptable, but
> remember we'll gradually remove existing filter types so we should avoid
> new dependencies on them. Just keep in mind this will be temporary.
> 

 i40e and ixgbe all use existing filter types in rte_flow_driver.h. if all existing filter types will be removed, we need to change the fiter info after applied.

> Please add full documentation as well in Doxygen style like for existing
> symbols. We have to maintain this API properly documented.
> 
> > > > +
> > > > +#define PATTERN_SKIP_VOID(filter, filter_struct, error_type)
> > > 	\
> > > > +	do {								\
> > > > +		if (!pattern) {						\
> > > > +			memset(filter, 0, sizeof(filter_struct));	\
> > > > +			error->type = error_type;                       \
> > > > +			return -EINVAL;
> > > 	\
> > > > +		}							\
> > > > +		item = pattern + i;					\
> > >
> > > I believe macros that relies on variables that not passed as
> > > argument is not good idea.
> >
> > Yes, I'm reworking the macros, and it will be changed in v2.
> >
> > >
> > > > +		while (item->type == RTE_FLOW_ITEM_TYPE_VOID) {
> > > 	\
> > > > +			i++;						\
> > > > +			item = pattern + i;				\
> > > > +		}							\
> > > > +	} while (0)
> > > > +
> > > > +#define ACTION_SKIP_VOID(filter, filter_struct, error_type)
> > > 	\
> > > > +	do {								\
> > > > +		if (!actions) {						\
> > > > +			memset(filter, 0, sizeof(filter_struct));	\
> > > > +			error->type = error_type;			\
> > > > +			return -EINVAL;
> > > 	\
> > > > +		}							\
> > > > +		act = actions + i;					\
> > > > +		while (act->type == RTE_FLOW_ACTION_TYPE_VOID) {	\
> > > > +			i++;						\
> > > > +			act = actions + i;				\
> > > > +		}							\
> > > > +	} while (0)
> > >
> > > Are these macros generic enough for all rte_flow consumers?
> > >
> > > What do you think separate this patch, and use these after applied,
> > > meanwhile keeping function and MACROS PMD internal?
> >
> > The main purpose of the macros is to reduce the code in PMD, otherwise
> > there'll be many such codes to get the next non-void item in all parse
> > functions, including the parse_ethertype_filter function in
> > rte_flow.c. But actually I'm not very sure if it's generic enough for
> > all consumers, although I think it's general at present:)
> 
> I'll concede skipping VOIDs can be tedious depending on the parser
> implementation, but I do not think these macros need to be exposed either.
> PMDs can duplicate some code such as this.
> 
> I think ixgbe and i40e share a fair amount of code already, and factoring it
> should be part of larger task to create a common Intel-specific library instead.


Good point. Thanks. We'll consider related implementation for the common code.
In V2 patch set, there'll be no common code temporarily since the implementation of parsing functions is different between ixgbe and i40e.

> 
> > Thanks for your advice, I'll move the macros to PMD currently, then there'll
> be no macros used in parse_ethertype_filter function, and optimize it after
> applied.
> >
> > BTW, I plan to send out V2 patch set in this week.
> >
> > Best Regards,
> > Beilei
> >
> > >
> > > > +
> > > >  #ifdef __cplusplus
> > > >  }
> > > >  #endif
> > > >
> >
> 
> --
> Adrien Mazarguil
> 6WIND
  

Patch

diff --git a/lib/librte_ether/rte_flow.c b/lib/librte_ether/rte_flow.c
index 064963d..acc9057 100644
--- a/lib/librte_ether/rte_flow.c
+++ b/lib/librte_ether/rte_flow.c
@@ -157,3 +157,139 @@  rte_flow_query(uint8_t port_id,
 			   NULL, rte_strerror(ENOTSUP));
 	return -rte_errno;
 }
+
+/**
+ * Parse the rule to see if it is a ethertype rule.
+ * And get the ethertype filter info BTW.
+ */
+int
+cons_parse_ethertype_filter(const struct rte_flow_attr *attr,
+			    const struct rte_flow_item *pattern,
+			    const struct rte_flow_action *actions,
+			    struct rte_eth_ethertype_filter *filter,
+			    struct rte_flow_error *error)
+{
+	const struct rte_flow_item *item;
+	const struct rte_flow_action *act;
+	const struct rte_flow_item_eth *eth_spec;
+	const struct rte_flow_item_eth *eth_mask;
+	const struct rte_flow_action_queue *act_q;
+	uint32_t i, j;
+
+	/************************************************
+	 * parse pattern
+	 ************************************************/
+	i = 0;
+
+	/* The first not void item should be MAC. */
+	PATTERN_SKIP_VOID(filter, struct rte_eth_ethertype_filter,
+			  RTE_FLOW_ERROR_TYPE_ITEM_NUM);
+	if (item->type != RTE_FLOW_ITEM_TYPE_ETH) {
+		error->type = RTE_FLOW_ERROR_TYPE_ITEM;
+		return -EINVAL;
+	}
+
+	/* Get the MAC info. */
+	if (!item->spec || !item->mask) {
+		error->type = RTE_FLOW_ERROR_TYPE_ITEM;
+		return -EINVAL;
+	}
+
+	eth_spec = (const struct rte_flow_item_eth *)item->spec;
+	eth_mask = (const struct rte_flow_item_eth *)item->mask;
+	/**
+	 * Source MAC address must be masked.
+	 * Destination MAC address must be totally masked or not.
+	 */
+	if (eth_mask->src.addr_bytes[0] ||
+	    (eth_mask->dst.addr_bytes[0] != 0xFF &&
+	     eth_mask->dst.addr_bytes[0])) {
+		error->type = RTE_FLOW_ERROR_TYPE_ITEM;
+		return -EINVAL;
+	}
+
+	for (j = 1; j < ETHER_ADDR_LEN; j++) {
+		if (eth_mask->src.addr_bytes[j] !=
+		    eth_mask->src.addr_bytes[0] ||
+		    eth_mask->dst.addr_bytes[j] !=
+		    eth_mask->dst.addr_bytes[0]) {
+			error->type = RTE_FLOW_ERROR_TYPE_ITEM;
+			return -EINVAL;
+		}
+	}
+
+	if ((eth_mask->type & 0xFFFF) != 0xFFFF) {
+		error->type = RTE_FLOW_ERROR_TYPE_ITEM;
+		return -EINVAL;
+	}
+
+	if (eth_mask->dst.addr_bytes[0]) {
+		filter->mac_addr = eth_spec->dst;
+		filter->flags |= RTE_ETHTYPE_FLAGS_MAC;
+	} else {
+		filter->flags &= ~RTE_ETHTYPE_FLAGS_MAC;
+	}
+	filter->ether_type = (uint16_t)eth_spec->type;
+
+	/* Check if the next not void item is END. */
+	i++;
+	PATTERN_SKIP_VOID(filter, struct rte_eth_ethertype_filter,
+			  RTE_FLOW_ERROR_TYPE_ITEM_NUM);
+	if (item->type != RTE_FLOW_ITEM_TYPE_END) {
+		error->type = RTE_FLOW_ERROR_TYPE_ITEM;
+		return -EINVAL;
+	}
+
+	/************************************************
+	 * parse action
+	 ************************************************/
+	i = 0;
+
+	/* Check if the first not void action is QUEUE or DROP. */
+	ACTION_SKIP_VOID(filter, struct rte_eth_ethertype_filter,
+			 RTE_FLOW_ERROR_TYPE_ACTION_NUM);
+	if (act->type != RTE_FLOW_ACTION_TYPE_QUEUE &&
+	    act->type != RTE_FLOW_ACTION_TYPE_DROP) {
+		error->type = RTE_FLOW_ERROR_TYPE_ACTION;
+		return -EINVAL;
+	}
+
+	if (act->type == RTE_FLOW_ACTION_TYPE_QUEUE) {
+		act_q = (const struct rte_flow_action_queue *)act->conf;
+		filter->queue = act_q->index;
+	} else {
+		filter->flags |= RTE_ETHTYPE_FLAGS_DROP;
+	}
+
+	/* Check if the next not void item is END */
+	i++;
+	ACTION_SKIP_VOID(filter, struct rte_eth_ethertype_filter,
+			 RTE_FLOW_ERROR_TYPE_ACTION_NUM);
+	if (act->type != RTE_FLOW_ACTION_TYPE_END) {
+		error->type = RTE_FLOW_ERROR_TYPE_ACTION;
+		return -EINVAL;
+	}
+
+	/************************************************
+	 * parse attr
+	 ************************************************/
+	/* Must be input direction */
+	if (!attr->ingress) {
+		error->type = RTE_FLOW_ERROR_TYPE_ATTR_INGRESS;
+		return -EINVAL;
+	}
+
+	/* Not supported */
+	if (attr->egress) {
+		error->type = RTE_FLOW_ERROR_TYPE_ATTR_EGRESS;
+		return -EINVAL;
+	}
+
+	/* Not supported */
+	if (attr->priority) {
+		error->type = RTE_FLOW_ERROR_TYPE_ATTR_PRIORITY;
+		return -EINVAL;
+	}
+
+	return 0;
+}
diff --git a/lib/librte_ether/rte_flow_driver.h b/lib/librte_ether/rte_flow_driver.h
index a88c621..2760c74 100644
--- a/lib/librte_ether/rte_flow_driver.h
+++ b/lib/librte_ether/rte_flow_driver.h
@@ -170,6 +170,40 @@  rte_flow_error_set(struct rte_flow_error *error,
 const struct rte_flow_ops *
 rte_flow_ops_get(uint8_t port_id, struct rte_flow_error *error);
 
+int cons_parse_ethertype_filter(const struct rte_flow_attr *attr,
+			    const struct rte_flow_item *pattern,
+			    const struct rte_flow_action *actions,
+			    struct rte_eth_ethertype_filter *filter,
+			    struct rte_flow_error *error);
+
+#define PATTERN_SKIP_VOID(filter, filter_struct, error_type)		\
+	do {								\
+		if (!pattern) {						\
+			memset(filter, 0, sizeof(filter_struct));	\
+			error->type = error_type;                       \
+			return -EINVAL;					\
+		}							\
+		item = pattern + i;					\
+		while (item->type == RTE_FLOW_ITEM_TYPE_VOID) {		\
+			i++;						\
+			item = pattern + i;				\
+		}							\
+	} while (0)
+
+#define ACTION_SKIP_VOID(filter, filter_struct, error_type)		\
+	do {								\
+		if (!actions) {						\
+			memset(filter, 0, sizeof(filter_struct));	\
+			error->type = error_type;			\
+			return -EINVAL;					\
+		}							\
+		act = actions + i;					\
+		while (act->type == RTE_FLOW_ACTION_TYPE_VOID) {	\
+			i++;						\
+			act = actions + i;				\
+		}							\
+	} while (0)
+
 #ifdef __cplusplus
 }
 #endif