DPDK patches and discussions
 help / color / mirror / Atom feed
From: Thomas Monjalon <thomas@monjalon.net>
To: "Chautru, Nicolas" <nicolas.chautru@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
	"gakhil@marvell.com" <gakhil@marvell.com>,
	"maxime.coquelin@redhat.com" <maxime.coquelin@redhat.com>,
	"trix@redhat.com" <trix@redhat.com>,
	"mdr@ashroe.eu" <mdr@ashroe.eu>,
	"Richardson, Bruce" <bruce.richardson@intel.com>,
	"david.marchand@redhat.com" <david.marchand@redhat.com>,
	"stephen@networkplumber.org" <stephen@networkplumber.org>,
	"Zhang, Mingshan" <mingshan.zhang@intel.com>,
	"hemant.agrawal@nxp.com" <hemant.agrawal@nxp.com>
Subject: Re: [PATCH v10 6/7] bbdev: add queue related warning and status information
Date: Mon, 03 Oct 2022 19:21:59 +0200	[thread overview]
Message-ID: <10152654.qUNvkh4Gvn@thomas> (raw)
In-Reply-To: <BY5PR11MB44519804C7BD4F8F0823A88AF85B9@BY5PR11MB4451.namprd11.prod.outlook.com>

03/10/2022 18:39, Chautru, Nicolas:
> Hi Thomas, 
> 
> I will update all your comments below today, thanks.

Please do a self review of other patches (I reviewed only this one),
I suspect there are a lot of other details to improve in other places.

> The one where we need your confirmation is specifically this comment from your, I believe we discussed but good to make sure we are all aligned: 

Yes let's be clear.

