DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Gaëtan Rivet" <gaetan.rivet@6wind.com>
To: Ferruh Yigit <ferruh.yigit@intel.com>
Cc: "Pattan, Reshma" <reshma.pattan@intel.com>,
	"dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v2 07/12] pdump: disabled by default
Date: Thu, 15 Jun 2017 01:01:25 +0200	[thread overview]
Message-ID: <20170614230124.GZ29091@bidouze.vm.6wind.com> (raw)
In-Reply-To: <e021f9c4-022e-6cc1-248a-49f8e6809fdf@intel.com>

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@dpdk.org] On Behalf Of Gaetan Rivet
> >>> Sent: Thursday, June 8, 2017 12:59 AM
> >>> To: dev@dpdk.org
> >>> Cc: Gaetan Rivet <gaetan.rivet@6wind.com>
> >>> Subject: [dpdk-dev] [PATCH v2 07/12] pdump: disabled by default
> >>>
> >>> Signed-off-by: Gaetan Rivet <gaetan.rivet@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.

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

-- 
Gaëtan Rivet
6WIND

  reply	other threads:[~2017-06-14 23:01 UTC|newest]

Thread overview: 63+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-01 10:14 [dpdk-dev] [PATCH 0/8] bus/pci: remove PCI bus from EAL Gaetan Rivet
2017-06-01 10:14 ` [dpdk-dev] [PATCH 1/8] eal: expose rte_eal_using_phys_addrs Gaetan Rivet
2017-06-30 19:05   ` Jan Blunck
2017-07-03  8:25     ` Olivier Matz
2017-07-03 10:04       ` Jan Blunck
2017-07-03 11:49         ` Olivier Matz
2017-06-01 10:14 ` [dpdk-dev] [PATCH 2/8] ethdev: remove useless PCI dependency Gaetan Rivet
2017-06-01 10:14 ` [dpdk-dev] [PATCH 3/8] pmdinfogen: move to drivers subdirectory Gaetan Rivet
2017-06-01 10:14 ` [dpdk-dev] [PATCH 4/8] cryptodev: disabled by default Gaetan Rivet
2017-06-01 10:14 ` [dpdk-dev] [PATCH 5/8] eventdev: " Gaetan Rivet
2017-06-01 10:14 ` [dpdk-dev] [PATCH 6/8] pdump: " Gaetan Rivet
2017-06-01 10:14 ` [dpdk-dev] [PATCH 7/8] kni: " Gaetan Rivet
2017-06-01 10:14 ` [dpdk-dev] [PATCH 8/8] bus/pci: introduce pci bus Gaetan Rivet
2017-06-07 23:58 ` [dpdk-dev] [PATCH 0/8] bus/pci: remove PCI bus from EAL Gaetan Rivet
2017-06-07 23:58   ` [dpdk-dev] [PATCH v2 01/12] eal: expose rte_eal_using_phys_addrs Gaetan Rivet
2017-06-07 23:58   ` [dpdk-dev] [PATCH v2 02/12] ethdev: remove useless PCI dependency Gaetan Rivet
2017-06-07 23:58   ` [dpdk-dev] [PATCH v2 03/12] bus: properly include rte_debug Gaetan Rivet
2017-06-07 23:59   ` [dpdk-dev] [PATCH v2 04/12] eal: remove references to PCI Gaetan Rivet
2017-06-07 23:59   ` [dpdk-dev] [PATCH v2 05/12] pmdinfogen: move to drivers subdirectory Gaetan Rivet
2017-06-07 23:59   ` [dpdk-dev] [PATCH v2 06/12] cryptodev: disabled by default Gaetan Rivet
2017-06-09 15:03     ` De Lara Guarch, Pablo
2017-06-14  8:00       ` Gaëtan Rivet
2017-06-07 23:59   ` [dpdk-dev] [PATCH v2 07/12] pdump: " Gaetan Rivet
2017-06-09 14:24     ` Pattan, Reshma
2017-06-11 19:42       ` Gaëtan Rivet
2017-06-13 17:15         ` Ferruh Yigit
2017-06-14 23:01           ` Gaëtan Rivet [this message]
2017-06-15 13:07             ` Ferruh Yigit
2017-06-14  9:09         ` Pattan, Reshma
2017-06-14  9:20           ` Gaëtan Rivet
2017-06-07 23:59   ` [dpdk-dev] [PATCH v2 08/12] kni: " Gaetan Rivet
2017-06-09  8:56     ` Ferruh Yigit
2017-06-09  9:06       ` Gaëtan Rivet
2017-06-15 13:09         ` Ferruh Yigit
2017-06-15 14:48           ` Gaëtan Rivet
2017-06-07 23:59   ` [dpdk-dev] [PATCH v2 09/12] bus/pci: introduce pci bus Gaetan Rivet
2017-06-07 23:59   ` [dpdk-dev] [PATCH v2 10/12] bus/pci: follow checkpatch Gaetan Rivet
2017-06-07 23:59   ` [dpdk-dev] [PATCH v2 11/12] drivers: update eventdev dependencies Gaetan Rivet
2017-06-07 23:59   ` [dpdk-dev] [PATCH v2 12/12] drivers: update cryptodev dependencies Gaetan Rivet
2017-06-20 23:36   ` [dpdk-dev] [PATCH v3 0/9] bus/pci: remove PCI bus from EAL Gaetan Rivet
2017-06-20 23:36     ` [dpdk-dev] [PATCH v3 1/9] kni: disabled by default Gaetan Rivet
2017-06-20 23:36     ` [dpdk-dev] [PATCH v3 2/9] eal: expose rte_eal_using_phys_addrs Gaetan Rivet
2017-06-21  7:44       ` Thomas Monjalon
2017-06-21  9:21         ` Gaëtan Rivet
2017-06-20 23:36     ` [dpdk-dev] [PATCH v3 3/9] ethdev: remove useless PCI dependency Gaetan Rivet
2017-06-20 23:36     ` [dpdk-dev] [PATCH v3 4/9] bus: properly include rte_debug Gaetan Rivet
2017-06-21  7:45       ` Thomas Monjalon
2017-06-21  9:26         ` Gaëtan Rivet
2017-06-20 23:36     ` [dpdk-dev] [PATCH v3 5/9] pmdinfogen: move to drivers subdirectory Gaetan Rivet
2017-06-21  7:57       ` Thomas Monjalon
2017-06-21  9:40         ` Gaëtan Rivet
2017-06-21 10:00           ` Thomas Monjalon
2017-06-21 11:39             ` Gaëtan Rivet
2017-06-21 12:14               ` Thomas Monjalon
2017-06-20 23:36     ` [dpdk-dev] [PATCH v3 6/9] bus/pci: introduce pci bus Gaetan Rivet
2017-06-20 23:36     ` [dpdk-dev] [PATCH v3 7/9] bus/pci: follow checkpatch Gaetan Rivet
2017-06-20 23:36     ` [dpdk-dev] [PATCH v3 8/9] drivers: update eventdev dependencies Gaetan Rivet
2017-06-20 23:36     ` [dpdk-dev] [PATCH v3 9/9] drivers: update cryptodev dependencies Gaetan Rivet
2017-06-23  3:29     ` [dpdk-dev] [PATCH v3 0/9] bus/pci: remove PCI bus from EAL Tan, Jianfeng
2017-06-23  8:19       ` Gaëtan Rivet
2017-06-23 12:48         ` Thomas Monjalon
2017-06-23 14:35           ` Tan, Jianfeng
2017-06-23 14:45             ` Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170614230124.GZ29091@bidouze.vm.6wind.com \
    --to=gaetan.rivet@6wind.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=reshma.pattan@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).