[dpdk-dev] [PATCH v2 07/12] pdump: disabled by default

[Ferruh Yigit]

On 6/15/2017 12:01 AM, Gaëtan Rivet wrote:
> Hi Ferruh,
> 
> On Tue, Jun 13, 2017 at 06:15:45PM +0100, Ferruh Yigit wrote:
>> On 6/11/2017 8:42 PM, Gaëtan Rivet wrote:
>>> Hi Reshma,
>>>
>>> On Fri, Jun 09, 2017 at 02:24:58PM +0000, Pattan, Reshma wrote:
>>>> Hi,
>>>>
>>>>> -----Original Message-----
>>>>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Gaetan Rivet
>>>>> Sent: Thursday, June 8, 2017 12:59 AM
>>>>> To: dev at dpdk.org
>>>>> Cc: Gaetan Rivet <gaetan.rivet at 6wind.com>
>>>>> Subject: [dpdk-dev] [PATCH v2 07/12] pdump: disabled by default
>>>>>
>>>>> Signed-off-by: Gaetan Rivet <gaetan.rivet at 6wind.com>
>>>>> ---
>>>>>  config/common_base | 2 +-
>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/config/common_base b/config/common_base index
>>>>> cade611..8ec5e4e 100644
>>>>> --- a/config/common_base
>>>>> +++ b/config/common_base
>>>>> @@ -700,7 +700,7 @@ CONFIG_RTE_KNI_PREEMPT_DEFAULT=y  #  # Compile
>>>>> the pdump library  # -CONFIG_RTE_LIBRTE_PDUMP=y
>>>>> +CONFIG_RTE_LIBRTE_PDUMP=n
>>>>>
>>>>>  #
>>>>>  # Compile vhost user library
>>>>> --
>>>>> 2.1.4
>>>>
>>>> Since, you already mentioned in other mail to Ferruh that config flag disabling patches are only for testers compilation purpose and you have plans to make proper fix by end of June.  I will wait on for actual patch. 
>>>>
>>>
>>> I said I planned to do so, but found out that I would not have enough
>>> time before the end of June. Sorry about the ambiguous phrasing.
>>>
>>> Do you think you will be able to fix this library in time?
>>
>> KNI uses / depends pci, I am not sure what to fix here.
>>
>> The problem to enable the KNI is build dependency problem, right?
>>
>> I guess problem will be fixes if we can build in following order:
>> - lib/eal
>> - drivers/bus
>> - lib
>> - drivers
>>
>> This was the case when bus drives compiled within eal. What do you think
>> about this build order?
>>
> 
> Yes, that build order would fix the issue.
> However, IMO this is not the proper way to proceed.
> It obscures the architecture, the distinction between DPDK abstractions
> and their implementations.
> 
> Looking quickly into this dependency, it seems that the PCI info is only
> used during allocation, and only to register PCI information within
> device infos. They do not seem used afterward at the library level,
> except to print some device description upon device start.
> 
> They can be completely removed from KNI (both from the lib and the
> driver), without breaking the compilation.
> This however changes the API of rte_kni_alloc() and the ABI of
> rte_kni_conf.
> 
> But it seems better than changing the build order and opening a can of
> all kind of worms, allowing a few libraries to skirt around their duty to
> remain generic and independent from abstraction implementations.
> 
> Ideally KNI interfaces should be able to use any rte_device, not only
> PCI. But if it is forced to use only PCI devices, then pointing to an
> rte_pci_device seems a better way to proceed, as it has all those infos
> readily available. It would allow the PCI device to grow and evolve without
> breaking the KNI lib.
> 
> Anyway, I think there are several possible solutions to this, before
> resorting to modifying the build order.

I started the discussion in wrong thread, I will copy your mail and
answer from correct thread, hoping this won't make things more confusing.

For future reference, moving to:
http://dpdk.org/ml/archives/dev/2017-June/067688.html

> 
>>>
>>>> Please see if rte_pci.h can be removed from the includes of rte_pdump.c , might be unnecessary include.
>>>>
>>>> Thanks,
>>>> Reshma
>>>
>>
>