[dpdk-dev] [PATCHv6 02/33] drivers/common/dpaa2: adding qbman driver

[Shreyansh Jain]

Hello Ferruh,

On Monday 23 January 2017 11:00 PM, Ferruh Yigit wrote:
> On 1/23/2017 11:59 AM, Hemant Agrawal wrote:
>> QBMAN, is a hardware block which interfaces with the other
>> accelerating hardware blocks (For e.g., WRIOP) on NXP's DPAA2
>> SoC for queue, buffer and packet scheduling.
>>
>> This patch introduces a userspace driver for interfacing with
>> the QBMAN hw block.
>>
>> The qbman-portal component provides APIs to do the low level
>> hardware bit twiddling for operations such as:
>>       -initializing Qman software portals
>>       -building and sending portal commands
>>       -portal interrupt configuration and processing
>>
>> This same/similar code is used in kernel and compat file is used
>> to make it working in user space.
>>
>> Signed-off-by: Geoff Thorpe <Geoff.Thorpe at nxp.com>
>> Signed-off-by: Roy Pledge <Roy.Pledge at nxp.com>
>> Signed-off-by: Hemant Agrawal <hemant.agrawal at nxp.com>
>> ---
> <...>
>
>> --- a/config/common_base
>> +++ b/config/common_base
>> @@ -287,7 +287,6 @@ CONFIG_RTE_LIBRTE_THUNDERX_NICVF_DEBUG_TX=n
>>  CONFIG_RTE_LIBRTE_THUNDERX_NICVF_DEBUG_DRIVER=n
>>  CONFIG_RTE_LIBRTE_THUNDERX_NICVF_DEBUG_MBOX=n
>>
>> -#

Ah, yes, stray removal. We will remove it.
(Thanks for review - really appreciate your help).

>
> Minor typo ..
>
>>  # Compile burst-oriented VIRTIO PMD driver
>>  #
>>  CONFIG_RTE_LIBRTE_VIRTIO_PMD=y
>
> <...>
>
>> --- /dev/null
>> +++ b/drivers/common/dpaa2/qbman/rte_common_dpaa2_qbman_version.map
>> @@ -0,0 +1,27 @@
>> +DPDK_17.02 {
>> +	global:
>> +
>> +	qbman_check_command_complete;
>> +	qbman_eq_desc_clear;
>> +	qbman_eq_desc_set_fq;
>> +	qbman_eq_desc_set_no_orp;
>> +	qbman_eq_desc_set_qd;
>> +	qbman_eq_desc_set_response;
>> +	qbman_get_version;
>> +	qbman_pull_desc_clear;
>> +	qbman_pull_desc_set_fq;
>> +	qbman_pull_desc_set_numframes;
>> +	qbman_pull_desc_set_storage;
>> +	qbman_release_desc_clear;
>> +	qbman_release_desc_set_bpid;
>> +	qbman_result_DQ_fd;
>> +	qbman_result_DQ_flags;
>> +	qbman_result_has_new_result;
>> +	qbman_swp_acquire;
>> +	qbman_swp_init;
>> +	qbman_swp_pull;
>> +	qbman_swp_release;
>> +	qbman_swp_send_multiple;
>
> Overall, dpdk library exported APIs not having DPDK prefix (rte_) is a
> concern, which already pointed by Thomas.
>
> I guess only user of this library will be other dpaa2 code, so these are
> not really APIs. Not sure how to proceed.
> I think I have seen "_rte" prefix used in some APIs to say that is
> internal API, does it make sense to use that API here?

We had a detailed discussion on this internally after Thomas' comments.
It is very difficult to convert all such symbols - primarily because it
will break the 'linkage' between our Linux upstream candidate code,
internal repositories and what we are pushing to DPDK.

This is happening because we are spreading our common libraries beyond
the drivers/net/* scope. If we combine all of them together (no
drivers/common, drivers/pool etc), these symbols would not be exposed.

There was a discussion on ML as well for this kind of structure on our
first RFC submission.
We were in favor of a split design because that makes the overall PMD
look clean. But, this rte_* prefix issue is something we never
anticipated.

Further, we took hint from existing map files which also had similar
non rte_* symbols (per_lcore_*, devargs_list, pci_device_list etc) to
convince ourselves that this is not a strict rule.

As for "_rte" - once again, we need to define what internal is when it
comes to drivers/common, drivers/bus, drivers/pool .. kind of code
layout having functionality divided. It might be internal to framework
(limited to drivers/*) but would be external for a drivers/net/*.

Overall, a serious discussion on this is indeed needed for clarity as
more non-PCI drivers might contributed in near future with basic bus
infra in place.

>
>> +
>> +	local: *;
>> +};
>>
>
>