RE: [RFC PATCH v1 0/5] Adjust wording for NUMA vs. socket ID in DPDK

["Morten Brørup" <mb@smartsharesystems.com>]

> From: Burakov, Anatoly [mailto:anatoly.burakov@intel.com]
> Sent: Friday, 6 September 2024 15.18
> Subject: Re: [RFC PATCH v1 0/5] Adjust wording for NUMA vs. socket ID in DPDK
> 
> On 9/6/2024 3:07 PM, Bruce Richardson wrote:
> > On Fri, Sep 06, 2024 at 03:02:53PM +0200, Morten Brørup wrote:
> >>> From: Burakov, Anatoly [mailto:anatoly.burakov@intel.com]
> >>> Sent: Friday, 6 September 2024 14.46
> >>>
> >>> On 9/6/2024 2:37 PM, Morten Brørup wrote:
> >>>>> From: Anatoly Burakov [mailto:anatoly.burakov@intel.com]
> >>>>> Sent: Friday, 6 September 2024 13.47
> >>>>> To: dev@dpdk.org
> >>>>> Subject: [RFC PATCH v1 0/5] Adjust wording for NUMA vs. socket ID in
> DPDK
> >>>>>
> >>>>> While initially, DPDK has used the term "socket ID" to refer to physical
> >>>>> package
> >>>>> ID, the last time DPDK read "physical_package_id" for socket ID was ~9
> >>> years
> >>>>> ago, so it's been a while since we've actually switched over to using
> the
> >>> term
> >>>>> "socket" to mean "NUMA node".
> >>>>>
> >>>>> This wasn't a problem before, as most systems had one NUMA node per
> >>> physical
> >>>>> socket. However, in the last few years, more and more systems have
> multiple
> >>>>> NUMA
> >>>>> nodes per physical CPU socket. Since DPDK used NUMA nodes already, the
> >>>>> transition was pretty seamless, however now we're faced with a situation
> >>> when
> >>>>> most of our documentation still uses outdated terms, and our API is ripe
> >>> with
> >>>>> references to "sockets" when in actuality we mean "NUMA nodes". This
> could
> >>> be
> >>>>> a
> >>>>> source of confusion.
> >>>>>
> >>>>> While completely renaming all of our API's would be a huge effort, will
> >>> take a
> >>>>> long time and arguably wouldn't even be worth the API breakages (given
> that
> >>>>> this
> >>>>> mismatch between terminology and reality is implicitly understood by
> most
> >>>>> people
> >>>>> working on DPDK, and so this isn't so much of a problem in practice), we
> >>> can
> >>>>> do
> >>>>> some tweaks around the edges and at least document this unfortunate
> >>> reality.
> >>>>>
> >>>>> This patchset suggests the following changes:
> >>>>>
> >>>>> - Update rte_socket/rte_lcore documentation to refer to NUMA nodes
> rather
> >>> than
> >>>>> sockets - Rename internal structures' fields to better reflect this
> >>> intention
> >>>>> -
> >>>>> Rename --socket-mem/--socket-limit flags to refer to NUMA rather than
> >>> sockets
> >>>>> -
> >>>>> Add internal API to get physical package ID [1]
> >>>>>
> >>>>> The documentation is updated to refer to new EAL flags, but is otherwise
> >>> left
> >>>>> untouched, and instead the entry in "glossary" is amended to indicate
> that
> >>>>> when
> >>>>> DPDK documentation refers to "sockets", it actually means "NUMA ID's".
> As
> >>> next
> >>>>> steps, we could rename all API parameters to refer to NUMA ID rather
> than
> >>>>> socket
> >>>>> ID - this would not break neither API nor ABI, and instead would be a
> >>>>> documentation change in practice.
> >>>>>
> >>>>> [1] This could be used to group lcores by physical package, see e.g.
> >>>>> discussion
> >>>>>       under this patch:
> >>>>> https://patches.dpdk.org/project/dpdk/cover/20240827151014.201-1-
> >>>>> vipin.varghese@amd.com/
> >>>>
> >>>> Thank you for cleaning this up, Anatoly.
> >>>>
> >>>> I would prefer to take one more step and also rename functions and
> >>> parameters, e.g. rte_socket_id() -> rte_numa_id().
> >>>>
> >>>> For backwards compatibility, macros/functions with the old names can be
> >>> added.
> >>>>
> >>>
> >>> I don't think we can do such changes without deprecation notices, but
> >>> it's a good candidate for next release.
> >>
> >> Perhaps we can keep ABI compatibility by adding wrapper functions with the
> old names/parameters, which simply call the same functions with the new
> names/parameters.
> >>
> >> The Devil is in the details, and I haven't looked deeply into this. So take
> with a grain of salt.
> >>
> >>>
> >>> I have thought about including parameter renames in this patchset, but
> >>> for now I decided against doing so. I can certainly include this in the
> >>> next revision if that's something community is willing to accept.
> >>
> >> I agree with your decision on this. Renaming the parameters without
> renaming the functions could be confusing.
> >>
> >
> > I actually wonder if that is true. If we are simply renaming the parameters
> > without:
> > a) changing their types
> > b) changing the function behaviour
> > then it is neither an API nor an ABI break. If we were to do so, it would
> > be like changing a comment, since the actual parameter name is purely a
> > convenience to hint to the user what the value being passed actually does.
> >
> > That only applies for function parameters though. For any defines or macros
> > that need renaming, then we are into API break territory and we would want
> > backward compatible versions of same.
> >
> 
> To be clear, I was referring to the former rather than the latter;
> renaming public API function parameters/structure fields can be done
> relatively easily and won't break anything.

Agree.
By "confusing" I was referring to source code readability; e.g. an application calls some rte_socket_id() and passes the returned value on as a numa_id parameter to some other rte_socket_xyz() function.

I have no strong opinion on this.

But I would prefer going all the way, and also fix the API, i.e. rename the functions etc.
And I'm hoping ABI/API backwards compatibility can be preserved by adding wrappers with the previous names.

> If there is consensus on
> going further than I have with this patchset, I can certainly do so.