DPDK patches and discussions
 help / color / mirror / Atom feed
From: Ferruh Yigit <ferruh.yigit@amd.com>
To: shaibran@amazon.com
Cc: dev@dpdk.org
Subject: Re: [PATCH v3 08/33] net/ena/hal: exponential backoff exp limit
Date: Fri, 8 Mar 2024 17:24:22 +0000	[thread overview]
Message-ID: <76154018-868a-481e-9922-ed5bd166d90a@amd.com> (raw)
In-Reply-To: <20240306122445.4350-9-shaibran@amazon.com>

On 3/6/2024 12:24 PM, shaibran@amazon.com wrote:
> From: Shai Brandes <shaibran@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 <shaibran@amazon.com>
> Reviewed-by: Amit Bernstein <amitbern@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);
>  }
>  


  reply	other threads:[~2024-03-08 17:24 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-06 12:24 [PATCH v3 00/33] net/ena: v2.9.0 driver release shaibran
2024-03-06 12:24 ` [PATCH v3 01/33] net/ena: rework the metrics multi-process functions shaibran
2024-03-06 12:24 ` [PATCH v3 02/33] net/ena: report new supported link speed capabilities shaibran
2024-03-06 12:24 ` [PATCH v3 03/33] net/ena: update imissed stat with Rx overruns shaibran
2024-03-06 12:24 ` [PATCH v3 04/33] net/ena: sub-optimal configuration notifications support shaibran
2024-03-08 17:23   ` Ferruh Yigit
2024-03-10 14:43     ` Brandes, Shai
2024-03-13 11:18       ` Ferruh Yigit
2024-03-06 12:24 ` [PATCH v3 05/33] net/ena: fix fast mbuf free shaibran
2024-03-08 17:23   ` Ferruh Yigit
2024-03-10 14:58     ` Brandes, Shai
2024-03-13 11:28       ` Ferruh Yigit
2024-03-06 12:24 ` [PATCH v3 06/33] net/ena: rename base folder to hal shaibran
2024-03-08 17:23   ` Ferruh Yigit
2024-03-10 14:23     ` Brandes, Shai
2024-03-06 12:24 ` [PATCH v3 07/33] net/ena: restructure the llq policy setting process shaibran
2024-03-08 17:24   ` Ferruh Yigit
2024-03-10 14:29     ` Brandes, Shai
2024-03-13 11:21       ` Ferruh Yigit
2024-03-06 12:24 ` [PATCH v3 08/33] net/ena/hal: exponential backoff exp limit shaibran
2024-03-08 17:24   ` Ferruh Yigit [this message]
2024-03-10 14:53     ` Brandes, Shai
2024-03-12 16:53       ` Brandes, Shai
2024-03-13 11:25         ` Ferruh Yigit
2024-03-06 12:24 ` [PATCH v3 09/33] net/ena/hal: add a new csum offload bit shaibran
2024-03-08 17:24   ` Ferruh Yigit
2024-03-06 12:24 ` [PATCH v3 10/33] net/ena/hal: added a bus parameter to ena memcpy macro shaibran
2024-03-08 17:25   ` Ferruh Yigit
2024-03-10 15:08     ` Brandes, Shai
2024-03-13 11:27       ` Ferruh Yigit
2024-03-06 12:24 ` [PATCH v3 11/33] net/ena/hal: optimize Rx ring submission queue shaibran
2024-03-06 12:24 ` [PATCH v3 12/33] net/ena/hal: rename fields in completion descriptors shaibran
2024-03-06 12:24 ` [PATCH v3 13/33] net/ena/hal: use correct read once on u8 field shaibran
2024-03-06 12:24 ` [PATCH v3 14/33] net/ena/hal: add completion descriptor corruption check shaibran
2024-03-06 12:24 ` [PATCH v3 15/33] net/ena/hal: malformed Tx descriptor error reason shaibran
2024-03-06 12:24 ` [PATCH v3 16/33] net/ena/hal: phc feature modifications shaibran
2024-03-06 12:24 ` [PATCH v3 17/33] net/ena/hal: restructure interrupt handling shaibran
2024-03-06 12:24 ` [PATCH v3 18/33] net/ena/hal: add unlikely to error checks shaibran
2024-03-06 12:24 ` [PATCH v3 19/33] net/ena/hal: missing admin interrupt reset reason shaibran
2024-03-06 12:24 ` [PATCH v3 20/33] net/ena/hal: check for existing keep alive notification shaibran
2024-03-06 12:24 ` [PATCH v3 21/33] net/ena/hal: modify memory barrier comment shaibran
2024-03-06 12:24 ` [PATCH v3 22/33] net/ena/hal: rework Rx ring submission queue shaibran
2024-03-06 12:24 ` [PATCH v3 23/33] net/ena/hal: remove operating system type enum shaibran
2024-03-06 12:24 ` [PATCH v3 24/33] net/ena/hal: handle command abort shaibran
2024-03-06 12:24 ` [PATCH v3 25/33] net/ena/hal: add support for device reset request shaibran
2024-03-06 12:24 ` [PATCH v3 26/33] net/ena: cosmetic changes shaibran
2024-03-08 11:17   ` Ferruh Yigit
2024-03-08 13:19     ` Brandes, Shai
2024-03-08 14:50       ` Ferruh Yigit
2024-03-06 12:24 ` [PATCH v3 27/33] net/ena/hal: modify customer metrics memory management shaibran
2024-03-06 12:24 ` [PATCH v3 28/33] net/ena/hal: cosmetic changes shaibran
2024-03-08 17:43   ` Ferruh Yigit
2024-03-08 17:44     ` Ferruh Yigit
2024-03-12 17:12       ` Brandes, Shai
2024-03-06 12:24 ` [PATCH v3 29/33] net/ena: update device-preferred size of rings shaibran
2024-03-06 12:24 ` [PATCH v3 30/33] net/ena: exhaust interrupt callbacks in device close shaibran
2024-03-06 12:24 ` [PATCH v3 31/33] net/ena: support max large llq depth from the device shaibran
2024-03-06 12:24 ` [PATCH v3 32/33] net/ena: control path pure polling mode shaibran
2024-03-06 12:24 ` [PATCH v3 33/33] net/ena: upgrade driver version to 2.9.0 shaibran
2024-03-08 17:36 ` [PATCH v3 00/33] net/ena: v2.9.0 driver release Ferruh Yigit
2024-03-08 20:26   ` Brandes, Shai
2024-03-10 14:21     ` Brandes, Shai
2024-03-13 11:28       ` Ferruh Yigit
2024-03-13 13:38         ` Brandes, Shai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=76154018-868a-481e-9922-ed5bd166d90a@amd.com \
    --to=ferruh.yigit@amd.com \
    --cc=dev@dpdk.org \
    --cc=shaibran@amazon.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).