From: Dariusz Sosnowski <dsosnowski@nvidia.com>
To: Gregory Etelson <getelson@nvidia.com>, "dev@dpdk.org" <dev@dpdk.org>
Cc: Maayan Kashani <mkashani@nvidia.com>,
Slava Ovsiienko <viacheslavo@nvidia.com>,
Ori Kam <orika@nvidia.com>, Suanming Mou <suanmingm@nvidia.com>,
Matan Azrad <matan@nvidia.com>
Subject: RE: [PATCH 2/2] net/mlx5: improve pattern template validation
Date: Tue, 6 Feb 2024 12:26:45 +0000 [thread overview]
Message-ID: <IA1PR12MB8311D580488896920FFCE146A4462@IA1PR12MB8311.namprd12.prod.outlook.com> (raw)
In-Reply-To: <20240202150617.328603-3-getelson@nvidia.com>
Hi Gregory,
> -----Original Message-----
> From: Gregory Etelson <getelson@nvidia.com>
> Sent: Friday, February 2, 2024 16:06
> To: dev@dpdk.org
> Cc: Gregory Etelson <getelson@nvidia.com>; Maayan Kashani
> <mkashani@nvidia.com>; Dariusz Sosnowski <dsosnowski@nvidia.com>;
> Slava Ovsiienko <viacheslavo@nvidia.com>; Ori Kam <orika@nvidia.com>;
> Suanming Mou <suanmingm@nvidia.com>; Matan Azrad
> <matan@nvidia.com>
> Subject: [PATCH 2/2] net/mlx5: improve pattern template validation
>
> Current PMD implementation validates pattern templates that will always be
> rejected during table template creation.
>
> The patch adds basic HWS verifications to pattern validation to ensure that the
> pattern can be used in table template.
>
> PMD updates `rte_errno` if pattern template validation failed:
>
> E2BIG - pattern too big for PMD
> ENOTSUP - pattern not supported by PMD
> ENOMEM - PMD allocation failure
>
> Signed-off-by: Gregory Etelson <getelson@nvidia.com>
>
> [snip]
>
> --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
> index da873ae2e2..443aa5fcf0 100644
> --- a/drivers/net/mlx5/mlx5_flow_hw.c
> +++ b/drivers/net/mlx5/mlx5_flow_hw.c
> @@ -6840,6 +6840,46 @@ flow_hw_pattern_has_sq_match(const struct
> rte_flow_item *items)
> return false;
> }
>
> +static int
> +pattern_template_validate(struct rte_eth_dev *dev,
> + struct rte_flow_pattern_template *pt[], uint32_t
> pt_num) {
> + uint32_t group = 0;
> + struct rte_flow_error error;
> + struct rte_flow_template_table_attr tbl_attr = {
> + .nb_flows = 64,
> + .insertion_type =
> RTE_FLOW_TABLE_INSERTION_TYPE_PATTERN,
> + .hash_func = RTE_FLOW_TABLE_HASH_FUNC_DEFAULT,
> + .flow_attr = {
> + .ingress = pt[0]->attr.ingress,
> + .egress = pt[0]->attr.egress,
> + .transfer = pt[0]->attr.transfer
> + }
> + };
> + struct mlx5_priv *priv = dev->data->dev_private;
> + struct rte_flow_actions_template *action_template;
> +
> + if (pt[0]->attr.ingress)
> + action_template = priv-
> >action_template_drop[MLX5DR_TABLE_TYPE_NIC_RX];
> + else if (pt[0]->attr.egress)
> + action_template = priv-
> >action_template_drop[MLX5DR_TABLE_TYPE_NIC_TX];
> + else if (pt[0]->attr.transfer)
> + action_template = priv-
> >action_template_drop[MLX5DR_TABLE_TYPE_FDB];
> + else
> + return EINVAL;
> + do {
> + struct rte_flow_template_table *tmpl_tbl;
> +
> + tbl_attr.flow_attr.group = group;
> + tmpl_tbl = flow_hw_template_table_create(dev, &tbl_attr, pt, pt_num,
flow_hw_table_create() should be called here.
If E-Switch is enabled, flow_hw_template_table_create() will perform internal group translation for FDB and TX domain,
so group 0 will be untested.
> + &action_template, 1,
> NULL);
> + if (!tmpl_tbl)
> + return rte_errno;
> + flow_hw_table_destroy(dev, tmpl_tbl, &error);
I don't think that passing error struct is needed here, since this error is not propagated up.
> [snip]
>
> @@ -9184,6 +9235,66 @@ flow_hw_compare_config(const struct
> mlx5_flow_hw_attr *hw_attr,
> return true;
> }
>
> +/*
> + * No need to explicitly release drop action templates on port stop.
> + * Drop action templates release with other action templates during
> + * mlx5_dev_close -> flow_hw_resource_release ->
> +flow_hw_actions_template_destroy */ static void
> +action_template_drop_release(struct rte_eth_dev *dev) {
> + int i;
> + struct mlx5_priv *priv = dev->data->dev_private;
> +
> + for (i = 0; i < MLX5DR_TABLE_TYPE_MAX; i++) {
> + if (!priv->action_template_drop[i])
> + continue;
> + flow_hw_actions_template_destroy(dev,
> + priv-
> >action_template_drop[i],
> + NULL);
> + }
I'd suggest adding zeroing action_template_drop array entries here.
In case of failure inside rte_flow_configure(), rollback code called on error must
reset the affected fields in private data to allow safe call to rte_flow_configure() again.
> [snip]
>
> @@ -9621,10 +9735,7 @@ flow_hw_resource_release(struct rte_eth_dev
> *dev)
> flow_hw_flush_all_ctrl_flows(dev);
> flow_hw_cleanup_tx_repr_tagging(dev);
> flow_hw_cleanup_ctrl_rx_tables(dev);
> - while (!LIST_EMPTY(&priv->flow_hw_grp)) {
> - grp = LIST_FIRST(&priv->flow_hw_grp);
> - flow_hw_group_unset_miss_group(dev, grp, NULL);
> - }
> + action_template_drop_release(dev);
Why is the miss actions cleanup code removed? It does not seem related to the patch.
Best regards,
Dariusz Sosnowski
next prev parent reply other threads:[~2024-02-06 12:26 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-02 15:06 [PATCH 0/2] net/mlx5: update pattern validations Gregory Etelson
2024-02-02 15:06 ` [PATCH 1/2] net/mlx5/hws: definer, " Gregory Etelson
2024-02-06 12:27 ` Dariusz Sosnowski
2024-02-12 12:32 ` [PATCH v2 0/2] net/mlx5: " Gregory Etelson
2024-02-12 12:32 ` [PATCH v2 1/2] net/mlx5/hws: definer, " Gregory Etelson
2024-02-12 12:32 ` [PATCH v2 2/2] net/mlx5: improve pattern template validation Gregory Etelson
2024-02-02 15:06 ` [PATCH " Gregory Etelson
2024-02-06 12:26 ` Dariusz Sosnowski [this message]
2024-02-12 13:59 ` [PATCH v3 0/2] net/mlx5: update pattern validations Gregory Etelson
2024-02-12 13:59 ` [PATCH v3 1/2] net/mlx5/hws: definer, " Gregory Etelson
2024-02-12 13:59 ` [PATCH v3 2/2] net/mlx5: improve pattern template validation Gregory Etelson
2024-02-26 12:07 ` [PATCH v3 0/2] net/mlx5: update pattern validations Raslan Darawsheh
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=IA1PR12MB8311D580488896920FFCE146A4462@IA1PR12MB8311.namprd12.prod.outlook.com \
--to=dsosnowski@nvidia.com \
--cc=dev@dpdk.org \
--cc=getelson@nvidia.com \
--cc=matan@nvidia.com \
--cc=mkashani@nvidia.com \
--cc=orika@nvidia.com \
--cc=suanmingm@nvidia.com \
--cc=viacheslavo@nvidia.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).