[dpdk-dev] [PATCHv7 03/47] common/dpaa2: adding qbman driver

[Shreyansh Jain]

(Modified the subject to: 'Re: [PATCHv7 03/47] common/dpaa2: adding 
qbman driver' from 'Re: Hello Ferruh, Neil,')

Hello Ferruh,

On Tuesday 21 February 2017 08:09 PM, Ferruh Yigit wrote:
> On 2/21/2017 1:42 PM, Shreyansh Jain wrote:
>> Thanks for the suggestions about rte_* renaming in DPAA2 PMD.
>> I create a draft patch for a single symbol change. (applies over v7
>> of DPAA2 PMD)
>>
>> Can you tell me if this is the direction you were suggesting?
>>
>> I see two issues in this approach which are somewhat problematic for
>> me to change all the symbols:
>> 1) We saw a drop of over 5% when I replaced only 3 symbols (one
>>    of the most used ones, just for sampling). This also means that
>>    when more of such symbols are replaced, it would bring further
>>    drop. This was case when I used the Shared library approach.
>>    (*) I am not well versed with gcc symbol aliasing to comment for
>>        why such a drop would happen. But multiple test cycles confirm
>>        this.
>> 2) I have to include a new header in almost all the source files for PMD/
>>    Pool/Bus etc. This is besides the STATIC_SYMBOL macros across the
>>    code. Essentially, any internal repo patch cannot be directly
>>    transposed to DPDK repo. Increased effort for each internal->
>>    external release
>>
>> Overall, I would like you to consider if this effort for changing names
>> for exposed symbols is really useful or not.
>
> As you showed below, this works for exporting proper APIs, but not sure
> if this change worth or not.

Given such symbol aliasing is an impact on performance, probably there
is a need to discuss the strictness of rte_* appending for driver
symbols.
As for cost of maintaining such code base, it can be rationalized over
a period of time, but not performance.

>
>>
>> There is another approach - that of not using a drivers/common library.
>> This again is problematic for us - NXP DPAA2 being a hardware, the lib
>> and state for Crypto and Net hardware is tied together - so, having
>> multiple instances of library breaks either of Crypto or Net PMD.
>
> Isn't is possible to keep folder structure same, but produce single library.
> Because these don't provide any API to the user application, perhaps not
> need to be library at all.
>
> Assuming that bus and pool won't be required without a driver existing,
> is it possible have a single driver library that contains others?
>
> For net driver, dependency is as following:
>   bus_fslmc  --> common_dpaa2_qbman
>   pool_dpaa2 --> bus_fslmc, common_dpaa2_qbman
>   pmd_dpaa2  --> pool_dpaa2, bus_fslmc, common_dpaa2_qbman
>
> So generating only "librte_pmd_dpaa2" which include above dependent ones.
>
> For cryptodev pmd, I assume it has dependency to same modules:
>   pmd_crypto --> pool_dpaa2, bus_fslmc, common_dpaa2_qbman
>
> And this will generate only crypto pmd library, including all again.
>
>
> This will create duplication in binaries, but I think easier to manage
> library dependencies.
>
>
> And for above case, as far as I know, both net and crypto libraries can
> be linked against a binary even there are duplicate symbols. Are you
> getting error here?
>
<snip>

Thanks for your comments.

The key issue here is that driver/common is not actually a 'library' in
traditional sense. It is a driver support system. It provides
interfaces to interact with the hardware - and that includes the Net
and Crypto hardware.

Being a 'driver', this also has its own state. For example, a mem area
to interact with hardware queues, whether net or crypto - there is a
single instance of it.

This restricts its duplication as a library.
In fact, as of now the statefulness is quite limited, but once more
devices (like for eventdev) come into picture, this would become more
prominent.

Now, we have these possibility:
1. Have a shared library with non rte_* symbols
2. We have shared library with rte_* symbols
3. We have non-net devices (crypto, eventdev, ..) depend on net for 
these hardware interfaces

(2) is hitting performance significantly.
(3) it not a clean solution, having driver/crypto depend on driver/net. 
When new devices are there, more dependencies will occur.

In crux, probably we need to have a discussion on (1) and how strongly 
we feel about that (specially in context of drivers).

-
Shreyansh