> -----Original Message-----
> From: Ferruh Yigit <ferruh.yi...@amd.com>
> Sent: Friday, March 8, 2024 7:24 PM
> To: Brandes, Shai <shaib...@amazon.com>
> Cc: dev@dpdk.org
> Subject: RE: [EXTERNAL] [PATCH v3 08/33] net/ena/hal: exponential backoff
> exp limit
>
> CAUTION: This email originated from outside of the organization. Do not click
> links or open attachments unless you can confirm the sender and know the
> content is safe.
>
>
>
> 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?
>
[Brandes, Shai] I will restructure this patch, since this likely hides a fix in
hal.
It is originated from the HAL release, from which I took the patches one by
one, but the commit messages there tend to be (too) concise.
>
> 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);
> > }
> >
