Re: [dpdk-dev] [PATCH 1/7] common/dpaax: move internal symbols into INTERNAL section

[Ray Kinsella <mdr@ashroe.eu>]

On 07/05/2020 07:20, Hemant Agrawal wrote:
> Hi David
> 
>> On Tue, May 5, 2020 at 4:10 PM Hemant Agrawal
>> <hemant.agrawal@nxp.com> wrote:
>>>
>>> This patch moves the internal symbols to INTERNAL sections so that any
>>> change in them is not reported as ABI breakage.
>>
>> Talking about the series (not just this patch as I did not look at the details),
>> travis reported a build issue:
>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftravis-
>> ci.com%2Fgithub%2Fovsrobot%2Fdpdk%2Fbuilds%2F163985766&amp;data=02
>> %7C01%7Chemant.agrawal%40nxp.com%7C555963f208a3446deba708d7f116e
>> 1cf%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6372429528900581
>> 54&amp;sdata=j2fqNXovCPfaLlPtZwS9TaMBKMBP5l7inwX%2BsndavS4%3D&am
>> p;reserved=0
>>
>> It looks like there is a versioned symbol for a static function of one of those
>> drivers.
>> $ git grep dpaa2_get_qbman_swp origin/master
>> origin/master:drivers/bus/fslmc/portal/dpaa2_hw_dpio.c:static struct
>> dpaa2_dpio_dev *dpaa2_get_qbman_swp(int lcoreid)
>> origin/master:drivers/bus/fslmc/portal/dpaa2_hw_dpio.c:
>> dpaa2_io_portal[lcore_id].dpio_dev = dpaa2_get_qbman_swp(lcore_id);
>> origin/master:drivers/bus/fslmc/portal/dpaa2_hw_dpio.c:
>> dpaa2_get_qbman_swp(lcore_id);
>> origin/master:drivers/bus/fslmc/rte_bus_fslmc_version.map:
>> dpaa2_get_qbman_swp;
> 
> [Hemant] yes, I saw it. It will be fixed in v2
> 
>>
>>
>> Once fixed, I don't understand how you chose to move some symbols to
>> INTERNAL and not others.
>> Can you explain the differences?
> 
> [Hemant] In most of the libraries the symbols are internal only. i.e. they are suppose to be used by other dpdk internal drivers.
> Only very few cases rte_dpaa2_mempool.h, rte_pmd_dpaa2.h rte_pmd_dpaa.h have few functions, which are suppose to be used by the external entities or applications.
> 
>>
>>
>> Finally, I would still expect a failure in the ABI check even if using
>> __rte_internal marking.
>> Marking them internal will make you free of changing those symbols in the
>> future.
>> The problem is the transient state we are in.
> 
> [Hemant] I also expect it like this. But still facing few issues w.r.t variables.
> __rte_internal can not be used as a predecessor for variable names. 
> In map files dpaa have some of the variables also defined as part of symbols for the faster access across libraries. 
> I am still looking a right way to handle them. 

I think your least error prone way is going to be to use an internal function to return a pointer to the variable(s).
rte_dpaa2+get_my_important variables.

> 
>>
>> In 20.02 (and I suppose 19.11 too), those symbols were exported and
>> versioned as stable DPDK_20.
>> So with the current ABI check script, this move will still be seen as a removal.
>>
>>
>> --
>> David Marchand
>