From: Vlad Dogaru <vdogaru@nvidia.com>
The bwc layer will use `mlx5hws_rule_skip` to keep track of numbers of
RX and TX rules individually, so export this function for future usage.
While we're in there, reduce nesting by adding a couple of early return
statements.
Signed-off-by: Vlad Dogaru <vdogaru@nvidia.com>
Reviewed-by: Yevgeny Kliteynik <kliteyn@nvidia.com>
Signed-off-by: Mark Bloch <mbloch@nvidia.com>
---
.../mellanox/mlx5/core/steering/hws/rule.c | 35 ++++++++++---------
.../mellanox/mlx5/core/steering/hws/rule.h | 3 ++
2 files changed, 21 insertions(+), 17 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 5342a4cc7194..0370b9b87d4e 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/rule.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/rule.c
@@ -3,10 +3,8 @@
#include "internal.h"
-static void hws_rule_skip(struct mlx5hws_matcher *matcher,
- struct mlx5hws_match_template *mt,
- u32 flow_source,
- bool *skip_rx, bool *skip_tx)
+void mlx5hws_rule_skip(struct mlx5hws_matcher *matcher, u32 flow_source,
+ bool *skip_rx, bool *skip_tx)
{
/* By default FDB rules are added to both RX and TX */
*skip_rx = false;
@@ -14,20 +12,22 @@ static void hws_rule_skip(struct mlx5hws_matcher *matcher,
if (flow_source == MLX5_FLOW_CONTEXT_FLOW_SOURCE_LOCAL_VPORT) {
*skip_rx = true;
- } else if (flow_source == MLX5_FLOW_CONTEXT_FLOW_SOURCE_UPLINK) {
+ return;
+ }
+
+ if (flow_source == MLX5_FLOW_CONTEXT_FLOW_SOURCE_UPLINK) {
*skip_tx = true;
- } else {
- /* If no flow source was set for current rule,
- * check for flow source in matcher attributes.
- */
- if (matcher->attr.optimize_flow_src) {
- *skip_tx =
- matcher->attr.optimize_flow_src == MLX5HWS_MATCHER_FLOW_SRC_WIRE;
- *skip_rx =
- matcher->attr.optimize_flow_src == MLX5HWS_MATCHER_FLOW_SRC_VPORT;
- return;
- }
+ return;
}
+
+ /* If no flow source was set for the rule, check for flow source in
+ * matcher attributes.
+ */
+ if (matcher->attr.optimize_flow_src == MLX5HWS_MATCHER_FLOW_SRC_WIRE)
+ *skip_tx = true;
+ else if (matcher->attr.optimize_flow_src ==
+ MLX5HWS_MATCHER_FLOW_SRC_VPORT)
+ *skip_rx = true;
}
static void
@@ -66,7 +66,8 @@ static void hws_rule_init_dep_wqe(struct mlx5hws_send_ring_dep_wqe *dep_wqe,
attr->rule_idx : 0;
if (tbl->type == MLX5HWS_TABLE_TYPE_FDB) {
- hws_rule_skip(matcher, mt, attr->flow_source, &skip_rx, &skip_tx);
+ mlx5hws_rule_skip(matcher, attr->flow_source, &skip_rx,
+ &skip_tx);
if (!skip_rx) {
dep_wqe->rtc_0 = matcher->match_ste.rtc_0_id;
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 1c47a9c11572..d0f082b8dbf5 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/rule.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/rule.h
@@ -69,6 +69,9 @@ struct mlx5hws_rule {
*/
};
+void mlx5hws_rule_skip(struct mlx5hws_matcher *matcher, u32 flow_source,
+ bool *skip_rx, bool *skip_tx);
+
void mlx5hws_rule_free_action_ste(struct mlx5hws_action_ste_chunk *action_ste);
int mlx5hws_rule_move_hws_remove(struct mlx5hws_rule *rule,
--
2.34.1On Sun, Jun 22, 2025 at 08:22:21PM +0300, Mark Bloch wrote: > From: Vlad Dogaru <vdogaru@nvidia.com> > > The bwc layer will use `mlx5hws_rule_skip` to keep track of numbers of > RX and TX rules individually, so export this function for future usage. > > While we're in there, reduce nesting by adding a couple of early return > statements. I'm all for reducing nesting. But this patch has two distinct changes. Please consider splitting it into two patches. > > Signed-off-by: Vlad Dogaru <vdogaru@nvidia.com> > Reviewed-by: Yevgeny Kliteynik <kliteyn@nvidia.com> > Signed-off-by: Mark Bloch <mbloch@nvidia.com> ...
Hi Simon, Thanks for reviewing the patches! On 24-Jun-25 21:38, Simon Horman wrote: > On Sun, Jun 22, 2025 at 08:22:21PM +0300, Mark Bloch wrote: >> From: Vlad Dogaru <vdogaru@nvidia.com> >> >> The bwc layer will use `mlx5hws_rule_skip` to keep track of numbers of >> RX and TX rules individually, so export this function for future usage. >> >> While we're in there, reduce nesting by adding a couple of early return >> statements. > > I'm all for reducing nesting. But this patch has two distinct changes. > Please consider splitting it into two patches. Not sure I'd send the refactor thing alone - it isn't worth the effort IMHO... But since I'm already in here - sure, will sent it in a separate patch. >> >> Signed-off-by: Vlad Dogaru <vdogaru@nvidia.com> >> Reviewed-by: Yevgeny Kliteynik <kliteyn@nvidia.com> >> Signed-off-by: Mark Bloch <mbloch@nvidia.com> > > ...
On Wed, Jun 25, 2025 at 03:35:52AM +0300, Yevgeny Kliteynik wrote: > Hi Simon, > Thanks for reviewing the patches! > > On 24-Jun-25 21:38, Simon Horman wrote: > > On Sun, Jun 22, 2025 at 08:22:21PM +0300, Mark Bloch wrote: > > > From: Vlad Dogaru <vdogaru@nvidia.com> > > > > > > The bwc layer will use `mlx5hws_rule_skip` to keep track of numbers of > > > RX and TX rules individually, so export this function for future usage. > > > > > > While we're in there, reduce nesting by adding a couple of early return > > > statements. > > > > I'm all for reducing nesting. But this patch has two distinct changes. > > Please consider splitting it into two patches. > > Not sure I'd send the refactor thing alone - it isn't worth the effort > IMHO... But since I'm already in here - sure, will sent it in a separate > patch. FWIIW, I think the refactor is fine in the context of this patchset. But I do feel that it being a separate patch within the patchset is best. ...
On 25-Jun-25 12:45, Simon Horman wrote: > On Wed, Jun 25, 2025 at 03:35:52AM +0300, Yevgeny Kliteynik wrote: >> Hi Simon, >> Thanks for reviewing the patches! >> >> On 24-Jun-25 21:38, Simon Horman wrote: >>> On Sun, Jun 22, 2025 at 08:22:21PM +0300, Mark Bloch wrote: >>>> From: Vlad Dogaru <vdogaru@nvidia.com> >>>> >>>> The bwc layer will use `mlx5hws_rule_skip` to keep track of numbers of >>>> RX and TX rules individually, so export this function for future usage. >>>> >>>> While we're in there, reduce nesting by adding a couple of early return >>>> statements. >>> >>> I'm all for reducing nesting. But this patch has two distinct changes. >>> Please consider splitting it into two patches. >> >> Not sure I'd send the refactor thing alone - it isn't worth the effort >> IMHO... But since I'm already in here - sure, will sent it in a separate >> patch. > > FWIIW, I think the refactor is fine in the context of this patchset. > But I do feel that it being a separate patch within the patchset is best. Ack, thanks
On Wed, 25 Jun 2025 03:35:52 +0300 Yevgeny Kliteynik wrote: > >> The bwc layer will use `mlx5hws_rule_skip` to keep track of numbers of > >> RX and TX rules individually, so export this function for future usage. > >> > >> While we're in there, reduce nesting by adding a couple of early return > >> statements. > > > > I'm all for reducing nesting. But this patch has two distinct changes. > > Please consider splitting it into two patches. > > Not sure I'd send the refactor thing alone - it isn't worth the effort > IMHO... But since I'm already in here - sure, will sent it in a separate > patch. FWIW having a function which returns void but with 2 output parameters is in itself a bit awkward. I'd personally return a 2 bit bitmask of which mode is enabled. But there's no accounting for taste.
On 25-Jun-25 03:45, Jakub Kicinski wrote: > On Wed, 25 Jun 2025 03:35:52 +0300 Yevgeny Kliteynik wrote: >>>> The bwc layer will use `mlx5hws_rule_skip` to keep track of numbers of >>>> RX and TX rules individually, so export this function for future usage. >>>> >>>> While we're in there, reduce nesting by adding a couple of early return >>>> statements. >>> >>> I'm all for reducing nesting. But this patch has two distinct changes. >>> Please consider splitting it into two patches. >> >> Not sure I'd send the refactor thing alone - it isn't worth the effort >> IMHO... But since I'm already in here - sure, will sent it in a separate >> patch. > > FWIW having a function which returns void but with 2 output parameters > is in itself a bit awkward. I'd personally return a 2 bit bitmask of > which mode is enabled. But there's no accounting for taste. Indeed, it's a matter of taste. I see that it's kind of a style in this area of the code ¯\_(ツ)_/¯ There are more somewhat similar cases, and I'd like to be aligned with the existing style.
© 2016 - 2025 Red Hat, Inc.