* [PATCH net-next 01/12] net/mlx5: HWS, Fix matcher action template attach
2025-04-08 14:00 [PATCH net-next 00/12] net/mlx5: HWS, Refactor action STE handling Tariq Toukan
@ 2025-04-08 14:00 ` Tariq Toukan
2025-04-09 15:21 ` Michal Kubiak
2025-04-08 14:00 ` [PATCH net-next 02/12] net/mlx5: HWS, Remove unused element array Tariq Toukan
` (10 subsequent siblings)
11 siblings, 1 reply; 32+ messages in thread
From: Tariq Toukan @ 2025-04-08 14:00 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet,
Andrew Lunn
Cc: Gal Pressman, Leon Romanovsky, Saeed Mahameed, Leon Romanovsky,
Tariq Toukan, netdev, linux-rdma, linux-kernel, Moshe Shemesh,
Mark Bloch, Vlad Dogaru, Yevgeny Kliteynik
From: Vlad Dogaru <vdogaru@nvidia.com>
The procedure of attaching an action template to an existing matcher had
a few issues:
1. Attaching accidentally overran the `at` array in bwc_matcher, which
would result in memory corruption. This bug wasn't triggered, but it
is possible to trigger it by attaching action templates beyond the
initial buffer size of 8. Fix this by converting to a dynamically
sized buffer and reallocating if needed.
2. Similarly, the `at` array inside the native matcher was never
reallocated. Fix this the same as above.
3. The bwc layer treated any error in action template attach as a signal
that the matcher should be rehashed to account for a larger number of
action STEs. In reality, there are other unrelated errors that can
arise and they should be propagated upstack. Fix this by adding a
`need_rehash` output parameter that's orthogonal to error codes.
Fixes: 2111bb970c78 ("net/mlx5: HWS, added backward-compatible API handling")
Signed-off-by: Vlad Dogaru <vdogaru@nvidia.com>
Reviewed-by: Yevgeny Kliteynik <kliteyn@nvidia.com>
Reviewed-by: Mark Bloch <mbloch@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
.../mellanox/mlx5/core/steering/hws/bwc.c | 55 ++++++++++++++++---
.../mellanox/mlx5/core/steering/hws/bwc.h | 9 ++-
.../mellanox/mlx5/core/steering/hws/matcher.c | 48 +++++++++++++---
.../mellanox/mlx5/core/steering/hws/matcher.h | 4 ++
.../mellanox/mlx5/core/steering/hws/mlx5hws.h | 5 +-
5 files changed, 97 insertions(+), 24 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/bwc.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/bwc.c
index 19dce1ba512d..32de8bfc7644 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/bwc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/bwc.c
@@ -90,13 +90,19 @@ int mlx5hws_bwc_matcher_create_simple(struct mlx5hws_bwc_matcher *bwc_matcher,
bwc_matcher->priority = priority;
bwc_matcher->size_log = MLX5HWS_BWC_MATCHER_INIT_SIZE_LOG;
+ bwc_matcher->size_of_at_array = MLX5HWS_BWC_MATCHER_ATTACH_AT_NUM;
+ bwc_matcher->at = kcalloc(bwc_matcher->size_of_at_array,
+ sizeof(*bwc_matcher->at), GFP_KERNEL);
+ if (!bwc_matcher->at)
+ goto free_bwc_matcher_rules;
+
/* create dummy action template */
bwc_matcher->at[0] =
mlx5hws_action_template_create(action_types ?
action_types : init_action_types);
if (!bwc_matcher->at[0]) {
mlx5hws_err(table->ctx, "BWC matcher: failed creating action template\n");
- goto free_bwc_matcher_rules;
+ goto free_bwc_matcher_at_array;
}
bwc_matcher->num_of_at = 1;
@@ -126,6 +132,8 @@ int mlx5hws_bwc_matcher_create_simple(struct mlx5hws_bwc_matcher *bwc_matcher,
mlx5hws_match_template_destroy(bwc_matcher->mt);
free_at:
mlx5hws_action_template_destroy(bwc_matcher->at[0]);
+free_bwc_matcher_at_array:
+ kfree(bwc_matcher->at);
free_bwc_matcher_rules:
kfree(bwc_matcher->rules);
err:
@@ -192,6 +200,7 @@ int mlx5hws_bwc_matcher_destroy_simple(struct mlx5hws_bwc_matcher *bwc_matcher)
for (i = 0; i < bwc_matcher->num_of_at; i++)
mlx5hws_action_template_destroy(bwc_matcher->at[i]);
+ kfree(bwc_matcher->at);
mlx5hws_match_template_destroy(bwc_matcher->mt);
kfree(bwc_matcher->rules);
@@ -520,6 +529,23 @@ hws_bwc_matcher_extend_at(struct mlx5hws_bwc_matcher *bwc_matcher,
struct mlx5hws_rule_action rule_actions[])
{
enum mlx5hws_action_type action_types[MLX5HWS_BWC_MAX_ACTS];
+ void *p;
+
+ if (unlikely(bwc_matcher->num_of_at >= bwc_matcher->size_of_at_array)) {
+ if (bwc_matcher->size_of_at_array >= MLX5HWS_MATCHER_MAX_AT)
+ return -ENOMEM;
+ bwc_matcher->size_of_at_array *= 2;
+ p = krealloc(bwc_matcher->at,
+ bwc_matcher->size_of_at_array *
+ sizeof(*bwc_matcher->at),
+ __GFP_ZERO | GFP_KERNEL);
+ if (!p) {
+ bwc_matcher->size_of_at_array /= 2;
+ return -ENOMEM;
+ }
+
+ bwc_matcher->at = p;
+ }
hws_bwc_rule_actions_to_action_types(rule_actions, action_types);
@@ -777,6 +803,7 @@ int mlx5hws_bwc_rule_create_simple(struct mlx5hws_bwc_rule *bwc_rule,
struct mlx5hws_rule_attr rule_attr;
struct mutex *queue_lock; /* Protect the queue */
u32 num_of_rules;
+ bool need_rehash;
int ret = 0;
int at_idx;
@@ -803,10 +830,14 @@ int mlx5hws_bwc_rule_create_simple(struct mlx5hws_bwc_rule *bwc_rule,
at_idx = bwc_matcher->num_of_at - 1;
ret = mlx5hws_matcher_attach_at(bwc_matcher->matcher,
- bwc_matcher->at[at_idx]);
+ bwc_matcher->at[at_idx],
+ &need_rehash);
if (unlikely(ret)) {
- /* Action template attach failed, possibly due to
- * requiring more action STEs.
+ hws_bwc_unlock_all_queues(ctx);
+ return ret;
+ }
+ if (unlikely(need_rehash)) {
+ /* The new action template requires more action STEs.
* Need to attempt creating new matcher with all
* the action templates, including the new one.
*/
@@ -942,6 +973,7 @@ hws_bwc_rule_action_update(struct mlx5hws_bwc_rule *bwc_rule,
struct mlx5hws_context *ctx = bwc_matcher->matcher->tbl->ctx;
struct mlx5hws_rule_attr rule_attr;
struct mutex *queue_lock; /* Protect the queue */
+ bool need_rehash;
int at_idx, ret;
u16 idx;
@@ -973,12 +1005,17 @@ hws_bwc_rule_action_update(struct mlx5hws_bwc_rule *bwc_rule,
at_idx = bwc_matcher->num_of_at - 1;
ret = mlx5hws_matcher_attach_at(bwc_matcher->matcher,
- bwc_matcher->at[at_idx]);
+ bwc_matcher->at[at_idx],
+ &need_rehash);
if (unlikely(ret)) {
- /* Action template attach failed, possibly due to
- * requiring more action STEs.
- * Need to attempt creating new matcher with all
- * the action templates, including the new one.
+ hws_bwc_unlock_all_queues(ctx);
+ return ret;
+ }
+ if (unlikely(need_rehash)) {
+ /* The new action template requires more action
+ * STEs. Need to attempt creating new matcher
+ * with all the action templates, including the
+ * new one.
*/
ret = hws_bwc_matcher_rehash_at(bwc_matcher);
if (unlikely(ret)) {
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/bwc.h b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/bwc.h
index 47f7ed141553..bb0cf4b922ce 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/bwc.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/bwc.h
@@ -10,9 +10,7 @@
#define MLX5HWS_BWC_MATCHER_REHASH_BURST_TH 32
/* Max number of AT attach operations for the same matcher.
- * When the limit is reached, next attempt to attach new AT
- * will result in creation of a new matcher and moving all
- * the rules to this matcher.
+ * When the limit is reached, a larger buffer is allocated for the ATs.
*/
#define MLX5HWS_BWC_MATCHER_ATTACH_AT_NUM 8
@@ -23,10 +21,11 @@
struct mlx5hws_bwc_matcher {
struct mlx5hws_matcher *matcher;
struct mlx5hws_match_template *mt;
- struct mlx5hws_action_template *at[MLX5HWS_BWC_MATCHER_ATTACH_AT_NUM];
- u32 priority;
+ struct mlx5hws_action_template **at;
u8 num_of_at;
+ u8 size_of_at_array;
u8 size_log;
+ u32 priority;
atomic_t num_of_rules;
struct list_head *rules;
};
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/matcher.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/matcher.c
index b61864b32053..37a4497048a6 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/matcher.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/matcher.c
@@ -905,18 +905,48 @@ static int hws_matcher_uninit(struct mlx5hws_matcher *matcher)
return 0;
}
+static int hws_matcher_grow_at_array(struct mlx5hws_matcher *matcher)
+{
+ void *p;
+
+ if (matcher->size_of_at_array >= MLX5HWS_MATCHER_MAX_AT)
+ return -ENOMEM;
+
+ matcher->size_of_at_array *= 2;
+ p = krealloc(matcher->at,
+ matcher->size_of_at_array * sizeof(*matcher->at),
+ __GFP_ZERO | GFP_KERNEL);
+ if (!p) {
+ matcher->size_of_at_array /= 2;
+ return -ENOMEM;
+ }
+
+ matcher->at = p;
+
+ return 0;
+}
+
int mlx5hws_matcher_attach_at(struct mlx5hws_matcher *matcher,
- struct mlx5hws_action_template *at)
+ struct mlx5hws_action_template *at,
+ bool *need_rehash)
{
bool is_jumbo = mlx5hws_matcher_mt_is_jumbo(matcher->mt);
struct mlx5hws_context *ctx = matcher->tbl->ctx;
u32 required_stes;
int ret;
- if (!matcher->attr.max_num_of_at_attach) {
- mlx5hws_dbg(ctx, "Num of current at (%d) exceed allowed value\n",
- matcher->num_of_at);
- return -EOPNOTSUPP;
+ *need_rehash = false;
+
+ if (unlikely(matcher->num_of_at >= matcher->size_of_at_array)) {
+ ret = hws_matcher_grow_at_array(matcher);
+ if (ret)
+ return ret;
+
+ if (matcher->col_matcher) {
+ ret = hws_matcher_grow_at_array(matcher->col_matcher);
+ if (ret)
+ return ret;
+ }
}
ret = hws_matcher_check_and_process_at(matcher, at);
@@ -927,12 +957,11 @@ int mlx5hws_matcher_attach_at(struct mlx5hws_matcher *matcher,
if (matcher->action_ste.max_stes < required_stes) {
mlx5hws_dbg(ctx, "Required STEs [%d] exceeds initial action template STE [%d]\n",
required_stes, matcher->action_ste.max_stes);
- return -ENOMEM;
+ *need_rehash = true;
}
matcher->at[matcher->num_of_at] = *at;
matcher->num_of_at += 1;
- matcher->attr.max_num_of_at_attach -= 1;
if (matcher->col_matcher)
matcher->col_matcher->num_of_at = matcher->num_of_at;
@@ -960,8 +989,9 @@ hws_matcher_set_templates(struct mlx5hws_matcher *matcher,
if (!matcher->mt)
return -ENOMEM;
- matcher->at = kvcalloc(num_of_at + matcher->attr.max_num_of_at_attach,
- sizeof(*matcher->at),
+ matcher->size_of_at_array =
+ num_of_at + matcher->attr.max_num_of_at_attach;
+ matcher->at = kvcalloc(matcher->size_of_at_array, sizeof(*matcher->at),
GFP_KERNEL);
if (!matcher->at) {
mlx5hws_err(ctx, "Failed to allocate action template array\n");
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/matcher.h b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/matcher.h
index 020de70270c5..20b32012c418 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/matcher.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/matcher.h
@@ -23,6 +23,9 @@
*/
#define MLX5HWS_MATCHER_ACTION_RTC_UPDATE_MULT 1
+/* Maximum number of action templates that can be attached to a matcher. */
+#define MLX5HWS_MATCHER_MAX_AT 128
+
enum mlx5hws_matcher_offset {
MLX5HWS_MATCHER_OFFSET_TAG_DW1 = 12,
MLX5HWS_MATCHER_OFFSET_TAG_DW0 = 13,
@@ -72,6 +75,7 @@ struct mlx5hws_matcher {
struct mlx5hws_match_template *mt;
struct mlx5hws_action_template *at;
u8 num_of_at;
+ u8 size_of_at_array;
u8 num_of_mt;
/* enum mlx5hws_matcher_flags */
u8 flags;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/mlx5hws.h b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/mlx5hws.h
index 5121951f2778..8ed8a715a2eb 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/mlx5hws.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/mlx5hws.h
@@ -399,11 +399,14 @@ int mlx5hws_matcher_destroy(struct mlx5hws_matcher *matcher);
*
* @matcher: Matcher to attach the action template to.
* @at: Action template to be attached to the matcher.
+ * @need_rehash: Output parameter that tells callers if the matcher needs to be
+ * rehashed.
*
* Return: Zero on success, non-zero otherwise.
*/
int mlx5hws_matcher_attach_at(struct mlx5hws_matcher *matcher,
- struct mlx5hws_action_template *at);
+ struct mlx5hws_action_template *at,
+ bool *need_rehash);
/**
* mlx5hws_matcher_resize_set_target - Link two matchers and enable moving rules.
--
2.31.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH net-next 01/12] net/mlx5: HWS, Fix matcher action template attach
2025-04-08 14:00 ` [PATCH net-next 01/12] net/mlx5: HWS, Fix matcher action template attach Tariq Toukan
@ 2025-04-09 15:21 ` Michal Kubiak
2025-04-09 15:56 ` Vlad Dogaru
0 siblings, 1 reply; 32+ messages in thread
From: Michal Kubiak @ 2025-04-09 15:21 UTC (permalink / raw)
To: Tariq Toukan
Cc: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet,
Andrew Lunn, Gal Pressman, Leon Romanovsky, Saeed Mahameed,
Leon Romanovsky, netdev, linux-rdma, linux-kernel, Moshe Shemesh,
Mark Bloch, Vlad Dogaru, Yevgeny Kliteynik
On Tue, Apr 08, 2025 at 05:00:45PM +0300, Tariq Toukan wrote:
> From: Vlad Dogaru <vdogaru@nvidia.com>
>
> The procedure of attaching an action template to an existing matcher had
> a few issues:
>
> 1. Attaching accidentally overran the `at` array in bwc_matcher, which
> would result in memory corruption. This bug wasn't triggered, but it
> is possible to trigger it by attaching action templates beyond the
> initial buffer size of 8. Fix this by converting to a dynamically
> sized buffer and reallocating if needed.
>
> 2. Similarly, the `at` array inside the native matcher was never
> reallocated. Fix this the same as above.
>
> 3. The bwc layer treated any error in action template attach as a signal
> that the matcher should be rehashed to account for a larger number of
> action STEs. In reality, there are other unrelated errors that can
> arise and they should be propagated upstack. Fix this by adding a
> `need_rehash` output parameter that's orthogonal to error codes.
>
> Fixes: 2111bb970c78 ("net/mlx5: HWS, added backward-compatible API handling")
> Signed-off-by: Vlad Dogaru <vdogaru@nvidia.com>
> Reviewed-by: Yevgeny Kliteynik <kliteyn@nvidia.com>
> Reviewed-by: Mark Bloch <mbloch@nvidia.com>
> Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
In general the patch looks OK to me.
Just one request for clarification inline.
Thanks,
Michal
> ---
> .../mellanox/mlx5/core/steering/hws/bwc.c | 55 ++++++++++++++++---
> .../mellanox/mlx5/core/steering/hws/bwc.h | 9 ++-
> .../mellanox/mlx5/core/steering/hws/matcher.c | 48 +++++++++++++---
> .../mellanox/mlx5/core/steering/hws/matcher.h | 4 ++
> .../mellanox/mlx5/core/steering/hws/mlx5hws.h | 5 +-
> 5 files changed, 97 insertions(+), 24 deletions(-)
>
[...]
> @@ -520,6 +529,23 @@ hws_bwc_matcher_extend_at(struct mlx5hws_bwc_matcher *bwc_matcher,
> struct mlx5hws_rule_action rule_actions[])
> {
> enum mlx5hws_action_type action_types[MLX5HWS_BWC_MAX_ACTS];
> + void *p;
> +
> + if (unlikely(bwc_matcher->num_of_at >= bwc_matcher->size_of_at_array)) {
> + if (bwc_matcher->size_of_at_array >= MLX5HWS_MATCHER_MAX_AT)
> + return -ENOMEM;
> + bwc_matcher->size_of_at_array *= 2;
Is it possible that `num_of_at` is even greater than twice `size_of_array`?
If so, shouldn't you calculate how many multiplications by 2 you need to
do?
> + p = krealloc(bwc_matcher->at,
> + bwc_matcher->size_of_at_array *
> + sizeof(*bwc_matcher->at),
> + __GFP_ZERO | GFP_KERNEL);
> + if (!p) {
> + bwc_matcher->size_of_at_array /= 2;
> + return -ENOMEM;
> + }
> +
> + bwc_matcher->at = p;
> + }
>
> hws_bwc_rule_actions_to_action_types(rule_actions, action_types);
>
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH net-next 01/12] net/mlx5: HWS, Fix matcher action template attach
2025-04-09 15:21 ` Michal Kubiak
@ 2025-04-09 15:56 ` Vlad Dogaru
2025-04-09 16:00 ` Michal Kubiak
0 siblings, 1 reply; 32+ messages in thread
From: Vlad Dogaru @ 2025-04-09 15:56 UTC (permalink / raw)
To: Michal Kubiak, Tariq Toukan
Cc: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet,
Andrew Lunn, Gal Pressman, Leon Romanovsky, Saeed Mahameed,
Leon Romanovsky, netdev, linux-rdma, linux-kernel, Moshe Shemesh,
Mark Bloch, Yevgeny Kliteynik
On 4/9/25 17:21, Michal Kubiak wrote:
> On Tue, Apr 08, 2025 at 05:00:45PM +0300, Tariq Toukan wrote:
>> From: Vlad Dogaru <vdogaru@nvidia.com>
>>
>> The procedure of attaching an action template to an existing matcher had
>> a few issues:
>>
>> 1. Attaching accidentally overran the `at` array in bwc_matcher, which
>> would result in memory corruption. This bug wasn't triggered, but it
>> is possible to trigger it by attaching action templates beyond the
>> initial buffer size of 8. Fix this by converting to a dynamically
>> sized buffer and reallocating if needed.
>>
>> 2. Similarly, the `at` array inside the native matcher was never
>> reallocated. Fix this the same as above.
>>
>> 3. The bwc layer treated any error in action template attach as a signal
>> that the matcher should be rehashed to account for a larger number of
>> action STEs. In reality, there are other unrelated errors that can
>> arise and they should be propagated upstack. Fix this by adding a
>> `need_rehash` output parameter that's orthogonal to error codes.
>>
>> Fixes: 2111bb970c78 ("net/mlx5: HWS, added backward-compatible API handling")
>> Signed-off-by: Vlad Dogaru <vdogaru@nvidia.com>
>> Reviewed-by: Yevgeny Kliteynik <kliteyn@nvidia.com>
>> Reviewed-by: Mark Bloch <mbloch@nvidia.com>
>> Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
>
> In general the patch looks OK to me.
> Just one request for clarification inline.
Thank you for reviewing.
>> ---
>> .../mellanox/mlx5/core/steering/hws/bwc.c | 55 ++++++++++++++++---
>> .../mellanox/mlx5/core/steering/hws/bwc.h | 9 ++-
>> .../mellanox/mlx5/core/steering/hws/matcher.c | 48 +++++++++++++---
>> .../mellanox/mlx5/core/steering/hws/matcher.h | 4 ++
>> .../mellanox/mlx5/core/steering/hws/mlx5hws.h | 5 +-
>> 5 files changed, 97 insertions(+), 24 deletions(-)
>>
>
> [...]
>
>> @@ -520,6 +529,23 @@ hws_bwc_matcher_extend_at(struct mlx5hws_bwc_matcher *bwc_matcher,
>> struct mlx5hws_rule_action rule_actions[])
>> {
>> enum mlx5hws_action_type action_types[MLX5HWS_BWC_MAX_ACTS];
>> + void *p;
>> +
>> + if (unlikely(bwc_matcher->num_of_at >= bwc_matcher->size_of_at_array)) {
>> + if (bwc_matcher->size_of_at_array >= MLX5HWS_MATCHER_MAX_AT)
>> + return -ENOMEM;
>> + bwc_matcher->size_of_at_array *= 2;
>
> Is it possible that `num_of_at` is even greater than twice `size_of_array`?
> If so, shouldn't you calculate how many multiplications by 2 you need to
> do?
We only extend the array by one template at a time, immediately after
this check, so this can't happen.
Cheers,
Vlad
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH net-next 01/12] net/mlx5: HWS, Fix matcher action template attach
2025-04-09 15:56 ` Vlad Dogaru
@ 2025-04-09 16:00 ` Michal Kubiak
0 siblings, 0 replies; 32+ messages in thread
From: Michal Kubiak @ 2025-04-09 16:00 UTC (permalink / raw)
To: Vlad Dogaru
Cc: Tariq Toukan, David S. Miller, Jakub Kicinski, Paolo Abeni,
Eric Dumazet, Andrew Lunn, Gal Pressman, Leon Romanovsky,
Saeed Mahameed, Leon Romanovsky, netdev, linux-rdma, linux-kernel,
Moshe Shemesh, Mark Bloch, Yevgeny Kliteynik
On Wed, Apr 09, 2025 at 05:56:19PM +0200, Vlad Dogaru wrote:
> On 4/9/25 17:21, Michal Kubiak wrote:
> > On Tue, Apr 08, 2025 at 05:00:45PM +0300, Tariq Toukan wrote:
> > > From: Vlad Dogaru <vdogaru@nvidia.com>
> > >
> > > The procedure of attaching an action template to an existing matcher had
> > > a few issues:
> > >
> > > 1. Attaching accidentally overran the `at` array in bwc_matcher, which
> > > would result in memory corruption. This bug wasn't triggered, but it
> > > is possible to trigger it by attaching action templates beyond the
> > > initial buffer size of 8. Fix this by converting to a dynamically
> > > sized buffer and reallocating if needed.
> > >
> > > 2. Similarly, the `at` array inside the native matcher was never
> > > reallocated. Fix this the same as above.
> > >
> > > 3. The bwc layer treated any error in action template attach as a signal
> > > that the matcher should be rehashed to account for a larger number of
> > > action STEs. In reality, there are other unrelated errors that can
> > > arise and they should be propagated upstack. Fix this by adding a
> > > `need_rehash` output parameter that's orthogonal to error codes.
> > >
> > > Fixes: 2111bb970c78 ("net/mlx5: HWS, added backward-compatible API handling")
> > > Signed-off-by: Vlad Dogaru <vdogaru@nvidia.com>
> > > Reviewed-by: Yevgeny Kliteynik <kliteyn@nvidia.com>
> > > Reviewed-by: Mark Bloch <mbloch@nvidia.com>
> > > Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
> >
> > In general the patch looks OK to me.
> > Just one request for clarification inline.
>
> Thank you for reviewing.
>
> > > ---
> > > .../mellanox/mlx5/core/steering/hws/bwc.c | 55 ++++++++++++++++---
> > > .../mellanox/mlx5/core/steering/hws/bwc.h | 9 ++-
> > > .../mellanox/mlx5/core/steering/hws/matcher.c | 48 +++++++++++++---
> > > .../mellanox/mlx5/core/steering/hws/matcher.h | 4 ++
> > > .../mellanox/mlx5/core/steering/hws/mlx5hws.h | 5 +-
> > > 5 files changed, 97 insertions(+), 24 deletions(-)
> > >
> >
> > [...]
> >
> > > @@ -520,6 +529,23 @@ hws_bwc_matcher_extend_at(struct mlx5hws_bwc_matcher *bwc_matcher,
> > > struct mlx5hws_rule_action rule_actions[])
> > > {
> > > enum mlx5hws_action_type action_types[MLX5HWS_BWC_MAX_ACTS];
> > > + void *p;
> > > +
> > > + if (unlikely(bwc_matcher->num_of_at >= bwc_matcher->size_of_at_array)) {
> > > + if (bwc_matcher->size_of_at_array >= MLX5HWS_MATCHER_MAX_AT)
> > > + return -ENOMEM;
> > > + bwc_matcher->size_of_at_array *= 2;
> >
> > Is it possible that `num_of_at` is even greater than twice `size_of_array`?
> > If so, shouldn't you calculate how many multiplications by 2 you need to
> > do?
>
> We only extend the array by one template at a time, immediately after this
> check, so this can't happen.
>
> Cheers,
> Vlad
Thank you for the clarification! Just double checking :-).
Cheers,
Reviewed-by: Michal Kubiak <michal.kubiak@intel.com>
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH net-next 02/12] net/mlx5: HWS, Remove unused element array
2025-04-08 14:00 [PATCH net-next 00/12] net/mlx5: HWS, Refactor action STE handling Tariq Toukan
2025-04-08 14:00 ` [PATCH net-next 01/12] net/mlx5: HWS, Fix matcher action template attach Tariq Toukan
@ 2025-04-08 14:00 ` Tariq Toukan
2025-04-09 15:50 ` Michal Kubiak
2025-04-08 14:00 ` [PATCH net-next 03/12] net/mlx5: HWS, Make pool single resource Tariq Toukan
` (9 subsequent siblings)
11 siblings, 1 reply; 32+ messages in thread
From: Tariq Toukan @ 2025-04-08 14:00 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet,
Andrew Lunn
Cc: Gal Pressman, Leon Romanovsky, Saeed Mahameed, Leon Romanovsky,
Tariq Toukan, netdev, linux-rdma, linux-kernel, Moshe Shemesh,
Mark Bloch, Vlad Dogaru, Yevgeny Kliteynik
From: Vlad Dogaru <vdogaru@nvidia.com>
Remove the array of elements wrapped in a struct because in reality only
the first element was ever used.
Signed-off-by: Vlad Dogaru <vdogaru@nvidia.com>
Reviewed-by: Yevgeny Kliteynik <kliteyn@nvidia.com>
Reviewed-by: Mark Bloch <mbloch@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
.../mellanox/mlx5/core/steering/hws/pool.c | 55 ++++++++-----------
.../mellanox/mlx5/core/steering/hws/pool.h | 6 +-
2 files changed, 23 insertions(+), 38 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pool.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pool.c
index 50a81d360bb2..35ed9bee06a6 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pool.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pool.c
@@ -293,7 +293,7 @@ static int hws_pool_create_resource_on_index(struct mlx5hws_pool *pool,
}
static struct mlx5hws_pool_elements *
-hws_pool_element_create_new_elem(struct mlx5hws_pool *pool, u32 order, int idx)
+hws_pool_element_create_new_elem(struct mlx5hws_pool *pool, u32 order)
{
struct mlx5hws_pool_elements *elem;
u32 alloc_size;
@@ -311,21 +311,21 @@ hws_pool_element_create_new_elem(struct mlx5hws_pool *pool, u32 order, int idx)
elem->bitmap = hws_pool_create_and_init_bitmap(alloc_size - order);
if (!elem->bitmap) {
mlx5hws_err(pool->ctx,
- "Failed to create bitmap type: %d: size %d index: %d\n",
- pool->type, alloc_size, idx);
+ "Failed to create bitmap type: %d: size %d\n",
+ pool->type, alloc_size);
goto free_elem;
}
elem->log_size = alloc_size - order;
}
- if (hws_pool_create_resource_on_index(pool, alloc_size, idx)) {
- mlx5hws_err(pool->ctx, "Failed to create resource type: %d: size %d index: %d\n",
- pool->type, alloc_size, idx);
+ if (hws_pool_create_resource_on_index(pool, alloc_size, 0)) {
+ mlx5hws_err(pool->ctx, "Failed to create resource type: %d: size %d\n",
+ pool->type, alloc_size);
goto free_db;
}
- pool->db.element_manager->elements[idx] = elem;
+ pool->db.element = elem;
return elem;
@@ -359,9 +359,9 @@ hws_pool_onesize_element_get_mem_chunk(struct mlx5hws_pool *pool, u32 order,
{
struct mlx5hws_pool_elements *elem;
- elem = pool->db.element_manager->elements[0];
+ elem = pool->db.element;
if (!elem)
- elem = hws_pool_element_create_new_elem(pool, order, 0);
+ elem = hws_pool_element_create_new_elem(pool, order);
if (!elem)
goto err_no_elem;
@@ -451,16 +451,14 @@ static int hws_pool_general_element_db_init(struct mlx5hws_pool *pool)
return 0;
}
-static void hws_onesize_element_db_destroy_element(struct mlx5hws_pool *pool,
- struct mlx5hws_pool_elements *elem,
- struct mlx5hws_pool_chunk *chunk)
+static void
+hws_onesize_element_db_destroy_element(struct mlx5hws_pool *pool,
+ struct mlx5hws_pool_elements *elem)
{
- if (unlikely(!pool->resource[chunk->resource_idx]))
- pr_warn("HWS: invalid resource with index %d\n", chunk->resource_idx);
-
- hws_pool_resource_free(pool, chunk->resource_idx);
+ hws_pool_resource_free(pool, 0);
+ bitmap_free(elem->bitmap);
kfree(elem);
- pool->db.element_manager->elements[chunk->resource_idx] = NULL;
+ pool->db.element = NULL;
}
static void hws_onesize_element_db_put_chunk(struct mlx5hws_pool *pool,
@@ -471,7 +469,7 @@ static void hws_onesize_element_db_put_chunk(struct mlx5hws_pool *pool,
if (unlikely(chunk->resource_idx))
pr_warn("HWS: invalid resource with index %d\n", chunk->resource_idx);
- elem = pool->db.element_manager->elements[chunk->resource_idx];
+ elem = pool->db.element;
if (!elem) {
mlx5hws_err(pool->ctx, "No such element (%d)\n", chunk->resource_idx);
return;
@@ -483,7 +481,7 @@ static void hws_onesize_element_db_put_chunk(struct mlx5hws_pool *pool,
if (pool->flags & MLX5HWS_POOL_FLAGS_RELEASE_FREE_RESOURCE &&
!elem->num_of_elements)
- hws_onesize_element_db_destroy_element(pool, elem, chunk);
+ hws_onesize_element_db_destroy_element(pool, elem);
}
static int hws_onesize_element_db_get_chunk(struct mlx5hws_pool *pool,
@@ -504,18 +502,13 @@ static int hws_onesize_element_db_get_chunk(struct mlx5hws_pool *pool,
static void hws_onesize_element_db_uninit(struct mlx5hws_pool *pool)
{
- struct mlx5hws_pool_elements *elem;
- int i;
+ struct mlx5hws_pool_elements *elem = pool->db.element;
- for (i = 0; i < MLX5HWS_POOL_RESOURCE_ARR_SZ; i++) {
- elem = pool->db.element_manager->elements[i];
- if (elem) {
- bitmap_free(elem->bitmap);
- kfree(elem);
- pool->db.element_manager->elements[i] = NULL;
- }
+ if (elem) {
+ bitmap_free(elem->bitmap);
+ kfree(elem);
+ pool->db.element = NULL;
}
- kfree(pool->db.element_manager);
}
/* This memory management works as the following:
@@ -526,10 +519,6 @@ static void hws_onesize_element_db_uninit(struct mlx5hws_pool *pool)
*/
static int hws_pool_onesize_element_db_init(struct mlx5hws_pool *pool)
{
- pool->db.element_manager = kzalloc(sizeof(*pool->db.element_manager), GFP_KERNEL);
- if (!pool->db.element_manager)
- return -ENOMEM;
-
pool->p_db_uninit = &hws_onesize_element_db_uninit;
pool->p_get_chunk = &hws_onesize_element_db_get_chunk;
pool->p_put_chunk = &hws_onesize_element_db_put_chunk;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pool.h b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pool.h
index 621298b352b2..f4258f83fdbf 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pool.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pool.h
@@ -87,14 +87,10 @@ struct mlx5hws_pool_elements {
bool is_full;
};
-struct mlx5hws_element_manager {
- struct mlx5hws_pool_elements *elements[MLX5HWS_POOL_RESOURCE_ARR_SZ];
-};
-
struct mlx5hws_pool_db {
enum mlx5hws_db_type type;
union {
- struct mlx5hws_element_manager *element_manager;
+ struct mlx5hws_pool_elements *element;
struct mlx5hws_buddy_manager *buddy_manager;
};
};
--
2.31.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH net-next 02/12] net/mlx5: HWS, Remove unused element array
2025-04-08 14:00 ` [PATCH net-next 02/12] net/mlx5: HWS, Remove unused element array Tariq Toukan
@ 2025-04-09 15:50 ` Michal Kubiak
0 siblings, 0 replies; 32+ messages in thread
From: Michal Kubiak @ 2025-04-09 15:50 UTC (permalink / raw)
To: Tariq Toukan
Cc: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet,
Andrew Lunn, Gal Pressman, Leon Romanovsky, Saeed Mahameed,
Leon Romanovsky, netdev, linux-rdma, linux-kernel, Moshe Shemesh,
Mark Bloch, Vlad Dogaru, Yevgeny Kliteynik
On Tue, Apr 08, 2025 at 05:00:46PM +0300, Tariq Toukan wrote:
> From: Vlad Dogaru <vdogaru@nvidia.com>
>
> Remove the array of elements wrapped in a struct because in reality only
> the first element was ever used.
>
> Signed-off-by: Vlad Dogaru <vdogaru@nvidia.com>
> Reviewed-by: Yevgeny Kliteynik <kliteyn@nvidia.com>
> Reviewed-by: Mark Bloch <mbloch@nvidia.com>
> Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
> ---
> .../mellanox/mlx5/core/steering/hws/pool.c | 55 ++++++++-----------
> .../mellanox/mlx5/core/steering/hws/pool.h | 6 +-
> 2 files changed, 23 insertions(+), 38 deletions(-)
>
Reviewed-by: Michal Kubiak <michal.kubiak@intel.com>
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH net-next 03/12] net/mlx5: HWS, Make pool single resource
2025-04-08 14:00 [PATCH net-next 00/12] net/mlx5: HWS, Refactor action STE handling Tariq Toukan
2025-04-08 14:00 ` [PATCH net-next 01/12] net/mlx5: HWS, Fix matcher action template attach Tariq Toukan
2025-04-08 14:00 ` [PATCH net-next 02/12] net/mlx5: HWS, Remove unused element array Tariq Toukan
@ 2025-04-08 14:00 ` Tariq Toukan
2025-04-09 20:04 ` Michal Kubiak
2025-04-08 14:00 ` [PATCH net-next 04/12] net/mlx5: HWS, Refactor pool implementation Tariq Toukan
` (8 subsequent siblings)
11 siblings, 1 reply; 32+ messages in thread
From: Tariq Toukan @ 2025-04-08 14:00 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet,
Andrew Lunn
Cc: Gal Pressman, Leon Romanovsky, Saeed Mahameed, Leon Romanovsky,
Tariq Toukan, netdev, linux-rdma, linux-kernel, Moshe Shemesh,
Mark Bloch, Vlad Dogaru, Yevgeny Kliteynik
From: Vlad Dogaru <vdogaru@nvidia.com>
The pool implementation claimed to support multiple resources, but this
does not really make sense in context. Callers always allocate a single
STC or STE chunk of exactly the size provided.
The code that handled multiple resources was unused (and likely buggy)
due to the combination of flags passed by callers.
Simplify the pool by having it handle a single resource. As a result of
this simplification, chunks no longer contain a resource offset (there
is now only one resource per pool), and the get_base_id functions no
longer take a chunk parameter.
Signed-off-by: Vlad Dogaru <vdogaru@nvidia.com>
Reviewed-by: Yevgeny Kliteynik <kliteyn@nvidia.com>
Reviewed-by: Mark Bloch <mbloch@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
.../mellanox/mlx5/core/steering/hws/action.c | 27 ++-
.../mellanox/mlx5/core/steering/hws/debug.c | 22 +--
.../mellanox/mlx5/core/steering/hws/matcher.c | 10 +-
.../mellanox/mlx5/core/steering/hws/pool.c | 182 ++++++------------
.../mellanox/mlx5/core/steering/hws/pool.h | 28 +--
5 files changed, 91 insertions(+), 178 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/action.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/action.c
index b5332c54d4fb..781ba8c4f733 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/action.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/action.c
@@ -238,6 +238,7 @@ hws_action_fixup_stc_attr(struct mlx5hws_context *ctx,
enum mlx5hws_table_type table_type,
bool is_mirror)
{
+ struct mlx5hws_pool *pool;
bool use_fixup = false;
u32 fw_tbl_type;
u32 base_id;
@@ -253,13 +254,11 @@ hws_action_fixup_stc_attr(struct mlx5hws_context *ctx,
use_fixup = true;
break;
}
+ pool = stc_attr->ste_table.ste_pool;
if (!is_mirror)
- base_id = mlx5hws_pool_chunk_get_base_id(stc_attr->ste_table.ste_pool,
- &stc_attr->ste_table.ste);
+ base_id = mlx5hws_pool_get_base_id(pool);
else
- base_id =
- mlx5hws_pool_chunk_get_base_mirror_id(stc_attr->ste_table.ste_pool,
- &stc_attr->ste_table.ste);
+ base_id = mlx5hws_pool_get_base_mirror_id(pool);
*fixup_stc_attr = *stc_attr;
fixup_stc_attr->ste_table.ste_obj_id = base_id;
@@ -337,7 +336,7 @@ __must_hold(&ctx->ctrl_lock)
if (!mlx5hws_context_cap_dynamic_reparse(ctx))
stc_attr->reparse_mode = MLX5_IFC_STC_REPARSE_IGNORE;
- obj_0_id = mlx5hws_pool_chunk_get_base_id(stc_pool, stc);
+ obj_0_id = mlx5hws_pool_get_base_id(stc_pool);
/* According to table/action limitation change the stc_attr */
use_fixup = hws_action_fixup_stc_attr(ctx, stc_attr, &fixup_stc_attr, table_type, false);
@@ -353,7 +352,7 @@ __must_hold(&ctx->ctrl_lock)
if (table_type == MLX5HWS_TABLE_TYPE_FDB) {
u32 obj_1_id;
- obj_1_id = mlx5hws_pool_chunk_get_base_mirror_id(stc_pool, stc);
+ obj_1_id = mlx5hws_pool_get_base_mirror_id(stc_pool);
use_fixup = hws_action_fixup_stc_attr(ctx, stc_attr,
&fixup_stc_attr,
@@ -393,11 +392,11 @@ __must_hold(&ctx->ctrl_lock)
stc_attr.action_type = MLX5_IFC_STC_ACTION_TYPE_DROP;
stc_attr.action_offset = MLX5HWS_ACTION_OFFSET_HIT;
stc_attr.stc_offset = stc->offset;
- obj_id = mlx5hws_pool_chunk_get_base_id(stc_pool, stc);
+ obj_id = mlx5hws_pool_get_base_id(stc_pool);
mlx5hws_cmd_stc_modify(ctx->mdev, obj_id, &stc_attr);
if (table_type == MLX5HWS_TABLE_TYPE_FDB) {
- obj_id = mlx5hws_pool_chunk_get_base_mirror_id(stc_pool, stc);
+ obj_id = mlx5hws_pool_get_base_mirror_id(stc_pool);
mlx5hws_cmd_stc_modify(ctx->mdev, obj_id, &stc_attr);
}
@@ -1581,7 +1580,6 @@ hws_action_create_dest_match_range_table(struct mlx5hws_context *ctx,
u32 miss_ft_id)
{
struct mlx5hws_cmd_rtc_create_attr rtc_attr = {0};
- struct mlx5hws_action_default_stc *default_stc;
struct mlx5hws_matcher_action_ste *table_ste;
struct mlx5hws_pool_attr pool_attr = {0};
struct mlx5hws_pool *ste_pool, *stc_pool;
@@ -1629,7 +1627,7 @@ hws_action_create_dest_match_range_table(struct mlx5hws_context *ctx,
rtc_attr.fw_gen_wqe = true;
rtc_attr.is_scnd_range = true;
- obj_id = mlx5hws_pool_chunk_get_base_id(ste_pool, ste);
+ obj_id = mlx5hws_pool_get_base_id(ste_pool);
rtc_attr.pd = ctx->pd_num;
rtc_attr.ste_base = obj_id;
@@ -1639,8 +1637,7 @@ hws_action_create_dest_match_range_table(struct mlx5hws_context *ctx,
/* STC is a single resource (obj_id), use any STC for the ID */
stc_pool = ctx->stc_pool;
- default_stc = ctx->common_res.default_stc;
- obj_id = mlx5hws_pool_chunk_get_base_id(stc_pool, &default_stc->default_hit);
+ obj_id = mlx5hws_pool_get_base_id(stc_pool);
rtc_attr.stc_base = obj_id;
ret = mlx5hws_cmd_rtc_create(ctx->mdev, &rtc_attr, rtc_0_id);
@@ -1650,11 +1647,11 @@ hws_action_create_dest_match_range_table(struct mlx5hws_context *ctx,
}
/* Create mirror RTC */
- obj_id = mlx5hws_pool_chunk_get_base_mirror_id(ste_pool, ste);
+ obj_id = mlx5hws_pool_get_base_mirror_id(ste_pool);
rtc_attr.ste_base = obj_id;
rtc_attr.table_type = mlx5hws_table_get_res_fw_ft_type(MLX5HWS_TABLE_TYPE_FDB, true);
- obj_id = mlx5hws_pool_chunk_get_base_mirror_id(stc_pool, &default_stc->default_hit);
+ obj_id = mlx5hws_pool_get_base_mirror_id(stc_pool);
rtc_attr.stc_base = obj_id;
ret = mlx5hws_cmd_rtc_create(ctx->mdev, &rtc_attr, rtc_1_id);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/debug.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/debug.c
index 696275fd0ce2..3491408c5d84 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/debug.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/debug.c
@@ -118,7 +118,6 @@ static int hws_debug_dump_matcher(struct seq_file *f, struct mlx5hws_matcher *ma
{
enum mlx5hws_table_type tbl_type = matcher->tbl->type;
struct mlx5hws_cmd_ft_query_attr ft_attr = {0};
- struct mlx5hws_pool_chunk *ste;
struct mlx5hws_pool *ste_pool;
u64 icm_addr_0 = 0;
u64 icm_addr_1 = 0;
@@ -134,12 +133,11 @@ static int hws_debug_dump_matcher(struct seq_file *f, struct mlx5hws_matcher *ma
matcher->end_ft_id,
matcher->col_matcher ? HWS_PTR_TO_ID(matcher->col_matcher) : 0);
- ste = &matcher->match_ste.ste;
ste_pool = matcher->match_ste.pool;
if (ste_pool) {
- ste_0_id = mlx5hws_pool_chunk_get_base_id(ste_pool, ste);
+ ste_0_id = mlx5hws_pool_get_base_id(ste_pool);
if (tbl_type == MLX5HWS_TABLE_TYPE_FDB)
- ste_1_id = mlx5hws_pool_chunk_get_base_mirror_id(ste_pool, ste);
+ ste_1_id = mlx5hws_pool_get_base_mirror_id(ste_pool);
}
seq_printf(f, ",%d,%d,%d,%d",
@@ -148,12 +146,11 @@ static int hws_debug_dump_matcher(struct seq_file *f, struct mlx5hws_matcher *ma
matcher->match_ste.rtc_1_id,
(int)ste_1_id);
- ste = &matcher->action_ste.ste;
ste_pool = matcher->action_ste.pool;
if (ste_pool) {
- ste_0_id = mlx5hws_pool_chunk_get_base_id(ste_pool, ste);
+ ste_0_id = mlx5hws_pool_get_base_id(ste_pool);
if (tbl_type == MLX5HWS_TABLE_TYPE_FDB)
- ste_1_id = mlx5hws_pool_chunk_get_base_mirror_id(ste_pool, ste);
+ ste_1_id = mlx5hws_pool_get_base_mirror_id(ste_pool);
else
ste_1_id = -1;
} else {
@@ -387,14 +384,17 @@ static int hws_debug_dump_context_stc(struct seq_file *f, struct mlx5hws_context
if (!stc_pool)
return 0;
- if (stc_pool->resource[0]) {
- ret = hws_debug_dump_context_stc_resource(f, ctx, stc_pool->resource[0]);
+ if (stc_pool->resource) {
+ ret = hws_debug_dump_context_stc_resource(f, ctx,
+ stc_pool->resource);
if (ret)
return ret;
}
- if (stc_pool->mirror_resource[0]) {
- ret = hws_debug_dump_context_stc_resource(f, ctx, stc_pool->mirror_resource[0]);
+ if (stc_pool->mirror_resource) {
+ struct mlx5hws_pool_resource *res = stc_pool->mirror_resource;
+
+ ret = hws_debug_dump_context_stc_resource(f, ctx, res);
if (ret)
return ret;
}
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/matcher.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/matcher.c
index 37a4497048a6..59b14db427b4 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/matcher.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/matcher.c
@@ -223,7 +223,6 @@ static int hws_matcher_create_rtc(struct mlx5hws_matcher *matcher,
struct mlx5hws_cmd_rtc_create_attr rtc_attr = {0};
struct mlx5hws_match_template *mt = matcher->mt;
struct mlx5hws_context *ctx = matcher->tbl->ctx;
- struct mlx5hws_action_default_stc *default_stc;
struct mlx5hws_matcher_action_ste *action_ste;
struct mlx5hws_table *tbl = matcher->tbl;
struct mlx5hws_pool *ste_pool, *stc_pool;
@@ -305,7 +304,7 @@ static int hws_matcher_create_rtc(struct mlx5hws_matcher *matcher,
return -EINVAL;
}
- obj_id = mlx5hws_pool_chunk_get_base_id(ste_pool, ste);
+ obj_id = mlx5hws_pool_get_base_id(ste_pool);
rtc_attr.pd = ctx->pd_num;
rtc_attr.ste_base = obj_id;
@@ -316,8 +315,7 @@ static int hws_matcher_create_rtc(struct mlx5hws_matcher *matcher,
/* STC is a single resource (obj_id), use any STC for the ID */
stc_pool = ctx->stc_pool;
- default_stc = ctx->common_res.default_stc;
- obj_id = mlx5hws_pool_chunk_get_base_id(stc_pool, &default_stc->default_hit);
+ obj_id = mlx5hws_pool_get_base_id(stc_pool);
rtc_attr.stc_base = obj_id;
ret = mlx5hws_cmd_rtc_create(ctx->mdev, &rtc_attr, rtc_0_id);
@@ -328,11 +326,11 @@ static int hws_matcher_create_rtc(struct mlx5hws_matcher *matcher,
}
if (tbl->type == MLX5HWS_TABLE_TYPE_FDB) {
- obj_id = mlx5hws_pool_chunk_get_base_mirror_id(ste_pool, ste);
+ obj_id = mlx5hws_pool_get_base_mirror_id(ste_pool);
rtc_attr.ste_base = obj_id;
rtc_attr.table_type = mlx5hws_table_get_res_fw_ft_type(tbl->type, true);
- obj_id = mlx5hws_pool_chunk_get_base_mirror_id(stc_pool, &default_stc->default_hit);
+ obj_id = mlx5hws_pool_get_base_mirror_id(stc_pool);
rtc_attr.stc_base = obj_id;
hws_matcher_set_rtc_attr_sz(matcher, &rtc_attr, rtc_type, true);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pool.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pool.c
index 35ed9bee06a6..0de03e17624c 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pool.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pool.c
@@ -20,15 +20,14 @@ static void hws_pool_free_one_resource(struct mlx5hws_pool_resource *resource)
kfree(resource);
}
-static void hws_pool_resource_free(struct mlx5hws_pool *pool,
- int resource_idx)
+static void hws_pool_resource_free(struct mlx5hws_pool *pool)
{
- hws_pool_free_one_resource(pool->resource[resource_idx]);
- pool->resource[resource_idx] = NULL;
+ hws_pool_free_one_resource(pool->resource);
+ pool->resource = NULL;
if (pool->tbl_type == MLX5HWS_TABLE_TYPE_FDB) {
- hws_pool_free_one_resource(pool->mirror_resource[resource_idx]);
- pool->mirror_resource[resource_idx] = NULL;
+ hws_pool_free_one_resource(pool->mirror_resource);
+ pool->mirror_resource = NULL;
}
}
@@ -78,7 +77,7 @@ hws_pool_create_one_resource(struct mlx5hws_pool *pool, u32 log_range,
}
static int
-hws_pool_resource_alloc(struct mlx5hws_pool *pool, u32 log_range, int idx)
+hws_pool_resource_alloc(struct mlx5hws_pool *pool, u32 log_range)
{
struct mlx5hws_pool_resource *resource;
u32 fw_ft_type, opt_log_range;
@@ -91,7 +90,7 @@ hws_pool_resource_alloc(struct mlx5hws_pool *pool, u32 log_range, int idx)
return -EINVAL;
}
- pool->resource[idx] = resource;
+ pool->resource = resource;
if (pool->tbl_type == MLX5HWS_TABLE_TYPE_FDB) {
struct mlx5hws_pool_resource *mirror_resource;
@@ -102,10 +101,10 @@ hws_pool_resource_alloc(struct mlx5hws_pool *pool, u32 log_range, int idx)
if (!mirror_resource) {
mlx5hws_err(pool->ctx, "Failed allocating mirrored resource\n");
hws_pool_free_one_resource(resource);
- pool->resource[idx] = NULL;
+ pool->resource = NULL;
return -EINVAL;
}
- pool->mirror_resource[idx] = mirror_resource;
+ pool->mirror_resource = mirror_resource;
}
return 0;
@@ -129,9 +128,9 @@ static void hws_pool_buddy_db_put_chunk(struct mlx5hws_pool *pool,
{
struct mlx5hws_buddy_mem *buddy;
- buddy = pool->db.buddy_manager->buddies[chunk->resource_idx];
+ buddy = pool->db.buddy;
if (!buddy) {
- mlx5hws_err(pool->ctx, "No such buddy (%d)\n", chunk->resource_idx);
+ mlx5hws_err(pool->ctx, "Bad buddy state\n");
return;
}
@@ -139,86 +138,50 @@ static void hws_pool_buddy_db_put_chunk(struct mlx5hws_pool *pool,
}
static struct mlx5hws_buddy_mem *
-hws_pool_buddy_get_next_buddy(struct mlx5hws_pool *pool, int idx,
- u32 order, bool *is_new_buddy)
+hws_pool_buddy_get_buddy(struct mlx5hws_pool *pool, u32 order)
{
static struct mlx5hws_buddy_mem *buddy;
u32 new_buddy_size;
- buddy = pool->db.buddy_manager->buddies[idx];
+ buddy = pool->db.buddy;
if (buddy)
return buddy;
new_buddy_size = max(pool->alloc_log_sz, order);
- *is_new_buddy = true;
buddy = mlx5hws_buddy_create(new_buddy_size);
if (!buddy) {
- mlx5hws_err(pool->ctx, "Failed to create buddy order: %d index: %d\n",
- new_buddy_size, idx);
+ mlx5hws_err(pool->ctx, "Failed to create buddy order: %d\n",
+ new_buddy_size);
return NULL;
}
- if (hws_pool_resource_alloc(pool, new_buddy_size, idx) != 0) {
- mlx5hws_err(pool->ctx, "Failed to create resource type: %d: size %d index: %d\n",
- pool->type, new_buddy_size, idx);
+ if (hws_pool_resource_alloc(pool, new_buddy_size) != 0) {
+ mlx5hws_err(pool->ctx, "Failed to create resource type: %d: size %d\n",
+ pool->type, new_buddy_size);
mlx5hws_buddy_cleanup(buddy);
return NULL;
}
- pool->db.buddy_manager->buddies[idx] = buddy;
+ pool->db.buddy = buddy;
return buddy;
}
static int hws_pool_buddy_get_mem_chunk(struct mlx5hws_pool *pool,
int order,
- u32 *buddy_idx,
int *seg)
{
struct mlx5hws_buddy_mem *buddy;
- bool new_mem = false;
- int ret = 0;
- int i;
-
- *seg = -1;
-
- /* Find the next free place from the buddy array */
- while (*seg < 0) {
- for (i = 0; i < MLX5HWS_POOL_RESOURCE_ARR_SZ; i++) {
- buddy = hws_pool_buddy_get_next_buddy(pool, i,
- order,
- &new_mem);
- if (!buddy) {
- ret = -ENOMEM;
- goto out;
- }
-
- *seg = mlx5hws_buddy_alloc_mem(buddy, order);
- if (*seg >= 0)
- goto found;
-
- if (pool->flags & MLX5HWS_POOL_FLAGS_ONE_RESOURCE) {
- mlx5hws_err(pool->ctx,
- "Fail to allocate seg for one resource pool\n");
- ret = -ENOMEM;
- goto out;
- }
-
- if (new_mem) {
- /* We have new memory pool, should be place for us */
- mlx5hws_err(pool->ctx,
- "No memory for order: %d with buddy no: %d\n",
- order, i);
- ret = -ENOMEM;
- goto out;
- }
- }
- }
-found:
- *buddy_idx = i;
-out:
- return ret;
+ buddy = hws_pool_buddy_get_buddy(pool, order);
+ if (!buddy)
+ return -ENOMEM;
+
+ *seg = mlx5hws_buddy_alloc_mem(buddy, order);
+ if (*seg >= 0)
+ return 0;
+
+ return -ENOMEM;
}
static int hws_pool_buddy_db_get_chunk(struct mlx5hws_pool *pool,
@@ -226,9 +189,7 @@ static int hws_pool_buddy_db_get_chunk(struct mlx5hws_pool *pool,
{
int ret = 0;
- /* Go over the buddies and find next free slot */
ret = hws_pool_buddy_get_mem_chunk(pool, chunk->order,
- &chunk->resource_idx,
&chunk->offset);
if (ret)
mlx5hws_err(pool->ctx, "Failed to get free slot for chunk with order: %d\n",
@@ -240,33 +201,21 @@ static int hws_pool_buddy_db_get_chunk(struct mlx5hws_pool *pool,
static void hws_pool_buddy_db_uninit(struct mlx5hws_pool *pool)
{
struct mlx5hws_buddy_mem *buddy;
- int i;
-
- for (i = 0; i < MLX5HWS_POOL_RESOURCE_ARR_SZ; i++) {
- buddy = pool->db.buddy_manager->buddies[i];
- if (buddy) {
- mlx5hws_buddy_cleanup(buddy);
- kfree(buddy);
- pool->db.buddy_manager->buddies[i] = NULL;
- }
- }
- kfree(pool->db.buddy_manager);
+ buddy = pool->db.buddy;
+ if (buddy) {
+ mlx5hws_buddy_cleanup(buddy);
+ kfree(buddy);
+ pool->db.buddy = NULL;
+ }
}
static int hws_pool_buddy_db_init(struct mlx5hws_pool *pool, u32 log_range)
{
- pool->db.buddy_manager = kzalloc(sizeof(*pool->db.buddy_manager), GFP_KERNEL);
- if (!pool->db.buddy_manager)
- return -ENOMEM;
-
if (pool->flags & MLX5HWS_POOL_FLAGS_ALLOC_MEM_ON_CREATE) {
- bool new_buddy;
-
- if (!hws_pool_buddy_get_next_buddy(pool, 0, log_range, &new_buddy)) {
+ if (!hws_pool_buddy_get_buddy(pool, log_range)) {
mlx5hws_err(pool->ctx,
"Failed allocating memory on create log_sz: %d\n", log_range);
- kfree(pool->db.buddy_manager);
return -ENOMEM;
}
}
@@ -278,14 +227,13 @@ static int hws_pool_buddy_db_init(struct mlx5hws_pool *pool, u32 log_range)
return 0;
}
-static int hws_pool_create_resource_on_index(struct mlx5hws_pool *pool,
- u32 alloc_size, int idx)
+static int hws_pool_create_resource(struct mlx5hws_pool *pool, u32 alloc_size)
{
- int ret = hws_pool_resource_alloc(pool, alloc_size, idx);
+ int ret = hws_pool_resource_alloc(pool, alloc_size);
if (ret) {
- mlx5hws_err(pool->ctx, "Failed to create resource type: %d: size %d index: %d\n",
- pool->type, alloc_size, idx);
+ mlx5hws_err(pool->ctx, "Failed to create resource type: %d: size %d\n",
+ pool->type, alloc_size);
return ret;
}
@@ -319,7 +267,7 @@ hws_pool_element_create_new_elem(struct mlx5hws_pool *pool, u32 order)
elem->log_size = alloc_size - order;
}
- if (hws_pool_create_resource_on_index(pool, alloc_size, 0)) {
+ if (hws_pool_create_resource(pool, alloc_size)) {
mlx5hws_err(pool->ctx, "Failed to create resource type: %d: size %d\n",
pool->type, alloc_size);
goto free_db;
@@ -355,7 +303,7 @@ static int hws_pool_element_find_seg(struct mlx5hws_pool_elements *elem, int *se
static int
hws_pool_onesize_element_get_mem_chunk(struct mlx5hws_pool *pool, u32 order,
- u32 *idx, int *seg)
+ int *seg)
{
struct mlx5hws_pool_elements *elem;
@@ -370,7 +318,6 @@ hws_pool_onesize_element_get_mem_chunk(struct mlx5hws_pool *pool, u32 order,
return -ENOMEM;
}
- *idx = 0;
elem->num_of_elements++;
return 0;
@@ -379,21 +326,17 @@ hws_pool_onesize_element_get_mem_chunk(struct mlx5hws_pool *pool, u32 order,
return -ENOMEM;
}
-static int
-hws_pool_general_element_get_mem_chunk(struct mlx5hws_pool *pool, u32 order,
- u32 *idx, int *seg)
+static int hws_pool_general_element_get_mem_chunk(struct mlx5hws_pool *pool,
+ u32 order, int *seg)
{
- int ret, i;
-
- for (i = 0; i < MLX5HWS_POOL_RESOURCE_ARR_SZ; i++) {
- if (!pool->resource[i]) {
- ret = hws_pool_create_resource_on_index(pool, order, i);
- if (ret)
- goto err_no_res;
- *idx = i;
- *seg = 0; /* One memory slot in that element */
- return 0;
- }
+ int ret;
+
+ if (!pool->resource) {
+ ret = hws_pool_create_resource(pool, order);
+ if (ret)
+ goto err_no_res;
+ *seg = 0; /* One memory slot in that element */
+ return 0;
}
mlx5hws_err(pool->ctx, "No more resources (last request order: %d)\n", order);
@@ -409,9 +352,7 @@ static int hws_pool_general_element_db_get_chunk(struct mlx5hws_pool *pool,
{
int ret;
- /* Go over all memory elements and find/allocate free slot */
ret = hws_pool_general_element_get_mem_chunk(pool, chunk->order,
- &chunk->resource_idx,
&chunk->offset);
if (ret)
mlx5hws_err(pool->ctx, "Failed to get free slot for chunk with order: %d\n",
@@ -423,11 +364,8 @@ static int hws_pool_general_element_db_get_chunk(struct mlx5hws_pool *pool,
static void hws_pool_general_element_db_put_chunk(struct mlx5hws_pool *pool,
struct mlx5hws_pool_chunk *chunk)
{
- if (unlikely(!pool->resource[chunk->resource_idx]))
- pr_warn("HWS: invalid resource with index %d\n", chunk->resource_idx);
-
if (pool->flags & MLX5HWS_POOL_FLAGS_RELEASE_FREE_RESOURCE)
- hws_pool_resource_free(pool, chunk->resource_idx);
+ hws_pool_resource_free(pool);
}
static void hws_pool_general_element_db_uninit(struct mlx5hws_pool *pool)
@@ -455,7 +393,7 @@ static void
hws_onesize_element_db_destroy_element(struct mlx5hws_pool *pool,
struct mlx5hws_pool_elements *elem)
{
- hws_pool_resource_free(pool, 0);
+ hws_pool_resource_free(pool);
bitmap_free(elem->bitmap);
kfree(elem);
pool->db.element = NULL;
@@ -466,12 +404,9 @@ static void hws_onesize_element_db_put_chunk(struct mlx5hws_pool *pool,
{
struct mlx5hws_pool_elements *elem;
- if (unlikely(chunk->resource_idx))
- pr_warn("HWS: invalid resource with index %d\n", chunk->resource_idx);
-
elem = pool->db.element;
if (!elem) {
- mlx5hws_err(pool->ctx, "No such element (%d)\n", chunk->resource_idx);
+ mlx5hws_err(pool->ctx, "Pool element was not allocated\n");
return;
}
@@ -489,9 +424,7 @@ static int hws_onesize_element_db_get_chunk(struct mlx5hws_pool *pool,
{
int ret = 0;
- /* Go over all memory elements and find/allocate free slot */
ret = hws_pool_onesize_element_get_mem_chunk(pool, chunk->order,
- &chunk->resource_idx,
&chunk->offset);
if (ret)
mlx5hws_err(pool->ctx, "Failed to get free slot for chunk with order: %d\n",
@@ -614,13 +547,10 @@ mlx5hws_pool_create(struct mlx5hws_context *ctx, struct mlx5hws_pool_attr *pool_
int mlx5hws_pool_destroy(struct mlx5hws_pool *pool)
{
- int i;
-
mutex_destroy(&pool->lock);
- for (i = 0; i < MLX5HWS_POOL_RESOURCE_ARR_SZ; i++)
- if (pool->resource[i])
- hws_pool_resource_free(pool, i);
+ if (pool->resource)
+ hws_pool_resource_free(pool);
hws_pool_db_unint(pool);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pool.h b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pool.h
index f4258f83fdbf..112a61cd2997 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pool.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pool.h
@@ -6,16 +6,12 @@
#define MLX5HWS_POOL_STC_LOG_SZ 15
-#define MLX5HWS_POOL_RESOURCE_ARR_SZ 100
-
enum mlx5hws_pool_type {
MLX5HWS_POOL_TYPE_STE,
MLX5HWS_POOL_TYPE_STC,
};
struct mlx5hws_pool_chunk {
- u32 resource_idx;
- /* Internal offset, relative to base index */
int offset;
int order;
};
@@ -72,14 +68,10 @@ enum mlx5hws_db_type {
MLX5HWS_POOL_DB_TYPE_GENERAL_SIZE,
/* One resource only, all the elements are with same one size */
MLX5HWS_POOL_DB_TYPE_ONE_SIZE_RESOURCE,
- /* Many resources, the memory allocated with buddy mechanism */
+ /* Entries are managed using a buddy mechanism. */
MLX5HWS_POOL_DB_TYPE_BUDDY,
};
-struct mlx5hws_buddy_manager {
- struct mlx5hws_buddy_mem *buddies[MLX5HWS_POOL_RESOURCE_ARR_SZ];
-};
-
struct mlx5hws_pool_elements {
u32 num_of_elements;
unsigned long *bitmap;
@@ -91,7 +83,7 @@ struct mlx5hws_pool_db {
enum mlx5hws_db_type type;
union {
struct mlx5hws_pool_elements *element;
- struct mlx5hws_buddy_manager *buddy_manager;
+ struct mlx5hws_buddy_mem *buddy;
};
};
@@ -109,8 +101,8 @@ struct mlx5hws_pool {
size_t alloc_log_sz;
enum mlx5hws_table_type tbl_type;
enum mlx5hws_pool_optimize opt_type;
- struct mlx5hws_pool_resource *resource[MLX5HWS_POOL_RESOURCE_ARR_SZ];
- struct mlx5hws_pool_resource *mirror_resource[MLX5HWS_POOL_RESOURCE_ARR_SZ];
+ struct mlx5hws_pool_resource *resource;
+ struct mlx5hws_pool_resource *mirror_resource;
/* DB */
struct mlx5hws_pool_db db;
/* Functions */
@@ -131,17 +123,13 @@ int mlx5hws_pool_chunk_alloc(struct mlx5hws_pool *pool,
void mlx5hws_pool_chunk_free(struct mlx5hws_pool *pool,
struct mlx5hws_pool_chunk *chunk);
-static inline u32
-mlx5hws_pool_chunk_get_base_id(struct mlx5hws_pool *pool,
- struct mlx5hws_pool_chunk *chunk)
+static inline u32 mlx5hws_pool_get_base_id(struct mlx5hws_pool *pool)
{
- return pool->resource[chunk->resource_idx]->base_id;
+ return pool->resource->base_id;
}
-static inline u32
-mlx5hws_pool_chunk_get_base_mirror_id(struct mlx5hws_pool *pool,
- struct mlx5hws_pool_chunk *chunk)
+static inline u32 mlx5hws_pool_get_base_mirror_id(struct mlx5hws_pool *pool)
{
- return pool->mirror_resource[chunk->resource_idx]->base_id;
+ return pool->mirror_resource->base_id;
}
#endif /* MLX5HWS_POOL_H_ */
--
2.31.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH net-next 03/12] net/mlx5: HWS, Make pool single resource
2025-04-08 14:00 ` [PATCH net-next 03/12] net/mlx5: HWS, Make pool single resource Tariq Toukan
@ 2025-04-09 20:04 ` Michal Kubiak
0 siblings, 0 replies; 32+ messages in thread
From: Michal Kubiak @ 2025-04-09 20:04 UTC (permalink / raw)
To: Tariq Toukan
Cc: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet,
Andrew Lunn, Gal Pressman, Leon Romanovsky, Saeed Mahameed,
Leon Romanovsky, netdev, linux-rdma, linux-kernel, Moshe Shemesh,
Mark Bloch, Vlad Dogaru, Yevgeny Kliteynik
On Tue, Apr 08, 2025 at 05:00:47PM +0300, Tariq Toukan wrote:
> From: Vlad Dogaru <vdogaru@nvidia.com>
>
> The pool implementation claimed to support multiple resources, but this
> does not really make sense in context. Callers always allocate a single
> STC or STE chunk of exactly the size provided.
>
> The code that handled multiple resources was unused (and likely buggy)
> due to the combination of flags passed by callers.
>
> Simplify the pool by having it handle a single resource. As a result of
> this simplification, chunks no longer contain a resource offset (there
> is now only one resource per pool), and the get_base_id functions no
> longer take a chunk parameter.
>
> Signed-off-by: Vlad Dogaru <vdogaru@nvidia.com>
> Reviewed-by: Yevgeny Kliteynik <kliteyn@nvidia.com>
> Reviewed-by: Mark Bloch <mbloch@nvidia.com>
> Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
Nice code simplification!
Thanks,
Reviewed-by: Michal Kubiak <michal.kubiak@intel.com>
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH net-next 04/12] net/mlx5: HWS, Refactor pool implementation
2025-04-08 14:00 [PATCH net-next 00/12] net/mlx5: HWS, Refactor action STE handling Tariq Toukan
` (2 preceding siblings ...)
2025-04-08 14:00 ` [PATCH net-next 03/12] net/mlx5: HWS, Make pool single resource Tariq Toukan
@ 2025-04-08 14:00 ` Tariq Toukan
2025-04-09 21:24 ` Michal Kubiak
2025-04-08 14:00 ` [PATCH net-next 05/12] net/mlx5: HWS, Cleanup after pool refactoring Tariq Toukan
` (7 subsequent siblings)
11 siblings, 1 reply; 32+ messages in thread
From: Tariq Toukan @ 2025-04-08 14:00 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet,
Andrew Lunn
Cc: Gal Pressman, Leon Romanovsky, Saeed Mahameed, Leon Romanovsky,
Tariq Toukan, netdev, linux-rdma, linux-kernel, Moshe Shemesh,
Mark Bloch, Vlad Dogaru, Yevgeny Kliteynik
From: Vlad Dogaru <vdogaru@nvidia.com>
Refactor the pool implementation to remove unused flags and clarify its
usage. A pool represents a single range of STEs or STCs which are
allocated at pool creation time.
Pools are used under three patterns:
1. STCs are allocated one at a time from a global pool using a bitmap
based implementation.
2. Action STEs are allocated in power-of-two blocks using a buddy
algorithm.
3. Match STEs do not use allocation, since insertion into these tables
is based on hashes or direct addressing. In such cases we use a pool
only to create the STE range.
Signed-off-by: Vlad Dogaru <vdogaru@nvidia.com>
Reviewed-by: Yevgeny Kliteynik <kliteyn@nvidia.com>
Reviewed-by: Mark Bloch <mbloch@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
.../mellanox/mlx5/core/steering/hws/action.c | 1 -
.../mellanox/mlx5/core/steering/hws/context.c | 1 -
.../mellanox/mlx5/core/steering/hws/matcher.c | 19 +-
.../mellanox/mlx5/core/steering/hws/pool.c | 387 +++++-------------
.../mellanox/mlx5/core/steering/hws/pool.h | 45 +-
5 files changed, 116 insertions(+), 337 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/action.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/action.c
index 781ba8c4f733..39904b337b81 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/action.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/action.c
@@ -1602,7 +1602,6 @@ hws_action_create_dest_match_range_table(struct mlx5hws_context *ctx,
pool_attr.table_type = MLX5HWS_TABLE_TYPE_FDB;
pool_attr.pool_type = MLX5HWS_POOL_TYPE_STE;
- pool_attr.flags = MLX5HWS_POOL_FLAGS_FOR_STE_ACTION_POOL;
pool_attr.alloc_log_sz = 1;
table_ste->pool = mlx5hws_pool_create(ctx, &pool_attr);
if (!table_ste->pool) {
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/context.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/context.c
index 9cda2774fd64..b7cb736b74d7 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/context.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/context.c
@@ -34,7 +34,6 @@ static int hws_context_pools_init(struct mlx5hws_context *ctx)
/* Create an STC pool per FT type */
pool_attr.pool_type = MLX5HWS_POOL_TYPE_STC;
- pool_attr.flags = MLX5HWS_POOL_FLAGS_FOR_STC_POOL;
max_log_sz = min(MLX5HWS_POOL_STC_LOG_SZ, ctx->caps->stc_alloc_log_max);
pool_attr.alloc_log_sz = max(max_log_sz, ctx->caps->stc_alloc_log_gran);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/matcher.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/matcher.c
index 59b14db427b4..95d31fd6c976 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/matcher.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/matcher.c
@@ -265,14 +265,6 @@ static int hws_matcher_create_rtc(struct mlx5hws_matcher *matcher,
rtc_attr.match_definer_0 = ctx->caps->linear_match_definer;
}
}
-
- /* Match pool requires implicit allocation */
- ret = mlx5hws_pool_chunk_alloc(ste_pool, ste);
- if (ret) {
- mlx5hws_err(ctx, "Failed to allocate STE for %s RTC",
- hws_matcher_rtc_type_to_str(rtc_type));
- return ret;
- }
break;
case HWS_MATCHER_RTC_TYPE_STE_ARRAY:
@@ -357,23 +349,17 @@ static void hws_matcher_destroy_rtc(struct mlx5hws_matcher *matcher,
{
struct mlx5hws_matcher_action_ste *action_ste;
struct mlx5hws_table *tbl = matcher->tbl;
- struct mlx5hws_pool_chunk *ste;
- struct mlx5hws_pool *ste_pool;
u32 rtc_0_id, rtc_1_id;
switch (rtc_type) {
case HWS_MATCHER_RTC_TYPE_MATCH:
rtc_0_id = matcher->match_ste.rtc_0_id;
rtc_1_id = matcher->match_ste.rtc_1_id;
- ste_pool = matcher->match_ste.pool;
- ste = &matcher->match_ste.ste;
break;
case HWS_MATCHER_RTC_TYPE_STE_ARRAY:
action_ste = &matcher->action_ste;
rtc_0_id = action_ste->rtc_0_id;
rtc_1_id = action_ste->rtc_1_id;
- ste_pool = action_ste->pool;
- ste = &action_ste->ste;
break;
default:
return;
@@ -383,8 +369,6 @@ static void hws_matcher_destroy_rtc(struct mlx5hws_matcher *matcher,
mlx5hws_cmd_rtc_destroy(matcher->tbl->ctx->mdev, rtc_1_id);
mlx5hws_cmd_rtc_destroy(matcher->tbl->ctx->mdev, rtc_0_id);
- if (rtc_type == HWS_MATCHER_RTC_TYPE_MATCH)
- mlx5hws_pool_chunk_free(ste_pool, ste);
}
static int
@@ -557,7 +541,7 @@ static int hws_matcher_bind_at(struct mlx5hws_matcher *matcher)
/* Allocate action STE mempool */
pool_attr.table_type = tbl->type;
pool_attr.pool_type = MLX5HWS_POOL_TYPE_STE;
- pool_attr.flags = MLX5HWS_POOL_FLAGS_FOR_STE_ACTION_POOL;
+ pool_attr.flags = MLX5HWS_POOL_FLAG_BUDDY;
/* Pool size is similar to action RTC size */
pool_attr.alloc_log_sz = ilog2(roundup_pow_of_two(action_ste->max_stes)) +
matcher->attr.table.sz_row_log +
@@ -636,7 +620,6 @@ static int hws_matcher_bind_mt(struct mlx5hws_matcher *matcher)
/* Create an STE pool per matcher*/
pool_attr.table_type = matcher->tbl->type;
pool_attr.pool_type = MLX5HWS_POOL_TYPE_STE;
- pool_attr.flags = MLX5HWS_POOL_FLAGS_FOR_MATCHER_STE_POOL;
pool_attr.alloc_log_sz = matcher->attr.table.sz_col_log +
matcher->attr.table.sz_row_log;
hws_matcher_set_pool_attr(&pool_attr, matcher);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pool.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pool.c
index 0de03e17624c..270b333faab3 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pool.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pool.c
@@ -60,10 +60,8 @@ hws_pool_create_one_resource(struct mlx5hws_pool *pool, u32 log_range,
ret = -EINVAL;
}
- if (ret) {
- mlx5hws_err(pool->ctx, "Failed to allocate resource objects\n");
+ if (ret)
goto free_resource;
- }
resource->pool = pool;
resource->range = 1 << log_range;
@@ -76,17 +74,17 @@ hws_pool_create_one_resource(struct mlx5hws_pool *pool, u32 log_range,
return NULL;
}
-static int
-hws_pool_resource_alloc(struct mlx5hws_pool *pool, u32 log_range)
+static int hws_pool_resource_alloc(struct mlx5hws_pool *pool)
{
struct mlx5hws_pool_resource *resource;
u32 fw_ft_type, opt_log_range;
fw_ft_type = mlx5hws_table_get_res_fw_ft_type(pool->tbl_type, false);
- opt_log_range = pool->opt_type == MLX5HWS_POOL_OPTIMIZE_ORIG ? 0 : log_range;
+ opt_log_range = pool->opt_type == MLX5HWS_POOL_OPTIMIZE_ORIG ?
+ 0 : pool->alloc_log_sz;
resource = hws_pool_create_one_resource(pool, opt_log_range, fw_ft_type);
if (!resource) {
- mlx5hws_err(pool->ctx, "Failed allocating resource\n");
+ mlx5hws_err(pool->ctx, "Failed to allocate resource\n");
return -EINVAL;
}
@@ -96,10 +94,11 @@ hws_pool_resource_alloc(struct mlx5hws_pool *pool, u32 log_range)
struct mlx5hws_pool_resource *mirror_resource;
fw_ft_type = mlx5hws_table_get_res_fw_ft_type(pool->tbl_type, true);
- opt_log_range = pool->opt_type == MLX5HWS_POOL_OPTIMIZE_MIRROR ? 0 : log_range;
+ opt_log_range = pool->opt_type == MLX5HWS_POOL_OPTIMIZE_MIRROR ?
+ 0 : pool->alloc_log_sz;
mirror_resource = hws_pool_create_one_resource(pool, opt_log_range, fw_ft_type);
if (!mirror_resource) {
- mlx5hws_err(pool->ctx, "Failed allocating mirrored resource\n");
+ mlx5hws_err(pool->ctx, "Failed to allocate mirrored resource\n");
hws_pool_free_one_resource(resource);
pool->resource = NULL;
return -EINVAL;
@@ -110,92 +109,58 @@ hws_pool_resource_alloc(struct mlx5hws_pool *pool, u32 log_range)
return 0;
}
-static unsigned long *hws_pool_create_and_init_bitmap(u32 log_range)
-{
- unsigned long *cur_bmp;
-
- cur_bmp = bitmap_zalloc(1 << log_range, GFP_KERNEL);
- if (!cur_bmp)
- return NULL;
-
- bitmap_fill(cur_bmp, 1 << log_range);
-
- return cur_bmp;
-}
-
-static void hws_pool_buddy_db_put_chunk(struct mlx5hws_pool *pool,
- struct mlx5hws_pool_chunk *chunk)
+static int hws_pool_buddy_init(struct mlx5hws_pool *pool)
{
struct mlx5hws_buddy_mem *buddy;
- buddy = pool->db.buddy;
+ buddy = mlx5hws_buddy_create(pool->alloc_log_sz);
if (!buddy) {
- mlx5hws_err(pool->ctx, "Bad buddy state\n");
- return;
- }
-
- mlx5hws_buddy_free_mem(buddy, chunk->offset, chunk->order);
-}
-
-static struct mlx5hws_buddy_mem *
-hws_pool_buddy_get_buddy(struct mlx5hws_pool *pool, u32 order)
-{
- static struct mlx5hws_buddy_mem *buddy;
- u32 new_buddy_size;
-
- buddy = pool->db.buddy;
- if (buddy)
- return buddy;
-
- new_buddy_size = max(pool->alloc_log_sz, order);
- buddy = mlx5hws_buddy_create(new_buddy_size);
- if (!buddy) {
- mlx5hws_err(pool->ctx, "Failed to create buddy order: %d\n",
- new_buddy_size);
- return NULL;
+ mlx5hws_err(pool->ctx, "Failed to create buddy order: %zu\n",
+ pool->alloc_log_sz);
+ return -ENOMEM;
}
- if (hws_pool_resource_alloc(pool, new_buddy_size) != 0) {
- mlx5hws_err(pool->ctx, "Failed to create resource type: %d: size %d\n",
- pool->type, new_buddy_size);
+ if (hws_pool_resource_alloc(pool) != 0) {
+ mlx5hws_err(pool->ctx, "Failed to create resource type: %d size %zu\n",
+ pool->type, pool->alloc_log_sz);
mlx5hws_buddy_cleanup(buddy);
- return NULL;
+ return -ENOMEM;
}
pool->db.buddy = buddy;
- return buddy;
+ return 0;
}
-static int hws_pool_buddy_get_mem_chunk(struct mlx5hws_pool *pool,
- int order,
- int *seg)
+static int hws_pool_buddy_db_get_chunk(struct mlx5hws_pool *pool,
+ struct mlx5hws_pool_chunk *chunk)
{
- struct mlx5hws_buddy_mem *buddy;
+ struct mlx5hws_buddy_mem *buddy = pool->db.buddy;
- buddy = hws_pool_buddy_get_buddy(pool, order);
- if (!buddy)
- return -ENOMEM;
+ if (!buddy) {
+ mlx5hws_err(pool->ctx, "Bad buddy state\n");
+ return -EINVAL;
+ }
- *seg = mlx5hws_buddy_alloc_mem(buddy, order);
- if (*seg >= 0)
+ chunk->offset = mlx5hws_buddy_alloc_mem(buddy, chunk->order);
+ if (chunk->offset >= 0)
return 0;
return -ENOMEM;
}
-static int hws_pool_buddy_db_get_chunk(struct mlx5hws_pool *pool,
- struct mlx5hws_pool_chunk *chunk)
+static void hws_pool_buddy_db_put_chunk(struct mlx5hws_pool *pool,
+ struct mlx5hws_pool_chunk *chunk)
{
- int ret = 0;
+ struct mlx5hws_buddy_mem *buddy;
- ret = hws_pool_buddy_get_mem_chunk(pool, chunk->order,
- &chunk->offset);
- if (ret)
- mlx5hws_err(pool->ctx, "Failed to get free slot for chunk with order: %d\n",
- chunk->order);
+ buddy = pool->db.buddy;
+ if (!buddy) {
+ mlx5hws_err(pool->ctx, "Bad buddy state\n");
+ return;
+ }
- return ret;
+ mlx5hws_buddy_free_mem(buddy, chunk->offset, chunk->order);
}
static void hws_pool_buddy_db_uninit(struct mlx5hws_pool *pool)
@@ -210,15 +175,13 @@ static void hws_pool_buddy_db_uninit(struct mlx5hws_pool *pool)
}
}
-static int hws_pool_buddy_db_init(struct mlx5hws_pool *pool, u32 log_range)
+static int hws_pool_buddy_db_init(struct mlx5hws_pool *pool)
{
- if (pool->flags & MLX5HWS_POOL_FLAGS_ALLOC_MEM_ON_CREATE) {
- if (!hws_pool_buddy_get_buddy(pool, log_range)) {
- mlx5hws_err(pool->ctx,
- "Failed allocating memory on create log_sz: %d\n", log_range);
- return -ENOMEM;
- }
- }
+ int ret;
+
+ ret = hws_pool_buddy_init(pool);
+ if (ret)
+ return ret;
pool->p_db_uninit = &hws_pool_buddy_db_uninit;
pool->p_get_chunk = &hws_pool_buddy_db_get_chunk;
@@ -227,234 +190,105 @@ static int hws_pool_buddy_db_init(struct mlx5hws_pool *pool, u32 log_range)
return 0;
}
-static int hws_pool_create_resource(struct mlx5hws_pool *pool, u32 alloc_size)
-{
- int ret = hws_pool_resource_alloc(pool, alloc_size);
-
- if (ret) {
- mlx5hws_err(pool->ctx, "Failed to create resource type: %d: size %d\n",
- pool->type, alloc_size);
- return ret;
- }
-
- return 0;
-}
-
-static struct mlx5hws_pool_elements *
-hws_pool_element_create_new_elem(struct mlx5hws_pool *pool, u32 order)
+static unsigned long *hws_pool_create_and_init_bitmap(u32 log_range)
{
- struct mlx5hws_pool_elements *elem;
- u32 alloc_size;
-
- alloc_size = pool->alloc_log_sz;
+ unsigned long *bitmap;
- elem = kzalloc(sizeof(*elem), GFP_KERNEL);
- if (!elem)
+ bitmap = bitmap_zalloc(1 << log_range, GFP_KERNEL);
+ if (!bitmap)
return NULL;
- /* Sharing the same resource, also means that all the elements are with size 1 */
- if ((pool->flags & MLX5HWS_POOL_FLAGS_FIXED_SIZE_OBJECTS) &&
- !(pool->flags & MLX5HWS_POOL_FLAGS_RESOURCE_PER_CHUNK)) {
- /* Currently all chunks in size 1 */
- elem->bitmap = hws_pool_create_and_init_bitmap(alloc_size - order);
- if (!elem->bitmap) {
- mlx5hws_err(pool->ctx,
- "Failed to create bitmap type: %d: size %d\n",
- pool->type, alloc_size);
- goto free_elem;
- }
-
- elem->log_size = alloc_size - order;
- }
-
- if (hws_pool_create_resource(pool, alloc_size)) {
- mlx5hws_err(pool->ctx, "Failed to create resource type: %d: size %d\n",
- pool->type, alloc_size);
- goto free_db;
- }
-
- pool->db.element = elem;
+ bitmap_fill(bitmap, 1 << log_range);
- return elem;
-
-free_db:
- bitmap_free(elem->bitmap);
-free_elem:
- kfree(elem);
- return NULL;
+ return bitmap;
}
-static int hws_pool_element_find_seg(struct mlx5hws_pool_elements *elem, int *seg)
+static int hws_pool_bitmap_init(struct mlx5hws_pool *pool)
{
- unsigned int segment, size;
+ unsigned long *bitmap;
- size = 1 << elem->log_size;
-
- segment = find_first_bit(elem->bitmap, size);
- if (segment >= size) {
- elem->is_full = true;
+ bitmap = hws_pool_create_and_init_bitmap(pool->alloc_log_sz);
+ if (!bitmap) {
+ mlx5hws_err(pool->ctx, "Failed to create bitmap order: %zu\n",
+ pool->alloc_log_sz);
return -ENOMEM;
}
- bitmap_clear(elem->bitmap, segment, 1);
- *seg = segment;
- return 0;
-}
-
-static int
-hws_pool_onesize_element_get_mem_chunk(struct mlx5hws_pool *pool, u32 order,
- int *seg)
-{
- struct mlx5hws_pool_elements *elem;
-
- elem = pool->db.element;
- if (!elem)
- elem = hws_pool_element_create_new_elem(pool, order);
- if (!elem)
- goto err_no_elem;
-
- if (hws_pool_element_find_seg(elem, seg) != 0) {
- mlx5hws_err(pool->ctx, "No more resources (last request order: %d)\n", order);
+ if (hws_pool_resource_alloc(pool) != 0) {
+ mlx5hws_err(pool->ctx, "Failed to create resource type: %d: size %zu\n",
+ pool->type, pool->alloc_log_sz);
+ bitmap_free(bitmap);
return -ENOMEM;
}
- elem->num_of_elements++;
- return 0;
+ pool->db.bitmap = bitmap;
-err_no_elem:
- mlx5hws_err(pool->ctx, "Failed to allocate element for order: %d\n", order);
- return -ENOMEM;
+ return 0;
}
-static int hws_pool_general_element_get_mem_chunk(struct mlx5hws_pool *pool,
- u32 order, int *seg)
+static int hws_pool_bitmap_db_get_chunk(struct mlx5hws_pool *pool,
+ struct mlx5hws_pool_chunk *chunk)
{
- int ret;
+ unsigned long *bitmap, size;
- if (!pool->resource) {
- ret = hws_pool_create_resource(pool, order);
- if (ret)
- goto err_no_res;
- *seg = 0; /* One memory slot in that element */
- return 0;
+ if (chunk->order != 0) {
+ mlx5hws_err(pool->ctx, "Pool only supports order 0 allocs\n");
+ return -EINVAL;
}
- mlx5hws_err(pool->ctx, "No more resources (last request order: %d)\n", order);
- return -ENOMEM;
-
-err_no_res:
- mlx5hws_err(pool->ctx, "Failed to allocate element for order: %d\n", order);
- return -ENOMEM;
-}
-
-static int hws_pool_general_element_db_get_chunk(struct mlx5hws_pool *pool,
- struct mlx5hws_pool_chunk *chunk)
-{
- int ret;
-
- ret = hws_pool_general_element_get_mem_chunk(pool, chunk->order,
- &chunk->offset);
- if (ret)
- mlx5hws_err(pool->ctx, "Failed to get free slot for chunk with order: %d\n",
- chunk->order);
-
- return ret;
-}
+ bitmap = pool->db.bitmap;
+ if (!bitmap) {
+ mlx5hws_err(pool->ctx, "Bad bitmap state\n");
+ return -EINVAL;
+ }
-static void hws_pool_general_element_db_put_chunk(struct mlx5hws_pool *pool,
- struct mlx5hws_pool_chunk *chunk)
-{
- if (pool->flags & MLX5HWS_POOL_FLAGS_RELEASE_FREE_RESOURCE)
- hws_pool_resource_free(pool);
-}
+ size = 1 << pool->alloc_log_sz;
-static void hws_pool_general_element_db_uninit(struct mlx5hws_pool *pool)
-{
- (void)pool;
-}
+ chunk->offset = find_first_bit(bitmap, size);
+ if (chunk->offset >= size)
+ return -ENOMEM;
-/* This memory management works as the following:
- * - At start doesn't allocate no mem at all.
- * - When new request for chunk arrived:
- * allocate resource and give it.
- * - When free that chunk:
- * the resource is freed.
- */
-static int hws_pool_general_element_db_init(struct mlx5hws_pool *pool)
-{
- pool->p_db_uninit = &hws_pool_general_element_db_uninit;
- pool->p_get_chunk = &hws_pool_general_element_db_get_chunk;
- pool->p_put_chunk = &hws_pool_general_element_db_put_chunk;
+ bitmap_clear(bitmap, chunk->offset, 1);
return 0;
}
-static void
-hws_onesize_element_db_destroy_element(struct mlx5hws_pool *pool,
- struct mlx5hws_pool_elements *elem)
-{
- hws_pool_resource_free(pool);
- bitmap_free(elem->bitmap);
- kfree(elem);
- pool->db.element = NULL;
-}
-
-static void hws_onesize_element_db_put_chunk(struct mlx5hws_pool *pool,
- struct mlx5hws_pool_chunk *chunk)
+static void hws_pool_bitmap_db_put_chunk(struct mlx5hws_pool *pool,
+ struct mlx5hws_pool_chunk *chunk)
{
- struct mlx5hws_pool_elements *elem;
+ unsigned long *bitmap;
- elem = pool->db.element;
- if (!elem) {
- mlx5hws_err(pool->ctx, "Pool element was not allocated\n");
+ bitmap = pool->db.bitmap;
+ if (!bitmap) {
+ mlx5hws_err(pool->ctx, "Bad bitmap state\n");
return;
}
- bitmap_set(elem->bitmap, chunk->offset, 1);
- elem->is_full = false;
- elem->num_of_elements--;
-
- if (pool->flags & MLX5HWS_POOL_FLAGS_RELEASE_FREE_RESOURCE &&
- !elem->num_of_elements)
- hws_onesize_element_db_destroy_element(pool, elem);
+ bitmap_set(bitmap, chunk->offset, 1);
}
-static int hws_onesize_element_db_get_chunk(struct mlx5hws_pool *pool,
- struct mlx5hws_pool_chunk *chunk)
+static void hws_pool_bitmap_db_uninit(struct mlx5hws_pool *pool)
{
- int ret = 0;
-
- ret = hws_pool_onesize_element_get_mem_chunk(pool, chunk->order,
- &chunk->offset);
- if (ret)
- mlx5hws_err(pool->ctx, "Failed to get free slot for chunk with order: %d\n",
- chunk->order);
+ unsigned long *bitmap;
- return ret;
+ bitmap = pool->db.bitmap;
+ if (bitmap) {
+ bitmap_free(bitmap);
+ pool->db.bitmap = NULL;
+ }
}
-static void hws_onesize_element_db_uninit(struct mlx5hws_pool *pool)
+static int hws_pool_bitmap_db_init(struct mlx5hws_pool *pool)
{
- struct mlx5hws_pool_elements *elem = pool->db.element;
+ int ret;
- if (elem) {
- bitmap_free(elem->bitmap);
- kfree(elem);
- pool->db.element = NULL;
- }
-}
+ ret = hws_pool_bitmap_init(pool);
+ if (ret)
+ return ret;
-/* This memory management works as the following:
- * - At start doesn't allocate no mem at all.
- * - When new request for chunk arrived:
- * aloocate the first and only slot of memory/resource
- * when it ended return error.
- */
-static int hws_pool_onesize_element_db_init(struct mlx5hws_pool *pool)
-{
- pool->p_db_uninit = &hws_onesize_element_db_uninit;
- pool->p_get_chunk = &hws_onesize_element_db_get_chunk;
- pool->p_put_chunk = &hws_onesize_element_db_put_chunk;
+ pool->p_db_uninit = &hws_pool_bitmap_db_uninit;
+ pool->p_get_chunk = &hws_pool_bitmap_db_get_chunk;
+ pool->p_put_chunk = &hws_pool_bitmap_db_put_chunk;
return 0;
}
@@ -464,15 +298,14 @@ static int hws_pool_db_init(struct mlx5hws_pool *pool,
{
int ret;
- if (db_type == MLX5HWS_POOL_DB_TYPE_GENERAL_SIZE)
- ret = hws_pool_general_element_db_init(pool);
- else if (db_type == MLX5HWS_POOL_DB_TYPE_ONE_SIZE_RESOURCE)
- ret = hws_pool_onesize_element_db_init(pool);
+ if (db_type == MLX5HWS_POOL_DB_TYPE_BITMAP)
+ ret = hws_pool_bitmap_db_init(pool);
else
- ret = hws_pool_buddy_db_init(pool, pool->alloc_log_sz);
+ ret = hws_pool_buddy_db_init(pool);
if (ret) {
- mlx5hws_err(pool->ctx, "Failed to init general db : %d (ret: %d)\n", db_type, ret);
+ mlx5hws_err(pool->ctx, "Failed to init pool type: %d (ret: %d)\n",
+ db_type, ret);
return ret;
}
@@ -521,15 +354,10 @@ mlx5hws_pool_create(struct mlx5hws_context *ctx, struct mlx5hws_pool_attr *pool_
pool->tbl_type = pool_attr->table_type;
pool->opt_type = pool_attr->opt_type;
- /* Support general db */
- if (pool->flags == (MLX5HWS_POOL_FLAGS_RELEASE_FREE_RESOURCE |
- MLX5HWS_POOL_FLAGS_RESOURCE_PER_CHUNK))
- res_db_type = MLX5HWS_POOL_DB_TYPE_GENERAL_SIZE;
- else if (pool->flags == (MLX5HWS_POOL_FLAGS_ONE_RESOURCE |
- MLX5HWS_POOL_FLAGS_FIXED_SIZE_OBJECTS))
- res_db_type = MLX5HWS_POOL_DB_TYPE_ONE_SIZE_RESOURCE;
- else
+ if (pool->flags & MLX5HWS_POOL_FLAG_BUDDY)
res_db_type = MLX5HWS_POOL_DB_TYPE_BUDDY;
+ else
+ res_db_type = MLX5HWS_POOL_DB_TYPE_BITMAP;
pool->alloc_log_sz = pool_attr->alloc_log_sz;
@@ -545,7 +373,7 @@ mlx5hws_pool_create(struct mlx5hws_context *ctx, struct mlx5hws_pool_attr *pool_
return NULL;
}
-int mlx5hws_pool_destroy(struct mlx5hws_pool *pool)
+void mlx5hws_pool_destroy(struct mlx5hws_pool *pool)
{
mutex_destroy(&pool->lock);
@@ -555,5 +383,4 @@ int mlx5hws_pool_destroy(struct mlx5hws_pool *pool)
hws_pool_db_unint(pool);
kfree(pool);
- return 0;
}
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pool.h b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pool.h
index 112a61cd2997..9a781a87f097 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pool.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pool.h
@@ -23,29 +23,10 @@ struct mlx5hws_pool_resource {
};
enum mlx5hws_pool_flags {
- /* Only a one resource in that pool */
- MLX5HWS_POOL_FLAGS_ONE_RESOURCE = 1 << 0,
- MLX5HWS_POOL_FLAGS_RELEASE_FREE_RESOURCE = 1 << 1,
- /* No sharing resources between chunks */
- MLX5HWS_POOL_FLAGS_RESOURCE_PER_CHUNK = 1 << 2,
- /* All objects are in the same size */
- MLX5HWS_POOL_FLAGS_FIXED_SIZE_OBJECTS = 1 << 3,
- /* Managed by buddy allocator */
- MLX5HWS_POOL_FLAGS_BUDDY_MANAGED = 1 << 4,
- /* Allocate pool_type memory on pool creation */
- MLX5HWS_POOL_FLAGS_ALLOC_MEM_ON_CREATE = 1 << 5,
-
- /* These values should be used by the caller */
- MLX5HWS_POOL_FLAGS_FOR_STC_POOL =
- MLX5HWS_POOL_FLAGS_ONE_RESOURCE |
- MLX5HWS_POOL_FLAGS_FIXED_SIZE_OBJECTS,
- MLX5HWS_POOL_FLAGS_FOR_MATCHER_STE_POOL =
- MLX5HWS_POOL_FLAGS_RELEASE_FREE_RESOURCE |
- MLX5HWS_POOL_FLAGS_RESOURCE_PER_CHUNK,
- MLX5HWS_POOL_FLAGS_FOR_STE_ACTION_POOL =
- MLX5HWS_POOL_FLAGS_ONE_RESOURCE |
- MLX5HWS_POOL_FLAGS_BUDDY_MANAGED |
- MLX5HWS_POOL_FLAGS_ALLOC_MEM_ON_CREATE,
+ /* Managed by a buddy allocator. If this is not set only allocations of
+ * order 0 are supported.
+ */
+ MLX5HWS_POOL_FLAG_BUDDY = BIT(0),
};
enum mlx5hws_pool_optimize {
@@ -64,25 +45,16 @@ struct mlx5hws_pool_attr {
};
enum mlx5hws_db_type {
- /* Uses for allocating chunk of big memory, each element has its own resource in the FW*/
- MLX5HWS_POOL_DB_TYPE_GENERAL_SIZE,
- /* One resource only, all the elements are with same one size */
- MLX5HWS_POOL_DB_TYPE_ONE_SIZE_RESOURCE,
+ /* Uses a bitmap, supports only allocations of order 0. */
+ MLX5HWS_POOL_DB_TYPE_BITMAP,
/* Entries are managed using a buddy mechanism. */
MLX5HWS_POOL_DB_TYPE_BUDDY,
};
-struct mlx5hws_pool_elements {
- u32 num_of_elements;
- unsigned long *bitmap;
- u32 log_size;
- bool is_full;
-};
-
struct mlx5hws_pool_db {
enum mlx5hws_db_type type;
union {
- struct mlx5hws_pool_elements *element;
+ unsigned long *bitmap;
struct mlx5hws_buddy_mem *buddy;
};
};
@@ -103,7 +75,6 @@ struct mlx5hws_pool {
enum mlx5hws_pool_optimize opt_type;
struct mlx5hws_pool_resource *resource;
struct mlx5hws_pool_resource *mirror_resource;
- /* DB */
struct mlx5hws_pool_db db;
/* Functions */
mlx5hws_pool_unint_db p_db_uninit;
@@ -115,7 +86,7 @@ struct mlx5hws_pool *
mlx5hws_pool_create(struct mlx5hws_context *ctx,
struct mlx5hws_pool_attr *pool_attr);
-int mlx5hws_pool_destroy(struct mlx5hws_pool *pool);
+void mlx5hws_pool_destroy(struct mlx5hws_pool *pool);
int mlx5hws_pool_chunk_alloc(struct mlx5hws_pool *pool,
struct mlx5hws_pool_chunk *chunk);
--
2.31.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH net-next 04/12] net/mlx5: HWS, Refactor pool implementation
2025-04-08 14:00 ` [PATCH net-next 04/12] net/mlx5: HWS, Refactor pool implementation Tariq Toukan
@ 2025-04-09 21:24 ` Michal Kubiak
0 siblings, 0 replies; 32+ messages in thread
From: Michal Kubiak @ 2025-04-09 21:24 UTC (permalink / raw)
To: Tariq Toukan
Cc: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet,
Andrew Lunn, Gal Pressman, Leon Romanovsky, Saeed Mahameed,
Leon Romanovsky, netdev, linux-rdma, linux-kernel, Moshe Shemesh,
Mark Bloch, Vlad Dogaru, Yevgeny Kliteynik
On Tue, Apr 08, 2025 at 05:00:48PM +0300, Tariq Toukan wrote:
> From: Vlad Dogaru <vdogaru@nvidia.com>
>
> Refactor the pool implementation to remove unused flags and clarify its
> usage. A pool represents a single range of STEs or STCs which are
> allocated at pool creation time.
>
> Pools are used under three patterns:
>
> 1. STCs are allocated one at a time from a global pool using a bitmap
> based implementation.
>
> 2. Action STEs are allocated in power-of-two blocks using a buddy
> algorithm.
>
> 3. Match STEs do not use allocation, since insertion into these tables
> is based on hashes or direct addressing. In such cases we use a pool
> only to create the STE range.
>
> Signed-off-by: Vlad Dogaru <vdogaru@nvidia.com>
> Reviewed-by: Yevgeny Kliteynik <kliteyn@nvidia.com>
> Reviewed-by: Mark Bloch <mbloch@nvidia.com>
> Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
Thanks,
Reviewed-by: Michal Kubiak <michal.kubiak@intel.com>
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH net-next 05/12] net/mlx5: HWS, Cleanup after pool refactoring
2025-04-08 14:00 [PATCH net-next 00/12] net/mlx5: HWS, Refactor action STE handling Tariq Toukan
` (3 preceding siblings ...)
2025-04-08 14:00 ` [PATCH net-next 04/12] net/mlx5: HWS, Refactor pool implementation Tariq Toukan
@ 2025-04-08 14:00 ` Tariq Toukan
2025-04-09 21:23 ` Michal Kubiak
2025-04-08 14:00 ` [PATCH net-next 06/12] net/mlx5: HWS, Add fullness tracking to pool Tariq Toukan
` (6 subsequent siblings)
11 siblings, 1 reply; 32+ messages in thread
From: Tariq Toukan @ 2025-04-08 14:00 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet,
Andrew Lunn
Cc: Gal Pressman, Leon Romanovsky, Saeed Mahameed, Leon Romanovsky,
Tariq Toukan, netdev, linux-rdma, linux-kernel, Moshe Shemesh,
Mark Bloch, Vlad Dogaru, Yevgeny Kliteynik
From: Vlad Dogaru <vdogaru@nvidia.com>
Remove members which are now no longer used. In fact, many of the
`struct mlx5hws_pool_chunk` were not even written to beyond being
initialized, but they were used in various internals.
Also cleanup some local variables which made more sense when the API was
thicker.
Signed-off-by: Vlad Dogaru <vdogaru@nvidia.com>
Reviewed-by: Yevgeny Kliteynik <kliteyn@nvidia.com>
Reviewed-by: Mark Bloch <mbloch@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
.../mellanox/mlx5/core/steering/hws/action.c | 6 +--
.../mellanox/mlx5/core/steering/hws/matcher.c | 48 ++++++-------------
.../mellanox/mlx5/core/steering/hws/matcher.h | 2 -
3 files changed, 16 insertions(+), 40 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/action.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/action.c
index 39904b337b81..44b4640b47db 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/action.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/action.c
@@ -1583,7 +1583,6 @@ hws_action_create_dest_match_range_table(struct mlx5hws_context *ctx,
struct mlx5hws_matcher_action_ste *table_ste;
struct mlx5hws_pool_attr pool_attr = {0};
struct mlx5hws_pool *ste_pool, *stc_pool;
- struct mlx5hws_pool_chunk *ste;
u32 *rtc_0_id, *rtc_1_id;
u32 obj_id;
int ret;
@@ -1613,8 +1612,6 @@ hws_action_create_dest_match_range_table(struct mlx5hws_context *ctx,
rtc_0_id = &table_ste->rtc_0_id;
rtc_1_id = &table_ste->rtc_1_id;
ste_pool = table_ste->pool;
- ste = &table_ste->ste;
- ste->order = 1;
rtc_attr.log_size = 0;
rtc_attr.log_depth = 0;
@@ -1630,7 +1627,7 @@ hws_action_create_dest_match_range_table(struct mlx5hws_context *ctx,
rtc_attr.pd = ctx->pd_num;
rtc_attr.ste_base = obj_id;
- rtc_attr.ste_offset = ste->offset;
+ rtc_attr.ste_offset = 0;
rtc_attr.reparse_mode = mlx5hws_context_get_reparse_mode(ctx);
rtc_attr.table_type = mlx5hws_table_get_res_fw_ft_type(MLX5HWS_TABLE_TYPE_FDB, false);
@@ -1833,7 +1830,6 @@ mlx5hws_action_create_dest_match_range(struct mlx5hws_context *ctx,
stc_attr.action_offset = MLX5HWS_ACTION_OFFSET_HIT;
stc_attr.action_type = MLX5_IFC_STC_ACTION_TYPE_JUMP_TO_STE_TABLE;
stc_attr.reparse_mode = MLX5_IFC_STC_REPARSE_IGNORE;
- stc_attr.ste_table.ste = table_ste->ste;
stc_attr.ste_table.ste_pool = table_ste->pool;
stc_attr.ste_table.match_definer_id = ctx->caps->trivial_match_definer;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/matcher.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/matcher.c
index 95d31fd6c976..54dd5433a3ca 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/matcher.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/matcher.c
@@ -197,22 +197,15 @@ static int hws_matcher_disconnect(struct mlx5hws_matcher *matcher)
static void hws_matcher_set_rtc_attr_sz(struct mlx5hws_matcher *matcher,
struct mlx5hws_cmd_rtc_create_attr *rtc_attr,
- enum mlx5hws_matcher_rtc_type rtc_type,
bool is_mirror)
{
- struct mlx5hws_pool_chunk *ste = &matcher->action_ste.ste;
enum mlx5hws_matcher_flow_src flow_src = matcher->attr.optimize_flow_src;
- bool is_match_rtc = rtc_type == HWS_MATCHER_RTC_TYPE_MATCH;
if ((flow_src == MLX5HWS_MATCHER_FLOW_SRC_VPORT && !is_mirror) ||
(flow_src == MLX5HWS_MATCHER_FLOW_SRC_WIRE && is_mirror)) {
/* Optimize FDB RTC */
rtc_attr->log_size = 0;
rtc_attr->log_depth = 0;
- } else {
- /* Keep original values */
- rtc_attr->log_size = is_match_rtc ? matcher->attr.table.sz_row_log : ste->order;
- rtc_attr->log_depth = is_match_rtc ? matcher->attr.table.sz_col_log : 0;
}
}
@@ -225,8 +218,7 @@ static int hws_matcher_create_rtc(struct mlx5hws_matcher *matcher,
struct mlx5hws_context *ctx = matcher->tbl->ctx;
struct mlx5hws_matcher_action_ste *action_ste;
struct mlx5hws_table *tbl = matcher->tbl;
- struct mlx5hws_pool *ste_pool, *stc_pool;
- struct mlx5hws_pool_chunk *ste;
+ struct mlx5hws_pool *ste_pool;
u32 *rtc_0_id, *rtc_1_id;
u32 obj_id;
int ret;
@@ -236,8 +228,6 @@ static int hws_matcher_create_rtc(struct mlx5hws_matcher *matcher,
rtc_0_id = &matcher->match_ste.rtc_0_id;
rtc_1_id = &matcher->match_ste.rtc_1_id;
ste_pool = matcher->match_ste.pool;
- ste = &matcher->match_ste.ste;
- ste->order = attr->table.sz_col_log + attr->table.sz_row_log;
rtc_attr.log_size = attr->table.sz_row_log;
rtc_attr.log_depth = attr->table.sz_col_log;
@@ -273,16 +263,15 @@ static int hws_matcher_create_rtc(struct mlx5hws_matcher *matcher,
rtc_0_id = &action_ste->rtc_0_id;
rtc_1_id = &action_ste->rtc_1_id;
ste_pool = action_ste->pool;
- ste = &action_ste->ste;
/* Action RTC size calculation:
* log((max number of rules in matcher) *
* (max number of action STEs per rule) *
* (2 to support writing new STEs for update rule))
*/
- ste->order = ilog2(roundup_pow_of_two(action_ste->max_stes)) +
- attr->table.sz_row_log +
- MLX5HWS_MATCHER_ACTION_RTC_UPDATE_MULT;
- rtc_attr.log_size = ste->order;
+ rtc_attr.log_size =
+ ilog2(roundup_pow_of_two(action_ste->max_stes)) +
+ attr->table.sz_row_log +
+ MLX5HWS_MATCHER_ACTION_RTC_UPDATE_MULT;
rtc_attr.log_depth = 0;
rtc_attr.update_index_mode = MLX5_IFC_RTC_STE_UPDATE_MODE_BY_OFFSET;
/* The action STEs use the default always hit definer */
@@ -300,21 +289,20 @@ static int hws_matcher_create_rtc(struct mlx5hws_matcher *matcher,
rtc_attr.pd = ctx->pd_num;
rtc_attr.ste_base = obj_id;
- rtc_attr.ste_offset = ste->offset;
+ rtc_attr.ste_offset = 0;
rtc_attr.reparse_mode = mlx5hws_context_get_reparse_mode(ctx);
rtc_attr.table_type = mlx5hws_table_get_res_fw_ft_type(tbl->type, false);
- hws_matcher_set_rtc_attr_sz(matcher, &rtc_attr, rtc_type, false);
+ hws_matcher_set_rtc_attr_sz(matcher, &rtc_attr, false);
/* STC is a single resource (obj_id), use any STC for the ID */
- stc_pool = ctx->stc_pool;
- obj_id = mlx5hws_pool_get_base_id(stc_pool);
+ obj_id = mlx5hws_pool_get_base_id(ctx->stc_pool);
rtc_attr.stc_base = obj_id;
ret = mlx5hws_cmd_rtc_create(ctx->mdev, &rtc_attr, rtc_0_id);
if (ret) {
mlx5hws_err(ctx, "Failed to create matcher RTC of type %s",
hws_matcher_rtc_type_to_str(rtc_type));
- goto free_ste;
+ return ret;
}
if (tbl->type == MLX5HWS_TABLE_TYPE_FDB) {
@@ -322,9 +310,9 @@ static int hws_matcher_create_rtc(struct mlx5hws_matcher *matcher,
rtc_attr.ste_base = obj_id;
rtc_attr.table_type = mlx5hws_table_get_res_fw_ft_type(tbl->type, true);
- obj_id = mlx5hws_pool_get_base_mirror_id(stc_pool);
+ obj_id = mlx5hws_pool_get_base_mirror_id(ctx->stc_pool);
rtc_attr.stc_base = obj_id;
- hws_matcher_set_rtc_attr_sz(matcher, &rtc_attr, rtc_type, true);
+ hws_matcher_set_rtc_attr_sz(matcher, &rtc_attr, true);
ret = mlx5hws_cmd_rtc_create(ctx->mdev, &rtc_attr, rtc_1_id);
if (ret) {
@@ -338,16 +326,12 @@ static int hws_matcher_create_rtc(struct mlx5hws_matcher *matcher,
destroy_rtc_0:
mlx5hws_cmd_rtc_destroy(ctx->mdev, *rtc_0_id);
-free_ste:
- if (rtc_type == HWS_MATCHER_RTC_TYPE_MATCH)
- mlx5hws_pool_chunk_free(ste_pool, ste);
return ret;
}
static void hws_matcher_destroy_rtc(struct mlx5hws_matcher *matcher,
enum mlx5hws_matcher_rtc_type rtc_type)
{
- struct mlx5hws_matcher_action_ste *action_ste;
struct mlx5hws_table *tbl = matcher->tbl;
u32 rtc_0_id, rtc_1_id;
@@ -357,18 +341,17 @@ static void hws_matcher_destroy_rtc(struct mlx5hws_matcher *matcher,
rtc_1_id = matcher->match_ste.rtc_1_id;
break;
case HWS_MATCHER_RTC_TYPE_STE_ARRAY:
- action_ste = &matcher->action_ste;
- rtc_0_id = action_ste->rtc_0_id;
- rtc_1_id = action_ste->rtc_1_id;
+ rtc_0_id = matcher->action_ste.rtc_0_id;
+ rtc_1_id = matcher->action_ste.rtc_1_id;
break;
default:
return;
}
if (tbl->type == MLX5HWS_TABLE_TYPE_FDB)
- mlx5hws_cmd_rtc_destroy(matcher->tbl->ctx->mdev, rtc_1_id);
+ mlx5hws_cmd_rtc_destroy(tbl->ctx->mdev, rtc_1_id);
- mlx5hws_cmd_rtc_destroy(matcher->tbl->ctx->mdev, rtc_0_id);
+ mlx5hws_cmd_rtc_destroy(tbl->ctx->mdev, rtc_0_id);
}
static int
@@ -564,7 +547,6 @@ static int hws_matcher_bind_at(struct mlx5hws_matcher *matcher)
stc_attr.action_offset = MLX5HWS_ACTION_OFFSET_HIT;
stc_attr.action_type = MLX5_IFC_STC_ACTION_TYPE_JUMP_TO_STE_TABLE;
stc_attr.reparse_mode = MLX5_IFC_STC_REPARSE_IGNORE;
- stc_attr.ste_table.ste = action_ste->ste;
stc_attr.ste_table.ste_pool = action_ste->pool;
stc_attr.ste_table.match_definer_id = ctx->caps->trivial_match_definer;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/matcher.h b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/matcher.h
index 20b32012c418..0450b6175ac9 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/matcher.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/matcher.h
@@ -45,14 +45,12 @@ struct mlx5hws_match_template {
};
struct mlx5hws_matcher_match_ste {
- struct mlx5hws_pool_chunk ste;
u32 rtc_0_id;
u32 rtc_1_id;
struct mlx5hws_pool *pool;
};
struct mlx5hws_matcher_action_ste {
- struct mlx5hws_pool_chunk ste;
struct mlx5hws_pool_chunk stc;
u32 rtc_0_id;
u32 rtc_1_id;
--
2.31.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH net-next 05/12] net/mlx5: HWS, Cleanup after pool refactoring
2025-04-08 14:00 ` [PATCH net-next 05/12] net/mlx5: HWS, Cleanup after pool refactoring Tariq Toukan
@ 2025-04-09 21:23 ` Michal Kubiak
2025-04-10 9:39 ` Vlad Dogaru
0 siblings, 1 reply; 32+ messages in thread
From: Michal Kubiak @ 2025-04-09 21:23 UTC (permalink / raw)
To: Tariq Toukan
Cc: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet,
Andrew Lunn, Gal Pressman, Leon Romanovsky, Saeed Mahameed,
Leon Romanovsky, netdev, linux-rdma, linux-kernel, Moshe Shemesh,
Mark Bloch, Vlad Dogaru, Yevgeny Kliteynik
On Tue, Apr 08, 2025 at 05:00:49PM +0300, Tariq Toukan wrote:
> From: Vlad Dogaru <vdogaru@nvidia.com>
>
> Remove members which are now no longer used. In fact, many of the
> `struct mlx5hws_pool_chunk` were not even written to beyond being
> initialized, but they were used in various internals.
>
> Also cleanup some local variables which made more sense when the API was
> thicker.
>
> Signed-off-by: Vlad Dogaru <vdogaru@nvidia.com>
> Reviewed-by: Yevgeny Kliteynik <kliteyn@nvidia.com>
> Reviewed-by: Mark Bloch <mbloch@nvidia.com>
> Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
> ---
> .../mellanox/mlx5/core/steering/hws/action.c | 6 +--
> .../mellanox/mlx5/core/steering/hws/matcher.c | 48 ++++++-------------
> .../mellanox/mlx5/core/steering/hws/matcher.h | 2 -
> 3 files changed, 16 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/action.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/action.c
> index 39904b337b81..44b4640b47db 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/action.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/action.c
> @@ -1583,7 +1583,6 @@ hws_action_create_dest_match_range_table(struct mlx5hws_context *ctx,
> struct mlx5hws_matcher_action_ste *table_ste;
> struct mlx5hws_pool_attr pool_attr = {0};
> struct mlx5hws_pool *ste_pool, *stc_pool;
> - struct mlx5hws_pool_chunk *ste;
> u32 *rtc_0_id, *rtc_1_id;
> u32 obj_id;
> int ret;
> @@ -1613,8 +1612,6 @@ hws_action_create_dest_match_range_table(struct mlx5hws_context *ctx,
> rtc_0_id = &table_ste->rtc_0_id;
> rtc_1_id = &table_ste->rtc_1_id;
> ste_pool = table_ste->pool;
> - ste = &table_ste->ste;
> - ste->order = 1;
>
> rtc_attr.log_size = 0;
> rtc_attr.log_depth = 0;
> @@ -1630,7 +1627,7 @@ hws_action_create_dest_match_range_table(struct mlx5hws_context *ctx,
>
> rtc_attr.pd = ctx->pd_num;
> rtc_attr.ste_base = obj_id;
> - rtc_attr.ste_offset = ste->offset;
> + rtc_attr.ste_offset = 0;
Is the `rtc_attr.ste_offset` member still needed? Can it be removed from
the "cmd.h" header? It's always zero right now, isn't it?
> rtc_attr.reparse_mode = mlx5hws_context_get_reparse_mode(ctx);
> rtc_attr.table_type = mlx5hws_table_get_res_fw_ft_type(MLX5HWS_TABLE_TYPE_FDB, false);
>
[...]
> @@ -300,21 +289,20 @@ static int hws_matcher_create_rtc(struct mlx5hws_matcher *matcher,
>
> rtc_attr.pd = ctx->pd_num;
> rtc_attr.ste_base = obj_id;
> - rtc_attr.ste_offset = ste->offset;
> + rtc_attr.ste_offset = 0;
Same question here.
Thanks,
Michal
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH net-next 05/12] net/mlx5: HWS, Cleanup after pool refactoring
2025-04-09 21:23 ` Michal Kubiak
@ 2025-04-10 9:39 ` Vlad Dogaru
0 siblings, 0 replies; 32+ messages in thread
From: Vlad Dogaru @ 2025-04-10 9:39 UTC (permalink / raw)
To: Michal Kubiak
Cc: Tariq Toukan, David S. Miller, Jakub Kicinski, Paolo Abeni,
Eric Dumazet, Andrew Lunn, Gal Pressman, Leon Romanovsky,
Saeed Mahameed, Leon Romanovsky, netdev, linux-rdma, linux-kernel,
Moshe Shemesh, Mark Bloch, Yevgeny Kliteynik
On Wed, Apr 09, 2025 at 11:23:04PM +0200, Michal Kubiak wrote:
> On Tue, Apr 08, 2025 at 05:00:49PM +0300, Tariq Toukan wrote:
> > From: Vlad Dogaru <vdogaru@nvidia.com>
> >
> > Remove members which are now no longer used. In fact, many of the
> > `struct mlx5hws_pool_chunk` were not even written to beyond being
> > initialized, but they were used in various internals.
> >
> > Also cleanup some local variables which made more sense when the API was
> > thicker.
> >
> > Signed-off-by: Vlad Dogaru <vdogaru@nvidia.com>
> > Reviewed-by: Yevgeny Kliteynik <kliteyn@nvidia.com>
> > Reviewed-by: Mark Bloch <mbloch@nvidia.com>
> > Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
> > ---
> > .../mellanox/mlx5/core/steering/hws/action.c | 6 +--
> > .../mellanox/mlx5/core/steering/hws/matcher.c | 48 ++++++-------------
> > .../mellanox/mlx5/core/steering/hws/matcher.h | 2 -
> > 3 files changed, 16 insertions(+), 40 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/action.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/action.c
> > index 39904b337b81..44b4640b47db 100644
> > --- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/action.c
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/action.c
> > @@ -1583,7 +1583,6 @@ hws_action_create_dest_match_range_table(struct mlx5hws_context *ctx,
> > struct mlx5hws_matcher_action_ste *table_ste;
> > struct mlx5hws_pool_attr pool_attr = {0};
> > struct mlx5hws_pool *ste_pool, *stc_pool;
> > - struct mlx5hws_pool_chunk *ste;
> > u32 *rtc_0_id, *rtc_1_id;
> > u32 obj_id;
> > int ret;
> > @@ -1613,8 +1612,6 @@ hws_action_create_dest_match_range_table(struct mlx5hws_context *ctx,
> > rtc_0_id = &table_ste->rtc_0_id;
> > rtc_1_id = &table_ste->rtc_1_id;
> > ste_pool = table_ste->pool;
> > - ste = &table_ste->ste;
> > - ste->order = 1;
> >
> > rtc_attr.log_size = 0;
> > rtc_attr.log_depth = 0;
> > @@ -1630,7 +1627,7 @@ hws_action_create_dest_match_range_table(struct mlx5hws_context *ctx,
> >
> > rtc_attr.pd = ctx->pd_num;
> > rtc_attr.ste_base = obj_id;
> > - rtc_attr.ste_offset = ste->offset;
> > + rtc_attr.ste_offset = 0;
>
> Is the `rtc_attr.ste_offset` member still needed? Can it be removed from
> the "cmd.h" header? It's always zero right now, isn't it?
That's right, nice catch. Will fix up in v2.
Thanks,
Vlad
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH net-next 06/12] net/mlx5: HWS, Add fullness tracking to pool
2025-04-08 14:00 [PATCH net-next 00/12] net/mlx5: HWS, Refactor action STE handling Tariq Toukan
` (4 preceding siblings ...)
2025-04-08 14:00 ` [PATCH net-next 05/12] net/mlx5: HWS, Cleanup after pool refactoring Tariq Toukan
@ 2025-04-08 14:00 ` Tariq Toukan
2025-04-10 11:38 ` Michal Kubiak
2025-04-08 14:00 ` [PATCH net-next 07/12] net/mlx5: HWS, Fix pool size optimization Tariq Toukan
` (5 subsequent siblings)
11 siblings, 1 reply; 32+ messages in thread
From: Tariq Toukan @ 2025-04-08 14:00 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet,
Andrew Lunn
Cc: Gal Pressman, Leon Romanovsky, Saeed Mahameed, Leon Romanovsky,
Tariq Toukan, netdev, linux-rdma, linux-kernel, Moshe Shemesh,
Mark Bloch, Vlad Dogaru, Yevgeny Kliteynik
From: Vlad Dogaru <vdogaru@nvidia.com>
Future users will need to query whether a pool is empty.
Signed-off-by: Vlad Dogaru <vdogaru@nvidia.com>
Reviewed-by: Yevgeny Kliteynik <kliteyn@nvidia.com>
Reviewed-by: Mark Bloch <mbloch@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
.../mellanox/mlx5/core/steering/hws/pool.c | 7 ++++++
.../mellanox/mlx5/core/steering/hws/pool.h | 25 +++++++++++++++++++
2 files changed, 32 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pool.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pool.c
index 270b333faab3..26d85fe3c417 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pool.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pool.c
@@ -324,6 +324,8 @@ int mlx5hws_pool_chunk_alloc(struct mlx5hws_pool *pool,
mutex_lock(&pool->lock);
ret = pool->p_get_chunk(pool, chunk);
+ if (ret == 0)
+ pool->available_elems -= 1 << chunk->order;
mutex_unlock(&pool->lock);
return ret;
@@ -334,6 +336,7 @@ void mlx5hws_pool_chunk_free(struct mlx5hws_pool *pool,
{
mutex_lock(&pool->lock);
pool->p_put_chunk(pool, chunk);
+ pool->available_elems += 1 << chunk->order;
mutex_unlock(&pool->lock);
}
@@ -360,6 +363,7 @@ mlx5hws_pool_create(struct mlx5hws_context *ctx, struct mlx5hws_pool_attr *pool_
res_db_type = MLX5HWS_POOL_DB_TYPE_BITMAP;
pool->alloc_log_sz = pool_attr->alloc_log_sz;
+ pool->available_elems = 1 << pool_attr->alloc_log_sz;
if (hws_pool_db_init(pool, res_db_type))
goto free_pool;
@@ -377,6 +381,9 @@ void mlx5hws_pool_destroy(struct mlx5hws_pool *pool)
{
mutex_destroy(&pool->lock);
+ if (pool->available_elems != 1 << pool->alloc_log_sz)
+ mlx5hws_err(pool->ctx, "Attempting to destroy non-empty pool\n");
+
if (pool->resource)
hws_pool_resource_free(pool);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pool.h b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pool.h
index 9a781a87f097..c82760d53e1a 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pool.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pool.h
@@ -71,6 +71,7 @@ struct mlx5hws_pool {
enum mlx5hws_pool_flags flags;
struct mutex lock; /* protect the pool */
size_t alloc_log_sz;
+ size_t available_elems;
enum mlx5hws_table_type tbl_type;
enum mlx5hws_pool_optimize opt_type;
struct mlx5hws_pool_resource *resource;
@@ -103,4 +104,28 @@ static inline u32 mlx5hws_pool_get_base_mirror_id(struct mlx5hws_pool *pool)
{
return pool->mirror_resource->base_id;
}
+
+static inline bool
+mlx5hws_pool_empty(struct mlx5hws_pool *pool)
+{
+ bool ret;
+
+ mutex_lock(&pool->lock);
+ ret = pool->available_elems == 0;
+ mutex_unlock(&pool->lock);
+
+ return ret;
+}
+
+static inline bool
+mlx5hws_pool_full(struct mlx5hws_pool *pool)
+{
+ bool ret;
+
+ mutex_lock(&pool->lock);
+ ret = pool->available_elems == (1 << pool->alloc_log_sz);
+ mutex_unlock(&pool->lock);
+
+ return ret;
+}
#endif /* MLX5HWS_POOL_H_ */
--
2.31.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH net-next 06/12] net/mlx5: HWS, Add fullness tracking to pool
2025-04-08 14:00 ` [PATCH net-next 06/12] net/mlx5: HWS, Add fullness tracking to pool Tariq Toukan
@ 2025-04-10 11:38 ` Michal Kubiak
0 siblings, 0 replies; 32+ messages in thread
From: Michal Kubiak @ 2025-04-10 11:38 UTC (permalink / raw)
To: Tariq Toukan
Cc: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet,
Andrew Lunn, Gal Pressman, Leon Romanovsky, Saeed Mahameed,
Leon Romanovsky, netdev, linux-rdma, linux-kernel, Moshe Shemesh,
Mark Bloch, Vlad Dogaru, Yevgeny Kliteynik
On Tue, Apr 08, 2025 at 05:00:50PM +0300, Tariq Toukan wrote:
> From: Vlad Dogaru <vdogaru@nvidia.com>
>
> Future users will need to query whether a pool is empty.
>
> Signed-off-by: Vlad Dogaru <vdogaru@nvidia.com>
> Reviewed-by: Yevgeny Kliteynik <kliteyn@nvidia.com>
> Reviewed-by: Mark Bloch <mbloch@nvidia.com>
> Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
> ---
> .../mellanox/mlx5/core/steering/hws/pool.c | 7 ++++++
> .../mellanox/mlx5/core/steering/hws/pool.h | 25 +++++++++++++++++++
> 2 files changed, 32 insertions(+)
>
Thanks,
Reviewed-by: Michal Kubiak <michal.kubiak@intel.com>
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH net-next 07/12] net/mlx5: HWS, Fix pool size optimization
2025-04-08 14:00 [PATCH net-next 00/12] net/mlx5: HWS, Refactor action STE handling Tariq Toukan
` (5 preceding siblings ...)
2025-04-08 14:00 ` [PATCH net-next 06/12] net/mlx5: HWS, Add fullness tracking to pool Tariq Toukan
@ 2025-04-08 14:00 ` Tariq Toukan
2025-04-10 12:21 ` Michal Kubiak
2025-04-08 14:00 ` [PATCH net-next 08/12] net/mlx5: HWS, Implement action STE pool Tariq Toukan
` (4 subsequent siblings)
11 siblings, 1 reply; 32+ messages in thread
From: Tariq Toukan @ 2025-04-08 14:00 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet,
Andrew Lunn
Cc: Gal Pressman, Leon Romanovsky, Saeed Mahameed, Leon Romanovsky,
Tariq Toukan, netdev, linux-rdma, linux-kernel, Moshe Shemesh,
Mark Bloch, Vlad Dogaru, Yevgeny Kliteynik
From: Vlad Dogaru <vdogaru@nvidia.com>
The optimization to create a size-one STE range for the unused direction
was broken. The hardware prevents us from creating RTCs over unallocated
STE space, so the only reason this has worked so far is because the
optimization was never used.
Signed-off-by: Vlad Dogaru <vdogaru@nvidia.com>
Reviewed-by: Yevgeny Kliteynik <kliteyn@nvidia.com>
Reviewed-by: Mark Bloch <mbloch@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pool.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pool.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pool.c
index 26d85fe3c417..7e37d6e9eb83 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pool.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pool.c
@@ -80,7 +80,7 @@ static int hws_pool_resource_alloc(struct mlx5hws_pool *pool)
u32 fw_ft_type, opt_log_range;
fw_ft_type = mlx5hws_table_get_res_fw_ft_type(pool->tbl_type, false);
- opt_log_range = pool->opt_type == MLX5HWS_POOL_OPTIMIZE_ORIG ?
+ opt_log_range = pool->opt_type == MLX5HWS_POOL_OPTIMIZE_MIRROR ?
0 : pool->alloc_log_sz;
resource = hws_pool_create_one_resource(pool, opt_log_range, fw_ft_type);
if (!resource) {
@@ -94,7 +94,7 @@ static int hws_pool_resource_alloc(struct mlx5hws_pool *pool)
struct mlx5hws_pool_resource *mirror_resource;
fw_ft_type = mlx5hws_table_get_res_fw_ft_type(pool->tbl_type, true);
- opt_log_range = pool->opt_type == MLX5HWS_POOL_OPTIMIZE_MIRROR ?
+ opt_log_range = pool->opt_type == MLX5HWS_POOL_OPTIMIZE_ORIG ?
0 : pool->alloc_log_sz;
mirror_resource = hws_pool_create_one_resource(pool, opt_log_range, fw_ft_type);
if (!mirror_resource) {
--
2.31.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH net-next 07/12] net/mlx5: HWS, Fix pool size optimization
2025-04-08 14:00 ` [PATCH net-next 07/12] net/mlx5: HWS, Fix pool size optimization Tariq Toukan
@ 2025-04-10 12:21 ` Michal Kubiak
2025-04-10 15:45 ` Vlad Dogaru
0 siblings, 1 reply; 32+ messages in thread
From: Michal Kubiak @ 2025-04-10 12:21 UTC (permalink / raw)
To: Tariq Toukan
Cc: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet,
Andrew Lunn, Gal Pressman, Leon Romanovsky, Saeed Mahameed,
Leon Romanovsky, netdev, linux-rdma, linux-kernel, Moshe Shemesh,
Mark Bloch, Vlad Dogaru, Yevgeny Kliteynik
On Tue, Apr 08, 2025 at 05:00:51PM +0300, Tariq Toukan wrote:
> From: Vlad Dogaru <vdogaru@nvidia.com>
>
> The optimization to create a size-one STE range for the unused direction
> was broken. The hardware prevents us from creating RTCs over unallocated
> STE space, so the only reason this has worked so far is because the
> optimization was never used.
>
Is there any chance that the optimization can be used (enabled) by
someone on previous kernels? If so, maybe the patch is a better candidate
for the "net" tree?
Thanks,
Michal
> Signed-off-by: Vlad Dogaru <vdogaru@nvidia.com>
> Reviewed-by: Yevgeny Kliteynik <kliteyn@nvidia.com>
> Reviewed-by: Mark Bloch <mbloch@nvidia.com>
> Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
> ---
> drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pool.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pool.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pool.c
> index 26d85fe3c417..7e37d6e9eb83 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pool.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pool.c
> @@ -80,7 +80,7 @@ static int hws_pool_resource_alloc(struct mlx5hws_pool *pool)
> u32 fw_ft_type, opt_log_range;
>
> fw_ft_type = mlx5hws_table_get_res_fw_ft_type(pool->tbl_type, false);
> - opt_log_range = pool->opt_type == MLX5HWS_POOL_OPTIMIZE_ORIG ?
> + opt_log_range = pool->opt_type == MLX5HWS_POOL_OPTIMIZE_MIRROR ?
> 0 : pool->alloc_log_sz;
> resource = hws_pool_create_one_resource(pool, opt_log_range, fw_ft_type);
> if (!resource) {
> @@ -94,7 +94,7 @@ static int hws_pool_resource_alloc(struct mlx5hws_pool *pool)
> struct mlx5hws_pool_resource *mirror_resource;
>
> fw_ft_type = mlx5hws_table_get_res_fw_ft_type(pool->tbl_type, true);
> - opt_log_range = pool->opt_type == MLX5HWS_POOL_OPTIMIZE_MIRROR ?
> + opt_log_range = pool->opt_type == MLX5HWS_POOL_OPTIMIZE_ORIG ?
> 0 : pool->alloc_log_sz;
> mirror_resource = hws_pool_create_one_resource(pool, opt_log_range, fw_ft_type);
> if (!mirror_resource) {
> --
> 2.31.1
>
>
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH net-next 07/12] net/mlx5: HWS, Fix pool size optimization
2025-04-10 12:21 ` Michal Kubiak
@ 2025-04-10 15:45 ` Vlad Dogaru
0 siblings, 0 replies; 32+ messages in thread
From: Vlad Dogaru @ 2025-04-10 15:45 UTC (permalink / raw)
To: Michal Kubiak
Cc: Tariq Toukan, David S. Miller, Jakub Kicinski, Paolo Abeni,
Eric Dumazet, Andrew Lunn, Gal Pressman, Leon Romanovsky,
Saeed Mahameed, Leon Romanovsky, netdev, linux-rdma, linux-kernel,
Moshe Shemesh, Mark Bloch, Yevgeny Kliteynik
On Thu, Apr 10, 2025 at 02:21:21PM +0200, Michal Kubiak wrote:
> On Tue, Apr 08, 2025 at 05:00:51PM +0300, Tariq Toukan wrote:
> > From: Vlad Dogaru <vdogaru@nvidia.com>
> >
> > The optimization to create a size-one STE range for the unused direction
> > was broken. The hardware prevents us from creating RTCs over unallocated
> > STE space, so the only reason this has worked so far is because the
> > optimization was never used.
> >
>
> Is there any chance that the optimization can be used (enabled) by
> someone on previous kernels? If so, maybe the patch is a better candidate
> for the "net" tree?
I strongly doubt it. Hardware steering has only been in the kernel since
6.14 and we are not aware of any users. The default is still to use the
older generation software steering, AFAIK.
Thanks,
Vlad
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH net-next 08/12] net/mlx5: HWS, Implement action STE pool
2025-04-08 14:00 [PATCH net-next 00/12] net/mlx5: HWS, Refactor action STE handling Tariq Toukan
` (6 preceding siblings ...)
2025-04-08 14:00 ` [PATCH net-next 07/12] net/mlx5: HWS, Fix pool size optimization Tariq Toukan
@ 2025-04-08 14:00 ` Tariq Toukan
2025-04-10 15:09 ` Michal Kubiak
2025-04-08 14:00 ` [PATCH net-next 09/12] net/mlx5: HWS, Use the new " Tariq Toukan
` (3 subsequent siblings)
11 siblings, 1 reply; 32+ messages in thread
From: Tariq Toukan @ 2025-04-08 14:00 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet,
Andrew Lunn
Cc: Gal Pressman, Leon Romanovsky, Saeed Mahameed, Leon Romanovsky,
Tariq Toukan, netdev, linux-rdma, linux-kernel, Moshe Shemesh,
Mark Bloch, Vlad Dogaru, Yevgeny Kliteynik
From: Vlad Dogaru <vdogaru@nvidia.com>
Implement a per-queue pool of action STEs that match STEs can link to,
regardless of matcher.
The code relies on hints to optimize whether a given rule is added to
rx-only, tx-only or both. Correspondingly, action STEs need to be added
to different RTC for ingress or egress paths. For rx-and-tx rules, the
current rule implementation dictates that the offsets for a given rule
must be the same in both RTCs.
To avoid wasting STEs, each action STE pool element holds 3 pools:
rx-only, tx-only, and rx-and-tx, corresponding to the possible values of
the pool optimization enum. The implementation then chooses at rule
creation / update which of these elements to allocate from.
Each element holds multiple action STE tables, which wrap an RTC, an STE
range, the logic to buddy-allocate offsets from the range, and an STC
that allows match STEs to point to this table. When allocating offsets
from an element, we iterate through available action STE tables and, if
needed, create a new table.
Similar to the previous implementation, this iteration does not free any
resources. This is implemented in a subsequent patch.
Signed-off-by: Vlad Dogaru <vdogaru@nvidia.com>
Reviewed-by: Yevgeny Kliteynik <kliteyn@nvidia.com>
Reviewed-by: Mark Bloch <mbloch@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
.../net/ethernet/mellanox/mlx5/core/Makefile | 3 +-
.../mlx5/core/steering/hws/action_ste_pool.c | 388 ++++++++++++++++++
.../mlx5/core/steering/hws/action_ste_pool.h | 58 +++
.../mellanox/mlx5/core/steering/hws/context.c | 7 +
.../mellanox/mlx5/core/steering/hws/context.h | 1 +
.../mlx5/core/steering/hws/internal.h | 1 +
.../mellanox/mlx5/core/steering/hws/pool.h | 1 +
7 files changed, 458 insertions(+), 1 deletion(-)
create mode 100644 drivers/net/ethernet/mellanox/mlx5/core/steering/hws/action_ste_pool.c
create mode 100644 drivers/net/ethernet/mellanox/mlx5/core/steering/hws/action_ste_pool.h
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/Makefile b/drivers/net/ethernet/mellanox/mlx5/core/Makefile
index 568bbe5f83f5..d292e6a9e22c 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/Makefile
+++ b/drivers/net/ethernet/mellanox/mlx5/core/Makefile
@@ -154,7 +154,8 @@ mlx5_core-$(CONFIG_MLX5_HW_STEERING) += steering/hws/cmd.o \
steering/hws/vport.o \
steering/hws/bwc_complex.o \
steering/hws/fs_hws_pools.o \
- steering/hws/fs_hws.o
+ steering/hws/fs_hws.o \
+ steering/hws/action_ste_pool.o
#
# SF device
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/action_ste_pool.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/action_ste_pool.c
new file mode 100644
index 000000000000..e5c453ec65b3
--- /dev/null
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/action_ste_pool.c
@@ -0,0 +1,388 @@
+// SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB
+/* Copyright (c) 2025 NVIDIA Corporation & Affiliates */
+
+#include "internal.h"
+
+static const char *
+hws_pool_opt_to_str(enum mlx5hws_pool_optimize opt)
+{
+ switch (opt) {
+ case MLX5HWS_POOL_OPTIMIZE_NONE:
+ return "rx-and-tx";
+ case MLX5HWS_POOL_OPTIMIZE_ORIG:
+ return "rx-only";
+ case MLX5HWS_POOL_OPTIMIZE_MIRROR:
+ return "tx-only";
+ default:
+ return "unknown";
+ }
+}
+
+static int
+hws_action_ste_table_create_pool(struct mlx5hws_context *ctx,
+ struct mlx5hws_action_ste_table *action_tbl,
+ enum mlx5hws_pool_optimize opt, size_t log_sz)
+{
+ struct mlx5hws_pool_attr pool_attr = { 0 };
+
+ pool_attr.pool_type = MLX5HWS_POOL_TYPE_STE;
+ pool_attr.table_type = MLX5HWS_TABLE_TYPE_FDB;
+ pool_attr.flags = MLX5HWS_POOL_FLAG_BUDDY;
+ pool_attr.opt_type = opt;
+ pool_attr.alloc_log_sz = log_sz;
+
+ action_tbl->pool = mlx5hws_pool_create(ctx, &pool_attr);
+ if (!action_tbl->pool) {
+ mlx5hws_err(ctx, "Failed to allocate STE pool\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int hws_action_ste_table_create_single_rtc(
+ struct mlx5hws_context *ctx,
+ struct mlx5hws_action_ste_table *action_tbl,
+ enum mlx5hws_pool_optimize opt, size_t log_sz, bool tx)
+{
+ struct mlx5hws_cmd_rtc_create_attr rtc_attr = { 0 };
+ u32 *rtc_id;
+
+ rtc_attr.log_depth = 0;
+ rtc_attr.update_index_mode = MLX5_IFC_RTC_STE_UPDATE_MODE_BY_OFFSET;
+ /* Action STEs use the default always hit definer. */
+ rtc_attr.match_definer_0 = ctx->caps->trivial_match_definer;
+ rtc_attr.is_frst_jumbo = false;
+ rtc_attr.miss_ft_id = 0;
+ rtc_attr.pd = ctx->pd_num;
+ rtc_attr.ste_offset = 0;
+ rtc_attr.reparse_mode = mlx5hws_context_get_reparse_mode(ctx);
+
+ if (tx) {
+ rtc_attr.table_type = FS_FT_FDB_TX;
+ rtc_attr.ste_base =
+ mlx5hws_pool_get_base_mirror_id(action_tbl->pool);
+ rtc_attr.stc_base =
+ mlx5hws_pool_get_base_mirror_id(ctx->stc_pool);
+ rtc_attr.log_size =
+ opt == MLX5HWS_POOL_OPTIMIZE_ORIG ? 0 : log_sz;
+ rtc_id = &action_tbl->rtc_1_id;
+ } else {
+ rtc_attr.table_type = FS_FT_FDB_RX;
+ rtc_attr.ste_base = mlx5hws_pool_get_base_id(action_tbl->pool);
+ rtc_attr.stc_base = mlx5hws_pool_get_base_id(ctx->stc_pool);
+ rtc_attr.log_size =
+ opt == MLX5HWS_POOL_OPTIMIZE_MIRROR ? 0 : log_sz;
+ rtc_id = &action_tbl->rtc_0_id;
+ }
+
+ return mlx5hws_cmd_rtc_create(ctx->mdev, &rtc_attr, rtc_id);
+}
+
+static int
+hws_action_ste_table_create_rtcs(struct mlx5hws_context *ctx,
+ struct mlx5hws_action_ste_table *action_tbl,
+ enum mlx5hws_pool_optimize opt, size_t log_sz)
+{
+ int err;
+
+ err = hws_action_ste_table_create_single_rtc(ctx, action_tbl, opt,
+ log_sz, false);
+ if (err)
+ return err;
+
+ err = hws_action_ste_table_create_single_rtc(ctx, action_tbl, opt,
+ log_sz, true);
+ if (err) {
+ mlx5hws_cmd_rtc_destroy(ctx->mdev, action_tbl->rtc_0_id);
+ return err;
+ }
+
+ return 0;
+}
+
+static void
+hws_action_ste_table_destroy_rtcs(struct mlx5hws_action_ste_table *action_tbl)
+{
+ mlx5hws_cmd_rtc_destroy(action_tbl->pool->ctx->mdev,
+ action_tbl->rtc_1_id);
+ mlx5hws_cmd_rtc_destroy(action_tbl->pool->ctx->mdev,
+ action_tbl->rtc_0_id);
+}
+
+static int
+hws_action_ste_table_create_stc(struct mlx5hws_context *ctx,
+ struct mlx5hws_action_ste_table *action_tbl)
+{
+ struct mlx5hws_cmd_stc_modify_attr stc_attr = { 0 };
+
+ stc_attr.action_offset = MLX5HWS_ACTION_OFFSET_HIT;
+ stc_attr.action_type = MLX5_IFC_STC_ACTION_TYPE_JUMP_TO_STE_TABLE;
+ stc_attr.reparse_mode = MLX5_IFC_STC_REPARSE_IGNORE;
+ stc_attr.ste_table.ste_pool = action_tbl->pool;
+ stc_attr.ste_table.match_definer_id = ctx->caps->trivial_match_definer;
+
+ return mlx5hws_action_alloc_single_stc(ctx, &stc_attr,
+ MLX5HWS_TABLE_TYPE_FDB,
+ &action_tbl->stc);
+}
+
+static struct mlx5hws_action_ste_table *
+hws_action_ste_table_alloc(struct mlx5hws_action_ste_pool_element *parent_elem)
+{
+ enum mlx5hws_pool_optimize opt = parent_elem->opt;
+ struct mlx5hws_context *ctx = parent_elem->ctx;
+ struct mlx5hws_action_ste_table *action_tbl;
+ size_t log_sz;
+ int err;
+
+ log_sz = min(parent_elem->log_sz ?
+ parent_elem->log_sz +
+ MLX5HWS_ACTION_STE_TABLE_STEP_LOG_SZ :
+ MLX5HWS_ACTION_STE_TABLE_INIT_LOG_SZ,
+ MLX5HWS_ACTION_STE_TABLE_MAX_LOG_SZ);
+
+ action_tbl = kzalloc(sizeof(*action_tbl), GFP_KERNEL);
+ if (!action_tbl)
+ return ERR_PTR(-ENOMEM);
+
+ err = hws_action_ste_table_create_pool(ctx, action_tbl, opt, log_sz);
+ if (err)
+ goto free_tbl;
+
+ err = hws_action_ste_table_create_rtcs(ctx, action_tbl, opt, log_sz);
+ if (err)
+ goto destroy_pool;
+
+ err = hws_action_ste_table_create_stc(ctx, action_tbl);
+ if (err)
+ goto destroy_rtcs;
+
+ action_tbl->parent_elem = parent_elem;
+ INIT_LIST_HEAD(&action_tbl->list_node);
+ list_add(&action_tbl->list_node, &parent_elem->available);
+ parent_elem->log_sz = log_sz;
+
+ mlx5hws_dbg(ctx,
+ "Allocated %s action STE table log_sz %zu; STEs (%d, %d); RTCs (%d, %d); STC %d\n",
+ hws_pool_opt_to_str(opt), log_sz,
+ mlx5hws_pool_get_base_id(action_tbl->pool),
+ mlx5hws_pool_get_base_mirror_id(action_tbl->pool),
+ action_tbl->rtc_0_id, action_tbl->rtc_1_id,
+ action_tbl->stc.offset);
+
+ return action_tbl;
+
+destroy_rtcs:
+ hws_action_ste_table_destroy_rtcs(action_tbl);
+destroy_pool:
+ mlx5hws_pool_destroy(action_tbl->pool);
+free_tbl:
+ kfree(action_tbl);
+
+ return ERR_PTR(err);
+}
+
+static void
+hws_action_ste_table_destroy(struct mlx5hws_action_ste_table *action_tbl)
+{
+ struct mlx5hws_context *ctx = action_tbl->parent_elem->ctx;
+
+ mlx5hws_dbg(ctx,
+ "Destroying %s action STE table: STEs (%d, %d); RTCs (%d, %d); STC %d\n",
+ hws_pool_opt_to_str(action_tbl->parent_elem->opt),
+ mlx5hws_pool_get_base_id(action_tbl->pool),
+ mlx5hws_pool_get_base_mirror_id(action_tbl->pool),
+ action_tbl->rtc_0_id, action_tbl->rtc_1_id,
+ action_tbl->stc.offset);
+
+ mlx5hws_action_free_single_stc(ctx, MLX5HWS_TABLE_TYPE_FDB,
+ &action_tbl->stc);
+ hws_action_ste_table_destroy_rtcs(action_tbl);
+ mlx5hws_pool_destroy(action_tbl->pool);
+
+ list_del(&action_tbl->list_node);
+ kfree(action_tbl);
+}
+
+static int
+hws_action_ste_pool_element_init(struct mlx5hws_context *ctx,
+ struct mlx5hws_action_ste_pool_element *elem,
+ enum mlx5hws_pool_optimize opt)
+{
+ elem->ctx = ctx;
+ elem->opt = opt;
+ INIT_LIST_HEAD(&elem->available);
+ INIT_LIST_HEAD(&elem->full);
+
+ return 0;
+}
+
+static void hws_action_ste_pool_element_destroy(
+ struct mlx5hws_action_ste_pool_element *elem)
+{
+ struct mlx5hws_action_ste_table *action_tbl, *p;
+
+ /* This should be empty, but attempt to free its elements anyway. */
+ list_for_each_entry_safe(action_tbl, p, &elem->full, list_node)
+ hws_action_ste_table_destroy(action_tbl);
+
+ list_for_each_entry_safe(action_tbl, p, &elem->available, list_node)
+ hws_action_ste_table_destroy(action_tbl);
+}
+
+static int hws_action_ste_pool_init(struct mlx5hws_context *ctx,
+ struct mlx5hws_action_ste_pool *pool)
+{
+ enum mlx5hws_pool_optimize opt;
+ int err;
+
+ /* Rules which are added for both RX and TX must use the same action STE
+ * indices for both. If we were to use a single table, then RX-only and
+ * TX-only rules would waste the unused entries. Thus, we use separate
+ * table sets for the three cases.
+ */
+ for (opt = MLX5HWS_POOL_OPTIMIZE_NONE; opt < MLX5HWS_POOL_OPTIMIZE_MAX;
+ opt++) {
+ err = hws_action_ste_pool_element_init(ctx, &pool->elems[opt],
+ opt);
+ if (err)
+ goto destroy_elems;
+ }
+
+ return 0;
+
+destroy_elems:
+ while (opt-- > MLX5HWS_POOL_OPTIMIZE_NONE)
+ hws_action_ste_pool_element_destroy(&pool->elems[opt]);
+
+ return err;
+}
+
+static void hws_action_ste_pool_destroy(struct mlx5hws_action_ste_pool *pool)
+{
+ int opt;
+
+ for (opt = MLX5HWS_POOL_OPTIMIZE_MAX - 1;
+ opt >= MLX5HWS_POOL_OPTIMIZE_NONE; opt--)
+ hws_action_ste_pool_element_destroy(&pool->elems[opt]);
+}
+
+int mlx5hws_action_ste_pool_init(struct mlx5hws_context *ctx)
+{
+ struct mlx5hws_action_ste_pool *pool;
+ size_t queues = ctx->queues;
+ int i, err;
+
+ pool = kcalloc(queues, sizeof(*pool), GFP_KERNEL);
+ if (!pool)
+ return -ENOMEM;
+
+ for (i = 0; i < queues; i++) {
+ err = hws_action_ste_pool_init(ctx, &pool[i]);
+ if (err)
+ goto free_pool;
+ }
+
+ ctx->action_ste_pool = pool;
+
+ return 0;
+
+free_pool:
+ while (i--)
+ hws_action_ste_pool_destroy(&pool[i]);
+ kfree(pool);
+
+ return err;
+}
+
+void mlx5hws_action_ste_pool_uninit(struct mlx5hws_context *ctx)
+{
+ size_t queues = ctx->queues;
+ int i;
+
+ for (i = 0; i < queues; i++)
+ hws_action_ste_pool_destroy(&ctx->action_ste_pool[i]);
+
+ kfree(ctx->action_ste_pool);
+}
+
+static struct mlx5hws_action_ste_pool_element *
+hws_action_ste_choose_elem(struct mlx5hws_action_ste_pool *pool,
+ bool skip_rx, bool skip_tx)
+{
+ if (skip_rx)
+ return &pool->elems[MLX5HWS_POOL_OPTIMIZE_MIRROR];
+
+ if (skip_tx)
+ return &pool->elems[MLX5HWS_POOL_OPTIMIZE_ORIG];
+
+ return &pool->elems[MLX5HWS_POOL_OPTIMIZE_NONE];
+}
+
+static int
+hws_action_ste_table_chunk_alloc(struct mlx5hws_action_ste_table *action_tbl,
+ struct mlx5hws_action_ste_chunk *chunk)
+{
+ int err;
+
+ err = mlx5hws_pool_chunk_alloc(action_tbl->pool, &chunk->ste);
+ if (err)
+ return err;
+
+ chunk->action_tbl = action_tbl;
+
+ return 0;
+}
+
+int mlx5hws_action_ste_chunk_alloc(struct mlx5hws_action_ste_pool *pool,
+ bool skip_rx, bool skip_tx,
+ struct mlx5hws_action_ste_chunk *chunk)
+{
+ struct mlx5hws_action_ste_pool_element *elem;
+ struct mlx5hws_action_ste_table *action_tbl;
+ bool found;
+ int err;
+
+ if (skip_rx && skip_tx)
+ return -EINVAL;
+
+ elem = hws_action_ste_choose_elem(pool, skip_rx, skip_tx);
+
+ mlx5hws_dbg(elem->ctx,
+ "Allocating action STEs skip_rx %d skip_tx %d order %d\n",
+ skip_rx, skip_tx, chunk->ste.order);
+
+ found = false;
+ list_for_each_entry(action_tbl, &elem->available, list_node) {
+ if (!hws_action_ste_table_chunk_alloc(action_tbl, chunk)) {
+ found = true;
+ break;
+ }
+ }
+
+ if (!found) {
+ action_tbl = hws_action_ste_table_alloc(elem);
+ if (IS_ERR(action_tbl))
+ return PTR_ERR(action_tbl);
+
+ err = hws_action_ste_table_chunk_alloc(action_tbl, chunk);
+ if (err)
+ return err;
+ }
+
+ if (mlx5hws_pool_empty(action_tbl->pool))
+ list_move(&action_tbl->list_node, &elem->full);
+
+ return 0;
+}
+
+void mlx5hws_action_ste_chunk_free(struct mlx5hws_action_ste_chunk *chunk)
+{
+ mlx5hws_dbg(chunk->action_tbl->pool->ctx,
+ "Freeing action STEs offset %d order %d\n",
+ chunk->ste.offset, chunk->ste.order);
+ mlx5hws_pool_chunk_free(chunk->action_tbl->pool, &chunk->ste);
+ list_move(&chunk->action_tbl->list_node,
+ &chunk->action_tbl->parent_elem->available);
+}
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/action_ste_pool.h b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/action_ste_pool.h
new file mode 100644
index 000000000000..2de660a63223
--- /dev/null
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/action_ste_pool.h
@@ -0,0 +1,58 @@
+/* SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB */
+/* Copyright (c) 2025 NVIDIA Corporation & Affiliates */
+
+#ifndef ACTION_STE_POOL_H_
+#define ACTION_STE_POOL_H_
+
+#define MLX5HWS_ACTION_STE_TABLE_INIT_LOG_SZ 10
+#define MLX5HWS_ACTION_STE_TABLE_STEP_LOG_SZ 1
+#define MLX5HWS_ACTION_STE_TABLE_MAX_LOG_SZ 20
+
+struct mlx5hws_action_ste_pool_element;
+
+struct mlx5hws_action_ste_table {
+ struct mlx5hws_action_ste_pool_element *parent_elem;
+ /* Wraps the RTC and STE range for this given action. */
+ struct mlx5hws_pool *pool;
+ /* Match STEs use this STC to jump to this pool's RTC. */
+ struct mlx5hws_pool_chunk stc;
+ u32 rtc_0_id;
+ u32 rtc_1_id;
+ struct list_head list_node;
+};
+
+struct mlx5hws_action_ste_pool_element {
+ struct mlx5hws_context *ctx;
+ size_t log_sz; /* Size of the largest table so far. */
+ enum mlx5hws_pool_optimize opt;
+ struct list_head available;
+ struct list_head full;
+};
+
+/* Central repository of action STEs. The context contains one of these pools
+ * per queue.
+ */
+struct mlx5hws_action_ste_pool {
+ struct mlx5hws_action_ste_pool_element elems[MLX5HWS_POOL_OPTIMIZE_MAX];
+};
+
+/* A chunk of STEs and the table it was allocated from. Used by rules. */
+struct mlx5hws_action_ste_chunk {
+ struct mlx5hws_action_ste_table *action_tbl;
+ struct mlx5hws_pool_chunk ste;
+};
+
+int mlx5hws_action_ste_pool_init(struct mlx5hws_context *ctx);
+
+void mlx5hws_action_ste_pool_uninit(struct mlx5hws_context *ctx);
+
+/* Callers are expected to fill chunk->ste.order. On success, this function
+ * populates chunk->tbl and chunk->ste.offset.
+ */
+int mlx5hws_action_ste_chunk_alloc(struct mlx5hws_action_ste_pool *pool,
+ bool skip_rx, bool skip_tx,
+ struct mlx5hws_action_ste_chunk *chunk);
+
+void mlx5hws_action_ste_chunk_free(struct mlx5hws_action_ste_chunk *chunk);
+
+#endif /* ACTION_STE_POOL_H_ */
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/context.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/context.c
index b7cb736b74d7..428dae869706 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/context.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/context.c
@@ -158,10 +158,16 @@ static int hws_context_init_hws(struct mlx5hws_context *ctx,
if (ret)
goto pools_uninit;
+ ret = mlx5hws_action_ste_pool_init(ctx);
+ if (ret)
+ goto close_queues;
+
INIT_LIST_HEAD(&ctx->tbl_list);
return 0;
+close_queues:
+ mlx5hws_send_queues_close(ctx);
pools_uninit:
hws_context_pools_uninit(ctx);
uninit_pd:
@@ -174,6 +180,7 @@ static void hws_context_uninit_hws(struct mlx5hws_context *ctx)
if (!(ctx->flags & MLX5HWS_CONTEXT_FLAG_HWS_SUPPORT))
return;
+ mlx5hws_action_ste_pool_uninit(ctx);
mlx5hws_send_queues_close(ctx);
hws_context_pools_uninit(ctx);
hws_context_uninit_pd(ctx);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/context.h b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/context.h
index 38c3647444ad..e987e93bbc6e 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/context.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/context.h
@@ -39,6 +39,7 @@ struct mlx5hws_context {
struct mlx5hws_cmd_query_caps *caps;
u32 pd_num;
struct mlx5hws_pool *stc_pool;
+ struct mlx5hws_action_ste_pool *action_ste_pool; /* One per queue */
struct mlx5hws_context_common_res common_res;
struct mlx5hws_pattern_cache *pattern_cache;
struct mlx5hws_definer_cache *definer_cache;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/internal.h b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/internal.h
index 30ccd635b505..21279d503117 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/internal.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/internal.h
@@ -17,6 +17,7 @@
#include "context.h"
#include "table.h"
#include "send.h"
+#include "action_ste_pool.h"
#include "rule.h"
#include "cmd.h"
#include "action.h"
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pool.h b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pool.h
index c82760d53e1a..33e33d5f1fb3 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pool.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pool.h
@@ -33,6 +33,7 @@ enum mlx5hws_pool_optimize {
MLX5HWS_POOL_OPTIMIZE_NONE = 0x0,
MLX5HWS_POOL_OPTIMIZE_ORIG = 0x1,
MLX5HWS_POOL_OPTIMIZE_MIRROR = 0x2,
+ MLX5HWS_POOL_OPTIMIZE_MAX = 0x3,
};
struct mlx5hws_pool_attr {
--
2.31.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH net-next 08/12] net/mlx5: HWS, Implement action STE pool
2025-04-08 14:00 ` [PATCH net-next 08/12] net/mlx5: HWS, Implement action STE pool Tariq Toukan
@ 2025-04-10 15:09 ` Michal Kubiak
0 siblings, 0 replies; 32+ messages in thread
From: Michal Kubiak @ 2025-04-10 15:09 UTC (permalink / raw)
To: Tariq Toukan
Cc: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet,
Andrew Lunn, Gal Pressman, Leon Romanovsky, Saeed Mahameed,
Leon Romanovsky, netdev, linux-rdma, linux-kernel, Moshe Shemesh,
Mark Bloch, Vlad Dogaru, Yevgeny Kliteynik
On Tue, Apr 08, 2025 at 05:00:52PM +0300, Tariq Toukan wrote:
> From: Vlad Dogaru <vdogaru@nvidia.com>
>
> Implement a per-queue pool of action STEs that match STEs can link to,
> regardless of matcher.
>
> The code relies on hints to optimize whether a given rule is added to
> rx-only, tx-only or both. Correspondingly, action STEs need to be added
> to different RTC for ingress or egress paths. For rx-and-tx rules, the
> current rule implementation dictates that the offsets for a given rule
> must be the same in both RTCs.
>
> To avoid wasting STEs, each action STE pool element holds 3 pools:
> rx-only, tx-only, and rx-and-tx, corresponding to the possible values of
> the pool optimization enum. The implementation then chooses at rule
> creation / update which of these elements to allocate from.
>
> Each element holds multiple action STE tables, which wrap an RTC, an STE
> range, the logic to buddy-allocate offsets from the range, and an STC
> that allows match STEs to point to this table. When allocating offsets
> from an element, we iterate through available action STE tables and, if
> needed, create a new table.
>
> Similar to the previous implementation, this iteration does not free any
> resources. This is implemented in a subsequent patch.
>
> Signed-off-by: Vlad Dogaru <vdogaru@nvidia.com>
> Reviewed-by: Yevgeny Kliteynik <kliteyn@nvidia.com>
> Reviewed-by: Mark Bloch <mbloch@nvidia.com>
> Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
> ---
The patch looks OK to me. It corresponds with the idea described in the
cover letter and commit message.
No new issues found.
> +static int hws_action_ste_table_create_single_rtc(
> + struct mlx5hws_context *ctx,
> + struct mlx5hws_action_ste_table *action_tbl,
> + enum mlx5hws_pool_optimize opt, size_t log_sz, bool tx)
> +{
> + struct mlx5hws_cmd_rtc_create_attr rtc_attr = { 0 };
> + u32 *rtc_id;
> +
> + rtc_attr.log_depth = 0;
> + rtc_attr.update_index_mode = MLX5_IFC_RTC_STE_UPDATE_MODE_BY_OFFSET;
> + /* Action STEs use the default always hit definer. */
> + rtc_attr.match_definer_0 = ctx->caps->trivial_match_definer;
> + rtc_attr.is_frst_jumbo = false;
> + rtc_attr.miss_ft_id = 0;
> + rtc_attr.pd = ctx->pd_num;
> + rtc_attr.ste_offset = 0;
As I mentioned in my review for patch #5, it's always zero.
Anyway, you've already said you're going to remove it in v2.
Thanks,
Reviewed-by: Michal Kubiak <michal.kubiak@intel.com>
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH net-next 09/12] net/mlx5: HWS, Use the new action STE pool
2025-04-08 14:00 [PATCH net-next 00/12] net/mlx5: HWS, Refactor action STE handling Tariq Toukan
` (7 preceding siblings ...)
2025-04-08 14:00 ` [PATCH net-next 08/12] net/mlx5: HWS, Implement action STE pool Tariq Toukan
@ 2025-04-08 14:00 ` Tariq Toukan
2025-04-10 16:48 ` Michal Kubiak
2025-04-08 14:00 ` [PATCH net-next 10/12] net/mlx5: HWS, Cleanup matcher action STE table Tariq Toukan
` (2 subsequent siblings)
11 siblings, 1 reply; 32+ messages in thread
From: Tariq Toukan @ 2025-04-08 14:00 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet,
Andrew Lunn
Cc: Gal Pressman, Leon Romanovsky, Saeed Mahameed, Leon Romanovsky,
Tariq Toukan, netdev, linux-rdma, linux-kernel, Moshe Shemesh,
Mark Bloch, Vlad Dogaru, Yevgeny Kliteynik
From: Vlad Dogaru <vdogaru@nvidia.com>
Use the central action STE pool when creating / updating rules.
Signed-off-by: Vlad Dogaru <vdogaru@nvidia.com>
Reviewed-by: Yevgeny Kliteynik <kliteyn@nvidia.com>
Reviewed-by: Mark Bloch <mbloch@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
.../mellanox/mlx5/core/steering/hws/rule.c | 69 ++++++++-----------
.../mellanox/mlx5/core/steering/hws/rule.h | 12 +---
2 files changed, 30 insertions(+), 51 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/rule.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/rule.c
index a27a2d5ffc7b..5b758467ed03 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/rule.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/rule.c
@@ -195,44 +195,30 @@ hws_rule_load_delete_info(struct mlx5hws_rule *rule,
}
}
-static int hws_rule_alloc_action_ste(struct mlx5hws_rule *rule)
+static int mlx5hws_rule_alloc_action_ste(struct mlx5hws_rule *rule,
+ u16 queue_id, bool skip_rx,
+ bool skip_tx)
{
struct mlx5hws_matcher *matcher = rule->matcher;
- struct mlx5hws_matcher_action_ste *action_ste;
- struct mlx5hws_pool_chunk ste = {0};
- int ret;
-
- action_ste = &matcher->action_ste;
- ste.order = ilog2(roundup_pow_of_two(action_ste->max_stes));
- ret = mlx5hws_pool_chunk_alloc(action_ste->pool, &ste);
- if (unlikely(ret)) {
- mlx5hws_err(matcher->tbl->ctx,
- "Failed to allocate STE for rule actions");
- return ret;
- }
-
- rule->action_ste.pool = matcher->action_ste.pool;
- rule->action_ste.num_stes = matcher->action_ste.max_stes;
- rule->action_ste.index = ste.offset;
+ struct mlx5hws_context *ctx = matcher->tbl->ctx;
- return 0;
+ rule->action_ste.ste.order =
+ ilog2(roundup_pow_of_two(matcher->action_ste.max_stes));
+ return mlx5hws_action_ste_chunk_alloc(&ctx->action_ste_pool[queue_id],
+ skip_rx, skip_tx,
+ &rule->action_ste);
}
-void mlx5hws_rule_free_action_ste(struct mlx5hws_rule_action_ste_info *action_ste)
+void mlx5hws_rule_free_action_ste(struct mlx5hws_action_ste_chunk *action_ste)
{
- struct mlx5hws_pool_chunk ste = {0};
-
- if (!action_ste->num_stes)
+ if (!action_ste->action_tbl)
return;
- ste.order = ilog2(roundup_pow_of_two(action_ste->num_stes));
- ste.offset = action_ste->index;
-
/* This release is safe only when the rule match STE was deleted
* (when the rule is being deleted) or replaced with the new STE that
* isn't pointing to old action STEs (when the rule is being updated).
*/
- mlx5hws_pool_chunk_free(action_ste->pool, &ste);
+ mlx5hws_action_ste_chunk_free(action_ste);
}
static void hws_rule_create_init(struct mlx5hws_rule *rule,
@@ -250,22 +236,15 @@ static void hws_rule_create_init(struct mlx5hws_rule *rule,
rule->rtc_0 = 0;
rule->rtc_1 = 0;
- rule->action_ste.pool = NULL;
- rule->action_ste.num_stes = 0;
- rule->action_ste.index = -1;
-
rule->status = MLX5HWS_RULE_STATUS_CREATING;
} else {
rule->status = MLX5HWS_RULE_STATUS_UPDATING;
+ /* Save the old action STE info so we can free it after writing
+ * new action STEs and a corresponding match STE.
+ */
+ rule->old_action_ste = rule->action_ste;
}
- /* Initialize the old action STE info - shallow-copy action_ste.
- * In create flow this will set old_action_ste fields to initial values.
- * In update flow this will save the existing action STE info,
- * so that we will later use it to free old STEs.
- */
- rule->old_action_ste = rule->action_ste;
-
rule->pending_wqes = 0;
/* Init default send STE attributes */
@@ -277,7 +256,6 @@ static void hws_rule_create_init(struct mlx5hws_rule *rule,
/* Init default action apply */
apply->tbl_type = tbl->type;
apply->common_res = &ctx->common_res;
- apply->jump_to_action_stc = matcher->action_ste.stc.offset;
apply->require_dep = 0;
}
@@ -353,17 +331,24 @@ static int hws_rule_create_hws(struct mlx5hws_rule *rule,
if (action_stes) {
/* Allocate action STEs for rules that need more than match STE */
- ret = hws_rule_alloc_action_ste(rule);
+ ret = mlx5hws_rule_alloc_action_ste(rule, attr->queue_id,
+ !!ste_attr.rtc_0,
+ !!ste_attr.rtc_1);
if (ret) {
mlx5hws_err(ctx, "Failed to allocate action memory %d", ret);
mlx5hws_send_abort_new_dep_wqe(queue);
return ret;
}
+ apply.jump_to_action_stc =
+ rule->action_ste.action_tbl->stc.offset;
/* Skip RX/TX based on the dep_wqe init */
- ste_attr.rtc_0 = dep_wqe->rtc_0 ? matcher->action_ste.rtc_0_id : 0;
- ste_attr.rtc_1 = dep_wqe->rtc_1 ? matcher->action_ste.rtc_1_id : 0;
+ ste_attr.rtc_0 = dep_wqe->rtc_0 ?
+ rule->action_ste.action_tbl->rtc_0_id : 0;
+ ste_attr.rtc_1 = dep_wqe->rtc_1 ?
+ rule->action_ste.action_tbl->rtc_1_id : 0;
/* Action STEs are written to a specific index last to first */
- ste_attr.direct_index = rule->action_ste.index + action_stes;
+ ste_attr.direct_index =
+ rule->action_ste.ste.offset + action_stes;
apply.next_direct_idx = ste_attr.direct_index;
} else {
apply.next_direct_idx = 0;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/rule.h b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/rule.h
index b5ee94ac449b..1c47a9c11572 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/rule.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/rule.h
@@ -43,12 +43,6 @@ struct mlx5hws_rule_match_tag {
};
};
-struct mlx5hws_rule_action_ste_info {
- struct mlx5hws_pool *pool;
- int index; /* STE array index */
- u8 num_stes;
-};
-
struct mlx5hws_rule_resize_info {
u32 rtc_0;
u32 rtc_1;
@@ -64,8 +58,8 @@ struct mlx5hws_rule {
struct mlx5hws_rule_match_tag tag;
struct mlx5hws_rule_resize_info *resize_info;
};
- struct mlx5hws_rule_action_ste_info action_ste;
- struct mlx5hws_rule_action_ste_info old_action_ste;
+ struct mlx5hws_action_ste_chunk action_ste;
+ struct mlx5hws_action_ste_chunk old_action_ste;
u32 rtc_0; /* The RTC into which the STE was inserted */
u32 rtc_1; /* The RTC into which the STE was inserted */
u8 status; /* enum mlx5hws_rule_status */
@@ -75,7 +69,7 @@ struct mlx5hws_rule {
*/
};
-void mlx5hws_rule_free_action_ste(struct mlx5hws_rule_action_ste_info *action_ste);
+void mlx5hws_rule_free_action_ste(struct mlx5hws_action_ste_chunk *action_ste);
int mlx5hws_rule_move_hws_remove(struct mlx5hws_rule *rule,
void *queue, void *user_data);
--
2.31.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH net-next 09/12] net/mlx5: HWS, Use the new action STE pool
2025-04-08 14:00 ` [PATCH net-next 09/12] net/mlx5: HWS, Use the new " Tariq Toukan
@ 2025-04-10 16:48 ` Michal Kubiak
0 siblings, 0 replies; 32+ messages in thread
From: Michal Kubiak @ 2025-04-10 16:48 UTC (permalink / raw)
To: Tariq Toukan
Cc: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet,
Andrew Lunn, Gal Pressman, Leon Romanovsky, Saeed Mahameed,
Leon Romanovsky, netdev, linux-rdma, linux-kernel, Moshe Shemesh,
Mark Bloch, Vlad Dogaru, Yevgeny Kliteynik
On Tue, Apr 08, 2025 at 05:00:53PM +0300, Tariq Toukan wrote:
> From: Vlad Dogaru <vdogaru@nvidia.com>
>
> Use the central action STE pool when creating / updating rules.
>
> Signed-off-by: Vlad Dogaru <vdogaru@nvidia.com>
> Reviewed-by: Yevgeny Kliteynik <kliteyn@nvidia.com>
> Reviewed-by: Mark Bloch <mbloch@nvidia.com>
> Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
Thanks,
Reviewed-by: Michal Kubiak <michal.kubiak@intel.com>
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH net-next 10/12] net/mlx5: HWS, Cleanup matcher action STE table
2025-04-08 14:00 [PATCH net-next 00/12] net/mlx5: HWS, Refactor action STE handling Tariq Toukan
` (8 preceding siblings ...)
2025-04-08 14:00 ` [PATCH net-next 09/12] net/mlx5: HWS, Use the new " Tariq Toukan
@ 2025-04-08 14:00 ` Tariq Toukan
2025-04-10 17:01 ` Michal Kubiak
2025-04-08 14:00 ` [PATCH net-next 11/12] net/mlx5: HWS, Free unused action STE tables Tariq Toukan
2025-04-08 14:00 ` [PATCH net-next 12/12] net/mlx5: HWS, Export action STE tables to debugfs Tariq Toukan
11 siblings, 1 reply; 32+ messages in thread
From: Tariq Toukan @ 2025-04-08 14:00 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet,
Andrew Lunn
Cc: Gal Pressman, Leon Romanovsky, Saeed Mahameed, Leon Romanovsky,
Tariq Toukan, netdev, linux-rdma, linux-kernel, Moshe Shemesh,
Mark Bloch, Vlad Dogaru, Yevgeny Kliteynik
From: Vlad Dogaru <vdogaru@nvidia.com>
Remove the matcher action STE implementation now that the code uses
per-queue action STE pools. This also allows simplifying matcher code
because it is now only handling a single type of RTC/STE.
The matcher resize data is also going away. Matchers were saving old
action STE data because the rules still used it, but now that data lives
in the action STE pool and is no longer coupled to a matcher.
Furthermore, matchers no longer need to rehash a due to action template
addition. If a new action template needs more action STEs, we simply
update the matcher's num_of_action_stes and future rules will allocate
the correct number. Existing rules are unaffected by such an operation
and can continue to use their existing action STEs.
The range action was using the matcher action STE implementation, but
there was no reason to do this other than the container fitting the
purpose. Extract that information to a separate structure.
Finally, stop dumping per-matcher information about action RTCs,
because they no longer exist. A later patch in this series will add
support for dumping action STE pools.
Signed-off-by: Vlad Dogaru <vdogaru@nvidia.com>
Reviewed-by: Yevgeny Kliteynik <kliteyn@nvidia.com>
Reviewed-by: Mark Bloch <mbloch@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
.../mellanox/mlx5/core/steering/hws/action.c | 23 +-
.../mellanox/mlx5/core/steering/hws/action.h | 8 +-
.../mellanox/mlx5/core/steering/hws/bwc.c | 77 +---
.../mellanox/mlx5/core/steering/hws/debug.c | 17 +-
.../mellanox/mlx5/core/steering/hws/matcher.c | 336 ++++--------------
.../mellanox/mlx5/core/steering/hws/matcher.h | 20 +-
.../mellanox/mlx5/core/steering/hws/mlx5hws.h | 5 +-
.../mellanox/mlx5/core/steering/hws/rule.c | 2 +-
8 files changed, 87 insertions(+), 401 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/action.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/action.c
index 44b4640b47db..f7732c4be7c5 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/action.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/action.c
@@ -1574,13 +1574,13 @@ hws_action_create_dest_match_range_definer(struct mlx5hws_context *ctx)
return definer;
}
-static struct mlx5hws_matcher_action_ste *
+static struct mlx5hws_range_action_table *
hws_action_create_dest_match_range_table(struct mlx5hws_context *ctx,
struct mlx5hws_definer *definer,
u32 miss_ft_id)
{
struct mlx5hws_cmd_rtc_create_attr rtc_attr = {0};
- struct mlx5hws_matcher_action_ste *table_ste;
+ struct mlx5hws_range_action_table *table_ste;
struct mlx5hws_pool_attr pool_attr = {0};
struct mlx5hws_pool *ste_pool, *stc_pool;
u32 *rtc_0_id, *rtc_1_id;
@@ -1670,9 +1670,9 @@ hws_action_create_dest_match_range_table(struct mlx5hws_context *ctx,
return NULL;
}
-static void
-hws_action_destroy_dest_match_range_table(struct mlx5hws_context *ctx,
- struct mlx5hws_matcher_action_ste *table_ste)
+static void hws_action_destroy_dest_match_range_table(
+ struct mlx5hws_context *ctx,
+ struct mlx5hws_range_action_table *table_ste)
{
mutex_lock(&ctx->ctrl_lock);
@@ -1684,12 +1684,11 @@ hws_action_destroy_dest_match_range_table(struct mlx5hws_context *ctx,
mutex_unlock(&ctx->ctrl_lock);
}
-static int
-hws_action_create_dest_match_range_fill_table(struct mlx5hws_context *ctx,
- struct mlx5hws_matcher_action_ste *table_ste,
- struct mlx5hws_action *hit_ft_action,
- struct mlx5hws_definer *range_definer,
- u32 min, u32 max)
+static int hws_action_create_dest_match_range_fill_table(
+ struct mlx5hws_context *ctx,
+ struct mlx5hws_range_action_table *table_ste,
+ struct mlx5hws_action *hit_ft_action,
+ struct mlx5hws_definer *range_definer, u32 min, u32 max)
{
struct mlx5hws_wqe_gta_data_seg_ste match_wqe_data = {0};
struct mlx5hws_wqe_gta_data_seg_ste range_wqe_data = {0};
@@ -1785,7 +1784,7 @@ mlx5hws_action_create_dest_match_range(struct mlx5hws_context *ctx,
u32 min, u32 max, u32 flags)
{
struct mlx5hws_cmd_stc_modify_attr stc_attr = {0};
- struct mlx5hws_matcher_action_ste *table_ste;
+ struct mlx5hws_range_action_table *table_ste;
struct mlx5hws_action *hit_ft_action;
struct mlx5hws_definer *definer;
struct mlx5hws_action *action;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/action.h b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/action.h
index 64b76075f7f8..25fa0d4c9221 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/action.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/action.h
@@ -118,6 +118,12 @@ struct mlx5hws_action_template {
u8 only_term;
};
+struct mlx5hws_range_action_table {
+ struct mlx5hws_pool *pool;
+ u32 rtc_0_id;
+ u32 rtc_1_id;
+};
+
struct mlx5hws_action {
u8 type;
u8 flags;
@@ -186,7 +192,7 @@ struct mlx5hws_action {
size_t size;
} remove_header;
struct {
- struct mlx5hws_matcher_action_ste *table_ste;
+ struct mlx5hws_range_action_table *table_ste;
struct mlx5hws_action *hit_ft_action;
struct mlx5hws_definer *definer;
} range;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/bwc.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/bwc.c
index 32de8bfc7644..510bfbbe5991 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/bwc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/bwc.c
@@ -478,21 +478,9 @@ hws_bwc_matcher_size_maxed_out(struct mlx5hws_bwc_matcher *bwc_matcher)
struct mlx5hws_cmd_query_caps *caps = bwc_matcher->matcher->tbl->ctx->caps;
/* check the match RTC size */
- if ((bwc_matcher->size_log +
- MLX5HWS_MATCHER_ASSURED_MAIN_TBL_DEPTH +
- MLX5HWS_BWC_MATCHER_SIZE_LOG_STEP) >
- (caps->ste_alloc_log_max - 1))
- return true;
-
- /* check the action RTC size */
- if ((bwc_matcher->size_log +
- MLX5HWS_BWC_MATCHER_SIZE_LOG_STEP +
- ilog2(roundup_pow_of_two(bwc_matcher->matcher->action_ste.max_stes)) +
- MLX5HWS_MATCHER_ACTION_RTC_UPDATE_MULT) >
- (caps->ste_alloc_log_max - 1))
- return true;
-
- return false;
+ return (bwc_matcher->size_log + MLX5HWS_MATCHER_ASSURED_MAIN_TBL_DEPTH +
+ MLX5HWS_BWC_MATCHER_SIZE_LOG_STEP) >
+ (caps->ste_alloc_log_max - 1);
}
static bool
@@ -779,19 +767,6 @@ hws_bwc_matcher_rehash_size(struct mlx5hws_bwc_matcher *bwc_matcher)
return hws_bwc_matcher_move(bwc_matcher);
}
-static int
-hws_bwc_matcher_rehash_at(struct mlx5hws_bwc_matcher *bwc_matcher)
-{
- /* Rehash by action template doesn't require any additional checking.
- * The bwc_matcher already contains the new action template.
- * Just do the usual rehash:
- * - create new matcher
- * - move all the rules to the new matcher
- * - destroy the old matcher
- */
- return hws_bwc_matcher_move(bwc_matcher);
-}
-
int mlx5hws_bwc_rule_create_simple(struct mlx5hws_bwc_rule *bwc_rule,
u32 *match_param,
struct mlx5hws_rule_action rule_actions[],
@@ -803,7 +778,6 @@ int mlx5hws_bwc_rule_create_simple(struct mlx5hws_bwc_rule *bwc_rule,
struct mlx5hws_rule_attr rule_attr;
struct mutex *queue_lock; /* Protect the queue */
u32 num_of_rules;
- bool need_rehash;
int ret = 0;
int at_idx;
@@ -830,30 +804,11 @@ int mlx5hws_bwc_rule_create_simple(struct mlx5hws_bwc_rule *bwc_rule,
at_idx = bwc_matcher->num_of_at - 1;
ret = mlx5hws_matcher_attach_at(bwc_matcher->matcher,
- bwc_matcher->at[at_idx],
- &need_rehash);
+ bwc_matcher->at[at_idx]);
if (unlikely(ret)) {
hws_bwc_unlock_all_queues(ctx);
return ret;
}
- if (unlikely(need_rehash)) {
- /* The new action template requires more action STEs.
- * Need to attempt creating new matcher with all
- * the action templates, including the new one.
- */
- ret = hws_bwc_matcher_rehash_at(bwc_matcher);
- if (unlikely(ret)) {
- mlx5hws_action_template_destroy(bwc_matcher->at[at_idx]);
- bwc_matcher->at[at_idx] = NULL;
- bwc_matcher->num_of_at--;
-
- hws_bwc_unlock_all_queues(ctx);
-
- mlx5hws_err(ctx,
- "BWC rule insertion: rehash AT failed (%d)\n", ret);
- return ret;
- }
- }
hws_bwc_unlock_all_queues(ctx);
mutex_lock(queue_lock);
@@ -973,7 +928,6 @@ hws_bwc_rule_action_update(struct mlx5hws_bwc_rule *bwc_rule,
struct mlx5hws_context *ctx = bwc_matcher->matcher->tbl->ctx;
struct mlx5hws_rule_attr rule_attr;
struct mutex *queue_lock; /* Protect the queue */
- bool need_rehash;
int at_idx, ret;
u16 idx;
@@ -1005,32 +959,11 @@ hws_bwc_rule_action_update(struct mlx5hws_bwc_rule *bwc_rule,
at_idx = bwc_matcher->num_of_at - 1;
ret = mlx5hws_matcher_attach_at(bwc_matcher->matcher,
- bwc_matcher->at[at_idx],
- &need_rehash);
+ bwc_matcher->at[at_idx]);
if (unlikely(ret)) {
hws_bwc_unlock_all_queues(ctx);
return ret;
}
- if (unlikely(need_rehash)) {
- /* The new action template requires more action
- * STEs. Need to attempt creating new matcher
- * with all the action templates, including the
- * new one.
- */
- ret = hws_bwc_matcher_rehash_at(bwc_matcher);
- if (unlikely(ret)) {
- mlx5hws_action_template_destroy(bwc_matcher->at[at_idx]);
- bwc_matcher->at[at_idx] = NULL;
- bwc_matcher->num_of_at--;
-
- hws_bwc_unlock_all_queues(ctx);
-
- mlx5hws_err(ctx,
- "BWC rule update: rehash AT failed (%d)\n",
- ret);
- return ret;
- }
- }
}
hws_bwc_unlock_all_queues(ctx);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/debug.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/debug.c
index 3491408c5d84..38f75dec9cfc 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/debug.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/debug.c
@@ -146,18 +146,6 @@ static int hws_debug_dump_matcher(struct seq_file *f, struct mlx5hws_matcher *ma
matcher->match_ste.rtc_1_id,
(int)ste_1_id);
- ste_pool = matcher->action_ste.pool;
- if (ste_pool) {
- ste_0_id = mlx5hws_pool_get_base_id(ste_pool);
- if (tbl_type == MLX5HWS_TABLE_TYPE_FDB)
- ste_1_id = mlx5hws_pool_get_base_mirror_id(ste_pool);
- else
- ste_1_id = -1;
- } else {
- ste_0_id = -1;
- ste_1_id = -1;
- }
-
ft_attr.type = matcher->tbl->fw_ft_type;
ret = mlx5hws_cmd_flow_table_query(matcher->tbl->ctx->mdev,
matcher->end_ft_id,
@@ -167,10 +155,7 @@ static int hws_debug_dump_matcher(struct seq_file *f, struct mlx5hws_matcher *ma
if (ret)
return ret;
- seq_printf(f, ",%d,%d,%d,%d,%d,0x%llx,0x%llx\n",
- matcher->action_ste.rtc_0_id, (int)ste_0_id,
- matcher->action_ste.rtc_1_id, (int)ste_1_id,
- 0,
+ seq_printf(f, ",-1,-1,-1,-1,0,0x%llx,0x%llx\n",
mlx5hws_debug_icm_to_idx(icm_addr_0),
mlx5hws_debug_icm_to_idx(icm_addr_1));
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/matcher.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/matcher.c
index 54dd5433a3ca..e8babbd58547 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/matcher.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/matcher.c
@@ -3,25 +3,6 @@
#include "internal.h"
-enum mlx5hws_matcher_rtc_type {
- HWS_MATCHER_RTC_TYPE_MATCH,
- HWS_MATCHER_RTC_TYPE_STE_ARRAY,
- HWS_MATCHER_RTC_TYPE_MAX,
-};
-
-static const char * const mlx5hws_matcher_rtc_type_str[] = {
- [HWS_MATCHER_RTC_TYPE_MATCH] = "MATCH",
- [HWS_MATCHER_RTC_TYPE_STE_ARRAY] = "STE_ARRAY",
- [HWS_MATCHER_RTC_TYPE_MAX] = "UNKNOWN",
-};
-
-static const char *hws_matcher_rtc_type_to_str(enum mlx5hws_matcher_rtc_type rtc_type)
-{
- if (rtc_type > HWS_MATCHER_RTC_TYPE_MAX)
- rtc_type = HWS_MATCHER_RTC_TYPE_MAX;
- return mlx5hws_matcher_rtc_type_str[rtc_type];
-}
-
static bool hws_matcher_requires_col_tbl(u8 log_num_of_rules)
{
/* Collision table concatenation is done only for large rule tables */
@@ -209,83 +190,52 @@ static void hws_matcher_set_rtc_attr_sz(struct mlx5hws_matcher *matcher,
}
}
-static int hws_matcher_create_rtc(struct mlx5hws_matcher *matcher,
- enum mlx5hws_matcher_rtc_type rtc_type)
+static int hws_matcher_create_rtc(struct mlx5hws_matcher *matcher)
{
struct mlx5hws_matcher_attr *attr = &matcher->attr;
struct mlx5hws_cmd_rtc_create_attr rtc_attr = {0};
struct mlx5hws_match_template *mt = matcher->mt;
struct mlx5hws_context *ctx = matcher->tbl->ctx;
- struct mlx5hws_matcher_action_ste *action_ste;
struct mlx5hws_table *tbl = matcher->tbl;
- struct mlx5hws_pool *ste_pool;
- u32 *rtc_0_id, *rtc_1_id;
u32 obj_id;
int ret;
- switch (rtc_type) {
- case HWS_MATCHER_RTC_TYPE_MATCH:
- rtc_0_id = &matcher->match_ste.rtc_0_id;
- rtc_1_id = &matcher->match_ste.rtc_1_id;
- ste_pool = matcher->match_ste.pool;
-
- rtc_attr.log_size = attr->table.sz_row_log;
- rtc_attr.log_depth = attr->table.sz_col_log;
- rtc_attr.is_frst_jumbo = mlx5hws_matcher_mt_is_jumbo(mt);
- rtc_attr.is_scnd_range = 0;
- rtc_attr.miss_ft_id = matcher->end_ft_id;
-
- if (attr->insert_mode == MLX5HWS_MATCHER_INSERT_BY_HASH) {
- /* The usual Hash Table */
- rtc_attr.update_index_mode = MLX5_IFC_RTC_STE_UPDATE_MODE_BY_HASH;
-
- /* The first mt is used since all share the same definer */
- rtc_attr.match_definer_0 = mlx5hws_definer_get_id(mt->definer);
- } else if (attr->insert_mode == MLX5HWS_MATCHER_INSERT_BY_INDEX) {
- rtc_attr.update_index_mode = MLX5_IFC_RTC_STE_UPDATE_MODE_BY_OFFSET;
- rtc_attr.num_hash_definer = 1;
-
- if (attr->distribute_mode == MLX5HWS_MATCHER_DISTRIBUTE_BY_HASH) {
- /* Hash Split Table */
- rtc_attr.access_index_mode = MLX5_IFC_RTC_STE_ACCESS_MODE_BY_HASH;
- rtc_attr.match_definer_0 = mlx5hws_definer_get_id(mt->definer);
- } else if (attr->distribute_mode == MLX5HWS_MATCHER_DISTRIBUTE_BY_LINEAR) {
- /* Linear Lookup Table */
- rtc_attr.access_index_mode = MLX5_IFC_RTC_STE_ACCESS_MODE_LINEAR;
- rtc_attr.match_definer_0 = ctx->caps->linear_match_definer;
- }
+ rtc_attr.log_size = attr->table.sz_row_log;
+ rtc_attr.log_depth = attr->table.sz_col_log;
+ rtc_attr.is_frst_jumbo = mlx5hws_matcher_mt_is_jumbo(mt);
+ rtc_attr.is_scnd_range = 0;
+ rtc_attr.miss_ft_id = matcher->end_ft_id;
+
+ if (attr->insert_mode == MLX5HWS_MATCHER_INSERT_BY_HASH) {
+ /* The usual Hash Table */
+ rtc_attr.update_index_mode =
+ MLX5_IFC_RTC_STE_UPDATE_MODE_BY_HASH;
+
+ /* The first mt is used since all share the same definer */
+ rtc_attr.match_definer_0 = mlx5hws_definer_get_id(mt->definer);
+ } else if (attr->insert_mode == MLX5HWS_MATCHER_INSERT_BY_INDEX) {
+ rtc_attr.update_index_mode =
+ MLX5_IFC_RTC_STE_UPDATE_MODE_BY_OFFSET;
+ rtc_attr.num_hash_definer = 1;
+
+ if (attr->distribute_mode ==
+ MLX5HWS_MATCHER_DISTRIBUTE_BY_HASH) {
+ /* Hash Split Table */
+ rtc_attr.access_index_mode =
+ MLX5_IFC_RTC_STE_ACCESS_MODE_BY_HASH;
+ rtc_attr.match_definer_0 =
+ mlx5hws_definer_get_id(mt->definer);
+ } else if (attr->distribute_mode ==
+ MLX5HWS_MATCHER_DISTRIBUTE_BY_LINEAR) {
+ /* Linear Lookup Table */
+ rtc_attr.access_index_mode =
+ MLX5_IFC_RTC_STE_ACCESS_MODE_LINEAR;
+ rtc_attr.match_definer_0 =
+ ctx->caps->linear_match_definer;
}
- break;
-
- case HWS_MATCHER_RTC_TYPE_STE_ARRAY:
- action_ste = &matcher->action_ste;
-
- rtc_0_id = &action_ste->rtc_0_id;
- rtc_1_id = &action_ste->rtc_1_id;
- ste_pool = action_ste->pool;
- /* Action RTC size calculation:
- * log((max number of rules in matcher) *
- * (max number of action STEs per rule) *
- * (2 to support writing new STEs for update rule))
- */
- rtc_attr.log_size =
- ilog2(roundup_pow_of_two(action_ste->max_stes)) +
- attr->table.sz_row_log +
- MLX5HWS_MATCHER_ACTION_RTC_UPDATE_MULT;
- rtc_attr.log_depth = 0;
- rtc_attr.update_index_mode = MLX5_IFC_RTC_STE_UPDATE_MODE_BY_OFFSET;
- /* The action STEs use the default always hit definer */
- rtc_attr.match_definer_0 = ctx->caps->trivial_match_definer;
- rtc_attr.is_frst_jumbo = false;
- rtc_attr.miss_ft_id = 0;
- break;
-
- default:
- mlx5hws_err(ctx, "HWS Invalid RTC type\n");
- return -EINVAL;
}
- obj_id = mlx5hws_pool_get_base_id(ste_pool);
+ obj_id = mlx5hws_pool_get_base_id(matcher->match_ste.pool);
rtc_attr.pd = ctx->pd_num;
rtc_attr.ste_base = obj_id;
@@ -298,15 +248,16 @@ static int hws_matcher_create_rtc(struct mlx5hws_matcher *matcher,
obj_id = mlx5hws_pool_get_base_id(ctx->stc_pool);
rtc_attr.stc_base = obj_id;
- ret = mlx5hws_cmd_rtc_create(ctx->mdev, &rtc_attr, rtc_0_id);
+ ret = mlx5hws_cmd_rtc_create(ctx->mdev, &rtc_attr,
+ &matcher->match_ste.rtc_0_id);
if (ret) {
- mlx5hws_err(ctx, "Failed to create matcher RTC of type %s",
- hws_matcher_rtc_type_to_str(rtc_type));
+ mlx5hws_err(ctx, "Failed to create matcher RTC\n");
return ret;
}
if (tbl->type == MLX5HWS_TABLE_TYPE_FDB) {
- obj_id = mlx5hws_pool_get_base_mirror_id(ste_pool);
+ obj_id = mlx5hws_pool_get_base_mirror_id(
+ matcher->match_ste.pool);
rtc_attr.ste_base = obj_id;
rtc_attr.table_type = mlx5hws_table_get_res_fw_ft_type(tbl->type, true);
@@ -314,10 +265,10 @@ static int hws_matcher_create_rtc(struct mlx5hws_matcher *matcher,
rtc_attr.stc_base = obj_id;
hws_matcher_set_rtc_attr_sz(matcher, &rtc_attr, true);
- ret = mlx5hws_cmd_rtc_create(ctx->mdev, &rtc_attr, rtc_1_id);
+ ret = mlx5hws_cmd_rtc_create(ctx->mdev, &rtc_attr,
+ &matcher->match_ste.rtc_1_id);
if (ret) {
- mlx5hws_err(ctx, "Failed to create peer matcher RTC of type %s",
- hws_matcher_rtc_type_to_str(rtc_type));
+ mlx5hws_err(ctx, "Failed to create mirror matcher RTC\n");
goto destroy_rtc_0;
}
}
@@ -325,33 +276,18 @@ static int hws_matcher_create_rtc(struct mlx5hws_matcher *matcher,
return 0;
destroy_rtc_0:
- mlx5hws_cmd_rtc_destroy(ctx->mdev, *rtc_0_id);
+ mlx5hws_cmd_rtc_destroy(ctx->mdev, matcher->match_ste.rtc_0_id);
return ret;
}
-static void hws_matcher_destroy_rtc(struct mlx5hws_matcher *matcher,
- enum mlx5hws_matcher_rtc_type rtc_type)
+static void hws_matcher_destroy_rtc(struct mlx5hws_matcher *matcher)
{
- struct mlx5hws_table *tbl = matcher->tbl;
- u32 rtc_0_id, rtc_1_id;
-
- switch (rtc_type) {
- case HWS_MATCHER_RTC_TYPE_MATCH:
- rtc_0_id = matcher->match_ste.rtc_0_id;
- rtc_1_id = matcher->match_ste.rtc_1_id;
- break;
- case HWS_MATCHER_RTC_TYPE_STE_ARRAY:
- rtc_0_id = matcher->action_ste.rtc_0_id;
- rtc_1_id = matcher->action_ste.rtc_1_id;
- break;
- default:
- return;
- }
+ struct mlx5_core_dev *mdev = matcher->tbl->ctx->mdev;
- if (tbl->type == MLX5HWS_TABLE_TYPE_FDB)
- mlx5hws_cmd_rtc_destroy(tbl->ctx->mdev, rtc_1_id);
+ if (matcher->tbl->type == MLX5HWS_TABLE_TYPE_FDB)
+ mlx5hws_cmd_rtc_destroy(mdev, matcher->match_ste.rtc_1_id);
- mlx5hws_cmd_rtc_destroy(tbl->ctx->mdev, rtc_0_id);
+ mlx5hws_cmd_rtc_destroy(mdev, matcher->match_ste.rtc_0_id);
}
static int
@@ -419,85 +355,17 @@ static int hws_matcher_check_and_process_at(struct mlx5hws_matcher *matcher,
return 0;
}
-static int hws_matcher_resize_init(struct mlx5hws_matcher *src_matcher)
-{
- struct mlx5hws_matcher_resize_data *resize_data;
-
- resize_data = kzalloc(sizeof(*resize_data), GFP_KERNEL);
- if (!resize_data)
- return -ENOMEM;
-
- resize_data->max_stes = src_matcher->action_ste.max_stes;
-
- resize_data->stc = src_matcher->action_ste.stc;
- resize_data->rtc_0_id = src_matcher->action_ste.rtc_0_id;
- resize_data->rtc_1_id = src_matcher->action_ste.rtc_1_id;
- resize_data->pool = src_matcher->action_ste.max_stes ?
- src_matcher->action_ste.pool : NULL;
-
- /* Place the new resized matcher on the dst matcher's list */
- list_add(&resize_data->list_node, &src_matcher->resize_dst->resize_data);
-
- /* Move all the previous resized matchers to the dst matcher's list */
- while (!list_empty(&src_matcher->resize_data)) {
- resize_data = list_first_entry(&src_matcher->resize_data,
- struct mlx5hws_matcher_resize_data,
- list_node);
- list_del_init(&resize_data->list_node);
- list_add(&resize_data->list_node, &src_matcher->resize_dst->resize_data);
- }
-
- return 0;
-}
-
-static void hws_matcher_resize_uninit(struct mlx5hws_matcher *matcher)
-{
- struct mlx5hws_matcher_resize_data *resize_data;
-
- if (!mlx5hws_matcher_is_resizable(matcher))
- return;
-
- while (!list_empty(&matcher->resize_data)) {
- resize_data = list_first_entry(&matcher->resize_data,
- struct mlx5hws_matcher_resize_data,
- list_node);
- list_del_init(&resize_data->list_node);
-
- if (resize_data->max_stes) {
- mlx5hws_action_free_single_stc(matcher->tbl->ctx,
- matcher->tbl->type,
- &resize_data->stc);
-
- if (matcher->tbl->type == MLX5HWS_TABLE_TYPE_FDB)
- mlx5hws_cmd_rtc_destroy(matcher->tbl->ctx->mdev,
- resize_data->rtc_1_id);
-
- mlx5hws_cmd_rtc_destroy(matcher->tbl->ctx->mdev,
- resize_data->rtc_0_id);
-
- if (resize_data->pool)
- mlx5hws_pool_destroy(resize_data->pool);
- }
-
- kfree(resize_data);
- }
-}
-
static int hws_matcher_bind_at(struct mlx5hws_matcher *matcher)
{
bool is_jumbo = mlx5hws_matcher_mt_is_jumbo(matcher->mt);
- struct mlx5hws_cmd_stc_modify_attr stc_attr = {0};
- struct mlx5hws_matcher_action_ste *action_ste;
- struct mlx5hws_table *tbl = matcher->tbl;
- struct mlx5hws_pool_attr pool_attr = {0};
- struct mlx5hws_context *ctx = tbl->ctx;
- u32 required_stes;
- u8 max_stes = 0;
+ struct mlx5hws_context *ctx = matcher->tbl->ctx;
+ u8 required_stes, max_stes;
int i, ret;
if (matcher->flags & MLX5HWS_MATCHER_FLAGS_COLLISION)
return 0;
+ max_stes = 0;
for (i = 0; i < matcher->num_of_at; i++) {
struct mlx5hws_action_template *at = &matcher->at[i];
@@ -513,74 +381,9 @@ static int hws_matcher_bind_at(struct mlx5hws_matcher *matcher)
/* Future: Optimize reparse */
}
- /* There are no additional STEs required for matcher */
- if (!max_stes)
- return 0;
-
- matcher->action_ste.max_stes = max_stes;
-
- action_ste = &matcher->action_ste;
-
- /* Allocate action STE mempool */
- pool_attr.table_type = tbl->type;
- pool_attr.pool_type = MLX5HWS_POOL_TYPE_STE;
- pool_attr.flags = MLX5HWS_POOL_FLAG_BUDDY;
- /* Pool size is similar to action RTC size */
- pool_attr.alloc_log_sz = ilog2(roundup_pow_of_two(action_ste->max_stes)) +
- matcher->attr.table.sz_row_log +
- MLX5HWS_MATCHER_ACTION_RTC_UPDATE_MULT;
- hws_matcher_set_pool_attr(&pool_attr, matcher);
- action_ste->pool = mlx5hws_pool_create(ctx, &pool_attr);
- if (!action_ste->pool) {
- mlx5hws_err(ctx, "Failed to create action ste pool\n");
- return -EINVAL;
- }
-
- /* Allocate action RTC */
- ret = hws_matcher_create_rtc(matcher, HWS_MATCHER_RTC_TYPE_STE_ARRAY);
- if (ret) {
- mlx5hws_err(ctx, "Failed to create action RTC\n");
- goto free_ste_pool;
- }
-
- /* Allocate STC for jumps to STE */
- stc_attr.action_offset = MLX5HWS_ACTION_OFFSET_HIT;
- stc_attr.action_type = MLX5_IFC_STC_ACTION_TYPE_JUMP_TO_STE_TABLE;
- stc_attr.reparse_mode = MLX5_IFC_STC_REPARSE_IGNORE;
- stc_attr.ste_table.ste_pool = action_ste->pool;
- stc_attr.ste_table.match_definer_id = ctx->caps->trivial_match_definer;
-
- ret = mlx5hws_action_alloc_single_stc(ctx, &stc_attr, tbl->type,
- &action_ste->stc);
- if (ret) {
- mlx5hws_err(ctx, "Failed to create action jump to table STC\n");
- goto free_rtc;
- }
+ matcher->num_of_action_stes = max_stes;
return 0;
-
-free_rtc:
- hws_matcher_destroy_rtc(matcher, HWS_MATCHER_RTC_TYPE_STE_ARRAY);
-free_ste_pool:
- mlx5hws_pool_destroy(action_ste->pool);
- return ret;
-}
-
-static void hws_matcher_unbind_at(struct mlx5hws_matcher *matcher)
-{
- struct mlx5hws_matcher_action_ste *action_ste;
- struct mlx5hws_table *tbl = matcher->tbl;
-
- action_ste = &matcher->action_ste;
-
- if (!action_ste->max_stes ||
- matcher->flags & MLX5HWS_MATCHER_FLAGS_COLLISION ||
- mlx5hws_matcher_is_in_resize(matcher))
- return;
-
- mlx5hws_action_free_single_stc(tbl->ctx, tbl->type, &action_ste->stc);
- hws_matcher_destroy_rtc(matcher, HWS_MATCHER_RTC_TYPE_STE_ARRAY);
- mlx5hws_pool_destroy(action_ste->pool);
}
static int hws_matcher_bind_mt(struct mlx5hws_matcher *matcher)
@@ -724,10 +527,10 @@ static int hws_matcher_create_and_connect(struct mlx5hws_matcher *matcher)
/* Create matcher end flow table anchor */
ret = hws_matcher_create_end_ft(matcher);
if (ret)
- goto unbind_at;
+ goto unbind_mt;
/* Allocate the RTC for the new matcher */
- ret = hws_matcher_create_rtc(matcher, HWS_MATCHER_RTC_TYPE_MATCH);
+ ret = hws_matcher_create_rtc(matcher);
if (ret)
goto destroy_end_ft;
@@ -739,11 +542,9 @@ static int hws_matcher_create_and_connect(struct mlx5hws_matcher *matcher)
return 0;
destroy_rtc:
- hws_matcher_destroy_rtc(matcher, HWS_MATCHER_RTC_TYPE_MATCH);
+ hws_matcher_destroy_rtc(matcher);
destroy_end_ft:
hws_matcher_destroy_end_ft(matcher);
-unbind_at:
- hws_matcher_unbind_at(matcher);
unbind_mt:
hws_matcher_unbind_mt(matcher);
return ret;
@@ -751,11 +552,9 @@ static int hws_matcher_create_and_connect(struct mlx5hws_matcher *matcher)
static void hws_matcher_destroy_and_disconnect(struct mlx5hws_matcher *matcher)
{
- hws_matcher_resize_uninit(matcher);
hws_matcher_disconnect(matcher);
- hws_matcher_destroy_rtc(matcher, HWS_MATCHER_RTC_TYPE_MATCH);
+ hws_matcher_destroy_rtc(matcher);
hws_matcher_destroy_end_ft(matcher);
- hws_matcher_unbind_at(matcher);
hws_matcher_unbind_mt(matcher);
}
@@ -777,8 +576,6 @@ hws_matcher_create_col_matcher(struct mlx5hws_matcher *matcher)
if (!col_matcher)
return -ENOMEM;
- INIT_LIST_HEAD(&col_matcher->resize_data);
-
col_matcher->tbl = matcher->tbl;
col_matcher->mt = matcher->mt;
col_matcher->at = matcher->at;
@@ -832,8 +629,6 @@ static int hws_matcher_init(struct mlx5hws_matcher *matcher)
struct mlx5hws_context *ctx = matcher->tbl->ctx;
int ret;
- INIT_LIST_HEAD(&matcher->resize_data);
-
mutex_lock(&ctx->ctrl_lock);
/* Allocate matcher resource and connect to the packet pipe */
@@ -890,16 +685,12 @@ static int hws_matcher_grow_at_array(struct mlx5hws_matcher *matcher)
}
int mlx5hws_matcher_attach_at(struct mlx5hws_matcher *matcher,
- struct mlx5hws_action_template *at,
- bool *need_rehash)
+ struct mlx5hws_action_template *at)
{
bool is_jumbo = mlx5hws_matcher_mt_is_jumbo(matcher->mt);
- struct mlx5hws_context *ctx = matcher->tbl->ctx;
u32 required_stes;
int ret;
- *need_rehash = false;
-
if (unlikely(matcher->num_of_at >= matcher->size_of_at_array)) {
ret = hws_matcher_grow_at_array(matcher);
if (ret)
@@ -917,11 +708,8 @@ int mlx5hws_matcher_attach_at(struct mlx5hws_matcher *matcher,
return ret;
required_stes = at->num_of_action_stes - (!is_jumbo || at->only_term);
- if (matcher->action_ste.max_stes < required_stes) {
- mlx5hws_dbg(ctx, "Required STEs [%d] exceeds initial action template STE [%d]\n",
- required_stes, matcher->action_ste.max_stes);
- *need_rehash = true;
- }
+ if (matcher->num_of_action_stes < required_stes)
+ matcher->num_of_action_stes = required_stes;
matcher->at[matcher->num_of_at] = *at;
matcher->num_of_at += 1;
@@ -1103,7 +891,7 @@ static int hws_matcher_resize_precheck(struct mlx5hws_matcher *src_matcher,
return -EINVAL;
}
- if (src_matcher->action_ste.max_stes > dst_matcher->action_ste.max_stes) {
+ if (src_matcher->num_of_action_stes > dst_matcher->num_of_action_stes) {
mlx5hws_err(ctx, "Src/dst matcher max STEs mismatch\n");
return -EINVAL;
}
@@ -1132,10 +920,6 @@ int mlx5hws_matcher_resize_set_target(struct mlx5hws_matcher *src_matcher,
src_matcher->resize_dst = dst_matcher;
- ret = hws_matcher_resize_init(src_matcher);
- if (ret)
- src_matcher->resize_dst = NULL;
-
out:
mutex_unlock(&src_matcher->tbl->ctx->ctrl_lock);
return ret;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/matcher.h b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/matcher.h
index 0450b6175ac9..bad1fa8f77fd 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/matcher.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/matcher.h
@@ -50,23 +50,6 @@ struct mlx5hws_matcher_match_ste {
struct mlx5hws_pool *pool;
};
-struct mlx5hws_matcher_action_ste {
- struct mlx5hws_pool_chunk stc;
- u32 rtc_0_id;
- u32 rtc_1_id;
- struct mlx5hws_pool *pool;
- u8 max_stes;
-};
-
-struct mlx5hws_matcher_resize_data {
- struct mlx5hws_pool_chunk stc;
- u32 rtc_0_id;
- u32 rtc_1_id;
- struct mlx5hws_pool *pool;
- u8 max_stes;
- struct list_head list_node;
-};
-
struct mlx5hws_matcher {
struct mlx5hws_table *tbl;
struct mlx5hws_matcher_attr attr;
@@ -75,15 +58,14 @@ struct mlx5hws_matcher {
u8 num_of_at;
u8 size_of_at_array;
u8 num_of_mt;
+ u8 num_of_action_stes;
/* enum mlx5hws_matcher_flags */
u8 flags;
u32 end_ft_id;
struct mlx5hws_matcher *col_matcher;
struct mlx5hws_matcher *resize_dst;
struct mlx5hws_matcher_match_ste match_ste;
- struct mlx5hws_matcher_action_ste action_ste;
struct list_head list_node;
- struct list_head resize_data;
};
static inline bool
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/mlx5hws.h b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/mlx5hws.h
index 8ed8a715a2eb..5121951f2778 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/mlx5hws.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/mlx5hws.h
@@ -399,14 +399,11 @@ int mlx5hws_matcher_destroy(struct mlx5hws_matcher *matcher);
*
* @matcher: Matcher to attach the action template to.
* @at: Action template to be attached to the matcher.
- * @need_rehash: Output parameter that tells callers if the matcher needs to be
- * rehashed.
*
* Return: Zero on success, non-zero otherwise.
*/
int mlx5hws_matcher_attach_at(struct mlx5hws_matcher *matcher,
- struct mlx5hws_action_template *at,
- bool *need_rehash);
+ struct mlx5hws_action_template *at);
/**
* mlx5hws_matcher_resize_set_target - Link two matchers and enable moving rules.
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/rule.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/rule.c
index 5b758467ed03..9e6f35d68445 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/rule.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/rule.c
@@ -203,7 +203,7 @@ static int mlx5hws_rule_alloc_action_ste(struct mlx5hws_rule *rule,
struct mlx5hws_context *ctx = matcher->tbl->ctx;
rule->action_ste.ste.order =
- ilog2(roundup_pow_of_two(matcher->action_ste.max_stes));
+ ilog2(roundup_pow_of_two(matcher->num_of_action_stes));
return mlx5hws_action_ste_chunk_alloc(&ctx->action_ste_pool[queue_id],
skip_rx, skip_tx,
&rule->action_ste);
--
2.31.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH net-next 10/12] net/mlx5: HWS, Cleanup matcher action STE table
2025-04-08 14:00 ` [PATCH net-next 10/12] net/mlx5: HWS, Cleanup matcher action STE table Tariq Toukan
@ 2025-04-10 17:01 ` Michal Kubiak
2025-04-10 18:45 ` Vlad Dogaru
0 siblings, 1 reply; 32+ messages in thread
From: Michal Kubiak @ 2025-04-10 17:01 UTC (permalink / raw)
To: Tariq Toukan
Cc: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet,
Andrew Lunn, Gal Pressman, Leon Romanovsky, Saeed Mahameed,
Leon Romanovsky, netdev, linux-rdma, linux-kernel, Moshe Shemesh,
Mark Bloch, Vlad Dogaru, Yevgeny Kliteynik
On Tue, Apr 08, 2025 at 05:00:54PM +0300, Tariq Toukan wrote:
> From: Vlad Dogaru <vdogaru@nvidia.com>
>
> Remove the matcher action STE implementation now that the code uses
> per-queue action STE pools. This also allows simplifying matcher code
> because it is now only handling a single type of RTC/STE.
>
> The matcher resize data is also going away. Matchers were saving old
> action STE data because the rules still used it, but now that data lives
> in the action STE pool and is no longer coupled to a matcher.
>
> Furthermore, matchers no longer need to rehash a due to action template
> addition. If a new action template needs more action STEs, we simply
> update the matcher's num_of_action_stes and future rules will allocate
> the correct number. Existing rules are unaffected by such an operation
> and can continue to use their existing action STEs.
>
> The range action was using the matcher action STE implementation, but
> there was no reason to do this other than the container fitting the
> purpose. Extract that information to a separate structure.
>
> Finally, stop dumping per-matcher information about action RTCs,
> because they no longer exist. A later patch in this series will add
> support for dumping action STE pools.
>
> Signed-off-by: Vlad Dogaru <vdogaru@nvidia.com>
> Reviewed-by: Yevgeny Kliteynik <kliteyn@nvidia.com>
> Reviewed-by: Mark Bloch <mbloch@nvidia.com>
> Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
> ---
> .../mellanox/mlx5/core/steering/hws/action.c | 23 +-
> .../mellanox/mlx5/core/steering/hws/action.h | 8 +-
> .../mellanox/mlx5/core/steering/hws/bwc.c | 77 +---
> .../mellanox/mlx5/core/steering/hws/debug.c | 17 +-
> .../mellanox/mlx5/core/steering/hws/matcher.c | 336 ++++--------------
> .../mellanox/mlx5/core/steering/hws/matcher.h | 20 +-
> .../mellanox/mlx5/core/steering/hws/mlx5hws.h | 5 +-
> .../mellanox/mlx5/core/steering/hws/rule.c | 2 +-
> 8 files changed, 87 insertions(+), 401 deletions(-)
>
[...]
> @@ -803,7 +778,6 @@ int mlx5hws_bwc_rule_create_simple(struct mlx5hws_bwc_rule *bwc_rule,
> struct mlx5hws_rule_attr rule_attr;
> struct mutex *queue_lock; /* Protect the queue */
> u32 num_of_rules;
> - bool need_rehash;
This flag (and the function parameter below) were added in the Patch #1 as part
of the fix for unnecessary rehashing. Now it is removed again.
Is this fix really necessary for this series to somehow make it consistent?
Maybe Patch #1 should go separately as an independent fix in the "net"
tree? What do you think?
> int ret = 0;
> int at_idx;
>
> @@ -830,30 +804,11 @@ int mlx5hws_bwc_rule_create_simple(struct mlx5hws_bwc_rule *bwc_rule,
> at_idx = bwc_matcher->num_of_at - 1;
>
> ret = mlx5hws_matcher_attach_at(bwc_matcher->matcher,
> - bwc_matcher->at[at_idx],
> - &need_rehash);
> + bwc_matcher->at[at_idx]);
> if (unlikely(ret)) {
> hws_bwc_unlock_all_queues(ctx);
> return ret;
> }
> - if (unlikely(need_rehash)) {
> - /* The new action template requires more action STEs.
> - * Need to attempt creating new matcher with all
> - * the action templates, including the new one.
> - */
[...]
Thanks,
Michal
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH net-next 10/12] net/mlx5: HWS, Cleanup matcher action STE table
2025-04-10 17:01 ` Michal Kubiak
@ 2025-04-10 18:45 ` Vlad Dogaru
0 siblings, 0 replies; 32+ messages in thread
From: Vlad Dogaru @ 2025-04-10 18:45 UTC (permalink / raw)
To: Michal Kubiak
Cc: Tariq Toukan, David S. Miller, Jakub Kicinski, Paolo Abeni,
Eric Dumazet, Andrew Lunn, Gal Pressman, Leon Romanovsky,
Saeed Mahameed, Leon Romanovsky, netdev, linux-rdma, linux-kernel,
Moshe Shemesh, Mark Bloch, Yevgeny Kliteynik
On Thu, Apr 10, 2025 at 07:01:58PM +0200, Michal Kubiak wrote:
> On Tue, Apr 08, 2025 at 05:00:54PM +0300, Tariq Toukan wrote:
> > From: Vlad Dogaru <vdogaru@nvidia.com>
> >
> > Remove the matcher action STE implementation now that the code uses
> > per-queue action STE pools. This also allows simplifying matcher code
> > because it is now only handling a single type of RTC/STE.
> >
> > The matcher resize data is also going away. Matchers were saving old
> > action STE data because the rules still used it, but now that data lives
> > in the action STE pool and is no longer coupled to a matcher.
> >
> > Furthermore, matchers no longer need to rehash a due to action template
> > addition. If a new action template needs more action STEs, we simply
> > update the matcher's num_of_action_stes and future rules will allocate
> > the correct number. Existing rules are unaffected by such an operation
> > and can continue to use their existing action STEs.
> >
> > The range action was using the matcher action STE implementation, but
> > there was no reason to do this other than the container fitting the
> > purpose. Extract that information to a separate structure.
> >
> > Finally, stop dumping per-matcher information about action RTCs,
> > because they no longer exist. A later patch in this series will add
> > support for dumping action STE pools.
> >
> > Signed-off-by: Vlad Dogaru <vdogaru@nvidia.com>
> > Reviewed-by: Yevgeny Kliteynik <kliteyn@nvidia.com>
> > Reviewed-by: Mark Bloch <mbloch@nvidia.com>
> > Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
> > ---
> > .../mellanox/mlx5/core/steering/hws/action.c | 23 +-
> > .../mellanox/mlx5/core/steering/hws/action.h | 8 +-
> > .../mellanox/mlx5/core/steering/hws/bwc.c | 77 +---
> > .../mellanox/mlx5/core/steering/hws/debug.c | 17 +-
> > .../mellanox/mlx5/core/steering/hws/matcher.c | 336 ++++--------------
> > .../mellanox/mlx5/core/steering/hws/matcher.h | 20 +-
> > .../mellanox/mlx5/core/steering/hws/mlx5hws.h | 5 +-
> > .../mellanox/mlx5/core/steering/hws/rule.c | 2 +-
> > 8 files changed, 87 insertions(+), 401 deletions(-)
> >
>
>
> [...]
>
> > @@ -803,7 +778,6 @@ int mlx5hws_bwc_rule_create_simple(struct mlx5hws_bwc_rule *bwc_rule,
> > struct mlx5hws_rule_attr rule_attr;
> > struct mutex *queue_lock; /* Protect the queue */
> > u32 num_of_rules;
> > - bool need_rehash;
>
> This flag (and the function parameter below) were added in the Patch #1 as part
> of the fix for unnecessary rehashing. Now it is removed again.
> Is this fix really necessary for this series to somehow make it consistent?
> Maybe Patch #1 should go separately as an independent fix in the "net"
> tree? What do you think?
Although patch 1 is indeed a bugfix, we've never seen it triggered in
our regressions. Since all of the code is relatively new and it's
unlikely we will see this in the wild, let's keep the series as is, I
hope that's ok.
Cheers,
Vlad
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH net-next 11/12] net/mlx5: HWS, Free unused action STE tables
2025-04-08 14:00 [PATCH net-next 00/12] net/mlx5: HWS, Refactor action STE handling Tariq Toukan
` (9 preceding siblings ...)
2025-04-08 14:00 ` [PATCH net-next 10/12] net/mlx5: HWS, Cleanup matcher action STE table Tariq Toukan
@ 2025-04-08 14:00 ` Tariq Toukan
2025-04-10 17:28 ` Michal Kubiak
2025-04-08 14:00 ` [PATCH net-next 12/12] net/mlx5: HWS, Export action STE tables to debugfs Tariq Toukan
11 siblings, 1 reply; 32+ messages in thread
From: Tariq Toukan @ 2025-04-08 14:00 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet,
Andrew Lunn
Cc: Gal Pressman, Leon Romanovsky, Saeed Mahameed, Leon Romanovsky,
Tariq Toukan, netdev, linux-rdma, linux-kernel, Moshe Shemesh,
Mark Bloch, Vlad Dogaru, Yevgeny Kliteynik
From: Vlad Dogaru <vdogaru@nvidia.com>
Periodically check for unused action STE tables and free their
associated resources. In order to do this safely, add a per-queue lock
to synchronize the garbage collect work with regular operations on
steering rules.
Signed-off-by: Vlad Dogaru <vdogaru@nvidia.com>
Reviewed-by: Yevgeny Kliteynik <kliteyn@nvidia.com>
Reviewed-by: Mark Bloch <mbloch@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
.../mlx5/core/steering/hws/action_ste_pool.c | 88 ++++++++++++++++++-
.../mlx5/core/steering/hws/action_ste_pool.h | 11 +++
.../mellanox/mlx5/core/steering/hws/context.h | 1 +
3 files changed, 96 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/action_ste_pool.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/action_ste_pool.c
index e5c453ec65b3..509e19ec101b 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/action_ste_pool.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/action_ste_pool.c
@@ -160,6 +160,7 @@ hws_action_ste_table_alloc(struct mlx5hws_action_ste_pool_element *parent_elem)
action_tbl->parent_elem = parent_elem;
INIT_LIST_HEAD(&action_tbl->list_node);
+ action_tbl->last_used = jiffies;
list_add(&action_tbl->list_node, &parent_elem->available);
parent_elem->log_sz = log_sz;
@@ -237,6 +238,8 @@ static int hws_action_ste_pool_init(struct mlx5hws_context *ctx,
enum mlx5hws_pool_optimize opt;
int err;
+ mutex_init(&pool->lock);
+
/* Rules which are added for both RX and TX must use the same action STE
* indices for both. If we were to use a single table, then RX-only and
* TX-only rules would waste the unused entries. Thus, we use separate
@@ -248,6 +251,7 @@ static int hws_action_ste_pool_init(struct mlx5hws_context *ctx,
opt);
if (err)
goto destroy_elems;
+ pool->elems[opt].parent_pool = pool;
}
return 0;
@@ -268,6 +272,58 @@ static void hws_action_ste_pool_destroy(struct mlx5hws_action_ste_pool *pool)
hws_action_ste_pool_element_destroy(&pool->elems[opt]);
}
+static void hws_action_ste_pool_element_collect_stale(
+ struct mlx5hws_action_ste_pool_element *elem, struct list_head *cleanup)
+{
+ struct mlx5hws_action_ste_table *action_tbl, *p;
+ unsigned long expire_time, now;
+
+ expire_time = secs_to_jiffies(MLX5HWS_ACTION_STE_POOL_EXPIRE_SECONDS);
+ now = jiffies;
+
+ list_for_each_entry_safe(action_tbl, p, &elem->available, list_node) {
+ if (mlx5hws_pool_full(action_tbl->pool) &&
+ time_before(action_tbl->last_used + expire_time, now))
+ list_move(&action_tbl->list_node, cleanup);
+ }
+}
+
+static void hws_action_ste_table_cleanup_list(struct list_head *cleanup)
+{
+ struct mlx5hws_action_ste_table *action_tbl, *p;
+
+ list_for_each_entry_safe(action_tbl, p, cleanup, list_node)
+ hws_action_ste_table_destroy(action_tbl);
+}
+
+static void hws_action_ste_pool_cleanup(struct work_struct *work)
+{
+ enum mlx5hws_pool_optimize opt;
+ struct mlx5hws_context *ctx;
+ LIST_HEAD(cleanup);
+ int i;
+
+ ctx = container_of(work, struct mlx5hws_context,
+ action_ste_cleanup.work);
+
+ for (i = 0; i < ctx->queues; i++) {
+ struct mlx5hws_action_ste_pool *p = &ctx->action_ste_pool[i];
+
+ mutex_lock(&p->lock);
+ for (opt = MLX5HWS_POOL_OPTIMIZE_NONE;
+ opt < MLX5HWS_POOL_OPTIMIZE_MAX; opt++)
+ hws_action_ste_pool_element_collect_stale(
+ &p->elems[opt], &cleanup);
+ mutex_unlock(&p->lock);
+ }
+
+ hws_action_ste_table_cleanup_list(&cleanup);
+
+ schedule_delayed_work(&ctx->action_ste_cleanup,
+ secs_to_jiffies(
+ MLX5HWS_ACTION_STE_POOL_CLEANUP_SECONDS));
+}
+
int mlx5hws_action_ste_pool_init(struct mlx5hws_context *ctx)
{
struct mlx5hws_action_ste_pool *pool;
@@ -286,6 +342,12 @@ int mlx5hws_action_ste_pool_init(struct mlx5hws_context *ctx)
ctx->action_ste_pool = pool;
+ INIT_DELAYED_WORK(&ctx->action_ste_cleanup,
+ hws_action_ste_pool_cleanup);
+ schedule_delayed_work(
+ &ctx->action_ste_cleanup,
+ secs_to_jiffies(MLX5HWS_ACTION_STE_POOL_CLEANUP_SECONDS));
+
return 0;
free_pool:
@@ -301,6 +363,8 @@ void mlx5hws_action_ste_pool_uninit(struct mlx5hws_context *ctx)
size_t queues = ctx->queues;
int i;
+ cancel_delayed_work_sync(&ctx->action_ste_cleanup);
+
for (i = 0; i < queues; i++)
hws_action_ste_pool_destroy(&ctx->action_ste_pool[i]);
@@ -331,6 +395,7 @@ hws_action_ste_table_chunk_alloc(struct mlx5hws_action_ste_table *action_tbl,
return err;
chunk->action_tbl = action_tbl;
+ action_tbl->last_used = jiffies;
return 0;
}
@@ -347,6 +412,8 @@ int mlx5hws_action_ste_chunk_alloc(struct mlx5hws_action_ste_pool *pool,
if (skip_rx && skip_tx)
return -EINVAL;
+ mutex_lock(&pool->lock);
+
elem = hws_action_ste_choose_elem(pool, skip_rx, skip_tx);
mlx5hws_dbg(elem->ctx,
@@ -363,26 +430,39 @@ int mlx5hws_action_ste_chunk_alloc(struct mlx5hws_action_ste_pool *pool,
if (!found) {
action_tbl = hws_action_ste_table_alloc(elem);
- if (IS_ERR(action_tbl))
- return PTR_ERR(action_tbl);
+ if (IS_ERR(action_tbl)) {
+ err = PTR_ERR(action_tbl);
+ goto out;
+ }
err = hws_action_ste_table_chunk_alloc(action_tbl, chunk);
if (err)
- return err;
+ goto out;
}
if (mlx5hws_pool_empty(action_tbl->pool))
list_move(&action_tbl->list_node, &elem->full);
- return 0;
+ err = 0;
+
+out:
+ mutex_unlock(&pool->lock);
+
+ return err;
}
void mlx5hws_action_ste_chunk_free(struct mlx5hws_action_ste_chunk *chunk)
{
+ struct mutex *lock = &chunk->action_tbl->parent_elem->parent_pool->lock;
+
mlx5hws_dbg(chunk->action_tbl->pool->ctx,
"Freeing action STEs offset %d order %d\n",
chunk->ste.offset, chunk->ste.order);
+
+ mutex_lock(lock);
mlx5hws_pool_chunk_free(chunk->action_tbl->pool, &chunk->ste);
+ chunk->action_tbl->last_used = jiffies;
list_move(&chunk->action_tbl->list_node,
&chunk->action_tbl->parent_elem->available);
+ mutex_unlock(lock);
}
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/action_ste_pool.h b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/action_ste_pool.h
index 2de660a63223..a8ba97359e31 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/action_ste_pool.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/action_ste_pool.h
@@ -8,6 +8,9 @@
#define MLX5HWS_ACTION_STE_TABLE_STEP_LOG_SZ 1
#define MLX5HWS_ACTION_STE_TABLE_MAX_LOG_SZ 20
+#define MLX5HWS_ACTION_STE_POOL_CLEANUP_SECONDS 300
+#define MLX5HWS_ACTION_STE_POOL_EXPIRE_SECONDS 300
+
struct mlx5hws_action_ste_pool_element;
struct mlx5hws_action_ste_table {
@@ -19,10 +22,12 @@ struct mlx5hws_action_ste_table {
u32 rtc_0_id;
u32 rtc_1_id;
struct list_head list_node;
+ unsigned long last_used;
};
struct mlx5hws_action_ste_pool_element {
struct mlx5hws_context *ctx;
+ struct mlx5hws_action_ste_pool *parent_pool;
size_t log_sz; /* Size of the largest table so far. */
enum mlx5hws_pool_optimize opt;
struct list_head available;
@@ -33,6 +38,12 @@ struct mlx5hws_action_ste_pool_element {
* per queue.
*/
struct mlx5hws_action_ste_pool {
+ /* Protects the entire pool. We have one pool per queue and only one
+ * operation can be active per rule at a given time. Thus this lock
+ * protects solely against concurrent garbage collection and we expect
+ * very little contention.
+ */
+ struct mutex lock;
struct mlx5hws_action_ste_pool_element elems[MLX5HWS_POOL_OPTIMIZE_MAX];
};
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/context.h b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/context.h
index e987e93bbc6e..3f8938c73dc0 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/context.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/context.h
@@ -40,6 +40,7 @@ struct mlx5hws_context {
u32 pd_num;
struct mlx5hws_pool *stc_pool;
struct mlx5hws_action_ste_pool *action_ste_pool; /* One per queue */
+ struct delayed_work action_ste_cleanup;
struct mlx5hws_context_common_res common_res;
struct mlx5hws_pattern_cache *pattern_cache;
struct mlx5hws_definer_cache *definer_cache;
--
2.31.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH net-next 11/12] net/mlx5: HWS, Free unused action STE tables
2025-04-08 14:00 ` [PATCH net-next 11/12] net/mlx5: HWS, Free unused action STE tables Tariq Toukan
@ 2025-04-10 17:28 ` Michal Kubiak
2025-04-10 18:20 ` Vlad Dogaru
0 siblings, 1 reply; 32+ messages in thread
From: Michal Kubiak @ 2025-04-10 17:28 UTC (permalink / raw)
To: Tariq Toukan
Cc: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet,
Andrew Lunn, Gal Pressman, Leon Romanovsky, Saeed Mahameed,
Leon Romanovsky, netdev, linux-rdma, linux-kernel, Moshe Shemesh,
Mark Bloch, Vlad Dogaru, Yevgeny Kliteynik
On Tue, Apr 08, 2025 at 05:00:55PM +0300, Tariq Toukan wrote:
> From: Vlad Dogaru <vdogaru@nvidia.com>
>
> Periodically check for unused action STE tables and free their
> associated resources. In order to do this safely, add a per-queue lock
> to synchronize the garbage collect work with regular operations on
> steering rules.
>
> Signed-off-by: Vlad Dogaru <vdogaru@nvidia.com>
> Reviewed-by: Yevgeny Kliteynik <kliteyn@nvidia.com>
> Reviewed-by: Mark Bloch <mbloch@nvidia.com>
> Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
> ---
> .../mlx5/core/steering/hws/action_ste_pool.c | 88 ++++++++++++++++++-
> .../mlx5/core/steering/hws/action_ste_pool.h | 11 +++
> .../mellanox/mlx5/core/steering/hws/context.h | 1 +
> 3 files changed, 96 insertions(+), 4 deletions(-)
[...]
> +
> +static void hws_action_ste_pool_cleanup(struct work_struct *work)
> +{
> + enum mlx5hws_pool_optimize opt;
> + struct mlx5hws_context *ctx;
> + LIST_HEAD(cleanup);
> + int i;
> +
> + ctx = container_of(work, struct mlx5hws_context,
> + action_ste_cleanup.work);
> +
> + for (i = 0; i < ctx->queues; i++) {
> + struct mlx5hws_action_ste_pool *p = &ctx->action_ste_pool[i];
> +
> + mutex_lock(&p->lock);
> + for (opt = MLX5HWS_POOL_OPTIMIZE_NONE;
> + opt < MLX5HWS_POOL_OPTIMIZE_MAX; opt++)
> + hws_action_ste_pool_element_collect_stale(
> + &p->elems[opt], &cleanup);
> + mutex_unlock(&p->lock);
> + }
As I understand, in the loop above all unused items are being collected
to remove them at the end of the function, using `hws_action_ste_table_cleanup_list()`.
I noticed that only the collecting of elements is protected with the mutex.
So I have a question: is it possible that in a very short period of time
(between `mutex_unlock()` and `hws_action_ste_table_cleanup_list()` calls),
the cleanup list can somehow be invalidated (by changing the STE state
in another thread)?
> +
> + hws_action_ste_table_cleanup_list(&cleanup);
> +
> + schedule_delayed_work(&ctx->action_ste_cleanup,
> + secs_to_jiffies(
> + MLX5HWS_ACTION_STE_POOL_CLEANUP_SECONDS));
> +}
> +
[...]
Thanks,
Michal
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH net-next 11/12] net/mlx5: HWS, Free unused action STE tables
2025-04-10 17:28 ` Michal Kubiak
@ 2025-04-10 18:20 ` Vlad Dogaru
2025-04-11 11:22 ` Michal Kubiak
0 siblings, 1 reply; 32+ messages in thread
From: Vlad Dogaru @ 2025-04-10 18:20 UTC (permalink / raw)
To: Michal Kubiak
Cc: Tariq Toukan, David S. Miller, Jakub Kicinski, Paolo Abeni,
Eric Dumazet, Andrew Lunn, Gal Pressman, Leon Romanovsky,
Saeed Mahameed, Leon Romanovsky, netdev, linux-rdma, linux-kernel,
Moshe Shemesh, Mark Bloch, Yevgeny Kliteynik
On Thu, Apr 10, 2025 at 07:28:18PM +0200, Michal Kubiak wrote:
> On Tue, Apr 08, 2025 at 05:00:55PM +0300, Tariq Toukan wrote:
> > From: Vlad Dogaru <vdogaru@nvidia.com>
> >
> > Periodically check for unused action STE tables and free their
> > associated resources. In order to do this safely, add a per-queue lock
> > to synchronize the garbage collect work with regular operations on
> > steering rules.
> >
> > Signed-off-by: Vlad Dogaru <vdogaru@nvidia.com>
> > Reviewed-by: Yevgeny Kliteynik <kliteyn@nvidia.com>
> > Reviewed-by: Mark Bloch <mbloch@nvidia.com>
> > Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
> > ---
> > .../mlx5/core/steering/hws/action_ste_pool.c | 88 ++++++++++++++++++-
> > .../mlx5/core/steering/hws/action_ste_pool.h | 11 +++
> > .../mellanox/mlx5/core/steering/hws/context.h | 1 +
> > 3 files changed, 96 insertions(+), 4 deletions(-)
>
> [...]
>
> > +
> > +static void hws_action_ste_pool_cleanup(struct work_struct *work)
> > +{
> > + enum mlx5hws_pool_optimize opt;
> > + struct mlx5hws_context *ctx;
> > + LIST_HEAD(cleanup);
> > + int i;
> > +
> > + ctx = container_of(work, struct mlx5hws_context,
> > + action_ste_cleanup.work);
> > +
> > + for (i = 0; i < ctx->queues; i++) {
> > + struct mlx5hws_action_ste_pool *p = &ctx->action_ste_pool[i];
> > +
> > + mutex_lock(&p->lock);
> > + for (opt = MLX5HWS_POOL_OPTIMIZE_NONE;
> > + opt < MLX5HWS_POOL_OPTIMIZE_MAX; opt++)
> > + hws_action_ste_pool_element_collect_stale(
> > + &p->elems[opt], &cleanup);
> > + mutex_unlock(&p->lock);
> > + }
>
> As I understand, in the loop above all unused items are being collected
> to remove them at the end of the function, using `hws_action_ste_table_cleanup_list()`.
>
> I noticed that only the collecting of elements is protected with the mutex.
> So I have a question: is it possible that in a very short period of time
> (between `mutex_unlock()` and `hws_action_ste_table_cleanup_list()` calls),
> the cleanup list can somehow be invalidated (by changing the STE state
> in another thread)?
An action_ste_table is either:
(a) in an action_ste_pool (indirectly, via a pool element); or
(b) in a cleanup list.
In situation (a), both the table's last_used timestamp and its offsets
are protected by the parent pool's mutex. The table can only be accessed
through its parent pool.
In situation (b), the table can only be accessed by its parent list, and
the only thing we do with it is free it.
There is only one transition, from state (a) to state (b), never the
other way around. This transition is done under the parent pool's mutex.
We only move tables to the cleanup list when all of their elements are
available, so there is no risk of a `chunk_free` call accessing a table
that's on a cleanup list: there are no chunks left to free.
I think this is airtight, but I'm happy to explore possible scenarios if
you have any in mind.
Thank you for the detailed review,
Vlad
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH net-next 11/12] net/mlx5: HWS, Free unused action STE tables
2025-04-10 18:20 ` Vlad Dogaru
@ 2025-04-11 11:22 ` Michal Kubiak
0 siblings, 0 replies; 32+ messages in thread
From: Michal Kubiak @ 2025-04-11 11:22 UTC (permalink / raw)
To: Vlad Dogaru
Cc: Tariq Toukan, David S. Miller, Jakub Kicinski, Paolo Abeni,
Eric Dumazet, Andrew Lunn, Gal Pressman, Leon Romanovsky,
Saeed Mahameed, Leon Romanovsky, netdev, linux-rdma, linux-kernel,
Moshe Shemesh, Mark Bloch, Yevgeny Kliteynik
On Thu, Apr 10, 2025 at 08:20:25PM +0200, Vlad Dogaru wrote:
> On Thu, Apr 10, 2025 at 07:28:18PM +0200, Michal Kubiak wrote:
> > On Tue, Apr 08, 2025 at 05:00:55PM +0300, Tariq Toukan wrote:
> > > From: Vlad Dogaru <vdogaru@nvidia.com>
> > >
> > > Periodically check for unused action STE tables and free their
> > > associated resources. In order to do this safely, add a per-queue lock
> > > to synchronize the garbage collect work with regular operations on
> > > steering rules.
> > >
> > > Signed-off-by: Vlad Dogaru <vdogaru@nvidia.com>
> > > Reviewed-by: Yevgeny Kliteynik <kliteyn@nvidia.com>
> > > Reviewed-by: Mark Bloch <mbloch@nvidia.com>
> > > Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
> > > ---
> > > .../mlx5/core/steering/hws/action_ste_pool.c | 88 ++++++++++++++++++-
> > > .../mlx5/core/steering/hws/action_ste_pool.h | 11 +++
> > > .../mellanox/mlx5/core/steering/hws/context.h | 1 +
> > > 3 files changed, 96 insertions(+), 4 deletions(-)
> >
> > [...]
> >
> > > +
> > > +static void hws_action_ste_pool_cleanup(struct work_struct *work)
> > > +{
> > > + enum mlx5hws_pool_optimize opt;
> > > + struct mlx5hws_context *ctx;
> > > + LIST_HEAD(cleanup);
> > > + int i;
> > > +
> > > + ctx = container_of(work, struct mlx5hws_context,
> > > + action_ste_cleanup.work);
> > > +
> > > + for (i = 0; i < ctx->queues; i++) {
> > > + struct mlx5hws_action_ste_pool *p = &ctx->action_ste_pool[i];
> > > +
> > > + mutex_lock(&p->lock);
> > > + for (opt = MLX5HWS_POOL_OPTIMIZE_NONE;
> > > + opt < MLX5HWS_POOL_OPTIMIZE_MAX; opt++)
> > > + hws_action_ste_pool_element_collect_stale(
> > > + &p->elems[opt], &cleanup);
> > > + mutex_unlock(&p->lock);
> > > + }
> >
> > As I understand, in the loop above all unused items are being collected
> > to remove them at the end of the function, using `hws_action_ste_table_cleanup_list()`.
> >
> > I noticed that only the collecting of elements is protected with the mutex.
> > So I have a question: is it possible that in a very short period of time
> > (between `mutex_unlock()` and `hws_action_ste_table_cleanup_list()` calls),
> > the cleanup list can somehow be invalidated (by changing the STE state
> > in another thread)?
>
> An action_ste_table is either:
> (a) in an action_ste_pool (indirectly, via a pool element); or
> (b) in a cleanup list.
>
> In situation (a), both the table's last_used timestamp and its offsets
> are protected by the parent pool's mutex. The table can only be accessed
> through its parent pool.
>
> In situation (b), the table can only be accessed by its parent list, and
> the only thing we do with it is free it.
>
> There is only one transition, from state (a) to state (b), never the
> other way around. This transition is done under the parent pool's mutex.
>
> We only move tables to the cleanup list when all of their elements are
> available, so there is no risk of a `chunk_free` call accessing a table
> that's on a cleanup list: there are no chunks left to free.
>
> I think this is airtight, but I'm happy to explore possible scenarios if
> you have any in mind.
>
> Thank you for the detailed review,
> Vlad
Hi Vlad,
Thanks for your detailed explanation!
I was under the mistaken assumption that the `cleanup` list only had
some sort of reference to actual data. Now I see that the function
`hws_action_ste_pool_element_collect_stale` moves the real STE data
from one list to another, so only that function call should be protected
by the mutex.
Perhaps, I should have inspected the implementation of
`hws_action_ste_pool_element_collect_stale()` more closely. :-)
Thanks,
Reviewed-by: Michal Kubiak <michal.kubiak@intel.com>
PS. I saw your V2 - will take a look.
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH net-next 12/12] net/mlx5: HWS, Export action STE tables to debugfs
2025-04-08 14:00 [PATCH net-next 00/12] net/mlx5: HWS, Refactor action STE handling Tariq Toukan
` (10 preceding siblings ...)
2025-04-08 14:00 ` [PATCH net-next 11/12] net/mlx5: HWS, Free unused action STE tables Tariq Toukan
@ 2025-04-08 14:00 ` Tariq Toukan
2025-04-10 17:06 ` Michal Kubiak
11 siblings, 1 reply; 32+ messages in thread
From: Tariq Toukan @ 2025-04-08 14:00 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet,
Andrew Lunn
Cc: Gal Pressman, Leon Romanovsky, Saeed Mahameed, Leon Romanovsky,
Tariq Toukan, netdev, linux-rdma, linux-kernel, Moshe Shemesh,
Mark Bloch, Vlad Dogaru, Yevgeny Kliteynik
From: Vlad Dogaru <vdogaru@nvidia.com>
Introduce a new type of dump object and dump all action STE tables,
along with information on their RTCs and STEs.
Signed-off-by: Vlad Dogaru <vdogaru@nvidia.com>
Reviewed-by: Hamdan Agbariya <hamdani@nvidia.com>
Reviewed-by: Mark Bloch <mbloch@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
.../mellanox/mlx5/core/steering/hws/debug.c | 36 ++++++++++++++++++-
.../mellanox/mlx5/core/steering/hws/debug.h | 2 ++
2 files changed, 37 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/debug.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/debug.c
index 38f75dec9cfc..91568d6c1dac 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/debug.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/debug.c
@@ -387,10 +387,41 @@ static int hws_debug_dump_context_stc(struct seq_file *f, struct mlx5hws_context
return 0;
}
+static void
+hws_debug_dump_action_ste_table(struct seq_file *f,
+ struct mlx5hws_action_ste_table *action_tbl)
+{
+ int ste_0_id = mlx5hws_pool_get_base_id(action_tbl->pool);
+ int ste_1_id = mlx5hws_pool_get_base_mirror_id(action_tbl->pool);
+
+ seq_printf(f, "%d,0x%llx,%d,%d,%d,%d\n",
+ MLX5HWS_DEBUG_RES_TYPE_ACTION_STE_TABLE,
+ HWS_PTR_TO_ID(action_tbl),
+ action_tbl->rtc_0_id, ste_0_id,
+ action_tbl->rtc_1_id, ste_1_id);
+}
+
+static void hws_debug_dump_action_ste_pool(struct seq_file *f,
+ struct mlx5hws_action_ste_pool *pool)
+{
+ struct mlx5hws_action_ste_table *action_tbl;
+ enum mlx5hws_pool_optimize opt;
+
+ mutex_lock(&pool->lock);
+ for (opt = MLX5HWS_POOL_OPTIMIZE_NONE; opt < MLX5HWS_POOL_OPTIMIZE_MAX;
+ opt++) {
+ list_for_each_entry(action_tbl, &pool->elems[opt].available,
+ list_node) {
+ hws_debug_dump_action_ste_table(f, action_tbl);
+ }
+ }
+ mutex_unlock(&pool->lock);
+}
+
static int hws_debug_dump_context(struct seq_file *f, struct mlx5hws_context *ctx)
{
struct mlx5hws_table *tbl;
- int ret;
+ int ret, i;
ret = hws_debug_dump_context_info(f, ctx);
if (ret)
@@ -410,6 +441,9 @@ static int hws_debug_dump_context(struct seq_file *f, struct mlx5hws_context *ct
return ret;
}
+ for (i = 0; i < ctx->queues; i++)
+ hws_debug_dump_action_ste_pool(f, &ctx->action_ste_pool[i]);
+
return 0;
}
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/debug.h b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/debug.h
index e44e7ae28f93..89c396f9f266 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/debug.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/debug.h
@@ -26,6 +26,8 @@ enum mlx5hws_debug_res_type {
MLX5HWS_DEBUG_RES_TYPE_MATCHER_TEMPLATE_HASH_DEFINER = 4205,
MLX5HWS_DEBUG_RES_TYPE_MATCHER_TEMPLATE_RANGE_DEFINER = 4206,
MLX5HWS_DEBUG_RES_TYPE_MATCHER_TEMPLATE_COMPARE_MATCH_DEFINER = 4207,
+
+ MLX5HWS_DEBUG_RES_TYPE_ACTION_STE_TABLE = 4300,
};
static inline u64
--
2.31.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH net-next 12/12] net/mlx5: HWS, Export action STE tables to debugfs
2025-04-08 14:00 ` [PATCH net-next 12/12] net/mlx5: HWS, Export action STE tables to debugfs Tariq Toukan
@ 2025-04-10 17:06 ` Michal Kubiak
0 siblings, 0 replies; 32+ messages in thread
From: Michal Kubiak @ 2025-04-10 17:06 UTC (permalink / raw)
To: Tariq Toukan
Cc: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet,
Andrew Lunn, Gal Pressman, Leon Romanovsky, Saeed Mahameed,
Leon Romanovsky, netdev, linux-rdma, linux-kernel, Moshe Shemesh,
Mark Bloch, Vlad Dogaru, Yevgeny Kliteynik
On Tue, Apr 08, 2025 at 05:00:56PM +0300, Tariq Toukan wrote:
> From: Vlad Dogaru <vdogaru@nvidia.com>
>
> Introduce a new type of dump object and dump all action STE tables,
> along with information on their RTCs and STEs.
>
> Signed-off-by: Vlad Dogaru <vdogaru@nvidia.com>
> Reviewed-by: Hamdan Agbariya <hamdani@nvidia.com>
> Reviewed-by: Mark Bloch <mbloch@nvidia.com>
> Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
Thanks,
Reviewed-by: Michal Kubiak <michal.kubiak@intel.com>
^ permalink raw reply [flat|nested] 32+ messages in thread