> -----Original Message-----
> From: dev <dev-boun...@dpdk.org> On Behalf Of Yongseok Koh
> Sent: Friday, November 2, 2018 11:08 PM
> To: Shahaf Shuler <shah...@mellanox.com>
> Cc: dev@dpdk.org; Yongseok Koh <ys...@mellanox.com>
> Subject: [dpdk-dev] [PATCH 3/3] net/mlx5: remove flags setting from flow
> preparation
> 
> Even though flow_drv_prepare() takes item_flags and action_flags to be
> filled in, those are not used and will be overwritten by parsing of
> flow_drv_translate(). There's no reason to keep the flags and fill it.
> Appropriate notes are added to the documentation of flow_drv_prepare() and
> flow_drv_translate().
> 
> Signed-off-by: Yongseok Koh <ys...@mellanox.com>
> ---
>  drivers/net/mlx5/mlx5_flow.c       | 38 ++++++++++++--------------
>  drivers/net/mlx5/mlx5_flow.h       |  3 +--
>  drivers/net/mlx5/mlx5_flow_dv.c    |  6 -----
>  drivers/net/mlx5/mlx5_flow_tcf.c   | 55 
> +++++---------------------------------
>  drivers/net/mlx5/mlx5_flow_verbs.c | 52 +++--------------------------------
>  5 files changed, 29 insertions(+), 125 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
> index 107a4f02f8..fae3bc92dd 100644
> --- a/drivers/net/mlx5/mlx5_flow.c
> +++ b/drivers/net/mlx5/mlx5_flow.c
> @@ -1657,8 +1657,6 @@ static struct mlx5_flow *
>  flow_null_prepare(const struct rte_flow_attr *attr __rte_unused,
>                 const struct rte_flow_item items[] __rte_unused,
>                 const struct rte_flow_action actions[] __rte_unused,
> -               uint64_t *item_flags __rte_unused,
> -               uint64_t *action_flags __rte_unused,
>                 struct rte_flow_error *error __rte_unused)
>  {
>       rte_errno = ENOTSUP;
> @@ -1786,16 +1784,19 @@ flow_drv_validate(struct rte_eth_dev *dev,
>   * calculates the size of memory required for device flow, allocates the
> memory,
>   * initializes the device flow and returns the pointer.
>   *
> + * @note
> + *   This function initializes device flow structure such as dv, tcf or 
> verbs in
> + *   struct mlx5_flow. However, it is callee's responsibility to initialize 
> the
> + *   rest. For example, adding returning device flow to flow->dev_flow list 
> and
> + *   setting backward reference to the flow should be done out of this 
> function.
> + *   layers field is not filled either.
> + *
>   * @param[in] attr
>   *   Pointer to the flow attributes.
>   * @param[in] items
>   *   Pointer to the list of items.
>   * @param[in] actions
>   *   Pointer to the list of actions.
> - * @param[out] item_flags
> - *   Pointer to bit mask of all items detected.
> - * @param[out] action_flags
> - *   Pointer to bit mask of all actions detected.
>   * @param[out] error
>   *   Pointer to the error structure.
>   *
> @@ -1803,12 +1804,10 @@ flow_drv_validate(struct rte_eth_dev *dev,
>   *   Pointer to device flow on success, otherwise NULL and rte_ernno is set.
>   */
>  static inline struct mlx5_flow *
> -flow_drv_prepare(struct rte_flow *flow,
> +flow_drv_prepare(const struct rte_flow *flow,
>                const struct rte_flow_attr *attr,
>                const struct rte_flow_item items[],
>                const struct rte_flow_action actions[],
> -              uint64_t *item_flags,
> -              uint64_t *action_flags,
>                struct rte_flow_error *error)
>  {
>       const struct mlx5_flow_driver_ops *fops;
> @@ -1816,8 +1815,7 @@ flow_drv_prepare(struct rte_flow *flow,
> 
>       assert(type > MLX5_FLOW_TYPE_MIN && type <
> MLX5_FLOW_TYPE_MAX);
>       fops = flow_get_drv_ops(type);
> -     return fops->prepare(attr, items, actions, item_flags, action_flags,
> -                          error);
> +     return fops->prepare(attr, items, actions, error);
>  }
> 
>  /**
> @@ -1826,6 +1824,12 @@ flow_drv_prepare(struct rte_flow *flow,
>   * translates a generic flow into a driver flow. flow_drv_prepare() must
>   * precede.
>   *
> + * @note
> + *   dev_flow->layers could be filled as a result of parsing during 
> translation
> + *   if needed by flow_drv_apply(). dev_flow->flow->actions can also be 
> filled
> + *   if necessary. As a flow can have multiple dev_flows by RSS flow 
> expansion,
> + *   flow->actions could be overwritten even though all the expanded
> dev_flows
> + *   have the same actions.
>   *
>   * @param[in] dev
>   *   Pointer to the rte dev structure.
> @@ -1889,7 +1893,7 @@ flow_drv_apply(struct rte_eth_dev *dev, struct
> rte_flow *flow,
>   * Flow driver remove API. This abstracts calling driver specific functions.
>   * Parent flow (rte_flow) should have driver type (drv_type). It removes a 
> flow
>   * on device. All the resources of the flow should be freed by calling
> - * flow_dv_destroy().
> + * flow_drv_destroy().
>   *
>   * @param[in] dev
>   *   Pointer to Ethernet device.
> @@ -2020,8 +2024,6 @@ flow_list_create(struct rte_eth_dev *dev, struct
> mlx5_flows *list,
>  {
>       struct rte_flow *flow = NULL;
>       struct mlx5_flow *dev_flow;
> -     uint64_t action_flags = 0;
> -     uint64_t item_flags = 0;
>       const struct rte_flow_action_rss *rss;
>       union {
>               struct rte_flow_expand_rss buf;
> @@ -2064,16 +2066,10 @@ flow_list_create(struct rte_eth_dev *dev, struct
> mlx5_flows *list,
>       }
>       for (i = 0; i < buf->entries; ++i) {
>               dev_flow = flow_drv_prepare(flow, attr, buf->entry[i].pattern,
> -                                         actions, &item_flags, &action_flags,
> -                                         error);
> +                                         actions, error);
>               if (!dev_flow)
>                       goto error;
>               dev_flow->flow = flow;
> -             dev_flow->layers = item_flags;
> -             /* Store actions once as expanded flows have same actions. */
> -             if (i == 0)
> -                     flow->actions = action_flags;
> -             assert(flow->actions == action_flags);
>               LIST_INSERT_HEAD(&flow->dev_flows, dev_flow, next);
>               ret = flow_drv_translate(dev, dev_flow, attr,
>                                        buf->entry[i].pattern,
> diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
> index fadde552c2..f976bff427 100644
> --- a/drivers/net/mlx5/mlx5_flow.h
> +++ b/drivers/net/mlx5/mlx5_flow.h
> @@ -293,8 +293,7 @@ typedef int (*mlx5_flow_validate_t)(struct rte_eth_dev
> *dev,
>                                   struct rte_flow_error *error);
>  typedef struct mlx5_flow *(*mlx5_flow_prepare_t)
>       (const struct rte_flow_attr *attr, const struct rte_flow_item items[],
> -      const struct rte_flow_action actions[], uint64_t *item_flags,
> -      uint64_t *action_flags, struct rte_flow_error *error);
> +      const struct rte_flow_action actions[], struct rte_flow_error *error);
>  typedef int (*mlx5_flow_translate_t)(struct rte_eth_dev *dev,
>                                    struct mlx5_flow *dev_flow,
>                                    const struct rte_flow_attr *attr,
> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
> b/drivers/net/mlx5/mlx5_flow_dv.c
> index 44e2a920eb..0fb791eafa 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -1014,10 +1014,6 @@ flow_dv_validate(struct rte_eth_dev *dev, const
> struct rte_flow_attr *attr,
>   *   Pointer to the list of items.
>   * @param[in] actions
>   *   Pointer to the list of actions.
> - * @param[out] item_flags
> - *   Pointer to bit mask of all items detected.
> - * @param[out] action_flags
> - *   Pointer to bit mask of all actions detected.
>   * @param[out] error
>   *   Pointer to the error structure.
>   *
> @@ -1029,8 +1025,6 @@ static struct mlx5_flow *
>  flow_dv_prepare(const struct rte_flow_attr *attr __rte_unused,
>               const struct rte_flow_item items[] __rte_unused,
>               const struct rte_flow_action actions[] __rte_unused,
> -             uint64_t *item_flags __rte_unused,
> -             uint64_t *action_flags __rte_unused,
>               struct rte_flow_error *error)
>  {
>       uint32_t size = sizeof(struct mlx5_flow);
> diff --git a/drivers/net/mlx5/mlx5_flow_tcf.c
> b/drivers/net/mlx5/mlx5_flow_tcf.c
> index 719fb10632..483e490843 100644
> --- a/drivers/net/mlx5/mlx5_flow_tcf.c
> +++ b/drivers/net/mlx5/mlx5_flow_tcf.c
> @@ -664,20 +664,15 @@ flow_tcf_create_pedit_mnl_msg(struct nlmsghdr
> *nl,
>   *
>   * @param[in,out] actions
>   *   actions specification.
> - * @param[in,out] action_flags
> - *   actions flags
> - * @param[in,out] size
> - *   accumulated size
> + *
>   * @return
>   *   Max memory size of one TC-pedit action
>   */
>  static int
> -flow_tcf_get_pedit_actions_size(const struct rte_flow_action **actions,
> -                             uint64_t *action_flags)
> +flow_tcf_get_pedit_actions_size(const struct rte_flow_action **actions)
>  {
>       int pedit_size = 0;
>       int keys = 0;
> -     uint64_t flags = 0;
> 
>       pedit_size += SZ_NLATTR_NEST + /* na_act_index. */
>                     SZ_NLATTR_STRZ_OF("pedit") +
> @@ -686,45 +681,35 @@ flow_tcf_get_pedit_actions_size(const struct
> rte_flow_action **actions,
>               switch ((*actions)->type) {
>               case RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC:
>                       keys += NUM_OF_PEDIT_KEYS(IPV4_ADDR_LEN);
> -                     flags |= MLX5_FLOW_ACTION_SET_IPV4_SRC;
>                       break;
>               case RTE_FLOW_ACTION_TYPE_SET_IPV4_DST:
>                       keys += NUM_OF_PEDIT_KEYS(IPV4_ADDR_LEN);
> -                     flags |= MLX5_FLOW_ACTION_SET_IPV4_DST;
>                       break;
>               case RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC:
>                       keys += NUM_OF_PEDIT_KEYS(IPV6_ADDR_LEN);
> -                     flags |= MLX5_FLOW_ACTION_SET_IPV6_SRC;
>                       break;
>               case RTE_FLOW_ACTION_TYPE_SET_IPV6_DST:
>                       keys += NUM_OF_PEDIT_KEYS(IPV6_ADDR_LEN);
> -                     flags |= MLX5_FLOW_ACTION_SET_IPV6_DST;
>                       break;
>               case RTE_FLOW_ACTION_TYPE_SET_TP_SRC:
>                       /* TCP is as same as UDP */
>                       keys += NUM_OF_PEDIT_KEYS(TP_PORT_LEN);
> -                     flags |= MLX5_FLOW_ACTION_SET_TP_SRC;
>                       break;
>               case RTE_FLOW_ACTION_TYPE_SET_TP_DST:
>                       /* TCP is as same as UDP */
>                       keys += NUM_OF_PEDIT_KEYS(TP_PORT_LEN);
> -                     flags |= MLX5_FLOW_ACTION_SET_TP_DST;
>                       break;
>               case RTE_FLOW_ACTION_TYPE_SET_TTL:
>                       keys += NUM_OF_PEDIT_KEYS(TTL_LEN);
> -                     flags |= MLX5_FLOW_ACTION_SET_TTL;
>                       break;
>               case RTE_FLOW_ACTION_TYPE_DEC_TTL:
>                       keys += NUM_OF_PEDIT_KEYS(TTL_LEN);
> -                     flags |= MLX5_FLOW_ACTION_DEC_TTL;
>                       break;
>               case RTE_FLOW_ACTION_TYPE_SET_MAC_SRC:
>                       keys += NUM_OF_PEDIT_KEYS(ETHER_ADDR_LEN);
> -                     flags |= MLX5_FLOW_ACTION_SET_MAC_SRC;
>                       break;
>               case RTE_FLOW_ACTION_TYPE_SET_MAC_DST:
>                       keys += NUM_OF_PEDIT_KEYS(ETHER_ADDR_LEN);
> -                     flags |= MLX5_FLOW_ACTION_SET_MAC_DST;
>                       break;
>               default:
>                       goto get_pedit_action_size_done;
> @@ -740,7 +725,6 @@ flow_tcf_get_pedit_actions_size(const struct
> rte_flow_action **actions,
>                     /* TCA_PEDIT_KEY_EX + HTYPE + CMD */
>                     (SZ_NLATTR_NEST + SZ_NLATTR_DATA_OF(2) +
>                      SZ_NLATTR_DATA_OF(2));
> -     (*action_flags) |= flags;
>       (*actions)--;
>       return pedit_size;
>  }
> @@ -1415,11 +1399,9 @@ flow_tcf_validate(struct rte_eth_dev *dev,
>   */
>  static int
>  flow_tcf_get_items_and_size(const struct rte_flow_attr *attr,
> -                         const struct rte_flow_item items[],
> -                         uint64_t *item_flags)
> +                         const struct rte_flow_item items[])
>  {
>       int size = 0;
> -     uint64_t flags = 0;
> 
>       size += SZ_NLATTR_STRZ_OF("flower") +
>               SZ_NLATTR_NEST + /* TCA_OPTIONS. */
> @@ -1436,7 +1418,6 @@ flow_tcf_get_items_and_size(const struct
> rte_flow_attr *attr,
>                       size += SZ_NLATTR_TYPE_OF(uint16_t) + /* Ether type.
> */
>                               SZ_NLATTR_DATA_OF(ETHER_ADDR_LEN) * 4;
>                               /* dst/src MAC addr and mask. */
> -                     flags |= MLX5_FLOW_LAYER_OUTER_L2;
>                       break;
>               case RTE_FLOW_ITEM_TYPE_VLAN:
>                       size += SZ_NLATTR_TYPE_OF(uint16_t) + /* Ether type.
> */
> @@ -1444,33 +1425,28 @@ flow_tcf_get_items_and_size(const struct
> rte_flow_attr *attr,
>                               /* VLAN Ether type. */
>                               SZ_NLATTR_TYPE_OF(uint8_t) + /* VLAN prio.
> */
>                               SZ_NLATTR_TYPE_OF(uint16_t); /* VLAN ID. */
> -                     flags |= MLX5_FLOW_LAYER_OUTER_VLAN;
>                       break;
>               case RTE_FLOW_ITEM_TYPE_IPV4:
>                       size += SZ_NLATTR_TYPE_OF(uint16_t) + /* Ether type.
> */
>                               SZ_NLATTR_TYPE_OF(uint8_t) + /* IP proto. */
>                               SZ_NLATTR_TYPE_OF(uint32_t) * 4;
>                               /* dst/src IP addr and mask. */
> -                     flags |= MLX5_FLOW_LAYER_OUTER_L3_IPV4;
>                       break;
>               case RTE_FLOW_ITEM_TYPE_IPV6:
>                       size += SZ_NLATTR_TYPE_OF(uint16_t) + /* Ether type.
> */
>                               SZ_NLATTR_TYPE_OF(uint8_t) + /* IP proto. */
>                               SZ_NLATTR_TYPE_OF(IPV6_ADDR_LEN) * 4;
>                               /* dst/src IP addr and mask. */
> -                     flags |= MLX5_FLOW_LAYER_OUTER_L3_IPV6;
>                       break;
>               case RTE_FLOW_ITEM_TYPE_UDP:
>                       size += SZ_NLATTR_TYPE_OF(uint8_t) + /* IP proto. */
>                               SZ_NLATTR_TYPE_OF(uint16_t) * 4;
>                               /* dst/src port and mask. */
> -                     flags |= MLX5_FLOW_LAYER_OUTER_L4_UDP;
>                       break;
>               case RTE_FLOW_ITEM_TYPE_TCP:
>                       size += SZ_NLATTR_TYPE_OF(uint8_t) + /* IP proto. */
>                               SZ_NLATTR_TYPE_OF(uint16_t) * 4;
>                               /* dst/src port and mask. */
> -                     flags |= MLX5_FLOW_LAYER_OUTER_L4_TCP;
>                       break;
>               default:
>                       DRV_LOG(WARNING,
> @@ -1480,7 +1456,6 @@ flow_tcf_get_items_and_size(const struct
> rte_flow_attr *attr,
>                       break;
>               }
>       }
> -     *item_flags = flags;
>       return size;
>  }
> 
> @@ -1497,11 +1472,9 @@ flow_tcf_get_items_and_size(const struct
> rte_flow_attr *attr,
>   *   Maximum size of memory for actions.
>   */
>  static int
> -flow_tcf_get_actions_and_size(const struct rte_flow_action actions[],
> -                           uint64_t *action_flags)
> +flow_tcf_get_actions_and_size(const struct rte_flow_action actions[])
>  {
>       int size = 0;
> -     uint64_t flags = 0;
> 
>       size += SZ_NLATTR_NEST; /* TCA_FLOWER_ACT. */
>       for (; actions->type != RTE_FLOW_ACTION_TYPE_END; actions++) {
> @@ -1513,35 +1486,28 @@ flow_tcf_get_actions_and_size(const struct
> rte_flow_action actions[],
>                               SZ_NLATTR_STRZ_OF("mirred") +
>                               SZ_NLATTR_NEST + /* TCA_ACT_OPTIONS. */
>                               SZ_NLATTR_TYPE_OF(struct tc_mirred);
> -                     flags |= MLX5_FLOW_ACTION_PORT_ID;
>                       break;
>               case RTE_FLOW_ACTION_TYPE_JUMP:
>                       size += SZ_NLATTR_NEST + /* na_act_index. */
>                               SZ_NLATTR_STRZ_OF("gact") +
>                               SZ_NLATTR_NEST + /* TCA_ACT_OPTIONS. */
>                               SZ_NLATTR_TYPE_OF(struct tc_gact);
> -                     flags |= MLX5_FLOW_ACTION_JUMP;
>                       break;
>               case RTE_FLOW_ACTION_TYPE_DROP:
>                       size += SZ_NLATTR_NEST + /* na_act_index. */
>                               SZ_NLATTR_STRZ_OF("gact") +
>                               SZ_NLATTR_NEST + /* TCA_ACT_OPTIONS. */
>                               SZ_NLATTR_TYPE_OF(struct tc_gact);
> -                     flags |= MLX5_FLOW_ACTION_DROP;
>                       break;
>               case RTE_FLOW_ACTION_TYPE_COUNT:
>                       break;
>               case RTE_FLOW_ACTION_TYPE_OF_POP_VLAN:
> -                     flags |= MLX5_FLOW_ACTION_OF_POP_VLAN;
>                       goto action_of_vlan;
>               case RTE_FLOW_ACTION_TYPE_OF_PUSH_VLAN:
> -                     flags |= MLX5_FLOW_ACTION_OF_PUSH_VLAN;
>                       goto action_of_vlan;
>               case RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_VID:
> -                     flags |= MLX5_FLOW_ACTION_OF_SET_VLAN_VID;
>                       goto action_of_vlan;
>               case RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_PCP:
> -                     flags |= MLX5_FLOW_ACTION_OF_SET_VLAN_PCP;
>                       goto action_of_vlan;
>  action_of_vlan:
>                       size += SZ_NLATTR_NEST + /* na_act_index. */
> @@ -1563,8 +1529,7 @@ flow_tcf_get_actions_and_size(const struct
> rte_flow_action actions[],
>               case RTE_FLOW_ACTION_TYPE_DEC_TTL:
>               case RTE_FLOW_ACTION_TYPE_SET_MAC_SRC:
>               case RTE_FLOW_ACTION_TYPE_SET_MAC_DST:
> -                     size += flow_tcf_get_pedit_actions_size(&actions,
> -                                                             &flags);
> +                     size += flow_tcf_get_pedit_actions_size(&actions);
>                       break;
>               default:
>                       DRV_LOG(WARNING,
> @@ -1574,7 +1539,6 @@ flow_tcf_get_actions_and_size(const struct
> rte_flow_action actions[],
>                       break;
>               }
>       }
> -     *action_flags = flags;
>       return size;
>  }
> 
> @@ -1610,10 +1574,6 @@ flow_tcf_nl_brand(struct nlmsghdr *nlh, uint32_t
> handle)
>   *   Pointer to the list of items.
>   * @param[in] actions
>   *   Pointer to the list of actions.
> - * @param[out] item_flags
> - *   Pointer to bit mask of all items detected.
> - * @param[out] action_flags
> - *   Pointer to bit mask of all actions detected.
>   * @param[out] error
>   *   Pointer to the error structure.
>   *
> @@ -1625,7 +1585,6 @@ static struct mlx5_flow *
>  flow_tcf_prepare(const struct rte_flow_attr *attr,
>                const struct rte_flow_item items[],
>                const struct rte_flow_action actions[],
> -              uint64_t *item_flags, uint64_t *action_flags,
>                struct rte_flow_error *error)
>  {
>       size_t size = sizeof(struct mlx5_flow) +
> @@ -1635,8 +1594,8 @@ flow_tcf_prepare(const struct rte_flow_attr *attr,
>       struct nlmsghdr *nlh;
>       struct tcmsg *tcm;
> 
> -     size += flow_tcf_get_items_and_size(attr, items, item_flags);
> -     size += flow_tcf_get_actions_and_size(actions, action_flags);
> +     size += flow_tcf_get_items_and_size(attr, items);
> +     size += flow_tcf_get_actions_and_size(actions);
>       dev_flow = rte_zmalloc(__func__, size, MNL_ALIGNTO);
>       if (!dev_flow) {
>               rte_flow_error_set(error, ENOMEM,
> diff --git a/drivers/net/mlx5/mlx5_flow_verbs.c
> b/drivers/net/mlx5/mlx5_flow_verbs.c
> index ab58c04db5..453c89e347 100644
> --- a/drivers/net/mlx5/mlx5_flow_verbs.c
> +++ b/drivers/net/mlx5/mlx5_flow_verbs.c
> @@ -1217,11 +1217,9 @@ flow_verbs_validate(struct rte_eth_dev *dev,
>   *   The size of the memory needed for all actions.
>   */
>  static int
> -flow_verbs_get_actions_and_size(const struct rte_flow_action actions[],
> -                             uint64_t *action_flags)
> +flow_verbs_get_actions_and_size(const struct rte_flow_action actions[])
>  {
>       int size = 0;
> -     uint64_t detected_actions = 0;
> 
>       for (; actions->type != RTE_FLOW_ACTION_TYPE_END; actions++) {
>               switch (actions->type) {
> @@ -1229,34 +1227,27 @@ flow_verbs_get_actions_and_size(const struct
> rte_flow_action actions[],
>                       break;
>               case RTE_FLOW_ACTION_TYPE_FLAG:
>                       size += sizeof(struct ibv_flow_spec_action_tag);
> -                     detected_actions |= MLX5_FLOW_ACTION_FLAG;
>                       break;
>               case RTE_FLOW_ACTION_TYPE_MARK:
>                       size += sizeof(struct ibv_flow_spec_action_tag);
> -                     detected_actions |= MLX5_FLOW_ACTION_MARK;
>                       break;
>               case RTE_FLOW_ACTION_TYPE_DROP:
>                       size += sizeof(struct ibv_flow_spec_action_drop);
> -                     detected_actions |= MLX5_FLOW_ACTION_DROP;
>                       break;
>               case RTE_FLOW_ACTION_TYPE_QUEUE:
> -                     detected_actions |= MLX5_FLOW_ACTION_QUEUE;
>                       break;
>               case RTE_FLOW_ACTION_TYPE_RSS:
> -                     detected_actions |= MLX5_FLOW_ACTION_RSS;
>                       break;
>               case RTE_FLOW_ACTION_TYPE_COUNT:
>  #if defined(HAVE_IBV_DEVICE_COUNTERS_SET_V42) || \
>       defined(HAVE_IBV_DEVICE_COUNTERS_SET_V45)
>                       size += sizeof(struct ibv_flow_spec_counter_action);
>  #endif
> -                     detected_actions |= MLX5_FLOW_ACTION_COUNT;
>                       break;
>               default:
>                       break;
>               }
>       }
> -     *action_flags = detected_actions;
>       return size;
>  }
> 
> @@ -1274,83 +1265,54 @@ flow_verbs_get_actions_and_size(const struct
> rte_flow_action actions[],
>   *   The size of the memory needed for all items.
>   */
>  static int
> -flow_verbs_get_items_and_size(const struct rte_flow_item items[],
> -                           uint64_t *item_flags)
> +flow_verbs_get_items_and_size(const struct rte_flow_item items[])
>  {
>       int size = 0;
> -     uint64_t detected_items = 0;
> 
>       for (; items->type != RTE_FLOW_ITEM_TYPE_END; items++) {
> -             int tunnel = !!(detected_items &
> MLX5_FLOW_LAYER_TUNNEL);
> -
>               switch (items->type) {
>               case RTE_FLOW_ITEM_TYPE_VOID:
>                       break;
>               case RTE_FLOW_ITEM_TYPE_ETH:
>                       size += sizeof(struct ibv_flow_spec_eth);
> -                     detected_items |= tunnel ?
> MLX5_FLOW_LAYER_INNER_L2 :
> -
> MLX5_FLOW_LAYER_OUTER_L2;
>                       break;
>               case RTE_FLOW_ITEM_TYPE_VLAN:
>                       size += sizeof(struct ibv_flow_spec_eth);
> -                     detected_items |=
> -                             tunnel ? (MLX5_FLOW_LAYER_INNER_L2 |
> -                                       MLX5_FLOW_LAYER_INNER_VLAN) :
> -                                      (MLX5_FLOW_LAYER_OUTER_L2 |
> -                                       MLX5_FLOW_LAYER_OUTER_VLAN);
>                       break;
>               case RTE_FLOW_ITEM_TYPE_IPV4:
>                       size += sizeof(struct ibv_flow_spec_ipv4_ext);
> -                     detected_items |= tunnel ?
> -                                       MLX5_FLOW_LAYER_INNER_L3_IPV4
> :
> -
> MLX5_FLOW_LAYER_OUTER_L3_IPV4;
>                       break;
>               case RTE_FLOW_ITEM_TYPE_IPV6:
>                       size += sizeof(struct ibv_flow_spec_ipv6);
> -                     detected_items |= tunnel ?
> -                                       MLX5_FLOW_LAYER_INNER_L3_IPV6
> :
> -
> MLX5_FLOW_LAYER_OUTER_L3_IPV6;
>                       break;
>               case RTE_FLOW_ITEM_TYPE_UDP:
>                       size += sizeof(struct ibv_flow_spec_tcp_udp);
> -                     detected_items |= tunnel ?
> -                                       MLX5_FLOW_LAYER_INNER_L4_UDP
> :
> -
> MLX5_FLOW_LAYER_OUTER_L4_UDP;
>                       break;
>               case RTE_FLOW_ITEM_TYPE_TCP:
>                       size += sizeof(struct ibv_flow_spec_tcp_udp);
> -                     detected_items |= tunnel ?
> -                                       MLX5_FLOW_LAYER_INNER_L4_TCP :
> -
> MLX5_FLOW_LAYER_OUTER_L4_TCP;
>                       break;
>               case RTE_FLOW_ITEM_TYPE_VXLAN:
>                       size += sizeof(struct ibv_flow_spec_tunnel);
> -                     detected_items |= MLX5_FLOW_LAYER_VXLAN;
>                       break;
>               case RTE_FLOW_ITEM_TYPE_VXLAN_GPE:
>                       size += sizeof(struct ibv_flow_spec_tunnel);
> -                     detected_items |= MLX5_FLOW_LAYER_VXLAN_GPE;
>                       break;
>  #ifdef HAVE_IBV_DEVICE_MPLS_SUPPORT
>               case RTE_FLOW_ITEM_TYPE_GRE:
>                       size += sizeof(struct ibv_flow_spec_gre);
> -                     detected_items |= MLX5_FLOW_LAYER_GRE;
>                       break;
>               case RTE_FLOW_ITEM_TYPE_MPLS:
>                       size += sizeof(struct ibv_flow_spec_mpls);
> -                     detected_items |= MLX5_FLOW_LAYER_MPLS;
>                       break;
>  #else
>               case RTE_FLOW_ITEM_TYPE_GRE:
>                       size += sizeof(struct ibv_flow_spec_tunnel);
> -                     detected_items |= MLX5_FLOW_LAYER_TUNNEL;
>                       break;
>  #endif
>               default:
>                       break;
>               }
>       }
> -     *item_flags = detected_items;
>       return size;
>  }
> 
> @@ -1365,10 +1327,6 @@ flow_verbs_get_items_and_size(const struct
> rte_flow_item items[],
>   *   Pointer to the list of items.
>   * @param[in] actions
>   *   Pointer to the list of actions.
> - * @param[out] item_flags
> - *   Pointer to bit mask of all items detected.
> - * @param[out] action_flags
> - *   Pointer to bit mask of all actions detected.
>   * @param[out] error
>   *   Pointer to the error structure.
>   *
> @@ -1380,15 +1338,13 @@ static struct mlx5_flow *
>  flow_verbs_prepare(const struct rte_flow_attr *attr __rte_unused,
>                  const struct rte_flow_item items[],
>                  const struct rte_flow_action actions[],
> -                uint64_t *item_flags,
> -                uint64_t *action_flags,
>                  struct rte_flow_error *error)
>  {
>       uint32_t size = sizeof(struct mlx5_flow) + sizeof(struct ibv_flow_attr);
>       struct mlx5_flow *flow;
> 
> -     size += flow_verbs_get_actions_and_size(actions, action_flags);
> -     size += flow_verbs_get_items_and_size(items, item_flags);
> +     size += flow_verbs_get_actions_and_size(actions);

I think the function name should be changed since it only returns the size.

> +     size += flow_verbs_get_items_and_size(items);

I think the function name should be changed since it only returns the size.

>       flow = rte_calloc(__func__, 1, size, 0);
>       if (!flow) {
>               rte_flow_error_set(error, ENOMEM,
> --
> 2.11.0

Best,
Ori Kam

Reply via email to