diff mbox series

[RFC,net-next,v3,1/2] net/mlx5e: Add helpers to calculate txq and ch idx

Message ID 20240529031628.324117-2-jdamato@fastly.com (mailing list archive)
State Superseded
Headers show
Series mlx5: Add netdev-genl queue stats | expand

Commit Message

Joe Damato May 29, 2024, 3:16 a.m. UTC
Add two helpers to:

1. Compute the txq_ix given a channel and a tc offset (tc_to_txq_ix).
2. Compute the channel index and tc offset given a txq_ix
   (txq_ix_to_chtc_ix).

The first helper, tc_to_txq_ix, is used in place of the mathematical
expressionin mlx5e_open_sqs when txq_ix values are computed.

The second helper, txq_ix_to_chtc_ix, will be used in a following patch.

Signed-off-by: Joe Damato <jdamato@fastly.com>
---
 .../net/ethernet/mellanox/mlx5/core/en_main.c  | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

Comments

Jacob Keller May 30, 2024, 9:15 p.m. UTC | #1
On 5/28/2024 8:16 PM, Joe Damato wrote:
> Add two helpers to:
> 
> 1. Compute the txq_ix given a channel and a tc offset (tc_to_txq_ix).
> 2. Compute the channel index and tc offset given a txq_ix
>    (txq_ix_to_chtc_ix).
> 
> The first helper, tc_to_txq_ix, is used in place of the mathematical
> expressionin mlx5e_open_sqs when txq_ix values are computed.
> 
> The second helper, txq_ix_to_chtc_ix, will be used in a following patch.
> 
> Signed-off-by: Joe Damato <jdamato@fastly.com>
> ---

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Simon Horman June 1, 2024, 11:35 a.m. UTC | #2
On Wed, May 29, 2024 at 03:16:26AM +0000, Joe Damato wrote:
> Add two helpers to:
> 
> 1. Compute the txq_ix given a channel and a tc offset (tc_to_txq_ix).
> 2. Compute the channel index and tc offset given a txq_ix
>    (txq_ix_to_chtc_ix).
> 
> The first helper, tc_to_txq_ix, is used in place of the mathematical
> expressionin mlx5e_open_sqs when txq_ix values are computed.
> 
> The second helper, txq_ix_to_chtc_ix, will be used in a following patch.

Hi Joe,

I think it would be best to add txq_ix_to_chtc_ix as part of patch that
uses it, because the current arrangement will cause allmodconfigs with
clang-18 and W=1 to fail due to txq_ix_to_chtc_ix being unused.

...
Simon Horman June 1, 2024, 11:39 a.m. UTC | #3
On Sat, Jun 01, 2024 at 12:35:57PM +0100, Simon Horman wrote:
> On Wed, May 29, 2024 at 03:16:26AM +0000, Joe Damato wrote:
> > Add two helpers to:
> > 
> > 1. Compute the txq_ix given a channel and a tc offset (tc_to_txq_ix).
> > 2. Compute the channel index and tc offset given a txq_ix
> >    (txq_ix_to_chtc_ix).
> > 
> > The first helper, tc_to_txq_ix, is used in place of the mathematical
> > expressionin mlx5e_open_sqs when txq_ix values are computed.
> > 
> > The second helper, txq_ix_to_chtc_ix, will be used in a following patch.
> 
> Hi Joe,
> 
> I think it would be best to add txq_ix_to_chtc_ix as part of patch that
> uses it, because the current arrangement will cause allmodconfigs with
> clang-18 and W=1 to fail due to txq_ix_to_chtc_ix being unused.
> 
> ...

Sorry, one more thing.

Please don't use inline in .c files unless there is a demonstrable
reason - f.e. performance - to do so. Rather, let the compiler figure
out when to inline functions.
Joe Damato June 2, 2024, 7:23 p.m. UTC | #4
On Sat, Jun 01, 2024 at 12:39:13PM +0100, Simon Horman wrote:
> On Sat, Jun 01, 2024 at 12:35:57PM +0100, Simon Horman wrote:
> > On Wed, May 29, 2024 at 03:16:26AM +0000, Joe Damato wrote:
> > > Add two helpers to:
> > > 
> > > 1. Compute the txq_ix given a channel and a tc offset (tc_to_txq_ix).
> > > 2. Compute the channel index and tc offset given a txq_ix
> > >    (txq_ix_to_chtc_ix).
> > > 
> > > The first helper, tc_to_txq_ix, is used in place of the mathematical
> > > expressionin mlx5e_open_sqs when txq_ix values are computed.
> > > 
> > > The second helper, txq_ix_to_chtc_ix, will be used in a following patch.
> > 
> > Hi Joe,
> > 
> > I think it would be best to add txq_ix_to_chtc_ix as part of patch that
> > uses it, because the current arrangement will cause allmodconfigs with
> > clang-18 and W=1 to fail due to txq_ix_to_chtc_ix being unused.
> > 
> > ...
> 
> Sorry, one more thing.
> 
> Please don't use inline in .c files unless there is a demonstrable
> reason - f.e. performance - to do so. Rather, let the compiler figure
> out when to inline functions.

Sure, I'll make sure in the next revision to include the second
helper in the second patch instead and avoid using "inline" in both
cases.
diff mbox series

Patch

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index b758bc72ac36..ce15805ad55a 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -2312,6 +2312,22 @@  static int mlx5e_txq_get_qos_node_hw_id(struct mlx5e_params *params, int txq_ix,
 	return 0;
 }
 
+static inline int tc_to_txq_ix(struct mlx5e_channel *c,
+			       struct mlx5e_params *params,
+			       int tc)
+{
+	return c->ix + tc * params->num_channels;
+}
+
+static inline void txq_ix_to_chtc_ix(struct mlx5e_params *params, int txq_ix,
+				     int *ch_ix, int *tc_ix)
+{
+	if (params->num_channels) {
+		*ch_ix = txq_ix % params->num_channels;
+		*tc_ix = txq_ix / params->num_channels;
+	}
+}
+
 static int mlx5e_open_sqs(struct mlx5e_channel *c,
 			  struct mlx5e_params *params,
 			  struct mlx5e_channel_param *cparam)
@@ -2319,7 +2335,7 @@  static int mlx5e_open_sqs(struct mlx5e_channel *c,
 	int err, tc;
 
 	for (tc = 0; tc < mlx5e_get_dcb_num_tc(params); tc++) {
-		int txq_ix = c->ix + tc * params->num_channels;
+		int txq_ix = tc_to_txq_ix(c, params, tc);
 		u32 qos_queue_group_id;
 		u32 tisn;