> -----Original Message-----
> From: Ori Kam <or...@mellanox.com>
> Sent: Wednesday, July 1, 2020 3:55 AM
> To: Jiawei(Jonny) Wang <jiaw...@mellanox.com>; Slava Ovsiienko
> <viachesl...@mellanox.com>; Matan Azrad <ma...@mellanox.com>
> Cc: dev@dpdk.org; Thomas Monjalon <tho...@monjalon.net>; Raslan
> Darawsheh <rasl...@mellanox.com>; ian.sto...@intel.com;
> f...@redhat.com; Jiawei(Jonny) Wang <jiaw...@mellanox.com>
> Subject: RE: [PATCH 6/8] net/mlx5: update translate function for sample
> action
>
> Hi Jiawei,
> PSB,
>
> Thanks,
> Ori
>
> > -----Original Message-----
> > From: Jiawei Wang <jiaw...@mellanox.com>
> > Sent: Thursday, June 25, 2020 7:26 PM
> > Subject: [PATCH 6/8] net/mlx5: update translate function for sample
> > action
> >
> > Translate the attribute of sample action that include sample ratio and
> > sub actions list, then create the sample DR action.
> >
> > Signed-off-by: Jiawei Wang <jiaw...@mellanox.com>
> > ---
> > drivers/net/mlx5/mlx5_flow.c | 16 +-
> > drivers/net/mlx5/mlx5_flow.h | 14 +-
> > drivers/net/mlx5/mlx5_flow_dv.c | 502
> > +++++++++++++++++++++++++++++++++++++++-
> > 3 files changed, 511 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/net/mlx5/mlx5_flow.c
> > b/drivers/net/mlx5/mlx5_flow.c index 7c65a9a..73ef290 100644
> > --- a/drivers/net/mlx5/mlx5_flow.c
> > +++ b/drivers/net/mlx5/mlx5_flow.c
> > @@ -4569,10 +4569,14 @@ uint32_t mlx5_flow_adjust_priority(struct
> > rte_eth_dev *dev, int32_t priority,
> > int hairpin_flow;
> > uint32_t hairpin_id = 0;
> > struct rte_flow_attr attr_tx = { .priority = 0 };
> > + struct rte_flow_attr attr_factor = {0};
> > int ret;
> >
> > - hairpin_flow = flow_check_hairpin_split(dev, attr, actions);
> > - ret = flow_drv_validate(dev, attr, items, p_actions_rx,
> > + memcpy((void *)&attr_factor, (const void *)attr, sizeof(*attr));
> > + if (external)
> > + attr_factor.group *= MLX5_FLOW_TABLE_FACTOR;
> > + hairpin_flow = flow_check_hairpin_split(dev, &attr_factor, actions);
> > + ret = flow_drv_validate(dev, &attr_factor, items, p_actions_rx,
> > external, hairpin_flow, error);
> > if (ret < 0)
> > return 0;
> > @@ -4591,7 +4595,7 @@ uint32_t mlx5_flow_adjust_priority(struct
> > rte_eth_dev *dev, int32_t priority,
> > rte_errno = ENOMEM;
> > goto error_before_flow;
> > }
> > - flow->drv_type = flow_get_drv_type(dev, attr);
> > + flow->drv_type = flow_get_drv_type(dev, &attr_factor);
> > if (hairpin_id != 0)
> > flow->hairpin_flow_id = hairpin_id;
> > MLX5_ASSERT(flow->drv_type > MLX5_FLOW_TYPE_MIN && @@ -
> 4637,7
> > +4641,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev,
> > int32_t priority,
> > * depending on configuration. In the simplest
> > * case it just creates unmodified original flow.
> > */
> > - ret = flow_create_split_outer(dev, flow, attr,
> > + ret = flow_create_split_outer(dev, flow, &attr_factor,
> > buf->entry[i].pattern,
> > p_actions_rx, external, idx,
> > error);
> > @@ -4674,8 +4678,8 @@ uint32_t mlx5_flow_adjust_priority(struct
> > rte_eth_dev *dev, int32_t priority,
> > * the egress Flows belong to the different device and
> > * copy table should be updated in peer NIC Rx domain.
> > */
> > - if (attr->ingress &&
> > - (external || attr->group != MLX5_FLOW_MREG_CP_TABLE_GROUP))
> > {
> > + if (attr_factor.ingress &&
> > + (external || attr_factor.group !=
> > MLX5_FLOW_MREG_CP_TABLE_GROUP)) {
> > ret = flow_mreg_update_copy_table(dev, flow, actions,
> error);
> > if (ret)
> > goto error;
> > diff --git a/drivers/net/mlx5/mlx5_flow.h
> > b/drivers/net/mlx5/mlx5_flow.h index 941de5f..4163183 100644
> > --- a/drivers/net/mlx5/mlx5_flow.h
> > +++ b/drivers/net/mlx5/mlx5_flow.h
> > @@ -369,6 +369,13 @@ enum mlx5_flow_fate_type {
> > MLX5_FLOW_FATE_MAX,
> > };
> >
> > +/*
> > + * Max number of actions per DV flow.
> > + * See CREATE_FLOW_MAX_FLOW_ACTIONS_SUPPORTED
> > + * in rdma-core file providers/mlx5/verbs.c.
> > + */
> > +#define MLX5_DV_MAX_NUMBER_OF_ACTIONS 8
> > +
> > /* Matcher PRM representation */
> > struct mlx5_flow_dv_match_params {
> > size_t size;
> > @@ -599,13 +606,6 @@ struct mlx5_flow_handle { #define
> > MLX5_FLOW_HANDLE_VERBS_SIZE (sizeof(struct mlx5_flow_handle))
> #endif
> >
> > -/*
> > - * Max number of actions per DV flow.
> > - * See CREATE_FLOW_MAX_FLOW_ACTIONS_SUPPORTED
> > - * in rdma-core file providers/mlx5/verbs.c.
> > - */
> > -#define MLX5_DV_MAX_NUMBER_OF_ACTIONS 8
> > -
> > /** Device flow structure only for DV flow creation. */ struct
> > mlx5_flow_dv_workspace {
> > uint32_t group; /**< The group index. */ diff --git
> > a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
> > index 710c0f3..62a4a3b 100644
> > --- a/drivers/net/mlx5/mlx5_flow_dv.c
> > +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> > @@ -79,6 +79,10 @@
> > flow_dv_tbl_resource_release(struct rte_eth_dev *dev,
> > struct mlx5_flow_tbl_resource *tbl);
> >
> > +static int
> > +flow_dv_encap_decap_resource_release(struct rte_eth_dev *dev,
> > + uint32_t encap_decap_idx);
> > +
> > /**
> > * Initialize flow attributes structure according to flow items' types.
> > *
> > @@ -7897,6 +7901,385 @@ struct field_modify_info modify_tcp[] = { }
> >
> > /**
> > + * Create an Rx Hash queue.
> > + *
> > + * @param dev
> > + * Pointer to Ethernet device.
> > + * @param[in] dev_flow
> > + * Pointer to the mlx5_flow.
> > + * @param[in] rss_desc
> > + * Pointer to the mlx5_flow_rss_desc.
> > + * @param[in, out] hrxq_idx
>
> I think this is only used as out.
>
right, will change it.
> > + * Hash Rx queue index.
> > + * @param[out] error
> > + * Pointer to error structure.
> > + *
> > + * @return
> > + * The Verbs/DevX object initialised, NULL otherwise and rte_errno is
> > set.
> > + */
> > +static struct mlx5_hrxq *
> > +flow_dv_handle_rx_queue(struct rte_eth_dev *dev,
> > + struct mlx5_flow *dev_flow,
> > + struct mlx5_flow_rss_desc *rss_desc,
> > + uint32_t *hrxq_idx,
> > + struct rte_flow_error *error)
> > +{
> > + struct mlx5_priv *priv = dev->data->dev_private;
> > + struct mlx5_flow_handle *dh = dev_flow->handle;
> > + struct mlx5_hrxq *hrxq;
> > +
> > + MLX5_ASSERT(rss_desc->queue_num);
> > + *hrxq_idx = mlx5_hrxq_get(dev, rss_desc->key,
> > + MLX5_RSS_HASH_KEY_LEN,
> > + dev_flow->hash_fields,
> > + rss_desc->queue,
> > + rss_desc->queue_num);
> > + if (!*hrxq_idx) {
> > + *hrxq_idx = mlx5_hrxq_new
> > + (dev, rss_desc->key,
> > + MLX5_RSS_HASH_KEY_LEN,
> > + dev_flow->hash_fields,
> > + rss_desc->queue,
> > + rss_desc->queue_num,
> > + !!(dh->layers &
> > + MLX5_FLOW_LAYER_TUNNEL));
> > + }
> > + hrxq = mlx5_ipool_get(priv->sh->ipool[MLX5_IPOOL_HRXQ],
> > + *hrxq_idx);
>
> Why do you need this line? You can compare the hrxq_idx to check for error.
>
Yes, we can check by *hrxq_idx==0 for error, or return corresponding hash rx
queue object if no error.
Thanks.
> > + if (!hrxq) {
> > + rte_flow_error_set
> > + (error, rte_errno,
> > + RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
> > + "cannot get hash queue");
> > + goto error;
> > + }
> > + dh->rix_hrxq = *hrxq_idx;
> > + return hrxq;
> > +error:
> > + /* hrxq is union, don't clear it if the flag is not set. */
> > + if (dh->rix_hrxq) {
> > + mlx5_hrxq_release(dev, dh->rix_hrxq);
> > + dh->rix_hrxq = 0;
> > + }
> > + return NULL;
> > +}
> > +
>
>
> [snap...]
