> From: Burakov, Anatoly [mailto:anatoly.bura...@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.bura...@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.bura...@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.vargh...@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.