> > But the big question is why do we need this "MAX" value?
> > The guideline is to avoid using such MAX value for long term compatibility.
> 
> This is not a _MAX enum but a _SIZE_MAX for array related to that enum. Note that the actual max value of the enum exists but is used a private macro. 
> The distinction is that the application cannot make any assumptions on what is the maximum enum value (ie. we don't want enum with MAX value, that is not future proof has captured in doc).
> But the application can make some assumption on the sizing of array based on such an enum. The difference being the padding which allows for the enum growth without breaking ABI or application.
> The previous name was _PADDED_MAX to make it clear this not a max enum but a padded value. Then more recenrtly the consensus in the community was to change this to _SIZE_MAX to be arguably more explicit this is to be used for array size. The comments I believe also make it clear this is not a MAX enum.
> 
> Does that make sense and do you agree this is best consensus so far to move forward?

It makes a lot of sense to me because this is what I proposed 2 or 3 years ago
to the techboard when we approached different general solutions.
But it has been said that it is not ideal for 2 reasons I think:
	- there is a compatibility breakage when we reach the maximum
	- there is no assumption on how the padding is filled

That's why the preferred way should be to avoid having any maximum value.
For instance, array allocation and iteration could be done in the API.

Anyway I'm fine with your solution for now.



  reply	other threads:[~2022-10-03 17:22 UTC|newest]

Thread overview: 174+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-09  0:22 [PATCH v1 0/2] bbdev: add device info on queue topology Nicolas Chautru
2022-03-09  0:22 ` [PATCH v1 1/2] " Nicolas Chautru
2022-03-09  1:28   ` Stephen Hemminger
2022-03-09  0:22 ` [PATCH v1 2/2] drivers/baseband: update PMDs to expose queue per operation Nicolas Chautru
2022-06-17 18:37   ` [PATCH v2 0/5] bbdev changes for 22.11 Nicolas Chautru
2022-06-17 18:37     ` [PATCH v2 1/5] bbdev: allow operation type enum for growth Nicolas Chautru
2022-06-17 18:37     ` [PATCH v2 2/5] bbdev: add device status info Nicolas Chautru
2022-06-17 18:37     ` [PATCH v2 3/5] bbdev: add device info on queue topology Nicolas Chautru
2022-06-17 18:37     ` [PATCH v2 4/5] drivers/baseband: update PMDs to expose queue per operation Nicolas Chautru
2022-06-17 18:37     ` [PATCH v2 5/5] bbdev: add new operation for FFT processing Nicolas Chautru
2022-06-28  1:35       ` [PATCH v3 0/7] bbdev changes for 22.11 Nicolas Chautru
2022-06-28  1:35         ` [PATCH v3 1/7] bbdev: allow operation type enum for growth Nicolas Chautru
2022-06-28  1:35         ` [PATCH v3 2/7] bbdev: add device status info Nicolas Chautru
2022-06-28  1:35         ` [PATCH v3 3/7] bbdev: add device info on queue topology Nicolas Chautru
2022-06-28  1:35         ` [PATCH v3 4/7] drivers/baseband: update PMDs to expose queue per operation Nicolas Chautru
2022-06-28  1:35         ` [PATCH v3 5/7] bbdev: add new operation for FFT processing Nicolas Chautru
2022-06-28  1:35         ` [PATCH v3 6/7] bbdev: add queue related warning and status information Nicolas Chautru
2022-06-28  1:35         ` [PATCH v3 7/7] bbdev: add a lock option for enqueue/dequeue operation Nicolas Chautru
2022-07-06  0:23       ` [PATCH v4 0/7] bbdev changes for 22.11 Nicolas Chautru
2022-07-06  0:23         ` [PATCH v4 1/7] bbdev: allow operation type enum for growth Nicolas Chautru
2022-07-06 12:50           ` Tom Rix
2022-07-06 21:20             ` Chautru, Nicolas
2022-07-06  0:23         ` [PATCH v4 2/7] bbdev: add device status info Nicolas Chautru
2022-07-06 15:38           ` Tom Rix
2022-07-06 21:16             ` Chautru, Nicolas
2022-07-07 13:37               ` Tom Rix
2022-07-07 17:15                 ` Chautru, Nicolas
2022-07-18 13:09                   ` Tom Rix
2022-08-25 14:08               ` Maxime Coquelin
2022-07-06  0:23         ` [PATCH v4 3/7] bbdev: add device info on queue topology Nicolas Chautru
2022-07-06 16:06           ` Tom Rix
2022-07-06 21:12             ` Chautru, Nicolas
2022-07-07 13:34               ` Tom Rix
2022-07-07 17:13                 ` Chautru, Nicolas
2022-07-18 13:04                   ` Tom Rix
2022-07-06  0:23         ` [PATCH v4 4/7] drivers/baseband: update PMDs to expose queue per operation Nicolas Chautru
2022-07-06 16:15           ` Tom Rix
2022-07-06 21:10             ` Chautru, Nicolas
2022-07-07 13:20               ` Tom Rix
2022-07-07 17:19                 ` Chautru, Nicolas
2022-07-18 13:21                   ` Tom Rix
2022-08-15 17:28                     ` Chautru, Nicolas
2022-07-06  0:23         ` [PATCH v4 5/7] bbdev: add new operation for FFT processing Nicolas Chautru
2022-07-06 18:47           ` Tom Rix
2022-07-06 21:04             ` Chautru, Nicolas
2022-07-07 13:09               ` Tom Rix
2022-07-07 16:57                 ` Chautru, Nicolas
2022-07-18 22:38                   ` Tom Rix
2022-07-06  0:23         ` [PATCH v4 6/7] bbdev: add queue related warning and status information Nicolas Chautru
2022-07-06 18:57           ` Tom Rix
2022-07-06 20:34             ` Chautru, Nicolas
2022-07-06  0:23         ` [PATCH v4 7/7] bbdev: add a lock option for enqueue/dequeue operation Nicolas Chautru
2022-07-06 19:01           ` Tom Rix
2022-07-06 19:20             ` Stephen Hemminger
2022-07-06 20:21               ` Chautru, Nicolas
2022-07-07 12:47                 ` Tom Rix
2022-07-06 23:28       ` [PATCH v5 0/7] bbdev changes for 22.11 Nicolas Chautru
2022-07-06 23:28         ` [PATCH v5 1/7] bbdev: allow operation type enum for growth Nicolas Chautru
2022-08-25 13:54           ` Maxime Coquelin
2022-07-06 23:28         ` [PATCH v5 2/7] bbdev: add device status info Nicolas Chautru
2022-08-25 14:18           ` Maxime Coquelin
2022-08-25 18:30             ` Chautru, Nicolas
2022-08-26 10:12               ` Maxime Coquelin
2022-08-29 16:10                 ` Chautru, Nicolas
2022-08-30  7:08                   ` Maxime Coquelin
2022-08-30 19:38                     ` Chautru, Nicolas
2022-07-06 23:28         ` [PATCH v5 3/7] bbdev: add device info on queue topology Nicolas Chautru
2022-08-25 15:23           ` Maxime Coquelin
2022-07-06 23:28         ` [PATCH v5 4/7] drivers/baseband: update PMDs to expose queue per operation Nicolas Chautru
2022-07-06 23:28         ` [PATCH v5 5/7] bbdev: add new operation for FFT processing Nicolas Chautru
2022-07-06 23:28         ` [PATCH v5 6/7] bbdev: add queue related warning and status information Nicolas Chautru
2022-07-06 23:28         ` [PATCH v5 7/7] bbdev: remove unnecessary if-check Nicolas Chautru
2022-08-15 17:54         ` [PATCH v5 0/7] bbdev changes for 22.11 Chautru, Nicolas
2022-08-25 18:24       ` [PATCH v6 " Nicolas Chautru
2022-08-25 18:24         ` [PATCH v6 1/7] bbdev: allow operation type enum for growth Nicolas Chautru
2022-08-25 18:24         ` [PATCH v6 2/7] bbdev: add device status info Nicolas Chautru
2022-08-25 18:24         ` [PATCH v6 3/7] bbdev: add device info on queue topology Nicolas Chautru
2022-08-25 18:24         ` [PATCH v6 4/7] drivers/baseband: update PMDs to expose queue per operation Nicolas Chautru
2022-08-26 11:53           ` Maxime Coquelin
2022-08-25 18:24         ` [PATCH v6 5/7] bbdev: add new operation for FFT processing Nicolas Chautru
2022-08-26 12:07           ` Maxime Coquelin
2022-08-29 18:18             ` Chautru, Nicolas
2022-08-25 18:24         ` [PATCH v6 6/7] bbdev: add queue related warning and status information Nicolas Chautru
2022-08-26 19:51           ` Maxime Coquelin
2022-08-25 18:24         ` [PATCH v6 7/7] bbdev: remove unnecessary if-check Nicolas Chautru
2022-08-26 19:52           ` Maxime Coquelin
2022-08-29 18:07       ` [PATCH v7 0/7] bbdev changes for 22.11 Nicolas Chautru
2022-08-29 18:07         ` [PATCH v7 1/7] bbdev: allow operation type enum for growth Nicolas Chautru
2022-08-29 18:07         ` [PATCH v7 2/7] bbdev: add device status info Nicolas Chautru
2022-08-30  2:19           ` Zhang, Mingshan
2022-08-30  4:43           ` Hemant Agrawal
2022-09-21 18:54           ` [EXT] " Akhil Goyal
2022-09-21 20:53             ` Chautru, Nicolas
2022-08-29 18:07         ` [PATCH v7 3/7] bbdev: add device info on queue topology Nicolas Chautru
2022-08-29 18:07         ` [PATCH v7 4/7] drivers/baseband: update PMDs to expose queue per operation Nicolas Chautru
2022-08-30  4:44           ` Hemant Agrawal
2022-09-21 19:00           ` [EXT] " Akhil Goyal
2022-09-21 20:53             ` Chautru, Nicolas
2022-08-29 18:07         ` [PATCH v7 5/7] bbdev: add new operation for FFT processing Nicolas Chautru
2022-09-21 19:14           ` [EXT] " Akhil Goyal
2022-09-21 20:56             ` Chautru, Nicolas
2022-09-22 14:19               ` Akhil Goyal
2022-09-22 16:39                 ` Chautru, Nicolas
2022-09-22 16:48                   ` Akhil Goyal
2022-09-22 17:25                     ` Chautru, Nicolas
2022-08-29 18:07         ` [PATCH v7 6/7] bbdev: add queue related warning and status information Nicolas Chautru
2022-09-21 19:21           ` [EXT] " Akhil Goyal
2022-09-21 20:57             ` Chautru, Nicolas
2022-09-23 10:57             ` Ferruh Yigit
     [not found]               ` <CO6PR18MB44848717BA4EA2FF8967D7CBD8509@CO6PR18MB4484.namprd18.prod.outlook.com>
2022-09-24 16:34                 ` Chautru, Nicolas
2022-09-27  9:43                   ` Ferruh Yigit
2022-09-27 20:59                   ` Chautru, Nicolas
2022-09-29 18:10                     ` Ferruh Yigit
2022-09-29 18:32                       ` Akhil Goyal
2022-09-29 19:48                         ` Chautru, Nicolas
2022-09-30  7:54                           ` Maxime Coquelin
2022-08-29 18:07         ` [PATCH v7 7/7] bbdev: remove unnecessary if-check Nicolas Chautru
2022-09-21 19:25           ` [EXT] " Akhil Goyal
2022-09-21 20:58             ` Chautru, Nicolas
2022-08-30  4:45         ` [PATCH v7 0/7] bbdev changes for 22.11 Hemant Agrawal
2022-09-06 16:47         ` Chautru, Nicolas
2022-09-21 21:02       ` [PATCH v8 " Nic Chautru
2022-09-21 21:02         ` [PATCH v8 1/7] bbdev: allow operation type enum for growth Nic Chautru
2022-09-21 21:02         ` [PATCH v8 2/7] bbdev: add device status info Nic Chautru
2022-09-21 21:02         ` [PATCH v8 3/7] bbdev: add device info on queue topology Nic Chautru
2022-09-21 21:02         ` [PATCH v8 4/7] drivers/baseband: update PMDs to expose queue per operation Nic Chautru
2022-09-21 21:02         ` [PATCH v8 5/7] bbdev: add new operation for FFT processing Nic Chautru
2022-09-21 21:02         ` [PATCH v8 6/7] bbdev: add queue related warning and status information Nic Chautru
2022-09-21 21:02         ` [PATCH v8 7/7] bbdev: remove unnecessary if-check Nic Chautru
2022-09-22 17:45       ` [PATCH v9 0/7] bbdev changes for 22.11 Nic Chautru
2022-09-22 17:45         ` [PATCH v9 1/7] bbdev: allow operation type enum for growth Nic Chautru
2022-09-22 17:45         ` [PATCH v9 2/7] bbdev: add device status info Nic Chautru
2022-09-22 17:45         ` [PATCH v9 3/7] bbdev: add device info on queue topology Nic Chautru
2022-09-22 17:45         ` [PATCH v9 4/7] drivers/baseband: update PMDs to expose queue per operation Nic Chautru
2022-09-22 17:45         ` [PATCH v9 5/7] bbdev: add new operation for FFT processing Nic Chautru
2022-09-22 17:45         ` [PATCH v9 6/7] bbdev: add queue related warning and status information Nic Chautru
2022-09-22 17:45         ` [PATCH v9 7/7] bbdev: remove unnecessary if-check Nic Chautru
2022-09-22 18:17         ` [EXT] [PATCH v9 0/7] bbdev changes for 22.11 Akhil Goyal
2022-09-22 20:59           ` Chautru, Nicolas
2022-09-30 18:45       ` [PATCH v10 " Nicolas Chautru
2022-09-30 18:45         ` [PATCH v10 1/7] bbdev: allow operation type enum for growth Nicolas Chautru
2022-09-30 18:46         ` [PATCH v10 2/7] bbdev: add device status info Nicolas Chautru
2022-09-30 18:46         ` [PATCH v10 3/7] bbdev: add device info on queue topology Nicolas Chautru
2022-09-30 18:46         ` [PATCH v10 4/7] drivers/baseband: update PMDs to expose queue per operation Nicolas Chautru
2022-09-30 18:46         ` [PATCH v10 5/7] bbdev: add new operation for FFT processing Nicolas Chautru
2022-09-30 18:46         ` [PATCH v10 6/7] bbdev: add queue related warning and status information Nicolas Chautru
2022-10-03  8:28           ` Thomas Monjalon
2022-10-03 16:39             ` Chautru, Nicolas
2022-10-03 17:21               ` Thomas Monjalon [this message]
2022-09-30 18:46         ` [PATCH v10 7/7] bbdev: remove unnecessary if-check Nicolas Chautru
2022-09-30 20:38         ` [EXT] [PATCH v10 0/7] bbdev changes for 22.11 Akhil Goyal
2022-10-03 18:00       ` [PATCH v11 " Nicolas Chautru
2022-10-03 18:00         ` [PATCH v11 1/7] bbdev: allow operation type enum for growth Nicolas Chautru
2022-10-03 18:00         ` [PATCH v11 2/7] bbdev: add device status info Nicolas Chautru
2022-10-03 18:00         ` [PATCH v11 3/7] bbdev: add device info on queue topology Nicolas Chautru
2022-10-03 18:00         ` [PATCH v11 4/7] drivers/baseband: update PMDs to expose queue per operation Nicolas Chautru
2022-10-03 18:00         ` [PATCH v11 5/7] bbdev: add new operation for FFT processing Nicolas Chautru
2022-10-03 18:00         ` [PATCH v11 6/7] bbdev: add queue related warning and status information Nicolas Chautru
2022-10-03 18:00         ` [PATCH v11 7/7] bbdev: remove unnecessary if-check Nicolas Chautru
2022-10-04 17:16       ` [PATCH v12 0/7] bbdev changes for 22.11 Nicolas Chautru
2022-10-04 17:16         ` [PATCH v12 1/7] bbdev: allow operation type enum for growth Nicolas Chautru
2022-10-04 17:16         ` [PATCH v12 2/7] bbdev: add device status info Nicolas Chautru
2022-10-05  7:16           ` Maxime Coquelin
2022-10-04 17:16         ` [PATCH v12 3/7] bbdev: add device info on queue topology Nicolas Chautru
2022-10-04 17:16         ` [PATCH v12 4/7] drivers/baseband: update PMDs to expose queue per operation Nicolas Chautru
2022-10-04 17:16         ` [PATCH v12 5/7] bbdev: add new operation for FFT processing Nicolas Chautru
2022-10-04 17:16         ` [PATCH v12 6/7] bbdev: add queue related warning and status information Nicolas Chautru
2022-10-04 17:16         ` [PATCH v12 7/7] bbdev: remove unnecessary if-check Nicolas Chautru
2022-10-06 17:31         ` [EXT] [PATCH v12 0/7] bbdev changes for 22.11 Akhil Goyal
2022-10-06 22:28           ` Chautru, Nicolas
2022-10-07  4:46             ` Akhil Goyal
2022-10-10  7:35           ` Thomas Monjalon
2022-10-10 17:07             ` Chautru, Nicolas
2022-06-06 16:15 ` [PATCH v1 0/2] bbdev: add device info on queue topology Chautru, Nicolas

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=10152654.qUNvkh4Gvn@thomas \
    --to=thomas@monjalon.net \
    --cc=bruce.richardson@intel.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=gakhil@marvell.com \
    --cc=hemant.agrawal@nxp.com \
    --cc=maxime.coquelin@redhat.com \
    --cc=mdr@ashroe.eu \
    --cc=mingshan.zhang@intel.com \
    --cc=nicolas.chautru@intel.com \
    --cc=stephen@networkplumber.org \
    --cc=trix@redhat.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).