[dpdk-dev,v2,07/17] net/i40e: add flow validate function
Checks
Commit Message
This patch adds i40e_flow_validation function to check if
a flow is valid according to the flow pattern.
i40e_parse_ethertype_filter is added first, it also gets
the ethertype info.
i40e_flow.c is added to handle all generic filter events.
Signed-off-by: Beilei Xing <beilei.xing@intel.com>
---
drivers/net/i40e/Makefile | 1 +
drivers/net/i40e/i40e_ethdev.c | 5 +
drivers/net/i40e/i40e_ethdev.h | 20 ++
drivers/net/i40e/i40e_flow.c | 431 +++++++++++++++++++++++++++++++++++++++++
4 files changed, 457 insertions(+)
create mode 100644 drivers/net/i40e/i40e_flow.c
Comments
Hi Beilei,
A few comments below.
On Tue, Dec 27, 2016 at 02:26:14PM +0800, Beilei Xing wrote:
> This patch adds i40e_flow_validation function to check if
> a flow is valid according to the flow pattern.
> i40e_parse_ethertype_filter is added first, it also gets
> the ethertype info.
> i40e_flow.c is added to handle all generic filter events.
>
> Signed-off-by: Beilei Xing <beilei.xing@intel.com>
> ---
> drivers/net/i40e/Makefile | 1 +
> drivers/net/i40e/i40e_ethdev.c | 5 +
> drivers/net/i40e/i40e_ethdev.h | 20 ++
> drivers/net/i40e/i40e_flow.c | 431 +++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 457 insertions(+)
> create mode 100644 drivers/net/i40e/i40e_flow.c
[...]
> diff --git a/drivers/net/i40e/i40e_flow.c b/drivers/net/i40e/i40e_flow.c
> new file mode 100644
> index 0000000..bf451ef
> --- /dev/null
> +++ b/drivers/net/i40e/i40e_flow.c
[...]
> + if (ethertype_filter->queue >= pf->dev_data->nb_rx_queues) {
> + rte_flow_error_set(error, EINVAL,
> + RTE_FLOW_ERROR_TYPE_ACTION,
> + NULL, "Invalid queue ID for"
> + " ethertype_filter.");
When setting an error type related to an existing object provided by the
application, you should set the related cause pointer to a non-NULL
value. In this particular case, retrieving the action object seems difficult
so it can remain that way, however there are many places in this series
where it can be done.
> + return -EINVAL;
While this is perfectly valid, you could also return -rte_errno to avoid
duplicating EINVAL.
[...]
> + }
> + if (ethertype_filter->ether_type == ETHER_TYPE_IPv4 ||
> + ethertype_filter->ether_type == ETHER_TYPE_IPv6) {
> + rte_flow_error_set(error, ENOTSUP,
> + RTE_FLOW_ERROR_TYPE_ITEM,
> + NULL, "Unsupported ether_type in"
> + " control packet filter.");
> + return -ENOTSUP;
> + }
> + if (ethertype_filter->ether_type == ETHER_TYPE_VLAN)
> + PMD_DRV_LOG(WARNING, "filter vlan ether_type in"
> + " first tag is not supported.");
> +
> + return ret;
> +}
[...]
> +/* Parse attributes */
> +static int
> +i40e_parse_attr(const struct rte_flow_attr *attr,
> + struct rte_flow_error *error)
> +{
> + /* Must be input direction */
> + if (!attr->ingress) {
> + rte_flow_error_set(error, EINVAL,
> + RTE_FLOW_ERROR_TYPE_ATTR_INGRESS,
> + NULL, "Only support ingress.");
Regarding my previous comment, &attr could replace NULL here as well as in
subsequent calls to rte_flow_error_set().
> + return -EINVAL;
> + }
> +
> + /* Not supported */
> + if (attr->egress) {
> + rte_flow_error_set(error, EINVAL,
> + RTE_FLOW_ERROR_TYPE_ATTR_EGRESS,
> + NULL, "Not support egress.");
> + return -EINVAL;
> + }
> +
> + /* Not supported */
> + if (attr->priority) {
> + rte_flow_error_set(error, EINVAL,
> + RTE_FLOW_ERROR_TYPE_ATTR_PRIORITY,
> + NULL, "Not support priority.");
> + return -EINVAL;
> + }
> +
> + /* Not supported */
> + if (attr->group) {
> + rte_flow_error_set(error, EINVAL,
> + RTE_FLOW_ERROR_TYPE_ATTR_GROUP,
> + NULL, "Not support group.");
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int
> +i40e_parse_ethertype_pattern(const struct rte_flow_item *pattern,
> + struct rte_flow_error *error,
> + struct rte_eth_ethertype_filter *filter)
> +{
> + const struct rte_flow_item *item = pattern;
> + const struct rte_flow_item_eth *eth_spec;
> + const struct rte_flow_item_eth *eth_mask;
> + enum rte_flow_item_type item_type;
> +
> + for (; item->type != RTE_FLOW_ITEM_TYPE_END; item++) {
> + item_type = item->type;
> + switch (item_type) {
> + case RTE_FLOW_ITEM_TYPE_ETH:
> + eth_spec = (const struct rte_flow_item_eth *)item->spec;
> + eth_mask = (const struct rte_flow_item_eth *)item->mask;
> + /* Get the MAC info. */
> + if (!eth_spec || !eth_mask) {
> + rte_flow_error_set(error, EINVAL,
> + RTE_FLOW_ERROR_TYPE_ITEM,
> + NULL,
> + "NULL ETH spec/mask");
> + return -EINVAL;
> + }
While optional, I think you should allow eth_spec and eth_mask to be NULL
here as described in [1]:
- If eth_spec is NULL, you can match anything that looks like a valid
Ethernet header.
- If eth_mask is NULL, you should assume a default mask (for Ethernet it
usually means matching source/destination MACs perfectly).
- You must check the "last" field as well, if non-NULL it may probably be
supported as long as the following condition is satisfied:
(spec & mask) == (last & mask)
[1] http://dpdk.org/doc/guides/prog_guide/rte_flow.html#pattern-item
[...]
> + const struct rte_flow_action_queue *act_q;
> + uint32_t index = 0;
> +
> + /* Check if the first non-void action is QUEUE or DROP. */
> + NEXT_ITEM_OF_ACTION(act, actions, index);
> + if (act->type != RTE_FLOW_ACTION_TYPE_QUEUE &&
> + act->type != RTE_FLOW_ACTION_TYPE_DROP) {
> + rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ACTION,
> + NULL, "Not supported action.");
Again, you could report &act instead of NULL here (please check all
remaining calls to rte_flow_error_set()).
[...]
>
> +union i40e_filter_t {
> + struct rte_eth_ethertype_filter ethertype_filter;
> + struct rte_eth_fdir_filter fdir_filter;
> + struct rte_eth_tunnel_filter_conf tunnel_filter; } cons_filter;
> +
> +typedef int (*parse_filter_t)(struct rte_eth_dev *dev,
> + const struct rte_flow_attr *attr,
> + const struct rte_flow_item pattern[],
> + const struct rte_flow_action actions[],
> + struct rte_flow_error *error,
> + union i40e_filter_t *filter);
You can use void* instead of define union i40e_filter_t.
> +struct i40e_valid_pattern {
> + enum rte_flow_item_type *items;
What the item points to? Add few comments
> +
> + ret = parse_filter(dev, attr, items, actions, error, &cons_filter);
Will you use cons_filter later? If not, it looks like we don't need the argument at all.
> +
> + rte_free(items);
> +
> + return ret;
> +}
> --
> 2.5.5
On Tue, Dec 27, 2016 at 02:26:14PM +0800, Beilei Xing wrote:
> This patch adds i40e_flow_validation function to check if
> a flow is valid according to the flow pattern.
> i40e_parse_ethertype_filter is added first, it also gets
> the ethertype info.
> i40e_flow.c is added to handle all generic filter events.
>
> Signed-off-by: Beilei Xing <beilei.xing@intel.com>
> ---
> drivers/net/i40e/Makefile | 1 +
> drivers/net/i40e/i40e_ethdev.c | 5 +
> drivers/net/i40e/i40e_ethdev.h | 20 ++
> drivers/net/i40e/i40e_flow.c | 431 +++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 457 insertions(+)
> create mode 100644 drivers/net/i40e/i40e_flow.c
>
> diff --git a/drivers/net/i40e/Makefile b/drivers/net/i40e/Makefile
> index 11175c4..89bd85a 100644
> --- a/drivers/net/i40e/Makefile
> +++ b/drivers/net/i40e/Makefile
> @@ -105,6 +105,7 @@ endif
> SRCS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += i40e_ethdev_vf.c
> SRCS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += i40e_pf.c
> SRCS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += i40e_fdir.c
> +SRCS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += i40e_flow.c
>
> # vector PMD driver needs SSE4.1 support
> ifeq ($(findstring RTE_MACHINE_CPUFLAG_SSE4_1,$(CFLAGS)),)
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 7f98b79..80024ed 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -8452,6 +8452,11 @@ i40e_dev_filter_ctrl(struct rte_eth_dev *dev,
> case RTE_ETH_FILTER_FDIR:
> ret = i40e_fdir_ctrl_func(dev, filter_op, arg);
> break;
> + case RTE_ETH_FILTER_GENERIC:
> + if (filter_op != RTE_ETH_FILTER_GET)
> + return -EINVAL;
> + *(const void **)arg = &i40e_flow_ops;
> + break;
> default:
> PMD_DRV_LOG(WARNING, "Filter type (%d) not supported",
> filter_type);
> diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
> index 6089895..bbe52f0 100644
> --- a/drivers/net/i40e/i40e_ethdev.h
> +++ b/drivers/net/i40e/i40e_ethdev.h
> @@ -38,6 +38,7 @@
> #include <rte_time.h>
> #include <rte_kvargs.h>
> #include <rte_hash.h>
> +#include <rte_flow_driver.h>
>
> #define I40E_VLAN_TAG_SIZE 4
>
> @@ -629,6 +630,23 @@ struct i40e_adapter {
> struct rte_timecounter tx_tstamp_tc;
> };
>
> +union i40e_filter_t {
> + struct rte_eth_ethertype_filter ethertype_filter;
> + struct rte_eth_fdir_filter fdir_filter;
> + struct rte_eth_tunnel_filter_conf tunnel_filter;
> +} cons_filter;
> +
Are you sure that you want to define a variable in i40e_ethdev.h?
> +typedef int (*parse_filter_t)(struct rte_eth_dev *dev,
> + const struct rte_flow_attr *attr,
> + const struct rte_flow_item pattern[],
> + const struct rte_flow_action actions[],
> + struct rte_flow_error *error,
> + union i40e_filter_t *filter);
> +struct i40e_valid_pattern {
> + enum rte_flow_item_type *items;
> + parse_filter_t parse_filter;
> +};
> +
> int i40e_dev_switch_queues(struct i40e_pf *pf, bool on);
> int i40e_vsi_release(struct i40e_vsi *vsi);
> struct i40e_vsi *i40e_vsi_setup(struct i40e_pf *pf,
> @@ -823,4 +841,6 @@ i40e_calc_itr_interval(int16_t interval)
> ((phy_type) & I40E_CAP_PHY_TYPE_25GBASE_SR) || \
> ((phy_type) & I40E_CAP_PHY_TYPE_25GBASE_LR))
>
> +const struct rte_flow_ops i40e_flow_ops;
> +
Same here. Are you sure that you want to define a variable in i40e_ethdev.h?
Maybe you should add the `extern' qualifier.
Best regards,
Tiwei Bie
> -----Original Message-----
> From: Wu, Jingjing
> Sent: Wednesday, December 28, 2016 10:52 AM
> To: Xing, Beilei <beilei.xing@intel.com>; Zhang, Helin
> <helin.zhang@intel.com>
> Cc: dev@dpdk.org
> Subject: RE: [PATCH v2 07/17] net/i40e: add flow validate function
>
>
> >
> > +union i40e_filter_t {
> > + struct rte_eth_ethertype_filter ethertype_filter;
> > + struct rte_eth_fdir_filter fdir_filter;
> > + struct rte_eth_tunnel_filter_conf tunnel_filter; } cons_filter;
> > +
> > +typedef int (*parse_filter_t)(struct rte_eth_dev *dev,
> > + const struct rte_flow_attr *attr,
> > + const struct rte_flow_item pattern[],
> > + const struct rte_flow_action actions[],
> > + struct rte_flow_error *error,
> > + union i40e_filter_t *filter);
> You can use void* instead of define union i40e_filter_t.
I tried the void * before, but I should determine the filter type when creating a flow. If using void*, I can get the filter info but I don't know which filer type it belongs to.
>
> > +struct i40e_valid_pattern {
> > + enum rte_flow_item_type *items;
> What the item points to? Add few comments
It's the pattern without VOID items. I'll add comments in next version.
> > +
> > + ret = parse_filter(dev, attr, items, actions, error, &cons_filter);
>
> Will you use cons_filter later? If not, it looks like we don't need the argument
> at all.
Yes, it's used to create flow. We us parse_filter to get the filter info. When creating a flow, flow_validate will be involved first to get filter info, then set filter according to the filter info.
> > +
> > + rte_free(items);
> > +
> > + return ret;
> > +}
> > --
> > 2.5.5
> -----Original Message-----
> From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com]
> Sent: Tuesday, December 27, 2016 8:40 PM
> To: Xing, Beilei <beilei.xing@intel.com>
> Cc: Wu, Jingjing <jingjing.wu@intel.com>; Zhang, Helin
> <helin.zhang@intel.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 07/17] net/i40e: add flow validate
> function
>
> Hi Beilei,
>
> A few comments below.
>
> On Tue, Dec 27, 2016 at 02:26:14PM +0800, Beilei Xing wrote:
> > This patch adds i40e_flow_validation function to check if a flow is
> > valid according to the flow pattern.
> > i40e_parse_ethertype_filter is added first, it also gets the ethertype
> > info.
> > i40e_flow.c is added to handle all generic filter events.
> >
> > Signed-off-by: Beilei Xing <beilei.xing@intel.com>
> > ---
> > drivers/net/i40e/Makefile | 1 +
> > drivers/net/i40e/i40e_ethdev.c | 5 +
> > drivers/net/i40e/i40e_ethdev.h | 20 ++
> > drivers/net/i40e/i40e_flow.c | 431
> +++++++++++++++++++++++++++++++++++++++++
> > 4 files changed, 457 insertions(+)
> > create mode 100644 drivers/net/i40e/i40e_flow.c
> [...]
> > diff --git a/drivers/net/i40e/i40e_flow.c
> > b/drivers/net/i40e/i40e_flow.c new file mode 100644 index
> > 0000000..bf451ef
> > --- /dev/null
> > +++ b/drivers/net/i40e/i40e_flow.c
> [...]
> > + if (ethertype_filter->queue >= pf->dev_data->nb_rx_queues) {
> > + rte_flow_error_set(error, EINVAL,
> > + RTE_FLOW_ERROR_TYPE_ACTION,
> > + NULL, "Invalid queue ID for"
> > + " ethertype_filter.");
>
> When setting an error type related to an existing object provided by the
> application, you should set the related cause pointer to a non-NULL value. In
> this particular case, retrieving the action object seems difficult so it can
> remain that way, however there are many places in this series where it can
> be done.
OK, I got the meaning and usage of cause pointer now. Thanks for the explaination.
>
> > + return -EINVAL;
>
> While this is perfectly valid, you could also return -rte_errno to avoid
> duplicating EINVAL.
Yes, agree.
>
> [...]
> > + }
> > + if (ethertype_filter->ether_type == ETHER_TYPE_IPv4 ||
> > + ethertype_filter->ether_type == ETHER_TYPE_IPv6) {
> > + rte_flow_error_set(error, ENOTSUP,
> > + RTE_FLOW_ERROR_TYPE_ITEM,
> > + NULL, "Unsupported ether_type in"
> > + " control packet filter.");
> > + return -ENOTSUP;
> > + }
> > + if (ethertype_filter->ether_type == ETHER_TYPE_VLAN)
> > + PMD_DRV_LOG(WARNING, "filter vlan ether_type in"
> > + " first tag is not supported.");
> > +
> > + return ret;
> > +}
> [...]
> > +/* Parse attributes */
> > +static int
> > +i40e_parse_attr(const struct rte_flow_attr *attr,
> > + struct rte_flow_error *error)
> > +{
> > + /* Must be input direction */
> > + if (!attr->ingress) {
> > + rte_flow_error_set(error, EINVAL,
> > + RTE_FLOW_ERROR_TYPE_ATTR_INGRESS,
> > + NULL, "Only support ingress.");
>
> Regarding my previous comment, &attr could replace NULL here as well as in
> subsequent calls to rte_flow_error_set().
Got it, thanks.
>
> > + return -EINVAL;
> > + }
> > +
> > + /* Not supported */
> > + if (attr->egress) {
> > + rte_flow_error_set(error, EINVAL,
> > + RTE_FLOW_ERROR_TYPE_ATTR_EGRESS,
> > + NULL, "Not support egress.");
> > + return -EINVAL;
> > + }
> > +
> > + /* Not supported */
> > + if (attr->priority) {
> > + rte_flow_error_set(error, EINVAL,
> > + RTE_FLOW_ERROR_TYPE_ATTR_PRIORITY,
> > + NULL, "Not support priority.");
> > + return -EINVAL;
> > + }
> > +
> > + /* Not supported */
> > + if (attr->group) {
> > + rte_flow_error_set(error, EINVAL,
> > + RTE_FLOW_ERROR_TYPE_ATTR_GROUP,
> > + NULL, "Not support group.");
> > + return -EINVAL;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int
> > +i40e_parse_ethertype_pattern(const struct rte_flow_item *pattern,
> > + struct rte_flow_error *error,
> > + struct rte_eth_ethertype_filter *filter) {
> > + const struct rte_flow_item *item = pattern;
> > + const struct rte_flow_item_eth *eth_spec;
> > + const struct rte_flow_item_eth *eth_mask;
> > + enum rte_flow_item_type item_type;
> > +
> > + for (; item->type != RTE_FLOW_ITEM_TYPE_END; item++) {
> > + item_type = item->type;
> > + switch (item_type) {
> > + case RTE_FLOW_ITEM_TYPE_ETH:
> > + eth_spec = (const struct rte_flow_item_eth *)item-
> >spec;
> > + eth_mask = (const struct rte_flow_item_eth *)item-
> >mask;
> > + /* Get the MAC info. */
> > + if (!eth_spec || !eth_mask) {
> > + rte_flow_error_set(error, EINVAL,
> > +
> RTE_FLOW_ERROR_TYPE_ITEM,
> > + NULL,
> > + "NULL ETH spec/mask");
> > + return -EINVAL;
> > + }
>
> While optional, I think you should allow eth_spec and eth_mask to be NULL
> here as described in [1]:
>
> - If eth_spec is NULL, you can match anything that looks like a valid
> Ethernet header.
>
> - If eth_mask is NULL, you should assume a default mask (for Ethernet it
> usually means matching source/destination MACs perfectly).
>
> - You must check the "last" field as well, if non-NULL it may probably be
> supported as long as the following condition is satisfied:
>
> (spec & mask) == (last & mask)
>
> [1] http://dpdk.org/doc/guides/prog_guide/rte_flow.html#pattern-item
>
Thanks for the specification. In fact, we don't support the "last" for both ixgbe and i40e currently according to the original design, so we only support perfect match till now. We will support it in the future, as the deadline is coming, what do you think?
> [...]
> > + const struct rte_flow_action_queue *act_q;
> > + uint32_t index = 0;
> > +
> > + /* Check if the first non-void action is QUEUE or DROP. */
> > + NEXT_ITEM_OF_ACTION(act, actions, index);
> > + if (act->type != RTE_FLOW_ACTION_TYPE_QUEUE &&
> > + act->type != RTE_FLOW_ACTION_TYPE_DROP) {
> > + rte_flow_error_set(error, EINVAL,
> RTE_FLOW_ERROR_TYPE_ACTION,
> > + NULL, "Not supported action.");
>
> Again, you could report &act instead of NULL here (please check all remaining
> calls to rte_flow_error_set()).
>
> [...]
>
> --
> Adrien Mazarguil
> 6WIND
Hi Beilei,
On Wed, Dec 28, 2016 at 09:00:03AM +0000, Xing, Beilei wrote:
>
>
> > -----Original Message-----
> > From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com]
> > Sent: Tuesday, December 27, 2016 8:40 PM
> > To: Xing, Beilei <beilei.xing@intel.com>
> > Cc: Wu, Jingjing <jingjing.wu@intel.com>; Zhang, Helin
> > <helin.zhang@intel.com>; dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v2 07/17] net/i40e: add flow validate
> > function
> >
> > Hi Beilei,
> >
> > A few comments below.
> >
> > On Tue, Dec 27, 2016 at 02:26:14PM +0800, Beilei Xing wrote:
> > > This patch adds i40e_flow_validation function to check if a flow is
> > > valid according to the flow pattern.
> > > i40e_parse_ethertype_filter is added first, it also gets the ethertype
> > > info.
> > > i40e_flow.c is added to handle all generic filter events.
> > >
> > > Signed-off-by: Beilei Xing <beilei.xing@intel.com>
> > > ---
> > > drivers/net/i40e/Makefile | 1 +
> > > drivers/net/i40e/i40e_ethdev.c | 5 +
> > > drivers/net/i40e/i40e_ethdev.h | 20 ++
> > > drivers/net/i40e/i40e_flow.c | 431
> > +++++++++++++++++++++++++++++++++++++++++
> > > 4 files changed, 457 insertions(+)
> > > create mode 100644 drivers/net/i40e/i40e_flow.c
> > [...]
> > > diff --git a/drivers/net/i40e/i40e_flow.c
> > > b/drivers/net/i40e/i40e_flow.c new file mode 100644 index
> > > 0000000..bf451ef
> > > --- /dev/null
> > > +++ b/drivers/net/i40e/i40e_flow.c
> > [...]
> > > + if (ethertype_filter->queue >= pf->dev_data->nb_rx_queues) {
> > > + rte_flow_error_set(error, EINVAL,
> > > + RTE_FLOW_ERROR_TYPE_ACTION,
> > > + NULL, "Invalid queue ID for"
> > > + " ethertype_filter.");
> >
> > When setting an error type related to an existing object provided by the
> > application, you should set the related cause pointer to a non-NULL value. In
> > this particular case, retrieving the action object seems difficult so it can
> > remain that way, however there are many places in this series where it can
> > be done.
>
> OK, I got the meaning and usage of cause pointer now. Thanks for the explaination.
>
> >
> > > + return -EINVAL;
> >
> > While this is perfectly valid, you could also return -rte_errno to avoid
> > duplicating EINVAL.
>
> Yes, agree.
>
> >
> > [...]
> > > + }
> > > + if (ethertype_filter->ether_type == ETHER_TYPE_IPv4 ||
> > > + ethertype_filter->ether_type == ETHER_TYPE_IPv6) {
> > > + rte_flow_error_set(error, ENOTSUP,
> > > + RTE_FLOW_ERROR_TYPE_ITEM,
> > > + NULL, "Unsupported ether_type in"
> > > + " control packet filter.");
> > > + return -ENOTSUP;
> > > + }
> > > + if (ethertype_filter->ether_type == ETHER_TYPE_VLAN)
> > > + PMD_DRV_LOG(WARNING, "filter vlan ether_type in"
> > > + " first tag is not supported.");
> > > +
> > > + return ret;
> > > +}
> > [...]
> > > +/* Parse attributes */
> > > +static int
> > > +i40e_parse_attr(const struct rte_flow_attr *attr,
> > > + struct rte_flow_error *error)
> > > +{
> > > + /* Must be input direction */
> > > + if (!attr->ingress) {
> > > + rte_flow_error_set(error, EINVAL,
> > > + RTE_FLOW_ERROR_TYPE_ATTR_INGRESS,
> > > + NULL, "Only support ingress.");
> >
> > Regarding my previous comment, &attr could replace NULL here as well as in
> > subsequent calls to rte_flow_error_set().
>
> Got it, thanks.
>
> >
> > > + return -EINVAL;
> > > + }
> > > +
> > > + /* Not supported */
> > > + if (attr->egress) {
> > > + rte_flow_error_set(error, EINVAL,
> > > + RTE_FLOW_ERROR_TYPE_ATTR_EGRESS,
> > > + NULL, "Not support egress.");
> > > + return -EINVAL;
> > > + }
> > > +
> > > + /* Not supported */
> > > + if (attr->priority) {
> > > + rte_flow_error_set(error, EINVAL,
> > > + RTE_FLOW_ERROR_TYPE_ATTR_PRIORITY,
> > > + NULL, "Not support priority.");
> > > + return -EINVAL;
> > > + }
> > > +
> > > + /* Not supported */
> > > + if (attr->group) {
> > > + rte_flow_error_set(error, EINVAL,
> > > + RTE_FLOW_ERROR_TYPE_ATTR_GROUP,
> > > + NULL, "Not support group.");
> > > + return -EINVAL;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int
> > > +i40e_parse_ethertype_pattern(const struct rte_flow_item *pattern,
> > > + struct rte_flow_error *error,
> > > + struct rte_eth_ethertype_filter *filter) {
> > > + const struct rte_flow_item *item = pattern;
> > > + const struct rte_flow_item_eth *eth_spec;
> > > + const struct rte_flow_item_eth *eth_mask;
> > > + enum rte_flow_item_type item_type;
> > > +
> > > + for (; item->type != RTE_FLOW_ITEM_TYPE_END; item++) {
> > > + item_type = item->type;
> > > + switch (item_type) {
> > > + case RTE_FLOW_ITEM_TYPE_ETH:
> > > + eth_spec = (const struct rte_flow_item_eth *)item-
> > >spec;
> > > + eth_mask = (const struct rte_flow_item_eth *)item-
> > >mask;
> > > + /* Get the MAC info. */
> > > + if (!eth_spec || !eth_mask) {
> > > + rte_flow_error_set(error, EINVAL,
> > > +
> > RTE_FLOW_ERROR_TYPE_ITEM,
> > > + NULL,
> > > + "NULL ETH spec/mask");
> > > + return -EINVAL;
> > > + }
> >
> > While optional, I think you should allow eth_spec and eth_mask to be NULL
> > here as described in [1]:
> >
> > - If eth_spec is NULL, you can match anything that looks like a valid
> > Ethernet header.
> >
> > - If eth_mask is NULL, you should assume a default mask (for Ethernet it
> > usually means matching source/destination MACs perfectly).
> >
> > - You must check the "last" field as well, if non-NULL it may probably be
> > supported as long as the following condition is satisfied:
> >
> > (spec & mask) == (last & mask)
> >
> > [1] http://dpdk.org/doc/guides/prog_guide/rte_flow.html#pattern-item
> >
>
> Thanks for the specification. In fact, we don't support the "last" for both ixgbe and i40e currently according to the original design, so we only support perfect match till now. We will support it in the future, as the deadline is coming, what do you think?
If you want to handle it later it's fine, however in that case you need to
at least generate an error when last is non-NULL (I did not see such a check
in this code).
Note that supporting it properly as defined in the API could be relatively
easy by implementing the above condition, it's just a small step above
simply checking for a NULL value.
> > [...]
> > > + const struct rte_flow_action_queue *act_q;
> > > + uint32_t index = 0;
> > > +
> > > + /* Check if the first non-void action is QUEUE or DROP. */
> > > + NEXT_ITEM_OF_ACTION(act, actions, index);
> > > + if (act->type != RTE_FLOW_ACTION_TYPE_QUEUE &&
> > > + act->type != RTE_FLOW_ACTION_TYPE_DROP) {
> > > + rte_flow_error_set(error, EINVAL,
> > RTE_FLOW_ERROR_TYPE_ACTION,
> > > + NULL, "Not supported action.");
> >
> > Again, you could report &act instead of NULL here (please check all remaining
> > calls to rte_flow_error_set()).
> >
> > [...]
> >
> > --
> > Adrien Mazarguil
> > 6WIND
> -----Original Message-----
> From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com]
> Sent: Wednesday, December 28, 2016 5:30 PM
> To: Xing, Beilei <beilei.xing@intel.com>
> Cc: 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 v2 07/17] net/i40e: add flow validate
> function
>
> Hi Beilei,
>
> On Wed, Dec 28, 2016 at 09:00:03AM +0000, Xing, Beilei wrote:
> >
> >
> > > -----Original Message-----
> > > From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com]
> > > Sent: Tuesday, December 27, 2016 8:40 PM
> > > To: Xing, Beilei <beilei.xing@intel.com>
> > > Cc: Wu, Jingjing <jingjing.wu@intel.com>; Zhang, Helin
> > > <helin.zhang@intel.com>; dev@dpdk.org
> > > Subject: Re: [dpdk-dev] [PATCH v2 07/17] net/i40e: add flow validate
> > > function
> > >
> > > Hi Beilei,
> > >
> > > A few comments below.
> > >
> > > On Tue, Dec 27, 2016 at 02:26:14PM +0800, Beilei Xing wrote:
> > > > This patch adds i40e_flow_validation function to check if a flow
> > > > is valid according to the flow pattern.
> > > > i40e_parse_ethertype_filter is added first, it also gets the
> > > > ethertype info.
> > > > i40e_flow.c is added to handle all generic filter events.
> > > >
> > > > Signed-off-by: Beilei Xing <beilei.xing@intel.com>
> > > > ---
> > > > drivers/net/i40e/Makefile | 1 +
> > > > drivers/net/i40e/i40e_ethdev.c | 5 +
> > > > drivers/net/i40e/i40e_ethdev.h | 20 ++
> > > > drivers/net/i40e/i40e_flow.c | 431
> > > +++++++++++++++++++++++++++++++++++++++++
> > > > 4 files changed, 457 insertions(+) create mode 100644
> > > > drivers/net/i40e/i40e_flow.c
> > > [...]
> > > > diff --git a/drivers/net/i40e/i40e_flow.c
> > > > b/drivers/net/i40e/i40e_flow.c new file mode 100644 index
> > > > 0000000..bf451ef
> > > > --- /dev/null
> > > > +++ b/drivers/net/i40e/i40e_flow.c
> > > [...]
> > > > + if (ethertype_filter->queue >= pf->dev_data->nb_rx_queues) {
> > > > + rte_flow_error_set(error, EINVAL,
> > > > + RTE_FLOW_ERROR_TYPE_ACTION,
> > > > + NULL, "Invalid queue ID for"
> > > > + " ethertype_filter.");
> > >
> > > When setting an error type related to an existing object provided by
> > > the application, you should set the related cause pointer to a
> > > non-NULL value. In this particular case, retrieving the action
> > > object seems difficult so it can remain that way, however there are
> > > many places in this series where it can be done.
> >
> > OK, I got the meaning and usage of cause pointer now. Thanks for the
> explaination.
> >
> > >
> > > > + return -EINVAL;
> > >
> > > While this is perfectly valid, you could also return -rte_errno to
> > > avoid duplicating EINVAL.
> >
> > Yes, agree.
> >
> > >
> > > [...]
> > > > + }
> > > > + if (ethertype_filter->ether_type == ETHER_TYPE_IPv4 ||
> > > > + ethertype_filter->ether_type == ETHER_TYPE_IPv6) {
> > > > + rte_flow_error_set(error, ENOTSUP,
> > > > + RTE_FLOW_ERROR_TYPE_ITEM,
> > > > + NULL, "Unsupported ether_type in"
> > > > + " control packet filter.");
> > > > + return -ENOTSUP;
> > > > + }
> > > > + if (ethertype_filter->ether_type == ETHER_TYPE_VLAN)
> > > > + PMD_DRV_LOG(WARNING, "filter vlan ether_type in"
> > > > + " first tag is not supported.");
> > > > +
> > > > + return ret;
> > > > +}
> > > [...]
> > > > +/* Parse attributes */
> > > > +static int
> > > > +i40e_parse_attr(const struct rte_flow_attr *attr,
> > > > + struct rte_flow_error *error)
> > > > +{
> > > > + /* Must be input direction */
> > > > + if (!attr->ingress) {
> > > > + rte_flow_error_set(error, EINVAL,
> > > > + RTE_FLOW_ERROR_TYPE_ATTR_INGRESS,
> > > > + NULL, "Only support ingress.");
> > >
> > > Regarding my previous comment, &attr could replace NULL here as well
> > > as in subsequent calls to rte_flow_error_set().
> >
> > Got it, thanks.
> >
> > >
> > > > + return -EINVAL;
> > > > + }
> > > > +
> > > > + /* Not supported */
> > > > + if (attr->egress) {
> > > > + rte_flow_error_set(error, EINVAL,
> > > > + RTE_FLOW_ERROR_TYPE_ATTR_EGRESS,
> > > > + NULL, "Not support egress.");
> > > > + return -EINVAL;
> > > > + }
> > > > +
> > > > + /* Not supported */
> > > > + if (attr->priority) {
> > > > + rte_flow_error_set(error, EINVAL,
> > > > + RTE_FLOW_ERROR_TYPE_ATTR_PRIORITY,
> > > > + NULL, "Not support priority.");
> > > > + return -EINVAL;
> > > > + }
> > > > +
> > > > + /* Not supported */
> > > > + if (attr->group) {
> > > > + rte_flow_error_set(error, EINVAL,
> > > > + RTE_FLOW_ERROR_TYPE_ATTR_GROUP,
> > > > + NULL, "Not support group.");
> > > > + return -EINVAL;
> > > > + }
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static int
> > > > +i40e_parse_ethertype_pattern(const struct rte_flow_item *pattern,
> > > > + struct rte_flow_error *error,
> > > > + struct rte_eth_ethertype_filter *filter) {
> > > > + const struct rte_flow_item *item = pattern;
> > > > + const struct rte_flow_item_eth *eth_spec;
> > > > + const struct rte_flow_item_eth *eth_mask;
> > > > + enum rte_flow_item_type item_type;
> > > > +
> > > > + for (; item->type != RTE_FLOW_ITEM_TYPE_END; item++) {
> > > > + item_type = item->type;
> > > > + switch (item_type) {
> > > > + case RTE_FLOW_ITEM_TYPE_ETH:
> > > > + eth_spec = (const struct rte_flow_item_eth *)item-
> > > >spec;
> > > > + eth_mask = (const struct rte_flow_item_eth *)item-
> > > >mask;
> > > > + /* Get the MAC info. */
> > > > + if (!eth_spec || !eth_mask) {
> > > > + rte_flow_error_set(error, EINVAL,
> > > > +
> > > RTE_FLOW_ERROR_TYPE_ITEM,
> > > > + NULL,
> > > > + "NULL ETH spec/mask");
> > > > + return -EINVAL;
> > > > + }
> > >
> > > While optional, I think you should allow eth_spec and eth_mask to be
> > > NULL here as described in [1]:
> > >
> > > - If eth_spec is NULL, you can match anything that looks like a valid
> > > Ethernet header.
> > >
> > > - If eth_mask is NULL, you should assume a default mask (for Ethernet it
> > > usually means matching source/destination MACs perfectly).
> > >
> > > - You must check the "last" field as well, if non-NULL it may probably be
> > > supported as long as the following condition is satisfied:
> > >
> > > (spec & mask) == (last & mask)
> > >
> > > [1] http://dpdk.org/doc/guides/prog_guide/rte_flow.html#pattern-item
> > >
> >
> > Thanks for the specification. In fact, we don't support the "last" for both
> ixgbe and i40e currently according to the original design, so we only support
> perfect match till now. We will support it in the future, as the deadline is
> coming, what do you think?
>
> If you want to handle it later it's fine, however in that case you need to at
> least generate an error when last is non-NULL (I did not see such a check in
> this code).
OK, will update the non-NULL condition in next version.
And thanks for all your comments.
>
> Note that supporting it properly as defined in the API could be relatively easy
> by implementing the above condition, it's just a small step above simply
> checking for a NULL value.
>
> > > [...]
> > > > + const struct rte_flow_action_queue *act_q;
> > > > + uint32_t index = 0;
> > > > +
> > > > + /* Check if the first non-void action is QUEUE or DROP. */
> > > > + NEXT_ITEM_OF_ACTION(act, actions, index);
> > > > + if (act->type != RTE_FLOW_ACTION_TYPE_QUEUE &&
> > > > + act->type != RTE_FLOW_ACTION_TYPE_DROP) {
> > > > + rte_flow_error_set(error, EINVAL,
> > > RTE_FLOW_ERROR_TYPE_ACTION,
> > > > + NULL, "Not supported action.");
> > >
> > > Again, you could report &act instead of NULL here (please check all
> > > remaining calls to rte_flow_error_set()).
> > >
> > > [...]
> > >
> > > --
> > > Adrien Mazarguil
> > > 6WIND
>
> --
> Adrien Mazarguil
> 6WIND
@@ -105,6 +105,7 @@ endif
SRCS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += i40e_ethdev_vf.c
SRCS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += i40e_pf.c
SRCS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += i40e_fdir.c
+SRCS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += i40e_flow.c
# vector PMD driver needs SSE4.1 support
ifeq ($(findstring RTE_MACHINE_CPUFLAG_SSE4_1,$(CFLAGS)),)
@@ -8452,6 +8452,11 @@ i40e_dev_filter_ctrl(struct rte_eth_dev *dev,
case RTE_ETH_FILTER_FDIR:
ret = i40e_fdir_ctrl_func(dev, filter_op, arg);
break;
+ case RTE_ETH_FILTER_GENERIC:
+ if (filter_op != RTE_ETH_FILTER_GET)
+ return -EINVAL;
+ *(const void **)arg = &i40e_flow_ops;
+ break;
default:
PMD_DRV_LOG(WARNING, "Filter type (%d) not supported",
filter_type);
@@ -38,6 +38,7 @@
#include <rte_time.h>
#include <rte_kvargs.h>
#include <rte_hash.h>
+#include <rte_flow_driver.h>
#define I40E_VLAN_TAG_SIZE 4
@@ -629,6 +630,23 @@ struct i40e_adapter {
struct rte_timecounter tx_tstamp_tc;
};
+union i40e_filter_t {
+ struct rte_eth_ethertype_filter ethertype_filter;
+ struct rte_eth_fdir_filter fdir_filter;
+ struct rte_eth_tunnel_filter_conf tunnel_filter;
+} cons_filter;
+
+typedef int (*parse_filter_t)(struct rte_eth_dev *dev,
+ const struct rte_flow_attr *attr,
+ const struct rte_flow_item pattern[],
+ const struct rte_flow_action actions[],
+ struct rte_flow_error *error,
+ union i40e_filter_t *filter);
+struct i40e_valid_pattern {
+ enum rte_flow_item_type *items;
+ parse_filter_t parse_filter;
+};
+
int i40e_dev_switch_queues(struct i40e_pf *pf, bool on);
int i40e_vsi_release(struct i40e_vsi *vsi);
struct i40e_vsi *i40e_vsi_setup(struct i40e_pf *pf,
@@ -823,4 +841,6 @@ i40e_calc_itr_interval(int16_t interval)
((phy_type) & I40E_CAP_PHY_TYPE_25GBASE_SR) || \
((phy_type) & I40E_CAP_PHY_TYPE_25GBASE_LR))
+const struct rte_flow_ops i40e_flow_ops;
+
#endif /* _I40E_ETHDEV_H_ */
new file mode 100644
@@ -0,0 +1,431 @@
+/*-
+ * BSD LICENSE
+ *
+ * Copyright (c) 2016 Intel Corporation. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in
+ * the documentation and/or other materials provided with the
+ * distribution.
+ * * Neither the name of Intel Corporation nor the names of its
+ * contributors may be used to endorse or promote products derived
+ * from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <sys/queue.h>
+#include <stdio.h>
+#include <errno.h>
+#include <stdint.h>
+#include <string.h>
+#include <unistd.h>
+#include <stdarg.h>
+
+#include <rte_ether.h>
+#include <rte_ethdev.h>
+#include <rte_log.h>
+#include <rte_memzone.h>
+#include <rte_malloc.h>
+#include <rte_eth_ctrl.h>
+#include <rte_tailq.h>
+#include <rte_flow_driver.h>
+
+#include "i40e_logs.h"
+#include "base/i40e_type.h"
+#include "i40e_ethdev.h"
+
+static int i40e_flow_validate(struct rte_eth_dev *dev,
+ const struct rte_flow_attr *attr,
+ const struct rte_flow_item pattern[],
+ const struct rte_flow_action actions[],
+ struct rte_flow_error *error);
+static int i40e_parse_ethertype_pattern(const struct rte_flow_item *pattern,
+ struct rte_flow_error *error,
+ struct rte_eth_ethertype_filter *filter);
+static int i40e_parse_ethertype_act(const struct rte_flow_action *actions,
+ struct rte_flow_error *error,
+ struct rte_eth_ethertype_filter *filter);
+static int i40e_parse_attr(const struct rte_flow_attr *attr,
+ struct rte_flow_error *error);
+
+const struct rte_flow_ops i40e_flow_ops = {
+ .validate = i40e_flow_validate,
+};
+
+/* Pattern matched ethertype filter */
+static enum rte_flow_item_type pattern_ethertype[] = {
+ RTE_FLOW_ITEM_TYPE_ETH,
+ RTE_FLOW_ITEM_TYPE_END,
+};
+
+static int
+i40e_parse_ethertype_filter(struct rte_eth_dev *dev,
+ const struct rte_flow_attr *attr,
+ const struct rte_flow_item pattern[],
+ const struct rte_flow_action actions[],
+ struct rte_flow_error *error,
+ union i40e_filter_t *filter)
+{
+ struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
+ struct rte_eth_ethertype_filter *ethertype_filter =
+ &filter->ethertype_filter;
+ int ret;
+
+ ret = i40e_parse_ethertype_pattern(pattern, error, ethertype_filter);
+ if (ret)
+ return ret;
+
+ ret = i40e_parse_ethertype_act(actions, error, ethertype_filter);
+ if (ret)
+ return ret;
+
+ ret = i40e_parse_attr(attr, error);
+ if (ret)
+ return ret;
+
+ if (ethertype_filter->queue >= pf->dev_data->nb_rx_queues) {
+ rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ACTION,
+ NULL, "Invalid queue ID for"
+ " ethertype_filter.");
+ return -EINVAL;
+ }
+ if (ethertype_filter->ether_type == ETHER_TYPE_IPv4 ||
+ ethertype_filter->ether_type == ETHER_TYPE_IPv6) {
+ rte_flow_error_set(error, ENOTSUP,
+ RTE_FLOW_ERROR_TYPE_ITEM,
+ NULL, "Unsupported ether_type in"
+ " control packet filter.");
+ return -ENOTSUP;
+ }
+ if (ethertype_filter->ether_type == ETHER_TYPE_VLAN)
+ PMD_DRV_LOG(WARNING, "filter vlan ether_type in"
+ " first tag is not supported.");
+
+ return ret;
+}
+
+static struct i40e_valid_pattern i40e_supported_patterns[] = {
+ /* Ethertype */
+ { pattern_ethertype, i40e_parse_ethertype_filter },
+};
+
+#define NEXT_ITEM_OF_ACTION(act, actions, index) \
+ do { \
+ act = actions + index; \
+ while (act->type == RTE_FLOW_ACTION_TYPE_VOID) { \
+ index++; \
+ act = actions + index; \
+ } \
+ } while (0)
+
+/* Find the first VOID or non-VOID item pointer */
+static const struct rte_flow_item *
+i40e_find_first_item(const struct rte_flow_item *item, bool is_void)
+{
+ bool is_find;
+
+ while (item->type != RTE_FLOW_ITEM_TYPE_END) {
+ if (is_void)
+ is_find = item->type == RTE_FLOW_ITEM_TYPE_VOID;
+ else
+ is_find = item->type != RTE_FLOW_ITEM_TYPE_VOID;
+ if (is_find)
+ break;
+ item++;
+ }
+ return item;
+}
+
+/* Skip all VOID items of the pattern */
+static void
+i40e_pattern_skip_void_item(struct rte_flow_item *items,
+ const struct rte_flow_item *pattern)
+{
+ uint32_t cpy_count = 0;
+ const struct rte_flow_item *pb = pattern, *pe = pattern;
+
+ for (;;) {
+ /* Find a non-void item first */
+ pb = i40e_find_first_item(pb, false);
+ if (pb->type == RTE_FLOW_ITEM_TYPE_END) {
+ pe = pb;
+ break;
+ }
+
+ /* Find a void item */
+ pe = i40e_find_first_item(pb + 1, true);
+
+ cpy_count = pe - pb;
+ rte_memcpy(items, pb, sizeof(struct rte_flow_item) * cpy_count);
+
+ items += cpy_count;
+
+ if (pe->type == RTE_FLOW_ITEM_TYPE_END) {
+ pb = pe;
+ break;
+ }
+
+ pb = pe + 1;
+ }
+ /* Copy the END item. */
+ rte_memcpy(items, pe, sizeof(struct rte_flow_item));
+}
+
+/* Check if the pattern matches a supported item type array */
+static bool
+i40e_match_pattern(enum rte_flow_item_type *item_array,
+ struct rte_flow_item *pattern)
+{
+ struct rte_flow_item *item = pattern;
+
+ while ((*item_array == item->type) &&
+ (*item_array != RTE_FLOW_ITEM_TYPE_END)) {
+ item_array++;
+ item++;
+ }
+
+ return (*item_array == RTE_FLOW_ITEM_TYPE_END &&
+ item->type == RTE_FLOW_ITEM_TYPE_END);
+}
+
+/* Find if there's parse filter function matched */
+static parse_filter_t
+i40e_find_parse_filter_func(struct rte_flow_item *pattern)
+{
+ parse_filter_t parse_filter = NULL;
+ uint8_t i = 0;
+
+ for (; i < RTE_DIM(i40e_supported_patterns); i++) {
+ if (i40e_match_pattern(i40e_supported_patterns[i].items,
+ pattern)) {
+ parse_filter = i40e_supported_patterns[i].parse_filter;
+ break;
+ }
+ }
+
+ return parse_filter;
+}
+
+/* Parse attributes */
+static int
+i40e_parse_attr(const struct rte_flow_attr *attr,
+ struct rte_flow_error *error)
+{
+ /* Must be input direction */
+ if (!attr->ingress) {
+ rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ATTR_INGRESS,
+ NULL, "Only support ingress.");
+ return -EINVAL;
+ }
+
+ /* Not supported */
+ if (attr->egress) {
+ rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ATTR_EGRESS,
+ NULL, "Not support egress.");
+ return -EINVAL;
+ }
+
+ /* Not supported */
+ if (attr->priority) {
+ rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ATTR_PRIORITY,
+ NULL, "Not support priority.");
+ return -EINVAL;
+ }
+
+ /* Not supported */
+ if (attr->group) {
+ rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ATTR_GROUP,
+ NULL, "Not support group.");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int
+i40e_parse_ethertype_pattern(const struct rte_flow_item *pattern,
+ struct rte_flow_error *error,
+ struct rte_eth_ethertype_filter *filter)
+{
+ const struct rte_flow_item *item = pattern;
+ const struct rte_flow_item_eth *eth_spec;
+ const struct rte_flow_item_eth *eth_mask;
+ enum rte_flow_item_type item_type;
+
+ for (; item->type != RTE_FLOW_ITEM_TYPE_END; item++) {
+ item_type = item->type;
+ switch (item_type) {
+ case RTE_FLOW_ITEM_TYPE_ETH:
+ eth_spec = (const struct rte_flow_item_eth *)item->spec;
+ eth_mask = (const struct rte_flow_item_eth *)item->mask;
+ /* Get the MAC info. */
+ if (!eth_spec || !eth_mask) {
+ rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ITEM,
+ NULL,
+ "NULL ETH spec/mask");
+ return -EINVAL;
+ }
+
+ /* Mask bits of source MAC address must be full of 0.
+ * Mask bits of destination MAC address must be full
+ * of 1 or full of 0.
+ */
+ if (!is_zero_ether_addr(ð_mask->src) ||
+ (!is_zero_ether_addr(ð_mask->dst) &&
+ !is_broadcast_ether_addr(ð_mask->dst))) {
+ rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ITEM,
+ NULL,
+ "Invalid MAC_addr mask");
+ return -EINVAL;
+ }
+
+ if ((eth_mask->type & UINT16_MAX) != UINT16_MAX) {
+ rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ITEM,
+ NULL,
+ "Invalid ethertype mask");
+ return -EINVAL;
+ }
+
+ /* If mask bits of destination MAC address
+ * are full of 1, set RTE_ETHTYPE_FLAGS_MAC.
+ */
+ if (is_broadcast_ether_addr(ð_mask->dst)) {
+ filter->mac_addr = eth_spec->dst;
+ filter->flags |= RTE_ETHTYPE_FLAGS_MAC;
+ } else {
+ filter->flags &= ~RTE_ETHTYPE_FLAGS_MAC;
+ }
+ filter->ether_type = rte_be_to_cpu_16(eth_spec->type);
+
+ break;
+ default:
+ break;
+ }
+ }
+
+ return 0;
+}
+
+static int
+i40e_parse_ethertype_act(const struct rte_flow_action *actions,
+ struct rte_flow_error *error,
+ struct rte_eth_ethertype_filter *filter)
+{
+ const struct rte_flow_action *act;
+ const struct rte_flow_action_queue *act_q;
+ uint32_t index = 0;
+
+ /* Check if the first non-void action is QUEUE or DROP. */
+ NEXT_ITEM_OF_ACTION(act, actions, index);
+ if (act->type != RTE_FLOW_ACTION_TYPE_QUEUE &&
+ act->type != RTE_FLOW_ACTION_TYPE_DROP) {
+ rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ACTION,
+ NULL, "Not supported 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 non-void item is END */
+ index++;
+ NEXT_ITEM_OF_ACTION(act, actions, index);
+ if (act->type != RTE_FLOW_ACTION_TYPE_END) {
+ rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ACTION,
+ NULL, "Not supported action.");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int
+i40e_flow_validate(struct rte_eth_dev *dev,
+ const struct rte_flow_attr *attr,
+ const struct rte_flow_item pattern[],
+ const struct rte_flow_action actions[],
+ struct rte_flow_error *error)
+{
+ struct rte_flow_item *items;
+ parse_filter_t parse_filter;
+ uint32_t item_num = 0; /* non-void item number of pattern*/
+ uint32_t i = 0;
+ int ret;
+
+ if (!pattern) {
+ rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ITEM_NUM,
+ NULL, "NULL pattern.");
+ return -EINVAL;
+ }
+
+ if (!actions) {
+ rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ACTION_NUM,
+ NULL, "NULL action.");
+ return -EINVAL;
+ }
+
+ memset(&cons_filter, 0, sizeof(cons_filter));
+
+ /* Get the non-void item number of pattern */
+ while ((pattern + i)->type != RTE_FLOW_ITEM_TYPE_END) {
+ if ((pattern + i)->type != RTE_FLOW_ITEM_TYPE_VOID)
+ item_num++;
+ i++;
+ }
+ item_num++;
+
+ items = rte_zmalloc("i40e_pattern",
+ item_num * sizeof(struct rte_flow_item), 0);
+ if (!items) {
+ rte_flow_error_set(error, ENOMEM, RTE_FLOW_ERROR_TYPE_ITEM_NUM,
+ NULL, "No memory for PMD internal items.");
+ return -ENOMEM;
+ }
+
+ i40e_pattern_skip_void_item(items, pattern);
+
+ /* Find if there's matched parse filter function */
+ parse_filter = i40e_find_parse_filter_func(items);
+ if (!parse_filter) {
+ rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ITEM,
+ NULL, "Unsupported pattern");
+ return -EINVAL;
+ }
+
+ ret = parse_filter(dev, attr, items, actions, error, &cons_filter);
+
+ rte_free(items);
+
+ return ret;
+}