On 3/6/2024 12:24 PM, shaib...@amazon.com wrote:
> From: Shai Brandes <shaib...@amazon.com>
>
> limits the exponent in the exponential backoff
> mechanism in order to avoid the value overflowing.
>
Is this a fix?
What was the impact of the overflowing if not limited? And is there a
significance of the value 16, can you please elaborate?
Also let me remind the patch subject format, (this may look
insignificant but helps to have more unified commit messages for
developers, and if not updated by author, maintainers update it and this
brings more overhead to maintainers):
"sub-module: verb object"
And we use verb 'fix' explicitly for all commits fixing something, and
that something can't be referenced as 'error', 'failure', 'issue',
'problem', etc... but it should be detailed.
Most of the times better to document NOT from driver internal
perspective, but impact of it, like "net/ena: set chain limit to 16" is
NOT a good one, it explains driver internal perspective (making all up)
but it can be something like:
"net/ena: support big packets by increasing link limit"
For this one, I am not sure impact of the change so hard for me to
propose an alternative, but just as example it can be something like:
"net/ena/base: avoid collision by limiting backoff delay"
> Signed-off-by: Shai Brandes <shaib...@amazon.com>
> Reviewed-by: Amit Bernstein <amitb...@amazon.com>
> ---
> drivers/net/ena/hal/ena_com.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ena/hal/ena_com.c b/drivers/net/ena/hal/ena_com.c
> index 6953a1fa33..31c37b0ab3 100644
> --- a/drivers/net/ena/hal/ena_com.c
> +++ b/drivers/net/ena/hal/ena_com.c
> @@ -34,6 +34,8 @@
>
> #define ENA_REGS_ADMIN_INTR_MASK 1
>
> +#define ENA_MAX_BACKOFF_DELAY_EXP 16U
> +
> #define ENA_MIN_ADMIN_POLL_US 100
>
> #define ENA_MAX_ADMIN_POLL_US 5000
> @@ -545,8 +547,9 @@ static int ena_com_comp_status_to_errno(struct
> ena_com_admin_queue *admin_queue,
>
> static void ena_delay_exponential_backoff_us(u32 exp, u32 delay_us)
> {
> + exp = ENA_MIN32(ENA_MAX_BACKOFF_DELAY_EXP, exp);
> delay_us = ENA_MAX32(ENA_MIN_ADMIN_POLL_US, delay_us);
> - delay_us = ENA_MIN32(delay_us * (1U << exp), ENA_MAX_ADMIN_POLL_US);
> + delay_us = ENA_MIN32(ENA_MAX_ADMIN_POLL_US, delay_us * (1U << exp));
> ENA_USLEEP(delay_us);
> }
>
