Why should DV prepare function return the list of actions?
The only reason I can think of, is if you want to remove the for loop in
dv_translate. And then in flow_dv_create_action change the switch case
to ifs.
Ori
> -----Original Message-----
> From: Yongseok Koh
> Sent: Sunday, October 28, 2018 7:35 PM
> To: Shahaf Shuler <shah...@mellanox.com>
> Cc: dev@dpdk.org; Yongseok Koh <ys...@mellanox.com>; Ori Kam
> <or...@mellanox.com>
> Subject: [PATCH] net/mlx5: fix Direct Verbs getting item and action flags
>
> Flow driver has to provide detected item flags and action flags via
> flow_drv_prepare(). DV doesn't return flags at all.
>
> Fixes: 865a0c15672c ("net/mlx5: add Direct Verbs prepare function")
> Cc: or...@mellanox.com
>
> Signed-off-by: Yongseok Koh <ys...@mellanox.com>
> ---
> drivers/net/mlx5/mlx5_flow_dv.c | 115
> ++++++++++++++++++++++++++++++++++++----
> 1 file changed, 106 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
> b/drivers/net/mlx5/mlx5_flow_dv.c
> index 8f729f44f8..67c133c2fb 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -354,6 +354,103 @@ flow_dv_validate(struct rte_eth_dev *dev, const
> struct rte_flow_attr *attr,
> }
>
> /**
> + * Extract item flags and action flags.
> + *
> + * @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.
> + */
> +static void
> +flow_dv_get_item_action_flags(const struct rte_flow_item items[],
> + const struct rte_flow_action actions[],
> + uint64_t *item_flags, uint64_t *action_flags)
> +{
> + uint64_t detected_items = 0;
> + uint64_t detected_actions = 0;
> + int tunnel;
> +
> + for (; items->type != RTE_FLOW_ITEM_TYPE_END; items++) {
> + tunnel = !!(detected_items & MLX5_FLOW_LAYER_TUNNEL);
> + switch (items->type) {
> + case RTE_FLOW_ITEM_TYPE_ETH:
> + detected_items |= tunnel ?
> MLX5_FLOW_LAYER_INNER_L2 :
> +
> MLX5_FLOW_LAYER_OUTER_L2;
> + break;
> + case RTE_FLOW_ITEM_TYPE_VLAN:
> + detected_items |= tunnel ?
> MLX5_FLOW_LAYER_INNER_VLAN :
> +
> MLX5_FLOW_LAYER_OUTER_VLAN;
> + break;
> + case RTE_FLOW_ITEM_TYPE_IPV4:
> + detected_items |= tunnel ?
> + MLX5_FLOW_LAYER_INNER_L3_IPV4
> :
> +
> MLX5_FLOW_LAYER_OUTER_L3_IPV4;
> + break;
> + case RTE_FLOW_ITEM_TYPE_IPV6:
> + detected_items |= tunnel ?
> + MLX5_FLOW_LAYER_INNER_L3_IPV6
> :
> +
> MLX5_FLOW_LAYER_OUTER_L3_IPV6;
> + break;
> + case RTE_FLOW_ITEM_TYPE_TCP:
> + detected_items |= tunnel ?
> + MLX5_FLOW_LAYER_INNER_L4_TCP :
> +
> MLX5_FLOW_LAYER_OUTER_L4_TCP;
> + break;
> + case RTE_FLOW_ITEM_TYPE_UDP:
> + detected_items |= tunnel ?
> + MLX5_FLOW_LAYER_INNER_L4_UDP
> :
> +
> MLX5_FLOW_LAYER_OUTER_L4_UDP;
> + break;
> + case RTE_FLOW_ITEM_TYPE_GRE:
> + case RTE_FLOW_ITEM_TYPE_NVGRE:
> + detected_items |= MLX5_FLOW_LAYER_GRE;
> + break;
> + case RTE_FLOW_ITEM_TYPE_VXLAN:
> + detected_items |= MLX5_FLOW_LAYER_VXLAN;
> + break;
> + case RTE_FLOW_ITEM_TYPE_VXLAN_GPE:
> + detected_items |= MLX5_FLOW_LAYER_VXLAN_GPE;
> + break;
> + case RTE_FLOW_ITEM_TYPE_META:
> + detected_items |= MLX5_FLOW_ITEM_METADATA;
> + break;
> + default:
> + break;
> + }
> + }
> + for (; actions->type != RTE_FLOW_ACTION_TYPE_END; actions++) {
> + switch (actions->type) {
> + case RTE_FLOW_ACTION_TYPE_FLAG:
> + detected_actions |= MLX5_FLOW_ACTION_FLAG;
> + break;
> + case RTE_FLOW_ACTION_TYPE_MARK:
> + detected_actions |= MLX5_FLOW_ACTION_MARK;
> + break;
> + case RTE_FLOW_ACTION_TYPE_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:
> + detected_actions |= MLX5_FLOW_ACTION_COUNT;
> + break;
> + default:
> + break;
> + }
> + }
> + *item_flags = detected_items;
> + *action_flags = detected_actions;
> +}
> +
> +/**
> * Internal preparation function. Allocates the DV flow size,
> * this size is constant.
> *
> @@ -376,15 +473,15 @@ flow_dv_validate(struct rte_eth_dev *dev, const
> struct rte_flow_attr *attr,
> */
> 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,
> + 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);
> struct mlx5_flow *flow;
>
> + flow_dv_get_item_action_flags(items, actions, item_flags,
> action_flags);
> flow = rte_calloc(__func__, 1, size, 0);
> if (!flow) {
> rte_flow_error_set(error, ENOMEM,
> @@ -1067,7 +1164,7 @@ flow_dv_create_action(const struct rte_flow_action
> *action,
> dev_flow->dv.actions[actions_n].tag_value =
> mlx5_flow_mark_set(MLX5_FLOW_MARK_DEFAULT);
> actions_n++;
> - flow->actions |= MLX5_FLOW_ACTION_FLAG;
> + assert(flow->actions & MLX5_FLOW_ACTION_FLAG);
> break;
> case RTE_FLOW_ACTION_TYPE_MARK:
> dev_flow->dv.actions[actions_n].type =
> MLX5DV_FLOW_ACTION_TAG;
> @@ -1075,18 +1172,18 @@ flow_dv_create_action(const struct
> rte_flow_action *action,
> mlx5_flow_mark_set
> (((const struct rte_flow_action_mark *)
> (action->conf))->id);
> - flow->actions |= MLX5_FLOW_ACTION_MARK;
> + assert(flow->actions & MLX5_FLOW_ACTION_MARK);
> actions_n++;
> break;
> case RTE_FLOW_ACTION_TYPE_DROP:
> dev_flow->dv.actions[actions_n].type =
> MLX5DV_FLOW_ACTION_DROP;
> - flow->actions |= MLX5_FLOW_ACTION_DROP;
> + assert(flow->actions & MLX5_FLOW_ACTION_DROP);
> break;
> case RTE_FLOW_ACTION_TYPE_QUEUE:
> queue = action->conf;
> flow->rss.queue_num = 1;
> (*flow->queue)[0] = queue->index;
> - flow->actions |= MLX5_FLOW_ACTION_QUEUE;
> + assert(flow->actions & MLX5_FLOW_ACTION_QUEUE);
> break;
> case RTE_FLOW_ACTION_TYPE_RSS:
> rss = action->conf;
> @@ -1098,7 +1195,7 @@ flow_dv_create_action(const struct rte_flow_action
> *action,
> flow->rss.types = rss->types;
> flow->rss.level = rss->level;
> /* Added to array only in apply since we need the QP */
> - flow->actions |= MLX5_FLOW_ACTION_RSS;
> + assert(flow->actions & MLX5_FLOW_ACTION_RSS);
> break;
> default:
> break;
> --
> 2.11.0
