DPDK patches and discussions
 help / color / mirror / Atom feed
From: Thomas Monjalon <thomas@monjalon.net>
To: "Morten Brørup" <mb@smartsharesystems.com>,
	"Jerin Jacob" <jerinjacobk@gmail.com>,
	"Van Haaren, Harry" <harry.van.haaren@intel.com>
Cc: Jerin Jacob <jerinj@marvell.com>, "dev@dpdk.org" <dev@dpdk.org>,
	"Li, WeiyuanX" <weiyuanx.li@intel.com>,
	Ferruh Yigit <ferruh.yigit@amd.com>,
	Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>,
	david.marchand@redhat.com
Subject: Re: rte_event_dev_xstats_reset id type
Date: Wed, 12 Oct 2022 17:13:00 +0200	[thread overview]
Message-ID: <8864071.MhkbZ0Pkbq@thomas> (raw)
In-Reply-To: <BN0PR11MB5712997B39C9946B64F50360D7229@BN0PR11MB5712.namprd11.prod.outlook.com>

12/10/2022 14:14, Van Haaren, Harry:
> From: Morten Brørup <mb@smartsharesystems.com>
> > From: Van Haaren, Harry [mailto:harry.van.haaren@intel.com]
> > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > On Wed, Oct 12, 2022 at 1:40 PM Morten Brørup wrote:
> > > > >
> > > > > Hi Jerin (eventdev maintainer),
> > > >
> > > > + harry.van.haaren@intel.com as the changes in drivers/event/sw.
> > >
> > > Thanks Jerin.
> > >
> > >
> > > > > While looking into bug #1101 [1], I noticed a mix of unsigned int
> > > and uint32_t in
> > > > the test code, which will fail on 64-bit big endian CPUs.
> > >
> > > Aha; that we can fix. I am curious why this isn't found in CI/reported
> > > before.
> > 
> > We probably don't test any 64-bit *big endian* architectures. Just a guess.
> 
> Seems so yes.
> 
> > > > > Specifically, rte_event_dev_xstats_reset() is called with the "ids"
> > > parameter
> > > > pointing to an unsigned int [2], but that parameter is a pointer to
> > > an uint32_t.
> > > > >
> > > > > I think the type of the ids array parameter to
> > > rte_event_dev_xstats_reset() should
> > > > be changed to unsigned int array, like in the other
> > > rte_event_dev_xxx() functions.
> > >
> > > In this case, we have the option to change the type of a variable in a
> > > test-case, or change API and cause API/ABI breakage.
> > 
> > Well.. yes, but I would phrase that last option: Change the API/ABI, so related
> > functions consistently use the same type for the same variable, instead of randomly
> > mixing uint64_t, uint32_t and unsigned int, depending on function.
> 
> Aah ok; I see your point now; there is inconsistent usage of uint32_t/unsigned int
> between the Eventdev APIs itself. Agree this is sub-optimal, and would have been
> nice to have spotted before the Eventdev API was stabilized.
> 
> 
> > Unfortunately, these functions are not marked experimental, so breaking API/ABI is
> > hard to do. :-(
> 
> Agreed again.

22.11 is a breaking release,
and changing type in the API is not much impactful,
so that's something you can change now,
or be quiet forever :)




  reply	other threads:[~2022-10-12 15:13 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-12  8:10 Morten Brørup
2022-10-12  9:45 ` Jerin Jacob
2022-10-12 10:29   ` Van Haaren, Harry
2022-10-12 10:41     ` Morten Brørup
2022-10-12 12:14       ` Van Haaren, Harry
2022-10-12 15:13         ` Thomas Monjalon [this message]
2022-10-12 15:35           ` Morten Brørup
2022-10-12 16:16             ` Jerin Jacob
2022-10-12 16:28               ` Thomas Monjalon
2022-10-12 16:47                 ` Jerin Jacob
2022-10-12 20:44                   ` Thomas Monjalon
2022-10-13  6:51                     ` xstats " Morten Brørup
2022-10-13  7:12                       ` Pavan Nikhilesh Bhagavatula
2022-10-13  8:26                         ` Thomas Monjalon
2022-10-13  8:33                           ` [EXT] " Pavan Nikhilesh Bhagavatula
2022-10-13  8:59                         ` Van Haaren, Harry

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=8864071.MhkbZ0Pkbq@thomas \
    --to=thomas@monjalon.net \
    --cc=andrew.rybchenko@oktetlabs.ru \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@amd.com \
    --cc=harry.van.haaren@intel.com \
    --cc=jerinj@marvell.com \
    --cc=jerinjacobk@gmail.com \
    --cc=mb@smartsharesystems.com \
    --cc=weiyuanx.li@intel.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).