> -----Original Message-----
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Wei Zhao
> Sent: Friday, December 30, 2016 3:53 PM
> To: dev@dpdk.org
> Cc: Zhao1, Wei <wei.zh...@intel.com>; Lu, Wenzhuo <wenzhuo...@intel.com>
> Subject: [dpdk-dev] [PATCH v2 11/18] net/ixgbe: parse n-tuple filter
>
> Add rule validate function and check if the rule is a n-tuple rule, and get
> the
> n-tuple info.
>
> Signed-off-by: Wei Zhao <wei.zh...@intel.com>
> Signed-off-by: Wenzhuo Lu <wenzhuo...@intel.com>
>
> ---
>
> v2:add new error set function
> ---
> drivers/net/ixgbe/ixgbe_ethdev.c | 414
> ++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 409 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> b/drivers/net/ixgbe/ixgbe_ethdev.c
> index 0de1318..198cc4b 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -388,6 +388,24 @@ static int ixgbe_dev_udp_tunnel_port_del(struct
> rte_eth_dev *dev,
> struct rte_eth_udp_tunnel
> *udp_tunnel); static int
> ixgbe_filter_restore(struct rte_eth_dev *dev); static void
> ixgbe_l2_tunnel_conf(struct rte_eth_dev *dev);
> +static int
> +cons_parse_ntuple_filter(const struct rte_flow_attr *attr,
> + const struct rte_flow_item pattern[],
> + const struct rte_flow_action actions[],
> + struct rte_eth_ntuple_filter *filter,
> + struct rte_flow_error *error);
Why do you declare cons_parse_ntuple_filter here? And seems it doesn't align
with the name rule.
> +static int
> +ixgbe_parse_ntuple_filter(const struct rte_flow_attr *attr,
> + const struct rte_flow_item pattern[],
> + const struct rte_flow_action actions[],
> + struct rte_eth_ntuple_filter *filter,
> + struct rte_flow_error *error);
> +static int
> +ixgbe_flow_validate(__rte_unused 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 ixgbe_flow_flush(struct rte_eth_dev *dev,
> struct rte_flow_error *error);
> /*
> @@ -769,7 +787,7 @@ static const struct rte_ixgbe_xstats_name_off
> rte_ixgbevf_stats_strings[] = {
> #define IXGBEVF_NB_XSTATS (sizeof(rte_ixgbevf_stats_strings) / \
> sizeof(rte_ixgbevf_stats_strings[0]))
> static const struct rte_flow_ops ixgbe_flow_ops = {
> - NULL,
> + ixgbe_flow_validate,
> NULL,
> NULL,
> ixgbe_flow_flush,
> @@ -8072,6 +8090,390 @@ ixgbe_clear_all_l2_tn_filter(struct rte_eth_dev
> *dev)
> return 0;
> }
>
> +static inline uint32_t
> +rte_be_to_cpu_24(uint32_t x)
> +{
> + return ((x & 0x000000ffUL) << 16) |
> + (x & 0x0000ff00UL) |
> + ((x & 0x00ff0000UL) >> 16);
> +}
Why do you define the function in PMD with rte_ prefixed? Do you want to move
it to rte library?
> +
> +
> +/**
> + * Parse the rule to see if it is a n-tuple rule.
> + * And get the n-tuple filter info BTW.
> + */
> +static int
> +cons_parse_ntuple_filter(const struct rte_flow_attr *attr,
> + const struct rte_flow_item pattern[],
> + const struct rte_flow_action actions[],
> + struct rte_eth_ntuple_filter *filter,
> + struct rte_flow_error *error)
How about splitting the function into three functions? Including parse
pattern/parse actions/parse attr.
> +
> +/* a specific function for ixgbe because the flags is specific */
> +static int ixgbe_parse_ntuple_filter(const struct rte_flow_attr *attr,
> + const struct rte_flow_item pattern[],
> + const struct rte_flow_action actions[],
> + struct rte_eth_ntuple_filter *filter,
> + struct rte_flow_error *error)
> +{
> + int ret;
> +
> + ret = cons_parse_ntuple_filter(attr, pattern, actions, filter, error);
> +
> + if (ret)
> + return ret;
> +
> + /* Ixgbe doesn't support tcp flags. */
> + if (filter->flags & RTE_NTUPLE_FLAGS_TCP_FLAG) {
> + memset(filter, 0, sizeof(struct rte_eth_ntuple_filter));
> + rte_flow_error_set(error, EINVAL,
> + RTE_FLOW_ERROR_TYPE_ITEM,
> + NULL, "Not supported by ntuple filter");
> + return -rte_errno;
> + }
> +
> + /* Ixgbe doesn't support many priorities. */
> + if (filter->priority < IXGBE_MIN_N_TUPLE_PRIO ||
> + filter->priority > IXGBE_MAX_N_TUPLE_PRIO) {
> + memset(filter, 0, sizeof(struct rte_eth_ntuple_filter));
> + rte_flow_error_set(error, EINVAL,
> + RTE_FLOW_ERROR_TYPE_ITEM,
> + NULL, "Priority not supported by ntuple filter");
> + return -rte_errno;
> + }
> +
> + if (filter->queue >= IXGBE_MAX_RX_QUEUE_NUM ||
> + filter->priority > IXGBE_5TUPLE_MAX_PRI ||
> + filter->priority < IXGBE_5TUPLE_MIN_PRI)
> + return -rte_errno;
> +
> + /* fixed value for ixgbe */
> + filter->flags = RTE_5TUPLE_FLAGS;
> + return 0;
> +}
> +
> +/**
> + * Check if the flow rule is supported by ixgbe.
> + * It only checkes the format. Don't guarantee the rule can be
Typo: checkes -> checks
> +programmed into
> + * the HW. Because there can be no enough room for the rule.
> + */
> +static int
> +ixgbe_flow_validate(__rte_unused 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_eth_ntuple_filter ntuple_filter;
> + int ret;
> +
> + memset(&ntuple_filter, 0, sizeof(struct rte_eth_ntuple_filter));
> + ret = ixgbe_parse_ntuple_filter(attr, pattern,
> + actions, &ntuple_filter, error);
> + if (!ret)
> + return 0;
> +
> + return ret;
> +}
> +
> /* Destroy all flow rules associated with a port on ixgbe. */ static int
> ixgbe_flow_flush(struct rte_eth_dev *dev, @@ -8085,15 +8487,17 @@
> ixgbe_flow_flush(struct rte_eth_dev *dev,
>
> ret = ixgbe_clear_all_fdir_filter(dev);
> if (ret < 0) {
> - rte_flow_error_set(error, EINVAL,
> RTE_FLOW_ERROR_TYPE_HANDLE,
> - NULL, "Failed to flush rule");
> + rte_flow_error_set(error, EINVAL,
> + RTE_FLOW_ERROR_TYPE_HANDLE,
> + NULL, "Failed to flush rule");
> return ret;
> }
>
> ret = ixgbe_clear_all_l2_tn_filter(dev);
> if (ret < 0) {
> - rte_flow_error_set(error, EINVAL,
> RTE_FLOW_ERROR_TYPE_HANDLE,
> - NULL, "Failed to flush rule");
> + rte_flow_error_set(error, EINVAL,
> + RTE_FLOW_ERROR_TYPE_HANDLE,
> + NULL, "Failed to flush rule");
> return ret;
> }
>
> --
> 2.5.5
