Thanks Ivan for the comments, patch updated.
> -----Original Message----- > From: Ivan Malov <ivan.ma...@oktetlabs.ru> > Sent: Wednesday, October 19, 2022 11:43 PM > To: Sean Zhang (Networking SW) <xiazh...@nvidia.com> > Cc: NBU-Contact-Thomas Monjalon (EXTERNAL) <tho...@monjalon.net>; > Matan Azrad <ma...@nvidia.com>; Slava Ovsiienko > <viachesl...@nvidia.com>; Andrew Rybchenko > <andrew.rybche...@oktetlabs.ru>; dev@dpdk.org > Subject: Re: [PATCH] net/mlx5: add port representor support > > External email: Use caution opening links or attachments > > > Hi, > > This patch might be missing the following part: > > diff --git a/doc/guides/nics/features/mlx5.ini > b/doc/guides/nics/features/mlx5.ini > index e5974063c8..5644626f06 100644 > --- a/doc/guides/nics/features/mlx5.ini > +++ b/doc/guides/nics/features/mlx5.ini > @@ -84,6 +84,7 @@ vlan = Y > vxlan = Y > vxlan_gpe = Y > represented_port = Y > +port_representor = Y > > [rte_flow actions] > age = I > > > Also, the summary line reads like adding support for representors in general. > Perhaps it pays to reword it as: > add port representor item support > > It's up to you, of course. > > > On Wed, 19 Oct 2022, Sean Zhang wrote: > > > Add support for port_representor item, it will match on traffic > > originated from representor port specified in the pattern. This item > > is supported in FDB steering domain only (in the flow with transfer > > attribute). > > > > For example, below flow will redirect the destination of traffic from > > port 1 to port 2. > > These "port 1" and "port 2" might read as "ethdev 1" and "ethdev 2", while in > reality the flow below asks to redirect traffic coming from ethdev 1 to a > switch port *represented by* ethdev 2. > > That's why it's important to use concrete terms instead of just "port". > > Though, I do not insist on rewording this. > > > > > testpmd> ... pattern eth / port_representor port_id is 1 / end actions > > represented_port ethdev_port_id 2 / ... > > > > To handle abovementioned item, tx queue matching is added in the > > driver, > > It would be better to spell "Tx" with the letter "T" capitalised. > > > and the flow will be expanded to number of the tx queues. If the spec > > of > > Same here. > > > port_representor is NULL, the flow will not be expanded and match on > > traffic from any representor port. > > > > Signed-off-by: Sean Zhang <xiazhang at nvidia.com> > > > > --- > > The depending patches as below: > > > > [1] > > http://patches.dpdk.org/project/dpdk/cover/20220930125315.5079-1- > suanm > > i...@nvidia.com > > --- > > drivers/net/mlx5/mlx5_flow.c | 116 ++++++++++++++++++++++++++++++- > - > > drivers/net/mlx5/mlx5_flow_dv.c | 11 ++- > > 2 files changed, 122 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/net/mlx5/mlx5_flow.c > > b/drivers/net/mlx5/mlx5_flow.c index 026d4eb9c0..d60b1490cc 100644 > > --- a/drivers/net/mlx5/mlx5_flow.c > > +++ b/drivers/net/mlx5/mlx5_flow.c > > @@ -126,6 +126,15 @@ struct mlx5_flow_expand_node { > > */ > > }; > > > > +/** Keep same format with mlx5_flow_expand_rss to share the buffer > > +for > > expansion. */ > > +struct mlx5_flow_expand_sqn { > > + uint32_t entries; /** Number of entries */ > > + struct { > > + struct rte_flow_item *pattern; /**< Expanded pattern array. > > */ > > + uint32_t priority; /**< Priority offset for each expansion. > > */ > > + } entry[]; > > +}; > > + > > /* Optional expand field. The expansion alg will not go deeper. */ > > #define MLX5_EXPANSION_NODE_OPTIONAL (UINT64_C(1) << 0) > > > > @@ -574,6 +583,88 @@ mlx5_flow_expand_rss(struct > mlx5_flow_expand_rss > > *buf, size_t size, > > return lsize; > > } > > > > +/** > > + * Expand SQN flows into several possible flows according to the tx > > +queue > > It would be better to spell "Tx" with the letter "T" capitalised. > > > + * number > > + * > > + * @param[in] buf > > + * Buffer to store the result expansion. > > + * @param[in] size > > + * Buffer size in bytes. If 0, @p buf can be NULL. > > + * @param[in] pattern > > + * User flow pattern. > > + * @param[in] sq_specs > > + * Buffer to store sq spec. > > + * > > + * @return > > + * 0 for success and negative value for failure > > + * > > + */ > > +static int > > +mlx5_flow_expand_sqn(struct mlx5_flow_expand_sqn *buf, size_t size, > > + const struct rte_flow_item *pattern, > > + struct mlx5_rte_flow_item_sq *sq_specs) { > > + const struct rte_flow_item *item; > > + bool port_representor = false; > > + size_t user_pattern_size = 0; > > + struct rte_eth_dev *dev; > > + struct mlx5_priv *priv; > > + void *addr = NULL; > > + uint16_t port_id; > > + size_t lsize; > > + int elt = 2; > > + uint16_t i; > > + > > + buf->entries = 0; > > + for (item = pattern; item->type != RTE_FLOW_ITEM_TYPE_END; item++) > { > > + if (item->type == RTE_FLOW_ITEM_TYPE_PORT_REPRESENTOR) { > > + const struct rte_flow_item_ethdev *pid_v = > > item->spec; > > + > > + if (!pid_v) > > + return 0; > > + port_id = pid_v->port_id; > > + port_representor = true; > > + } > > + user_pattern_size += sizeof(*item); > > + } > > + if (!port_representor) > > + return 0; > > + dev = &rte_eth_devices[port_id]; > > + priv = dev->data->dev_private; > > + buf->entry[0].pattern = (void *)&buf->entry[priv->txqs_n]; > > + lsize = offsetof(struct mlx5_flow_expand_sqn, entry) + > > + sizeof(buf->entry[0]) * priv->txqs_n; > > + if (lsize + (user_pattern_size + sizeof(struct rte_flow_item) * > > + elt) > > * priv->txqs_n > size) > > + return -EINVAL; > > + addr = buf->entry[0].pattern; > > + for (i = 0; i != priv->txqs_n; ++i) { > > + struct rte_flow_item pattern_add[] = { > > + { > > + .type = (enum rte_flow_item_type) > > + MLX5_RTE_FLOW_ITEM_TYPE_SQ, > > + .spec = &sq_specs[i], > > + }, > > + { > > + .type = RTE_FLOW_ITEM_TYPE_END, > > + }, > > + }; > > + struct mlx5_txq_ctrl *txq = mlx5_txq_get(dev, i); > > + > > + if (txq == NULL) > > + return -EINVAL; > > + buf->entry[i].pattern = addr; > > + sq_specs[i].queue = mlx5_txq_get_sqn(txq); > > + mlx5_txq_release(dev, i); > > + rte_memcpy(addr, pattern, user_pattern_size); > > + addr = (void *)(((uintptr_t)addr) + user_pattern_size); > > + rte_memcpy(addr, pattern_add, sizeof(struct > > + rte_flow_item) * > > elt); > > + addr = (void *)(((uintptr_t)addr) + sizeof(struct > > rte_flow_item) * elt); > > + buf->entries++; > > + } > > + return 0; > > +} > > + > > enum mlx5_expansion { > > MLX5_EXPANSION_ROOT, > > MLX5_EXPANSION_ROOT_OUTER, > > @@ -5421,6 +5512,11 @@ flow_meter_split_prep(struct rte_eth_dev *dev, > > memcpy(sfx_items, items, sizeof(*sfx_items)); > > sfx_items++; > > break; > > + case RTE_FLOW_ITEM_TYPE_PORT_REPRESENTOR: > > + flow_src_port = 0; > > + memcpy(sfx_items, items, sizeof(*sfx_items)); > > + sfx_items++; > > + break; > > case RTE_FLOW_ITEM_TYPE_VLAN: > > /* Determine if copy vlan item below. */ > > vlan_item_src = items; @@ -6076,7 +6172,8 @@ > > flow_sample_split_prep(struct rte_eth_dev *dev, > > }; > > /* Prepare the suffix subflow items. */ > > for (; items->type != RTE_FLOW_ITEM_TYPE_END; items++) { > > - if (items->type == RTE_FLOW_ITEM_TYPE_PORT_ID) { > > + if (items->type == RTE_FLOW_ITEM_TYPE_PORT_ID || > > + items->type == > > RTE_FLOW_ITEM_TYPE_PORT_REPRESENTOR) { > > memcpy(sfx_items, items, sizeof(*sfx_items)); > > sfx_items++; > > } > > @@ -6889,7 +6986,7 @@ flow_list_create(struct rte_eth_dev *dev, enum > > mlx5_flow_type type, > > int indir_actions_n = MLX5_MAX_INDIRECT_ACTIONS; > > union { > > struct mlx5_flow_expand_rss buf; > > - uint8_t buffer[4096]; > > + uint8_t buffer[8192]; > > } expand_buffer; > > union { > > struct rte_flow_action actions[MLX5_MAX_SPLIT_ACTIONS]; > > @@ -6903,6 +7000,7 @@ flow_list_create(struct rte_eth_dev *dev, enum > > mlx5_flow_type type, > > struct rte_flow_item items[MLX5_MAX_SPLIT_ITEMS]; > > uint8_t buffer[2048]; > > } items_tx; > > + struct mlx5_rte_flow_item_sq sq_specs[RTE_MAX_QUEUES_PER_PORT]; > > struct mlx5_flow_expand_rss *buf = &expand_buffer.buf; > > struct mlx5_flow_rss_desc *rss_desc; > > const struct rte_flow_action *p_actions_rx; @@ -6991,8 +7089,18 > > @@ flow_list_create(struct rte_eth_dev *dev, enum mlx5_flow_type type, > > > > mlx5_dbg__print_pattern(buf->entry[i].pattern); > > } > > } else { > > - buf->entries = 1; > > - buf->entry[0].pattern = (void *)(uintptr_t)items; > > + ret = mlx5_flow_expand_sqn((struct mlx5_flow_expand_sqn > > *)buf, > > + sizeof(expand_buffer.buffer), > > + items, sq_specs); > > + if (ret) { > > + rte_flow_error_set(error, ENOMEM, > > RTE_FLOW_ERROR_TYPE_HANDLE, > > + NULL, "not enough memory for > > rte_flow"); > > + goto error; > > + } > > + if (buf->entries == 0) { > > + buf->entries = 1; > > + buf->entry[0].pattern = (void *)(uintptr_t)items; > > + } > > } > > rss_desc->shared_rss = flow_get_shared_rss_action(dev, indir_actions, > > indir_actions_n); > > diff --git a/drivers/net/mlx5/mlx5_flow_dv.c > > b/drivers/net/mlx5/mlx5_flow_dv.c index 0f6fd34a8b..c4a2eb69a4 100644 > > --- a/drivers/net/mlx5/mlx5_flow_dv.c > > +++ b/drivers/net/mlx5/mlx5_flow_dv.c > > @@ -7189,6 +7189,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const > > struct rte_flow_attr *attr, > > port_id_item = items; > > break; > > case RTE_FLOW_ITEM_TYPE_REPRESENTED_PORT: > > + case RTE_FLOW_ITEM_TYPE_PORT_REPRESENTOR: > > ret = flow_dv_validate_item_represented_port > > (dev, items, attr, item_flags, > > error); > > if (ret < 0) > > @@ -13560,6 +13561,7 @@ flow_dv_translate_items_sws(struct > rte_eth_dev *dev, > > mlx5_flow_get_thread_workspace())->rss_desc, > > }; > > struct mlx5_dv_matcher_workspace wks_m = wks; > > + int item_type; > > int ret = 0; > > int tunnel; > > > > @@ -13569,7 +13571,8 @@ flow_dv_translate_items_sws(struct > rte_eth_dev *dev, > > RTE_FLOW_ERROR_TYPE_ITEM, > > NULL, "item not > > supported"); > > tunnel = !!(wks.item_flags & MLX5_FLOW_LAYER_TUNNEL); > > - switch (items->type) { > > + item_type = items->type; > > + switch (item_type) { > > case RTE_FLOW_ITEM_TYPE_CONNTRACK: > > flow_dv_translate_item_aso_ct(dev, match_mask, > > match_value, > > items); @@ -13581,6 +13584,12 @@ flow_dv_translate_items_sws(struct > rte_eth_dev *dev, > > wks.last_item = tunnel ? MLX5_FLOW_ITEM_INNER_FLEX : > > MLX5_FLOW_ITEM_OUTER_FLEX; > > break; > > + case MLX5_RTE_FLOW_ITEM_TYPE_SQ: > > + flow_dv_translate_item_sq(match_value, items, > > + MLX5_SET_MATCHER_SW_V); > > + flow_dv_translate_item_sq(match_mask, items, > > + MLX5_SET_MATCHER_SW_M); > > + break; > > default: > > ret = flow_dv_translate_items(dev, items, &wks_m, > > match_mask, MLX5_SET_MATCHER_SW_M, > > error); > > -- > > 2.25.1 > > > > > > Thank you.
