|SUCCESS| net/bnxt: add cast to wider type before multiplication

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/163282

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-04-24

git sha: Not directly available from patch format
Author: Denis Lyulin <lyulin.2003 at mail.ru>
Subject: [PATCH] net/bnxt: add cast to wider type before multiplication

This patch adds uint64_t casts before multiplication operations to
prevent potential integer overflow warnings from static analyzers when
calling rte_eal_alarm_set(). The first parameter of rte_eal_alarm_set()
is uint64_t, and the patch ensures uint32_t values are cast before
multiplication.

> diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
> index b677f9491d..bdfab3eef2 100644
> --- a/drivers/net/bnxt/bnxt_ethdev.c
> +++ b/drivers/net/bnxt/bnxt_ethdev.c
> @@ -4861,7 +4861,7 @@ static void bnxt_dev_recover(void *arg)
>  void bnxt_dev_reset_and_resume(void *arg)
>  {
>  	struct bnxt *bp = arg;
> -	uint32_t us = US_PER_MS * bp->fw_reset_min_msecs;
> +	uint64_t us = US_PER_MS * (uint64_t)bp->fw_reset_min_msecs;

Does this code still risk overflow? The cast happens after US_PER_MS has
already been multiplied by bp->fw_reset_min_msecs, so if both operands
are uint32_t, the multiplication occurs in 32-bit space before the cast
to uint64_t takes effect.

Should the cast be applied before the multiplication, like this:
    uint64_t us = (uint64_t)US_PER_MS * bp->fw_reset_min_msecs;
or:
    uint64_t us = US_PER_MS * (uint64_t)bp->fw_reset_min_msecs;

Wait, looking more carefully at the placement of the cast in your change,
the cast is on bp->fw_reset_min_msecs before multiplication, so this
should promote the operation to 64-bit. The order of operations is:
1. Cast bp->fw_reset_min_msecs to uint64_t
2. Multiply US_PER_MS (as uint32_t promoted to uint64_t) by the cast value

This appears correct. However, in the same function body there's a usage
that may still have the issue - does the 'us' variable get used with
rte_eal_alarm_set() somewhere below?

[ ... ]

> @@ -5008,7 +5008,7 @@ static void bnxt_check_fw_health(void *arg)
>  
>  	info->last_reset_counter = val;
>  
> -	rte_eal_alarm_set(US_PER_MS * info->driver_polling_freq,
> +	rte_eal_alarm_set(US_PER_MS * (uint64_t)info->driver_polling_freq,
>  			  bnxt_check_fw_health, (void *)bp);

Same question as above - does this cast occur at the right point in the
evaluation order to prevent overflow?

[ ... ]

> @@ -5030,7 +5030,7 @@ static void bnxt_check_fw_health(void *arg)
>  	else
>  		wait_msec = info->normal_func_wait_period;
>  
> -	rte_eal_alarm_set(US_PER_MS * wait_msec,
> +	rte_eal_alarm_set(US_PER_MS * (uint64_t)wait_msec,
>  			  bnxt_fw_reset_cb, (void *)bp);
>  }

[ ... ]

> @@ -5048,7 +5048,7 @@ void bnxt_schedule_fw_health_check(struct bnxt *bp)
>  
>  	polling_freq = bp->recovery_info->driver_polling_freq;
>  
> -	rte_eal_alarm_set(US_PER_MS * polling_freq,
> +	rte_eal_alarm_set(US_PER_MS * (uint64_t)polling_freq,
>  			  bnxt_check_fw_health, (void *)bp);
>  	bp->flags |= BNXT_FLAG_FW_HEALTH_CHECK_SCHEDULED;

These casts all appear to follow the same pattern. Can you verify that
the parenthesization ensures the cast happens before the multiplication
in all cases? The expression `US_PER_MS * (uint64_t)info->driver_polling_freq`
should promote correctly, but it would be clearer to cast the first
operand instead: `(uint64_t)US_PER_MS * info->driver_polling_freq`.