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

["Burakov, Anatoly" <anatoly.burakov@intel.com>]

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. If there is consensus on 
going further than I have with this patchset, I can certainly do so.

-- 
Thanks,
Anatoly