[dpdk-stable] patch 'net/mlx5: fix modify actions support limitation' has been queued to stable release 19.11.1
Luca Boccassi
luca.boccassi at gmail.com
Tue Feb 11 15:22:56 CET 2020
Hi,
The rebasing is done and you can see the result in the mail you replied
to.
The new 19.11 branch will be pushed by the end of the week.
On Tue, 2020-02-11 at 13:48 +0000, Bing Zhao wrote:
> Hi Luca,
>
> Many thanks for your help.
>
> Yes, this needs to be also applied to the 19.11 stable branch. I want
> to rebase it but could not find the branch in
> http://git.dpdk.org/dpdk-stable/
> , so would you please share me the
> branch information then I can try to finish the rebasing today or
> early tomorrow.
>
> BR. Bing
>
> > -----Original Message-----
> > From:
> > luca.boccassi at gmail.com
> > <
> > luca.boccassi at gmail.com
> > >
> > Sent: Tuesday, February 11, 2020 7:21 PM
> > To: Bing Zhao <
> > bingz at mellanox.com
> > >
> > Cc: Ori Kam <
> > orika at mellanox.com
> > >; Slava Ovsiienko
> > <
> > viacheslavo at mellanox.com
> > >; dpdk stable <
> > stable at dpdk.org
> > >
> > Subject: patch 'net/mlx5: fix modify actions support limitation'
> > has
> > been queued to stable release 19.11.1
> >
> > Hi,
> >
> > FYI, your patch has been queued to stable release 19.11.1
> >
> > Note it hasn't been pushed to
> > https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2F
> >
> > dpdk.org%2Fbrowse%2Fdpdk-
> > stable&data=02%7C01%7Cbingz%40mellanox.com%7Cf31d20ac8c
> > 4e427386a108d7aee5e8c2%7Ca652971c7d2e4d9ba6a4d149256f461b%
> > 7C0%7C0%7C637170174781961835&sdata=no%2BG5nKNzaYL72n
> > 3vf5v0ZmfmJLRLNcPfwqgf4YJNU0%3D&reserved=0 yet.
> > It will be pushed if I get no objections before 02/13/20. So please
> > shout
> > if anyone has objections.
> >
> > Also note that after the patch there's a diff of the upstream
> > commit vs
> > the patch applied to the branch. This will indicate if there was
> > any
> > rebasing needed to apply to the stable branch. If there were code
> > changes for rebasing
> > (ie: not only metadata diffs), please double check that the rebase
> > was
> > correctly done.
> >
> > Thanks.
> >
> > Luca Boccassi
> >
> > ---
> > From e0de816f945e599a933081c53bb060d5dd4cbf13 Mon Sep 17
> > 00:00:00 2001
> > From: Bing Zhao <
> > bingz at mellanox.com
> > >
> > Date: Mon, 20 Jan 2020 11:43:07 +0200
> > Subject: [PATCH] net/mlx5: fix modify actions support limitation
> >
> > [ upstream commit 024e95759c16c67eee92efffeee34915dc94a9c0 ]
> >
> > In the root table, there is some limitation of total number of
> > header
> > modify actions, 16 or 8 for each. But in other tables, there is no
> > such
> > strict limitation. In an IPv6 case, the IP fields modifying will
> > occupy
> > more actions than that in IPv4, so the total support number should
> > be
> > increased in order to support as many actions as possible for an
> > IPv6 +
> > TCP packet.
> > And in the meanwhile, the memory consumption should also be taken
> > into consideration because sometimes only several actions are
> > needed.
> > The root table checking could also be done in low layer driver and
> > the
> > error code will be returned if the actions number is over the
> > maximal
> > supported value.
> >
> > Fixes: 0e9d00027686 ("net/mlx5: check maximum modify actions
> > number")
> >
> > Signed-off-by: Bing Zhao <
> > bingz at mellanox.com
> > >
> > Acked-by: Ori Kam <
> > orika at mellanox.com
> > >
> > Acked-by: Viacheslav Ovsiienko <
> > viacheslavo at mellanox.com
> > >
> > ---
> > drivers/net/mlx5/mlx5_flow.h | 15 +++--
> > drivers/net/mlx5/mlx5_flow_dv.c | 104 +++++++++++++++++-----------
> > --
> > --
> > 2 files changed, 66 insertions(+), 53 deletions(-)
> >
> > diff --git a/drivers/net/mlx5/mlx5_flow.h
> > b/drivers/net/mlx5/mlx5_flow.h index db12715ecc..13bed08d75
> > 100644
> > --- a/drivers/net/mlx5/mlx5_flow.h
> > +++ b/drivers/net/mlx5/mlx5_flow.h
> > @@ -391,11 +391,14 @@ struct mlx5_flow_dv_tag_resource {
> >
> > /*
> > * Number of modification commands.
> > - * If extensive metadata registers are supported
> > - * the maximal actions amount is 16 and 8 otherwise.
> > + * If extensive metadata registers are supported, the maximal
> > actions
> > + amount is
> > + * 16 and 8 otherwise on root table. The validation could also be
> > done
> > + in the
> > + * lower driver layer.
> > + * On non-root table, there is no limitation, but 32 is enough
> > right
> > now.
> > */
> > -#define MLX5_MODIFY_NUM 16
> > -#define MLX5_MODIFY_NUM_NO_MREG 8
> > +#define MLX5_MAX_MODIFY_NUM 32
> > +#define MLX5_ROOT_TBL_MODIFY_NUM 16
> > +#define MLX5_ROOT_TBL_MODIFY_NUM_NO_MREG 8
> >
> > /* Modify resource structure */
> > struct mlx5_flow_dv_modify_hdr_resource { @@ -406,9 +409,9 @@
> > struct mlx5_flow_dv_modify_hdr_resource {
> > /**< Verbs modify header action object. */
> > uint8_t ft_type; /**< Flow table type, Rx or Tx. */
> > uint32_t actions_num; /**< Number of modification actions.
> > */
> > - struct mlx5_modification_cmd actions[MLX5_MODIFY_NUM];
> > + uint64_t flags; /**< Flags for RDMA API. */
> > + struct mlx5_modification_cmd actions[];
> > /**< Modification actions. */
> > - uint64_t flags; /**< Flags for RDMA API. */
> > };
> >
> > /* Jump action resource structure. */
> > diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
> > b/drivers/net/mlx5/mlx5_flow_dv.c index 8f77909419..74d1a68e4a
> > 100644
> > --- a/drivers/net/mlx5/mlx5_flow_dv.c
> > +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> > @@ -363,7 +363,7 @@ flow_dv_convert_modify_action(struct
> > rte_flow_item *item,
> > uint32_t mask;
> > uint32_t data;
> >
> > - if (i >= MLX5_MODIFY_NUM)
> > + if (i >= MLX5_MAX_MODIFY_NUM)
> > return rte_flow_error_set(error, EINVAL,
> > RTE_FLOW_ERROR_TYPE_ACTION,
> > NULL,
> > "too many items to modify");
> > @@ -404,11 +404,11 @@ flow_dv_convert_modify_action(struct
> > rte_flow_item *item,
> > ++i;
> > ++field;
> > } while (field->size);
> > - resource->actions_num = i;
> > - if (!resource->actions_num)
> > + if (resource->actions_num == i)
> > return rte_flow_error_set(error, EINVAL,
> >
> > RTE_FLOW_ERROR_TYPE_ACTION, NULL,
> > "invalid modification flow
> > item");
> > + resource->actions_num = i;
> > return 0;
> > }
> >
> > @@ -569,7 +569,7 @@ flow_dv_convert_action_modify_vlan_vid
> > struct mlx5_modification_cmd *actions = &resource->actions[i];
> > struct field_modify_info *field = modify_vlan_out_first_vid;
> >
> > - if (i >= MLX5_MODIFY_NUM)
> > + if (i >= MLX5_MAX_MODIFY_NUM)
> > return rte_flow_error_set(error, EINVAL,
> > RTE_FLOW_ERROR_TYPE_ACTION, NULL,
> > "too many items to modify");
> > @@ -902,7 +902,7 @@ flow_dv_convert_action_set_reg
> > struct mlx5_modification_cmd *actions = resource->actions;
> > uint32_t i = resource->actions_num;
> >
> > - if (i >= MLX5_MODIFY_NUM)
> > + if (i >= MLX5_MAX_MODIFY_NUM)
> > return rte_flow_error_set(error, EINVAL,
> >
> > RTE_FLOW_ERROR_TYPE_ACTION, NULL,
> > "too many items to modify");
> > @@ -914,10 +914,6 @@ flow_dv_convert_action_set_reg
> > actions[i].data1 = rte_cpu_to_be_32(conf->data);
> > ++i;
> > resource->actions_num = i;
> > - if (!resource->actions_num)
> > - return rte_flow_error_set(error, EINVAL,
> > -
> > RTE_FLOW_ERROR_TYPE_ACTION, NULL,
> > - "invalid modification flow
> > item");
> > return 0;
> > }
> >
> > @@ -2256,7 +2252,6 @@ flow_dv_encap_decap_resource_register
> > domain = sh->rx_domain;
> > else
> > domain = sh->tx_domain;
> > -
> > /* Lookup a matching resource from cache. */
> > LIST_FOREACH(cache_resource, &sh->encaps_decaps, next) {
> > if (resource->reformat_type == cache_resource-
> > > reformat_type && @@ -3367,21 +3362,27 @@
> >
> > flow_dv_validate_action_port_id(struct rte_eth_dev *dev,
> > *
> > * @param dev
> > * Pointer to rte_eth_dev structure.
> > + * @param flags
> > + * Flags bits to check if root level.
> > *
> > * @return
> > * Max number of modify header actions device can support.
> > */
> > static unsigned int
> > -flow_dv_modify_hdr_action_max(struct rte_eth_dev *dev)
> > +flow_dv_modify_hdr_action_max(struct rte_eth_dev *dev, uint64_t
> > flags)
> > {
> > /*
> > * There's no way to directly query the max cap. Although it
> > has to be
> > * acquried by iterative trial, it is a safe assumption that
> > more
> > * actions are supported by FW if extensive metadata register
> > is
> > - * supported.
> > + * supported. (Only in the root table)
> > */
> > - return mlx5_flow_ext_mreg_supported(dev) ?
> > MLX5_MODIFY_NUM :
> > -
> > MLX5_MODIFY_NUM_NO_MREG;
> > + if (!(flags & MLX5DV_DR_ACTION_FLAGS_ROOT_LEVEL))
> > + return MLX5_MAX_MODIFY_NUM;
> > + else
> > + return mlx5_flow_ext_mreg_supported(dev) ?
> > +
> > MLX5_ROOT_TBL_MODIFY_NUM :
> > +
> > MLX5_ROOT_TBL_MODIFY_NUM_NO_MREG;
> > }
> >
> > /**
> > @@ -3472,8 +3473,12 @@ flow_dv_modify_hdr_resource_register
> > struct mlx5_ibv_shared *sh = priv->sh;
> > struct mlx5_flow_dv_modify_hdr_resource *cache_resource;
> > struct mlx5dv_dr_domain *ns;
> > + uint32_t actions_len;
> >
> > - if (resource->actions_num >
> > flow_dv_modify_hdr_action_max(dev))
> > + resource->flags =
> > + dev_flow->group ? 0 :
> > MLX5DV_DR_ACTION_FLAGS_ROOT_LEVEL;
> > + if (resource->actions_num >
> > flow_dv_modify_hdr_action_max(dev,
> > + resource->flags))
> > return rte_flow_error_set(error, EOVERFLOW,
> >
> > RTE_FLOW_ERROR_TYPE_ACTION, NULL,
> > "too many modify header
> > items"); @@ -3483,17 +3488,15 @@
> > flow_dv_modify_hdr_resource_register
> > ns = sh->tx_domain;
> > else
> > ns = sh->rx_domain;
> > - resource->flags =
> > - dev_flow->group ? 0 :
> > MLX5DV_DR_ACTION_FLAGS_ROOT_LEVEL;
> > /* Lookup a matching resource from cache. */
> > + actions_len = resource->actions_num * sizeof(resource-
> > > actions[0]);
> >
> > LIST_FOREACH(cache_resource, &sh->modify_cmds, next) {
> > if (resource->ft_type == cache_resource->ft_type &&
> > resource->actions_num == cache_resource-
> > > actions_num &&
> >
> > resource->flags == cache_resource->flags &&
> > !memcmp((const void *)resource->actions,
> > (const void *)cache_resource->actions,
> > - (resource->actions_num *
> > - sizeof(resource-
> > > actions[0])))) {
> >
> > + actions_len)) {
> > DRV_LOG(DEBUG, "modify-header
> > resource %p: refcnt %d++",
> > (void *)cache_resource,
> > rte_atomic32_read(&cache_resource-
> > > refcnt));
> >
> > @@ -3503,18 +3506,18 @@ flow_dv_modify_hdr_resource_register
> > }
> > }
> > /* Register new modify-header resource. */
> > - cache_resource = rte_calloc(__func__, 1,
> > sizeof(*cache_resource), 0);
> > + cache_resource = rte_calloc(__func__, 1,
> > + sizeof(*cache_resource) +
> > actions_len, 0);
> > if (!cache_resource)
> > return rte_flow_error_set(error, ENOMEM,
> >
> > RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
> > "cannot allocate resource
> > memory");
> > *cache_resource = *resource;
> > + rte_memcpy(cache_resource->actions, resource->actions,
> > actions_len);
> > cache_resource->verbs_action =
> > mlx5_glue->dv_create_flow_action_modify_header
> > - (sh->ctx, cache_resource-
> > > ft_type,
> >
> > - ns, cache_resource->flags,
> > - cache_resource->actions_num
> > *
> > - sizeof(cache_resource-
> > > actions[0]),
> >
> > + (sh->ctx, cache_resource-
> > > ft_type, ns,
> >
> > + cache_resource->flags,
> > actions_len,
> > (uint64_t *)cache_resource-
> > > actions);
> >
> > if (!cache_resource->verbs_action) {
> > rte_free(cache_resource);
> > @@ -6670,10 +6673,13 @@ __flow_dv_translate(struct rte_eth_dev
> > *dev,
> > };
> > int actions_n = 0;
> > bool actions_end = false;
> > - struct mlx5_flow_dv_modify_hdr_resource mhdr_res = {
> > - .ft_type = attr->egress ?
> > MLX5DV_FLOW_TABLE_TYPE_NIC_TX :
> > -
> > MLX5DV_FLOW_TABLE_TYPE_NIC_RX
> > - };
> > + union {
> > + struct mlx5_flow_dv_modify_hdr_resource res;
> > + uint8_t len[sizeof(struct
> > mlx5_flow_dv_modify_hdr_resource) +
> > + sizeof(struct mlx5_modification_cmd) *
> > + (MLX5_MAX_MODIFY_NUM + 1)];
> > + } mhdr_dummy;
> > + struct mlx5_flow_dv_modify_hdr_resource *mhdr_res =
> > &mhdr_dummy.res;
> > union flow_dv_attr flow_attr = { .attr = 0 };
> > uint32_t tag_be;
> > union mlx5_flow_tbl_key tbl_key;
> > @@ -6685,15 +6691,19 @@ __flow_dv_translate(struct rte_eth_dev
> > *dev,
> > uint32_t table;
> > int ret = 0;
> >
> > + mhdr_res->ft_type = attr->egress ?
> > MLX5DV_FLOW_TABLE_TYPE_NIC_TX :
> > +
> > MLX5DV_FLOW_TABLE_TYPE_NIC_RX;
> > ret = mlx5_flow_group_to_table(attr, dev_flow->external, attr-
> > > group,
> >
> > &table, error);
> > if (ret)
> > return ret;
> > dev_flow->group = table;
> > if (attr->transfer)
> > - mhdr_res.ft_type = MLX5DV_FLOW_TABLE_TYPE_FDB;
> > + mhdr_res->ft_type = MLX5DV_FLOW_TABLE_TYPE_FDB;
> > if (priority == MLX5_FLOW_PRIO_RSVD)
> > priority = dev_conf->flow_prio - 1;
> > + /* number of actions must be set to 0 in case of dirty stack.
> > */
> > + mhdr_res->actions_num = 0;
> > for (; !actions_end ; actions++) {
> > const struct rte_flow_action_queue *queue;
> > const struct rte_flow_action_rss *rss; @@ -6731,7
> > +6741,7 @@ __flow_dv_translate(struct rte_eth_dev *dev,
> > };
> >
> > if (flow_dv_convert_action_mark(dev,
> > &mark,
> > -
> > &mhdr_res,
> > +
> > mhdr_res,
> > error))
> > return -rte_errno;
> > action_flags |=
> > MLX5_FLOW_ACTION_MARK_EXT; @@ -6753,7 +6763,7 @@
> > __flow_dv_translate(struct rte_eth_dev *dev,
> > actions->conf;
> >
> > if (flow_dv_convert_action_mark(dev,
> > mark,
> > -
> > &mhdr_res,
> > +
> > mhdr_res,
> > error))
> > return -rte_errno;
> > action_flags |=
> > MLX5_FLOW_ACTION_MARK_EXT; @@ -6774,7 +6784,7 @@
> > __flow_dv_translate(struct rte_eth_dev *dev,
> > break;
> > case RTE_FLOW_ACTION_TYPE_SET_META:
> > if (flow_dv_convert_action_set_meta
> > - (dev, &mhdr_res, attr,
> > + (dev, mhdr_res, attr,
> > (const struct
> > rte_flow_action_set_meta *)
> > actions->conf, error))
> > return -rte_errno;
> > @@ -6782,7 +6792,7 @@ __flow_dv_translate(struct rte_eth_dev
> > *dev,
> > break;
> > case RTE_FLOW_ACTION_TYPE_SET_TAG:
> > if (flow_dv_convert_action_set_tag
> > - (dev, &mhdr_res,
> > + (dev, mhdr_res,
> > (const struct rte_flow_action_set_tag
> > *)
> > actions->conf, error))
> > return -rte_errno;
> > @@ -6882,7 +6892,7 @@ cnt_err:
> > mlx5_update_vlan_vid_pcp(actions, &vlan);
> > /* If no VLAN push - this is a modify header
> > action */
> > if (flow_dv_convert_action_modify_vlan_vid
> > - (&mhdr_res, actions,
> > error))
> > + (mhdr_res, actions,
> > error))
> > return -rte_errno;
> > action_flags |=
> > MLX5_FLOW_ACTION_OF_SET_VLAN_VID;
> > break;
> > @@ -6981,7 +6991,7 @@ cnt_err:
> > case RTE_FLOW_ACTION_TYPE_SET_MAC_SRC:
> > case RTE_FLOW_ACTION_TYPE_SET_MAC_DST:
> > if (flow_dv_convert_action_modify_mac
> > - (&mhdr_res, actions, error))
> > + (mhdr_res, actions, error))
> > return -rte_errno;
> > action_flags |= actions->type ==
> >
> > RTE_FLOW_ACTION_TYPE_SET_MAC_SRC ?
> > @@ -6991,7 +7001,7 @@ cnt_err:
> > case RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC:
> > case RTE_FLOW_ACTION_TYPE_SET_IPV4_DST:
> > if (flow_dv_convert_action_modify_ipv4
> > - (&mhdr_res, actions, error))
> > + (mhdr_res, actions, error))
> > return -rte_errno;
> > action_flags |= actions->type ==
> >
> > RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC ?
> > @@ -7001,7 +7011,7 @@ cnt_err:
> > case RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC:
> > case RTE_FLOW_ACTION_TYPE_SET_IPV6_DST:
> > if (flow_dv_convert_action_modify_ipv6
> > - (&mhdr_res, actions, error))
> > + (mhdr_res, actions, error))
> > return -rte_errno;
> > action_flags |= actions->type ==
> >
> > RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC ?
> > @@ -7011,7 +7021,7 @@ cnt_err:
> > case RTE_FLOW_ACTION_TYPE_SET_TP_SRC:
> > case RTE_FLOW_ACTION_TYPE_SET_TP_DST:
> > if (flow_dv_convert_action_modify_tp
> > - (&mhdr_res, actions, items,
> > + (mhdr_res, actions, items,
> > &flow_attr, error))
> > return -rte_errno;
> > action_flags |= actions->type ==
> > @@ -7021,13 +7031,13 @@ cnt_err:
> > break;
> > case RTE_FLOW_ACTION_TYPE_DEC_TTL:
> > if (flow_dv_convert_action_modify_dec_ttl
> > - (&mhdr_res, items, &flow_attr,
> > error))
> > + (mhdr_res, items, &flow_attr,
> > error))
> > return -rte_errno;
> > action_flags |= MLX5_FLOW_ACTION_DEC_TTL;
> > break;
> > case RTE_FLOW_ACTION_TYPE_SET_TTL:
> > if (flow_dv_convert_action_modify_ttl
> > - (&mhdr_res, actions, items,
> > + (mhdr_res, actions, items,
> > &flow_attr, error))
> > return -rte_errno;
> > action_flags |= MLX5_FLOW_ACTION_SET_TTL;
> > @@ -7035,7 +7045,7 @@ cnt_err:
> > case RTE_FLOW_ACTION_TYPE_INC_TCP_SEQ:
> > case RTE_FLOW_ACTION_TYPE_DEC_TCP_SEQ:
> > if (flow_dv_convert_action_modify_tcp_seq
> > - (&mhdr_res, actions, error))
> > + (mhdr_res, actions, error))
> > return -rte_errno;
> > action_flags |= actions->type ==
> >
> > RTE_FLOW_ACTION_TYPE_INC_TCP_SEQ ?
> > @@ -7046,7 +7056,7 @@ cnt_err:
> > case RTE_FLOW_ACTION_TYPE_INC_TCP_ACK:
> > case RTE_FLOW_ACTION_TYPE_DEC_TCP_ACK:
> > if (flow_dv_convert_action_modify_tcp_ack
> > - (&mhdr_res, actions, error))
> > + (mhdr_res, actions, error))
> > return -rte_errno;
> > action_flags |= actions->type ==
> >
> > RTE_FLOW_ACTION_TYPE_INC_TCP_ACK ?
> > @@ -7055,13 +7065,13 @@ cnt_err:
> > break;
> > case MLX5_RTE_FLOW_ACTION_TYPE_TAG:
> > if (flow_dv_convert_action_set_reg
> > - (&mhdr_res, actions, error))
> > + (mhdr_res, actions, error))
> > return -rte_errno;
> > action_flags |= MLX5_FLOW_ACTION_SET_TAG;
> > break;
> > case MLX5_RTE_FLOW_ACTION_TYPE_COPY_MREG:
> > if (flow_dv_convert_action_copy_mreg
> > - (dev, &mhdr_res, actions,
> > error))
> > + (dev, mhdr_res, actions,
> > error))
> > return -rte_errno;
> > action_flags |= MLX5_FLOW_ACTION_SET_TAG;
> > break;
> > @@ -7086,10 +7096,10 @@ cnt_err:
> > break;
> > case RTE_FLOW_ACTION_TYPE_END:
> > actions_end = true;
> > - if (mhdr_res.actions_num) {
> > + if (mhdr_res->actions_num) {
> > /* create modify action if needed. */
> > if
> > (flow_dv_modify_hdr_resource_register
> > - (dev, &mhdr_res, dev_flow,
> > error))
> > + (dev, mhdr_res, dev_flow,
> > error))
> > return -rte_errno;
> > dev_flow-
> > > dv.actions[modify_action_position] =
> >
> > dev_flow->dv.modify_hdr-
> > > verbs_action;
> >
> > @@ -7098,7 +7108,7 @@ cnt_err:
> > default:
> > break;
> > }
> > - if (mhdr_res.actions_num &&
> > + if (mhdr_res->actions_num &&
> > modify_action_position == UINT32_MAX)
> > modify_action_position = actions_n++;
> > }
> > --
> > 2.20.1
> >
> > ---
> > Diff of the applied patch vs upstream commit (please double-check
> > if
> > non-empty:
> > ---
> > --- - 2020-02-11 11:17:42.994804790 +0000
> > +++ 0122-net-mlx5-fix-modify-actions-support-limitation.patch
> > 2020-
> > 02-11 11:17:38.612005311 +0000
> > @@ -1,8 +1,10 @@
> > -From 024e95759c16c67eee92efffeee34915dc94a9c0 Mon Sep 17
> > 00:00:00 2001
> > +From e0de816f945e599a933081c53bb060d5dd4cbf13 Mon Sep 17
> > 00:00:00 2001
> > From: Bing Zhao <
> > bingz at mellanox.com
> > >
> > Date: Mon, 20 Jan 2020 11:43:07 +0200
> > Subject: [PATCH] net/mlx5: fix modify actions support limitation
> >
> > +[ upstream commit 024e95759c16c67eee92efffeee34915dc94a9c0 ]
> > +
> > In the root table, there is some limitation of total number of
> > header
> > modify actions, 16 or 8 for each. But in other tables, there is
> > no such
> > strict limitation. In an IPv6 case, the IP fields modifying @@
> > -16,21
> > +18,20 @@ maximal supported value.
> >
> > Fixes: 0e9d00027686 ("net/mlx5: check maximum modify actions
> > number")
> > -Cc:
> > stable at dpdk.org
> >
> >
> > Signed-off-by: Bing Zhao <
> > bingz at mellanox.com
> > >
> > Acked-by: Ori Kam <
> > orika at mellanox.com
> > >
> > Acked-by: Viacheslav Ovsiienko <
> > viacheslavo at mellanox.com
> > >
> > ---
> > drivers/net/mlx5/mlx5_flow.h | 15 +++--
> > - drivers/net/mlx5/mlx5_flow_dv.c | 108 +++++++++++++++++--------
> > ----
> > ---
> > - 2 files changed, 68 insertions(+), 55 deletions(-)
> > + drivers/net/mlx5/mlx5_flow_dv.c | 104 +++++++++++++++++--------
> > ---
> > ----
> > + 2 files changed, 66 insertions(+), 53 deletions(-)
> >
> > diff --git a/drivers/net/mlx5/mlx5_flow.h
> > b/drivers/net/mlx5/mlx5_flow.h -index a1c7b67488..9832542328
> > 100644
> > +index db12715ecc..13bed08d75 100644
> > --- a/drivers/net/mlx5/mlx5_flow.h
> > +++ b/drivers/net/mlx5/mlx5_flow.h
> > -@@ -392,11 +392,14 @@ struct mlx5_flow_dv_tag_resource {
> > +@@ -391,11 +391,14 @@ struct mlx5_flow_dv_tag_resource {
> >
> > /*
> > * Number of modification commands.
> > @@ -49,7 +50,7 @@
> >
> > /* Modify resource structure */
> > struct mlx5_flow_dv_modify_hdr_resource { -@@ -407,9 +410,9 @@
> > struct mlx5_flow_dv_modify_hdr_resource {
> > +@@ -406,9 +409,9 @@ struct mlx5_flow_dv_modify_hdr_resource {
> > /**< Verbs modify header action object. */
> > uint8_t ft_type; /**< Flow table type, Rx or Tx. */
> > uint32_t actions_num; /**< Number of modification actions.
> > */ @@ -62,10 +63,10 @@
> >
> > /* Jump action resource structure. */
> > diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
> > b/drivers/net/mlx5/mlx5_flow_dv.c -index 26dbaaf329..5a1b42698c
> > 100644
> > +index 8f77909419..74d1a68e4a 100644
> > --- a/drivers/net/mlx5/mlx5_flow_dv.c
> > +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> > -@@ -366,7 +366,7 @@ flow_dv_convert_modify_action(struct
> > rte_flow_item *item,
> > +@@ -363,7 +363,7 @@ flow_dv_convert_modify_action(struct
> > rte_flow_item
> > +*item,
> > uint32_t mask;
> > uint32_t data;
> >
> > @@ -74,7 +75,7 @@
> > return rte_flow_error_set(error, EINVAL,
> > RTE_FLOW_ERROR_TYPE_ACTION,
> > NULL,
> > "too many items to modify");
> > -@@ -407,11 +407,11 @@ flow_dv_convert_modify_action(struct
> > rte_flow_item *item,
> > +@@ -404,11 +404,11 @@ flow_dv_convert_modify_action(struct
> > +rte_flow_item *item,
> > ++i;
> > ++field;
> > } while (field->size);
> > @@ -88,7 +89,7 @@
> > return 0;
> > }
> >
> > -@@ -572,7 +572,7 @@ flow_dv_convert_action_modify_vlan_vid
> > +@@ -569,7 +569,7 @@ flow_dv_convert_action_modify_vlan_vid
> > struct mlx5_modification_cmd *actions = &resource->actions[i];
> > struct field_modify_info *field = modify_vlan_out_first_vid;
> >
> > @@ -97,7 +98,7 @@
> > return rte_flow_error_set(error, EINVAL,
> > RTE_FLOW_ERROR_TYPE_ACTION, NULL,
> > "too many items to modify");
> > -@@ -905,7 +905,7 @@ flow_dv_convert_action_set_reg
> > +@@ -902,7 +902,7 @@ flow_dv_convert_action_set_reg
> > struct mlx5_modification_cmd *actions = resource->actions;
> > uint32_t i = resource->actions_num;
> >
> > @@ -106,7 +107,7 @@
> > return rte_flow_error_set(error, EINVAL,
> >
> > RTE_FLOW_ERROR_TYPE_ACTION, NULL,
> > "too many items to modify");
> > -@@ -917,10 +917,6 @@ flow_dv_convert_action_set_reg
> > +@@ -914,10 +914,6 @@ flow_dv_convert_action_set_reg
> > actions[i].data1 = rte_cpu_to_be_32(conf->data);
> > ++i;
> > resource->actions_num = i;
> > @@ -117,7 +118,7 @@
> > return 0;
> > }
> >
> > -@@ -2385,7 +2381,6 @@ flow_dv_encap_decap_resource_register
> > +@@ -2256,7 +2252,6 @@ flow_dv_encap_decap_resource_register
> > domain = sh->rx_domain;
> > else
> > domain = sh->tx_domain;
> > @@ -125,7 +126,7 @@
> > /* Lookup a matching resource from cache. */
> > LIST_FOREACH(cache_resource, &sh->encaps_decaps, next) {
> > if (resource->reformat_type == cache_resource-
> > > reformat_type && -@@ -3496,21 +3491,27 @@
> >
> > flow_dv_validate_action_port_id(struct rte_eth_dev *dev,
> > +@@ -3367,21 +3362,27 @@ flow_dv_validate_action_port_id(struct
> > +rte_eth_dev *dev,
> > *
> > * @param dev
> > * Pointer to rte_eth_dev structure.
> > @@ -157,7 +158,7 @@
> > }
> >
> > /**
> > -@@ -3669,8 +3670,12 @@ flow_dv_modify_hdr_resource_register
> > +@@ -3472,8 +3473,12 @@ flow_dv_modify_hdr_resource_register
> > struct mlx5_ibv_shared *sh = priv->sh;
> > struct mlx5_flow_dv_modify_hdr_resource *cache_resource;
> > struct mlx5dv_dr_domain *ns;
> > @@ -171,7 +172,7 @@
> > return rte_flow_error_set(error, EOVERFLOW,
> >
> > RTE_FLOW_ERROR_TYPE_ACTION, NULL,
> > "too many modify header
> > items"); -@@ -3680,17 +3685,15 @@
> > flow_dv_modify_hdr_resource_register
> > +@@ -3483,17 +3488,15 @@ flow_dv_modify_hdr_resource_register
> > ns = sh->tx_domain;
> > else
> > ns = sh->rx_domain;
> > @@ -191,7 +192,7 @@
> > DRV_LOG(DEBUG, "modify-header
> > resource %p: refcnt %d++",
> > (void *)cache_resource,
> > rte_atomic32_read(&cache_resource-
> > > refcnt));
> >
> > -@@ -3700,18 +3703,18 @@ flow_dv_modify_hdr_resource_register
> > +@@ -3503,18 +3506,18 @@ flow_dv_modify_hdr_resource_register
> > }
> > }
> > /* Register new modify-header resource. */ @@ -215,7 +216,7
> > @@
> > (uint64_t *)cache_resource-
> > > actions);
> >
> > if (!cache_resource->verbs_action) {
> > rte_free(cache_resource);
> > -@@ -7020,10 +7023,13 @@ __flow_dv_translate(struct rte_eth_dev
> > *dev,
> > +@@ -6670,10 +6673,13 @@ __flow_dv_translate(struct rte_eth_dev
> > *dev,
> > };
> > int actions_n = 0;
> > bool actions_end = false;
> > @@ -233,7 +234,7 @@
> > union flow_dv_attr flow_attr = { .attr = 0 };
> > uint32_t tag_be;
> > union mlx5_flow_tbl_key tbl_key;
> > -@@ -7035,15 +7041,19 @@ __flow_dv_translate(struct rte_eth_dev
> > *dev,
> > +@@ -6685,15 +6691,19 @@ __flow_dv_translate(struct rte_eth_dev
> > *dev,
> > uint32_t table;
> > int ret = 0;
> >
> > @@ -254,7 +255,7 @@
> > for (; !actions_end ; actions++) {
> > const struct rte_flow_action_queue *queue;
> > const struct rte_flow_action_rss *rss; -@@ -7081,7
> > +7091,7 @@ __flow_dv_translate(struct rte_eth_dev *dev,
> > +@@ -6731,7 +6741,7 @@ __flow_dv_translate(struct rte_eth_dev
> > *dev,
> > };
> >
> > if (flow_dv_convert_action_mark(dev,
> > &mark, @@ -263,7 +264,7 @@
> > error))
> > return -rte_errno;
> > action_flags |=
> > MLX5_FLOW_ACTION_MARK_EXT; -@@ -7103,7 +7113,7 @@
> > __flow_dv_translate(struct rte_eth_dev *dev,
> > +@@ -6753,7 +6763,7 @@ __flow_dv_translate(struct rte_eth_dev
> > *dev,
> > actions->conf;
> >
> > if (flow_dv_convert_action_mark(dev,
> > mark, @@ -272,7 +273,7 @@
> > error))
> > return -rte_errno;
> > action_flags |=
> > MLX5_FLOW_ACTION_MARK_EXT; -@@ -7124,7 +7134,7 @@
> > __flow_dv_translate(struct rte_eth_dev *dev,
> > +@@ -6774,7 +6784,7 @@ __flow_dv_translate(struct rte_eth_dev
> > *dev,
> > break;
> > case RTE_FLOW_ACTION_TYPE_SET_META:
> > if (flow_dv_convert_action_set_meta @@ -
> > 281,7 +282,7 @@
> > (const struct
> > rte_flow_action_set_meta *)
> > actions->conf, error))
> > return -rte_errno;
> > -@@ -7132,7 +7142,7 @@ __flow_dv_translate(struct rte_eth_dev
> > *dev,
> > +@@ -6782,7 +6792,7 @@ __flow_dv_translate(struct rte_eth_dev
> > *dev,
> > break;
> > case RTE_FLOW_ACTION_TYPE_SET_TAG:
> > if (flow_dv_convert_action_set_tag
> > @@ -290,7 +291,7 @@
> > (const struct rte_flow_action_set_tag
> > *)
> > actions->conf, error))
> > return -rte_errno;
> > -@@ -7232,7 +7242,7 @@ cnt_err:
> > +@@ -6882,7 +6892,7 @@ cnt_err:
> > mlx5_update_vlan_vid_pcp(actions, &vlan);
> > /* If no VLAN push - this is a modify header
> > action */
> > if (flow_dv_convert_action_modify_vlan_vid
> > @@ -299,7 +300,7 @@
> > return -rte_errno;
> > action_flags |=
> > MLX5_FLOW_ACTION_OF_SET_VLAN_VID;
> > break;
> > -@@ -7331,7 +7341,7 @@ cnt_err:
> > +@@ -6981,7 +6991,7 @@ cnt_err:
> > case RTE_FLOW_ACTION_TYPE_SET_MAC_SRC:
> > case RTE_FLOW_ACTION_TYPE_SET_MAC_DST:
> > if (flow_dv_convert_action_modify_mac
> > @@ -308,7 +309,7 @@
> > return -rte_errno;
> > action_flags |= actions->type ==
> >
> > RTE_FLOW_ACTION_TYPE_SET_MAC_SRC ?
> > -@@ -7341,7 +7351,7 @@ cnt_err:
> > +@@ -6991,7 +7001,7 @@ cnt_err:
> > case RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC:
> > case RTE_FLOW_ACTION_TYPE_SET_IPV4_DST:
> > if (flow_dv_convert_action_modify_ipv4
> > @@ -317,7 +318,7 @@
> > return -rte_errno;
> > action_flags |= actions->type ==
> >
> > RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC ?
> > -@@ -7351,7 +7361,7 @@ cnt_err:
> > +@@ -7001,7 +7011,7 @@ cnt_err:
> > case RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC:
> > case RTE_FLOW_ACTION_TYPE_SET_IPV6_DST:
> > if (flow_dv_convert_action_modify_ipv6
> > @@ -326,7 +327,7 @@
> > return -rte_errno;
> > action_flags |= actions->type ==
> >
> > RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC ?
> > -@@ -7361,7 +7371,7 @@ cnt_err:
> > +@@ -7011,7 +7021,7 @@ cnt_err:
> > case RTE_FLOW_ACTION_TYPE_SET_TP_SRC:
> > case RTE_FLOW_ACTION_TYPE_SET_TP_DST:
> > if (flow_dv_convert_action_modify_tp @@ -
> > 335,7 +336,7 @@
> > &flow_attr, error))
> > return -rte_errno;
> > action_flags |= actions->type ==
> > -@@ -7371,13 +7381,13 @@ cnt_err:
> > +@@ -7021,13 +7031,13 @@ cnt_err:
> > break;
> > case RTE_FLOW_ACTION_TYPE_DEC_TTL:
> > if (flow_dv_convert_action_modify_dec_ttl
> > @@ -351,7 +352,7 @@
> > &flow_attr, error))
> > return -rte_errno;
> > action_flags |= MLX5_FLOW_ACTION_SET_TTL;
> > -@@ -7385,7 +7395,7 @@ cnt_err:
> > +@@ -7035,7 +7045,7 @@ cnt_err:
> > case RTE_FLOW_ACTION_TYPE_INC_TCP_SEQ:
> > case RTE_FLOW_ACTION_TYPE_DEC_TCP_SEQ:
> > if (flow_dv_convert_action_modify_tcp_seq
> > @@ -360,7 +361,7 @@
> > return -rte_errno;
> > action_flags |= actions->type ==
> >
> > RTE_FLOW_ACTION_TYPE_INC_TCP_SEQ ?
> > -@@ -7396,7 +7406,7 @@ cnt_err:
> > +@@ -7046,7 +7056,7 @@ cnt_err:
> > case RTE_FLOW_ACTION_TYPE_INC_TCP_ACK:
> > case RTE_FLOW_ACTION_TYPE_DEC_TCP_ACK:
> > if (flow_dv_convert_action_modify_tcp_ack
> > @@ -369,7 +370,7 @@
> > return -rte_errno;
> > action_flags |= actions->type ==
> >
> > RTE_FLOW_ACTION_TYPE_INC_TCP_ACK ?
> > -@@ -7405,13 +7415,13 @@ cnt_err:
> > +@@ -7055,13 +7065,13 @@ cnt_err:
> > break;
> > case MLX5_RTE_FLOW_ACTION_TYPE_TAG:
> > if (flow_dv_convert_action_set_reg
> > @@ -385,22 +386,7 @@
> > return -rte_errno;
> > action_flags |= MLX5_FLOW_ACTION_SET_TAG;
> > break;
> > -@@ -7435,23 +7445,23 @@ cnt_err:
> > - action_flags |= MLX5_FLOW_ACTION_METER;
> > - break;
> > - case RTE_FLOW_ACTION_TYPE_SET_IPV4_DSCP:
> > -- if
> > (flow_dv_convert_action_modify_ipv4_dscp(&mhdr_res,
> > -+ if
> > (flow_dv_convert_action_modify_ipv4_dscp(mhdr_res,
> > - actions,
> > error))
> > - return -rte_errno;
> > - action_flags |=
> > MLX5_FLOW_ACTION_SET_IPV4_DSCP;
> > - break;
> > - case RTE_FLOW_ACTION_TYPE_SET_IPV6_DSCP:
> > -- if
> > (flow_dv_convert_action_modify_ipv6_dscp(&mhdr_res,
> > -+ if
> > (flow_dv_convert_action_modify_ipv6_dscp(mhdr_res,
> > - actions,
> > error))
> > - return -rte_errno;
> > - action_flags |=
> > MLX5_FLOW_ACTION_SET_IPV6_DSCP;
> > +@@ -7086,10 +7096,10 @@ cnt_err:
> > break;
> > case RTE_FLOW_ACTION_TYPE_END:
> > actions_end = true;
> > @@ -413,7 +399,7 @@
> > return -rte_errno;
> > dev_flow-
> > > dv.actions[modify_action_position] =
> >
> > dev_flow->dv.modify_hdr-
> > > verbs_action;
> >
> > -@@ -7460,7 +7470,7 @@ cnt_err:
> > +@@ -7098,7 +7108,7 @@ cnt_err:
> > default:
> > break;
> > }
--
Kind regards,
Luca Boccassi
More information about the stable
mailing list