2023-05-07 14:49:46

by Tariq Toukan

[permalink] [raw]
Subject: [PATCH net] linux/dim: Do nothing if no time delta between samples

From: Roy Novich <[email protected]>

Add return value for dim_calc_stats. This is an indication for the
caller if curr_stats was assigned by the function. Avoid using
curr_stats uninitialized over {rdma/net}_dim, when no time delta between
samples. Coverity reported this potential use of an uninitialized
variable.

Fixes: 4c4dbb4a7363 ("net/mlx5e: Move dynamic interrupt coalescing code to include/linux")
Fixes: cb3c7fd4f839 ("net/mlx5e: Support adaptive RX coalescing")
Signed-off-by: Roy Novich <[email protected]>
Reviewed-by: Aya Levin <[email protected]>
Reviewed-by: Saeed Mahameed <[email protected]>
Signed-off-by: Tariq Toukan <[email protected]>
---
include/linux/dim.h | 3 ++-
lib/dim/dim.c | 5 +++--
lib/dim/net_dim.c | 3 ++-
lib/dim/rdma_dim.c | 3 ++-
4 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/include/linux/dim.h b/include/linux/dim.h
index 6c5733981563..f343bc9aa2ec 100644
--- a/include/linux/dim.h
+++ b/include/linux/dim.h
@@ -236,8 +236,9 @@ void dim_park_tired(struct dim *dim);
*
* Calculate the delta between two samples (in data rates).
* Takes into consideration counter wrap-around.
+ * Returned boolean indicates whether curr_stats are reliable.
*/
-void dim_calc_stats(struct dim_sample *start, struct dim_sample *end,
+bool dim_calc_stats(struct dim_sample *start, struct dim_sample *end,
struct dim_stats *curr_stats);

/**
diff --git a/lib/dim/dim.c b/lib/dim/dim.c
index 38045d6d0538..e89aaf07bde5 100644
--- a/lib/dim/dim.c
+++ b/lib/dim/dim.c
@@ -54,7 +54,7 @@ void dim_park_tired(struct dim *dim)
}
EXPORT_SYMBOL(dim_park_tired);

-void dim_calc_stats(struct dim_sample *start, struct dim_sample *end,
+bool dim_calc_stats(struct dim_sample *start, struct dim_sample *end,
struct dim_stats *curr_stats)
{
/* u32 holds up to 71 minutes, should be enough */
@@ -66,7 +66,7 @@ void dim_calc_stats(struct dim_sample *start, struct dim_sample *end,
start->comp_ctr);

if (!delta_us)
- return;
+ return false;

curr_stats->ppms = DIV_ROUND_UP(npkts * USEC_PER_MSEC, delta_us);
curr_stats->bpms = DIV_ROUND_UP(nbytes * USEC_PER_MSEC, delta_us);
@@ -79,5 +79,6 @@ void dim_calc_stats(struct dim_sample *start, struct dim_sample *end,
else
curr_stats->cpe_ratio = 0;

+ return true;
}
EXPORT_SYMBOL(dim_calc_stats);
diff --git a/lib/dim/net_dim.c b/lib/dim/net_dim.c
index 53f6b9c6e936..4e32f7aaac86 100644
--- a/lib/dim/net_dim.c
+++ b/lib/dim/net_dim.c
@@ -227,7 +227,8 @@ void net_dim(struct dim *dim, struct dim_sample end_sample)
dim->start_sample.event_ctr);
if (nevents < DIM_NEVENTS)
break;
- dim_calc_stats(&dim->start_sample, &end_sample, &curr_stats);
+ if (!dim_calc_stats(&dim->start_sample, &end_sample, &curr_stats))
+ break;
if (net_dim_decision(&curr_stats, dim)) {
dim->state = DIM_APPLY_NEW_PROFILE;
schedule_work(&dim->work);
diff --git a/lib/dim/rdma_dim.c b/lib/dim/rdma_dim.c
index 15462d54758d..88f779486707 100644
--- a/lib/dim/rdma_dim.c
+++ b/lib/dim/rdma_dim.c
@@ -88,7 +88,8 @@ void rdma_dim(struct dim *dim, u64 completions)
nevents = curr_sample->event_ctr - dim->start_sample.event_ctr;
if (nevents < DIM_NEVENTS)
break;
- dim_calc_stats(&dim->start_sample, curr_sample, &curr_stats);
+ if (!dim_calc_stats(&dim->start_sample, curr_sample, &curr_stats))
+ break;
if (rdma_dim_decision(&curr_stats, dim)) {
dim->state = DIM_APPLY_NEW_PROFILE;
schedule_work(&dim->work);
--
2.34.1


2023-05-08 06:27:19

by Leon Romanovsky

[permalink] [raw]
Subject: Re: [PATCH net] linux/dim: Do nothing if no time delta between samples

On Sun, May 07, 2023 at 04:57:43PM +0300, Tariq Toukan wrote:
> From: Roy Novich <[email protected]>
>
> Add return value for dim_calc_stats. This is an indication for the
> caller if curr_stats was assigned by the function. Avoid using
> curr_stats uninitialized over {rdma/net}_dim, when no time delta between
> samples. Coverity reported this potential use of an uninitialized
> variable.
>
> Fixes: 4c4dbb4a7363 ("net/mlx5e: Move dynamic interrupt coalescing code to include/linux")
> Fixes: cb3c7fd4f839 ("net/mlx5e: Support adaptive RX coalescing")
> Signed-off-by: Roy Novich <[email protected]>
> Reviewed-by: Aya Levin <[email protected]>
> Reviewed-by: Saeed Mahameed <[email protected]>
> Signed-off-by: Tariq Toukan <[email protected]>
> ---
> include/linux/dim.h | 3 ++-
> lib/dim/dim.c | 5 +++--
> lib/dim/net_dim.c | 3 ++-
> lib/dim/rdma_dim.c | 3 ++-
> 4 files changed, 9 insertions(+), 5 deletions(-)
>

Thanks,
Reviewed-by: Leon Romanovsky <[email protected]>

2023-05-08 12:52:29

by Michal Kubiak

[permalink] [raw]
Subject: Re: [PATCH net] linux/dim: Do nothing if no time delta between samples

On Sun, May 07, 2023 at 04:57:43PM +0300, Tariq Toukan wrote:
> From: Roy Novich <[email protected]>
>
> Add return value for dim_calc_stats. This is an indication for the
> caller if curr_stats was assigned by the function. Avoid using
> curr_stats uninitialized over {rdma/net}_dim, when no time delta between
> samples. Coverity reported this potential use of an uninitialized
> variable.
>
> Fixes: 4c4dbb4a7363 ("net/mlx5e: Move dynamic interrupt coalescing code to include/linux")
> Fixes: cb3c7fd4f839 ("net/mlx5e: Support adaptive RX coalescing")
> Signed-off-by: Roy Novich <[email protected]>
> Reviewed-by: Aya Levin <[email protected]>
> Reviewed-by: Saeed Mahameed <[email protected]>
> Signed-off-by: Tariq Toukan <[email protected]>
> ---

Looks good to me.

Thanks,
Reviewed-by: Michal Kubiak <[email protected]>

2023-05-09 09:52:18

by patchwork-bot+netdevbpf

[permalink] [raw]
Subject: Re: [PATCH net] linux/dim: Do nothing if no time delta between samples

Hello:

This patch was applied to netdev/net.git (main)
by Paolo Abeni <[email protected]>:

On Sun, 7 May 2023 16:57:43 +0300 you wrote:
> From: Roy Novich <[email protected]>
>
> Add return value for dim_calc_stats. This is an indication for the
> caller if curr_stats was assigned by the function. Avoid using
> curr_stats uninitialized over {rdma/net}_dim, when no time delta between
> samples. Coverity reported this potential use of an uninitialized
> variable.
>
> [...]

Here is the summary with links:
- [net] linux/dim: Do nothing if no time delta between samples
https://git.kernel.org/netdev/net/c/162bd18eb55a

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html