[3/3] net/mlx5: remove flags setting from flow preparation
Checks
Commit Message
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 <yskoh@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(-)
Comments
> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Yongseok Koh
> Sent: Friday, November 2, 2018 11:08 PM
> To: Shahaf Shuler <shahafs@mellanox.com>
> Cc: dev@dpdk.org; Yongseok Koh <yskoh@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 <yskoh@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
On Sun, Nov 04, 2018 at 01:29:13AM -0700, Ori Kam wrote:
>
>
> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Yongseok Koh
> > Sent: Friday, November 2, 2018 11:08 PM
> > To: Shahaf Shuler <shahafs@mellanox.com>
> > Cc: dev@dpdk.org; Yongseok Koh <yskoh@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 <yskoh@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.
Agree.
I have to rebase the code anyway as VXLAN has been merged. Will change it.
Thanks,
Yongseok
@@ -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,
@@ -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,
@@ -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);
@@ -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,
@@ -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);
+ size += flow_verbs_get_items_and_size(items);
flow = rte_calloc(__func__, 1, size, 0);
if (!flow) {
rte_flow_error_set(error, ENOMEM,