DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev]  [PATCH] bus/pci: optimize pci device probe
@ 2020-04-26 17:38 jerinj
  2020-04-26 18:08 ` Thomas Monjalon
  0 siblings, 1 reply; 15+ messages in thread
From: jerinj @ 2020-04-26 17:38 UTC (permalink / raw)
  Cc: dev, maxime.coquelin, zhihong.wang, xiaolong.ye, thomas,
	david.marchand, hkalra, Jerin Jacob

From: Jerin Jacob <jerinj@marvell.com>

If the PCI device is not attached to any driver then there is no
point in probing it. As an optimization, skip the PCI device probe if
the PCI device driver of type RTE_KDRV_NONE.

Signed-off-by: Jerin Jacob <jerinj@marvell.com>
---
Notes:
------
- virtio drivers does special treatment based on RTE_KDRV_UNKNOWN, That is
the reason allowing RTE_KDRV_UNKNOWN in this patch.
- virio devices uses RTE_KDRV_UNKNOWN for some special meaning, IMO, if it would
  be better, if
a) Introduce the KDRV for virio
b) If the PCIe device of driver type NONE or UNKNOWN then not even add in pci
list
in the scan, It will improve the boot time by avoiding operation on
unwanted device like sorting the PCI devices, scanning it, probe it, managing
it etc.

- Initial problem reported at http://patches.dpdk.org/patch/64999/ as
boot time printf clutter on octeontx2 devices with a lot PCI devices which are
of type RTE_KDRV_NONE.

 drivers/bus/pci/pci_common.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c
index 3f5542076..2fa3d85ae 100644
--- a/drivers/bus/pci/pci_common.c
+++ b/drivers/bus/pci/pci_common.c
@@ -271,6 +271,8 @@ pci_probe_all_drivers(struct rte_pci_device *dev)
 		return -EINVAL;

 	FOREACH_DRIVER_ON_PCIBUS(dr) {
+		if (dev->kdrv == RTE_KDRV_NONE)
+			continue;
 		rc = rte_pci_probe_one_driver(dr, dev);
 		if (rc < 0)
 			/* negative value is an error */
--
2.26.2


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-dev] [PATCH] bus/pci: optimize pci device probe
  2020-04-26 17:38 [dpdk-dev] [PATCH] bus/pci: optimize pci device probe jerinj
@ 2020-04-26 18:08 ` Thomas Monjalon
  2020-04-26 18:41   ` Jerin Jacob
  0 siblings, 1 reply; 15+ messages in thread
From: Thomas Monjalon @ 2020-04-26 18:08 UTC (permalink / raw)
  To: jerinj
  Cc: dev, maxime.coquelin, zhihong.wang, xiaolong.ye, david.marchand,
	hkalra, Jerin Jacob

26/04/2020 19:38, jerinj@marvell.com:
> From: Jerin Jacob <jerinj@marvell.com>
> 
> If the PCI device is not attached to any driver then there is no
> point in probing it. As an optimization, skip the PCI device probe if
> the PCI device driver of type RTE_KDRV_NONE.
> 
> Signed-off-by: Jerin Jacob <jerinj@marvell.com>
> ---
> Notes:
> ------
> - virtio drivers does special treatment based on RTE_KDRV_UNKNOWN, That is
> the reason allowing RTE_KDRV_UNKNOWN in this patch.
> - virio devices uses RTE_KDRV_UNKNOWN for some special meaning, IMO, if it would
>   be better, if
> a) Introduce the KDRV for virio
> b) If the PCIe device of driver type NONE or UNKNOWN then not even add in pci
> list
> in the scan, It will improve the boot time by avoiding operation on
> unwanted device like sorting the PCI devices, scanning it, probe it, managing
> it etc.

mlx4/mlx4 uses RTE_KDRV_UNKNOWN.

> - Initial problem reported at http://patches.dpdk.org/patch/64999/ as
> boot time printf clutter on octeontx2 devices with a lot PCI devices which are
> of type RTE_KDRV_NONE.

Add a logtype for PCI driver and adjust log level accordingly
to your preferences.

> @@ -271,6 +271,8 @@ pci_probe_all_drivers(struct rte_pci_device *dev)
>  	FOREACH_DRIVER_ON_PCIBUS(dr) {
> +		if (dev->kdrv == RTE_KDRV_NONE)
> +			continue;
>  		rc = rte_pci_probe_one_driver(dr, dev);

Nack



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-dev] [PATCH] bus/pci: optimize pci device probe
  2020-04-26 18:08 ` Thomas Monjalon
@ 2020-04-26 18:41   ` Jerin Jacob
  2020-04-26 20:06     ` Thomas Monjalon
  0 siblings, 1 reply; 15+ messages in thread
From: Jerin Jacob @ 2020-04-26 18:41 UTC (permalink / raw)
  To: Thomas Monjalon
  Cc: Jerin Jacob, dpdk-dev, Maxime Coquelin, Zhihong Wang, Ye,
	Xiaolong, David Marchand, Harman Kalra

On Sun, Apr 26, 2020 at 11:38 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 26/04/2020 19:38, jerinj@marvell.com:
> > From: Jerin Jacob <jerinj@marvell.com>
> >
> > If the PCI device is not attached to any driver then there is no
> > point in probing it. As an optimization, skip the PCI device probe if
> > the PCI device driver of type RTE_KDRV_NONE.
> >
> > Signed-off-by: Jerin Jacob <jerinj@marvell.com>
> > ---
> > Notes:
> > ------
> > - virtio drivers does special treatment based on RTE_KDRV_UNKNOWN, That is
> > the reason allowing RTE_KDRV_UNKNOWN in this patch.
> > - virio devices uses RTE_KDRV_UNKNOWN for some special meaning, IMO, if it would
> >   be better, if
> > a) Introduce the KDRV for virio
> > b) If the PCIe device of driver type NONE or UNKNOWN then not even add in pci
> > list
> > in the scan, It will improve the boot time by avoiding operation on
> > unwanted device like sorting the PCI devices, scanning it, probe it, managing
> > it etc.
>
> mlx4/mlx4 uses RTE_KDRV_UNKNOWN.

OK.

>
> > - Initial problem reported at http://patches.dpdk.org/patch/64999/ as
> > boot time printf clutter on octeontx2 devices with a lot PCI devices which are
> > of type RTE_KDRV_NONE.
>
> Add a logtype for PCI driver and adjust log level accordingly
> to your preferences.
>
> > @@ -271,6 +271,8 @@ pci_probe_all_drivers(struct rte_pci_device *dev)
> >       FOREACH_DRIVER_ON_PCIBUS(dr) {
> > +             if (dev->kdrv == RTE_KDRV_NONE)
> > +                     continue;
> >               rc = rte_pci_probe_one_driver(dr, dev);
>
> Nack

I understand mlx4/mlx5 is using RTE_KDRV_UNKNOWN, Here we are skipping
the RTE_KDRV_NONE,
What is the use case for probing the devices with RTE_KDRV_NONE?


>
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-dev] [PATCH] bus/pci: optimize pci device probe
  2020-04-26 18:41   ` Jerin Jacob
@ 2020-04-26 20:06     ` Thomas Monjalon
  2020-04-27 17:59       ` Jerin Jacob
  2020-04-28  8:50       ` David Marchand
  0 siblings, 2 replies; 15+ messages in thread
From: Thomas Monjalon @ 2020-04-26 20:06 UTC (permalink / raw)
  To: Jerin Jacob
  Cc: Jerin Jacob, dpdk-dev, Maxime Coquelin, Zhihong Wang, Ye,
	Xiaolong, David Marchand, Harman Kalra

26/04/2020 20:41, Jerin Jacob:
> On Sun, Apr 26, 2020 at 11:38 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> >
> > 26/04/2020 19:38, jerinj@marvell.com:
> > > From: Jerin Jacob <jerinj@marvell.com>
> > >
> > > If the PCI device is not attached to any driver then there is no
> > > point in probing it. As an optimization, skip the PCI device probe if
> > > the PCI device driver of type RTE_KDRV_NONE.
> > >
> > > Signed-off-by: Jerin Jacob <jerinj@marvell.com>
> > > ---
> > > Notes:
> > > ------
> > > - virtio drivers does special treatment based on RTE_KDRV_UNKNOWN, That is
> > > the reason allowing RTE_KDRV_UNKNOWN in this patch.
> > > - virio devices uses RTE_KDRV_UNKNOWN for some special meaning, IMO, if it would
> > >   be better, if
> > > a) Introduce the KDRV for virio
> > > b) If the PCIe device of driver type NONE or UNKNOWN then not even add in pci
> > > list
> > > in the scan, It will improve the boot time by avoiding operation on
> > > unwanted device like sorting the PCI devices, scanning it, probe it, managing
> > > it etc.
> >
> > mlx4/mlx4 uses RTE_KDRV_UNKNOWN.
> 
> OK.
> 
> > > - Initial problem reported at http://patches.dpdk.org/patch/64999/ as
> > > boot time printf clutter on octeontx2 devices with a lot PCI devices which are
> > > of type RTE_KDRV_NONE.
> >
> > Add a logtype for PCI driver and adjust log level accordingly
> > to your preferences.
> >
> > > @@ -271,6 +271,8 @@ pci_probe_all_drivers(struct rte_pci_device *dev)
> > >       FOREACH_DRIVER_ON_PCIBUS(dr) {
> > > +             if (dev->kdrv == RTE_KDRV_NONE)
> > > +                     continue;
> > >               rc = rte_pci_probe_one_driver(dr, dev);
> >
> > Nack
> 
> I understand mlx4/mlx5 is using RTE_KDRV_UNKNOWN, Here we are skipping
> the RTE_KDRV_NONE,
> What is the use case for probing the devices with RTE_KDRV_NONE?

Maybe you are right. I don't remember the use case.
I think I remember these virtio and vmxnet3 PMD were not using UIO:
	http://git.dpdk.org/old/virtio-net-pmd/tree/virtio_user.c
	http://git.dpdk.org/old/vmxnet3-usermap/tree/pmd/vmxnet3.c

We need to know which case is using following code:

    case RTE_KDRV_NONE:
#if defined(RTE_ARCH_X86)
        ret = pci_ioport_map(dev, bar, p);
#endif
        break;

David, please could you refresh our memory?



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-dev] [PATCH] bus/pci: optimize pci device probe
  2020-04-26 20:06     ` Thomas Monjalon
@ 2020-04-27 17:59       ` Jerin Jacob
  2020-04-28  8:50       ` David Marchand
  1 sibling, 0 replies; 15+ messages in thread
From: Jerin Jacob @ 2020-04-27 17:59 UTC (permalink / raw)
  To: Thomas Monjalon
  Cc: Jerin Jacob, dpdk-dev, Maxime Coquelin, Zhihong Wang, Ye,
	Xiaolong, David Marchand, Harman Kalra

On Mon, Apr 27, 2020 at 1:36 AM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 26/04/2020 20:41, Jerin Jacob:
> > On Sun, Apr 26, 2020 at 11:38 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> > >
> > > 26/04/2020 19:38, jerinj@marvell.com:
> > > > From: Jerin Jacob <jerinj@marvell.com>
> > > >
> > > > If the PCI device is not attached to any driver then there is no
> > > > point in probing it. As an optimization, skip the PCI device probe if
> > > > the PCI device driver of type RTE_KDRV_NONE.
> > > >
> > > > Signed-off-by: Jerin Jacob <jerinj@marvell.com>
> > > > ---
> > > > Notes:
> > > > ------
> > > > - virtio drivers does special treatment based on RTE_KDRV_UNKNOWN, That is
> > > > the reason allowing RTE_KDRV_UNKNOWN in this patch.
> > > > - virio devices uses RTE_KDRV_UNKNOWN for some special meaning, IMO, if it would
> > > >   be better, if
> > > > a) Introduce the KDRV for virio
> > > > b) If the PCIe device of driver type NONE or UNKNOWN then not even add in pci
> > > > list
> > > > in the scan, It will improve the boot time by avoiding operation on
> > > > unwanted device like sorting the PCI devices, scanning it, probe it, managing
> > > > it etc.
> > >
> > > mlx4/mlx4 uses RTE_KDRV_UNKNOWN.
> >
> > OK.
> >
> > > > - Initial problem reported at http://patches.dpdk.org/patch/64999/ as
> > > > boot time printf clutter on octeontx2 devices with a lot PCI devices which are
> > > > of type RTE_KDRV_NONE.
> > >
> > > Add a logtype for PCI driver and adjust log level accordingly
> > > to your preferences.
> > >
> > > > @@ -271,6 +271,8 @@ pci_probe_all_drivers(struct rte_pci_device *dev)
> > > >       FOREACH_DRIVER_ON_PCIBUS(dr) {
> > > > +             if (dev->kdrv == RTE_KDRV_NONE)
> > > > +                     continue;
> > > >               rc = rte_pci_probe_one_driver(dr, dev);
> > >
> > > Nack
> >
> > I understand mlx4/mlx5 is using RTE_KDRV_UNKNOWN, Here we are skipping
> > the RTE_KDRV_NONE,
> > What is the use case for probing the devices with RTE_KDRV_NONE?
>
> Maybe you are right. I don't remember the use case.
> I think I remember these virtio and vmxnet3 PMD were not using UIO:
>         http://git.dpdk.org/old/virtio-net-pmd/tree/virtio_user.c
>         http://git.dpdk.org/old/vmxnet3-usermap/tree/pmd/vmxnet3.c

Looks it is OLD stale drivers with DPDK 1.3 or something.

Currently, virtio is using, following drivers.

RTE_PMD_REGISTER_PCI_TABLE(net_virtio, pci_id_virtio_map);
RTE_PMD_REGISTER_KMOD_DEP(net_virtio, "* igb_uio | uio_pci_generic | vfio-pci");

>
> We need to know which case is using following code:
>
>     case RTE_KDRV_NONE:
> #if defined(RTE_ARCH_X86)
>         ret = pci_ioport_map(dev, bar, p);
> #endif
>         break;

AFAIK, it does not affect the scanning. virtio maintainers?

> David, please could you refresh our memory?
>
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-dev] [PATCH] bus/pci: optimize pci device probe
  2020-04-26 20:06     ` Thomas Monjalon
  2020-04-27 17:59       ` Jerin Jacob
@ 2020-04-28  8:50       ` David Marchand
  2020-04-28  9:34         ` Jerin Jacob
  1 sibling, 1 reply; 15+ messages in thread
From: David Marchand @ 2020-04-28  8:50 UTC (permalink / raw)
  To: Thomas Monjalon
  Cc: Jerin Jacob, Jerin Jacob, dpdk-dev, Maxime Coquelin,
	Zhihong Wang, Ye, Xiaolong, Harman Kalra

On Sun, Apr 26, 2020 at 10:06 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 26/04/2020 20:41, Jerin Jacob:
> > On Sun, Apr 26, 2020 at 11:38 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> > >
> > > 26/04/2020 19:38, jerinj@marvell.com:
> > > > From: Jerin Jacob <jerinj@marvell.com>
> > > >
> > > > If the PCI device is not attached to any driver then there is no
> > > > point in probing it. As an optimization, skip the PCI device probe if
> > > > the PCI device driver of type RTE_KDRV_NONE.
> > > >
> > > > Signed-off-by: Jerin Jacob <jerinj@marvell.com>
> > > > ---
> > > > Notes:
> > > > ------
> > > > - virtio drivers does special treatment based on RTE_KDRV_UNKNOWN, That is
> > > > the reason allowing RTE_KDRV_UNKNOWN in this patch.
> > > > - virio devices uses RTE_KDRV_UNKNOWN for some special meaning, IMO, if it would
> > > >   be better, if
> > > > a) Introduce the KDRV for virio
> > > > b) If the PCIe device of driver type NONE or UNKNOWN then not even add in pci
> > > > list
> > > > in the scan, It will improve the boot time by avoiding operation on
> > > > unwanted device like sorting the PCI devices, scanning it, probe it, managing
> > > > it etc.
> > >
> > > mlx4/mlx4 uses RTE_KDRV_UNKNOWN.
> >
> > OK.
> >
> > > > - Initial problem reported at http://patches.dpdk.org/patch/64999/ as
> > > > boot time printf clutter on octeontx2 devices with a lot PCI devices which are
> > > > of type RTE_KDRV_NONE.
> > >
> > > Add a logtype for PCI driver and adjust log level accordingly
> > > to your preferences.
> > >
> > > > @@ -271,6 +271,8 @@ pci_probe_all_drivers(struct rte_pci_device *dev)
> > > >       FOREACH_DRIVER_ON_PCIBUS(dr) {
> > > > +             if (dev->kdrv == RTE_KDRV_NONE)
> > > > +                     continue;
> > > >               rc = rte_pci_probe_one_driver(dr, dev);
> > >
> > > Nack
> >
> > I understand mlx4/mlx5 is using RTE_KDRV_UNKNOWN, Here we are skipping
> > the RTE_KDRV_NONE,
> > What is the use case for probing the devices with RTE_KDRV_NONE?
>
> Maybe you are right. I don't remember the use case.
> I think I remember these virtio and vmxnet3 PMD were not using UIO:
>         http://git.dpdk.org/old/virtio-net-pmd/tree/virtio_user.c
>         http://git.dpdk.org/old/vmxnet3-usermap/tree/pmd/vmxnet3.c
>
> We need to know which case is using following code:
>
>     case RTE_KDRV_NONE:
> #if defined(RTE_ARCH_X86)
>         ret = pci_ioport_map(dev, bar, p);
> #endif
>         break;
>
> David, please could you refresh our memory?

The in-tree virtio-net driver directly calls rte_pci_map_device /
rte_pci_ioport_map depending on virtio legacy/modern modes.
This is why the virtio pci driver does not ask for
RTE_PCI_DRV_NEED_MAPPING to the pci bus.


In ioport mode, there were two options for historical reasons because
of the virtio driver you mention.
This driver did not rely on uio, and this behavior was later merged to
the current in-tree driver.
It ended up (not clean) in the pci bus driver because virtio was the
only user of this code.


Removing this special case could break x86 applications running with
legacy virtio.


On the plus side, we have been announcing for some time in virtio:
RTE_PMD_REGISTER_KMOD_DEP(net_virtio, "* igb_uio | uio_pci_generic | vfio-pci");


-- 
David Marchand


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-dev] [PATCH] bus/pci: optimize pci device probe
  2020-04-28  8:50       ` David Marchand
@ 2020-04-28  9:34         ` Jerin Jacob
  2020-05-05 15:50           ` Jerin Jacob
  0 siblings, 1 reply; 15+ messages in thread
From: Jerin Jacob @ 2020-04-28  9:34 UTC (permalink / raw)
  To: David Marchand
  Cc: Thomas Monjalon, Jerin Jacob, dpdk-dev, Maxime Coquelin,
	Zhihong Wang, Ye, Xiaolong, Harman Kalra

On Tue, Apr 28, 2020 at 2:21 PM David Marchand
<david.marchand@redhat.com> wrote:
>
> On Sun, Apr 26, 2020 at 10:06 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> >
> > 26/04/2020 20:41, Jerin Jacob:
> > > On Sun, Apr 26, 2020 at 11:38 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> > > >
> > > > 26/04/2020 19:38, jerinj@marvell.com:
> > > > > From: Jerin Jacob <jerinj@marvell.com>
> > > > >
> > > > > If the PCI device is not attached to any driver then there is no
> > > > > point in probing it. As an optimization, skip the PCI device probe if
> > > > > the PCI device driver of type RTE_KDRV_NONE.
> > > > >
> > > > > Signed-off-by: Jerin Jacob <jerinj@marvell.com>
> > > > > ---
> > > > > Notes:
> > > > > ------
> > > > > - virtio drivers does special treatment based on RTE_KDRV_UNKNOWN, That is
> > > > > the reason allowing RTE_KDRV_UNKNOWN in this patch.
> > > > > - virio devices uses RTE_KDRV_UNKNOWN for some special meaning, IMO, if it would
> > > > >   be better, if
> > > > > a) Introduce the KDRV for virio
> > > > > b) If the PCIe device of driver type NONE or UNKNOWN then not even add in pci
> > > > > list
> > > > > in the scan, It will improve the boot time by avoiding operation on
> > > > > unwanted device like sorting the PCI devices, scanning it, probe it, managing
> > > > > it etc.
> > > >
> > > > mlx4/mlx4 uses RTE_KDRV_UNKNOWN.
> > >
> > > OK.
> > >
> > > > > - Initial problem reported at http://patches.dpdk.org/patch/64999/ as
> > > > > boot time printf clutter on octeontx2 devices with a lot PCI devices which are
> > > > > of type RTE_KDRV_NONE.
> > > >
> > > > Add a logtype for PCI driver and adjust log level accordingly
> > > > to your preferences.
> > > >
> > > > > @@ -271,6 +271,8 @@ pci_probe_all_drivers(struct rte_pci_device *dev)
> > > > >       FOREACH_DRIVER_ON_PCIBUS(dr) {
> > > > > +             if (dev->kdrv == RTE_KDRV_NONE)
> > > > > +                     continue;
> > > > >               rc = rte_pci_probe_one_driver(dr, dev);
> > > >
> > > > Nack
> > >
> > > I understand mlx4/mlx5 is using RTE_KDRV_UNKNOWN, Here we are skipping
> > > the RTE_KDRV_NONE,
> > > What is the use case for probing the devices with RTE_KDRV_NONE?
> >
> > Maybe you are right. I don't remember the use case.
> > I think I remember these virtio and vmxnet3 PMD were not using UIO:
> >         http://git.dpdk.org/old/virtio-net-pmd/tree/virtio_user.c
> >         http://git.dpdk.org/old/vmxnet3-usermap/tree/pmd/vmxnet3.c
> >
> > We need to know which case is using following code:
> >
> >     case RTE_KDRV_NONE:
> > #if defined(RTE_ARCH_X86)
> >         ret = pci_ioport_map(dev, bar, p);
> > #endif
> >         break;
> >
> > David, please could you refresh our memory?
>
> The in-tree virtio-net driver directly calls rte_pci_map_device /
> rte_pci_ioport_map depending on virtio legacy/modern modes.
> This is why the virtio pci driver does not ask for
> RTE_PCI_DRV_NEED_MAPPING to the pci bus.
>
>
> In ioport mode, there were two options for historical reasons because
> of the virtio driver you mention.
> This driver did not rely on uio, and this behavior was later merged to
> the current in-tree driver.
> It ended up (not clean) in the pci bus driver because virtio was the
> only user of this code.
>
>
> Removing this special case could break x86 applications running with
> legacy virtio.
>
>
> On the plus side, we have been announcing for some time in virtio:
> RTE_PMD_REGISTER_KMOD_DEP(net_virtio, "* igb_uio | uio_pci_generic | vfio-pci");

What is to conclude?
# The In-tree virtio driver uses ""* igb_uio | uio_pci_generic |
vfio-pci"" driver as backend and it does not need RTE_KDRV_NONE?
OR
# The in-tree, legacy virtio(const struct virtio_pci_ops legacy_op)
can work without any kernel driver in the backend. So RTE_KDRV_NONE
need?




>
>
> --
> David Marchand
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-dev] [PATCH] bus/pci: optimize pci device probe
  2020-04-28  9:34         ` Jerin Jacob
@ 2020-05-05 15:50           ` Jerin Jacob
  2020-05-05 16:16             ` David Marchand
  0 siblings, 1 reply; 15+ messages in thread
From: Jerin Jacob @ 2020-05-05 15:50 UTC (permalink / raw)
  To: David Marchand
  Cc: Thomas Monjalon, Jerin Jacob, dpdk-dev, Maxime Coquelin,
	Zhihong Wang, Ye, Xiaolong, Harman Kalra

On Tue, Apr 28, 2020 at 3:04 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
>
> On Tue, Apr 28, 2020 at 2:21 PM David Marchand
> <david.marchand@redhat.com> wrote:
> >
> > On Sun, Apr 26, 2020 at 10:06 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> > >
> > > 26/04/2020 20:41, Jerin Jacob:
> > > > On Sun, Apr 26, 2020 at 11:38 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> > > > >
> > > > > 26/04/2020 19:38, jerinj@marvell.com:
> > > > > > From: Jerin Jacob <jerinj@marvell.com>
> > > > > >
> > > > > > If the PCI device is not attached to any driver then there is no
> > > > > > point in probing it. As an optimization, skip the PCI device probe if
> > > > > > the PCI device driver of type RTE_KDRV_NONE.
> > > > > >
> > > > > > Signed-off-by: Jerin Jacob <jerinj@marvell.com>
> > > > > > ---
> > > > > > Notes:
> > > > > > ------
> > > > > > - virtio drivers does special treatment based on RTE_KDRV_UNKNOWN, That is
> > > > > > the reason allowing RTE_KDRV_UNKNOWN in this patch.
> > > > > > - virio devices uses RTE_KDRV_UNKNOWN for some special meaning, IMO, if it would
> > > > > >   be better, if
> > > > > > a) Introduce the KDRV for virio
> > > > > > b) If the PCIe device of driver type NONE or UNKNOWN then not even add in pci
> > > > > > list
> > > > > > in the scan, It will improve the boot time by avoiding operation on
> > > > > > unwanted device like sorting the PCI devices, scanning it, probe it, managing
> > > > > > it etc.
> > > > >
> > > > > mlx4/mlx4 uses RTE_KDRV_UNKNOWN.
> > > >
> > > > OK.
> > > >
> > > > > > - Initial problem reported at http://patches.dpdk.org/patch/64999/ as
> > > > > > boot time printf clutter on octeontx2 devices with a lot PCI devices which are
> > > > > > of type RTE_KDRV_NONE.
> > > > >
> > > > > Add a logtype for PCI driver and adjust log level accordingly
> > > > > to your preferences.
> > > > >
> > > > > > @@ -271,6 +271,8 @@ pci_probe_all_drivers(struct rte_pci_device *dev)
> > > > > >       FOREACH_DRIVER_ON_PCIBUS(dr) {
> > > > > > +             if (dev->kdrv == RTE_KDRV_NONE)
> > > > > > +                     continue;
> > > > > >               rc = rte_pci_probe_one_driver(dr, dev);
> > > > >
> > > > > Nack
> > > >
> > > > I understand mlx4/mlx5 is using RTE_KDRV_UNKNOWN, Here we are skipping
> > > > the RTE_KDRV_NONE,
> > > > What is the use case for probing the devices with RTE_KDRV_NONE?
> > >
> > > Maybe you are right. I don't remember the use case.
> > > I think I remember these virtio and vmxnet3 PMD were not using UIO:
> > >         http://git.dpdk.org/old/virtio-net-pmd/tree/virtio_user.c
> > >         http://git.dpdk.org/old/vmxnet3-usermap/tree/pmd/vmxnet3.c
> > >
> > > We need to know which case is using following code:
> > >
> > >     case RTE_KDRV_NONE:
> > > #if defined(RTE_ARCH_X86)
> > >         ret = pci_ioport_map(dev, bar, p);
> > > #endif
> > >         break;
> > >
> > > David, please could you refresh our memory?
> >
> > The in-tree virtio-net driver directly calls rte_pci_map_device /
> > rte_pci_ioport_map depending on virtio legacy/modern modes.
> > This is why the virtio pci driver does not ask for
> > RTE_PCI_DRV_NEED_MAPPING to the pci bus.
> >
> >
> > In ioport mode, there were two options for historical reasons because
> > of the virtio driver you mention.
> > This driver did not rely on uio, and this behavior was later merged to
> > the current in-tree driver.
> > It ended up (not clean) in the pci bus driver because virtio was the
> > only user of this code.
> >
> >
> > Removing this special case could break x86 applications running with
> > legacy virtio.
> >
> >
> > On the plus side, we have been announcing for some time in virtio:
> > RTE_PMD_REGISTER_KMOD_DEP(net_virtio, "* igb_uio | uio_pci_generic | vfio-pci");
>
> What is to conclude?
> # The In-tree virtio driver uses ""* igb_uio | uio_pci_generic |
> vfio-pci"" driver as backend and it does not need RTE_KDRV_NONE?
> OR
> # The in-tree, legacy virtio(const struct virtio_pci_ops legacy_op)
> can work without any kernel driver in the backend. So RTE_KDRV_NONE
> need?

Ping. What is the conclusion? If it is former then this patch is valid.



>
>
>
>
> >
> >
> > --
> > David Marchand
> >

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-dev] [PATCH] bus/pci: optimize pci device probe
  2020-05-05 15:50           ` Jerin Jacob
@ 2020-05-05 16:16             ` David Marchand
  2020-05-06  6:34               ` Maxime Coquelin
  0 siblings, 1 reply; 15+ messages in thread
From: David Marchand @ 2020-05-05 16:16 UTC (permalink / raw)
  To: Jerin Jacob, Maxime Coquelin
  Cc: Thomas Monjalon, Jerin Jacob, dpdk-dev, Zhihong Wang, Ye,
	Xiaolong, Harman Kalra, Kevin Traynor, Luca Boccassi,
	Gaetan Rivet

On Tue, May 5, 2020 at 5:50 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > >
> > > Removing this special case could break x86 applications running with
> > > legacy virtio.
> > >
> > >
> > > On the plus side, we have been announcing for some time in virtio:
> > > RTE_PMD_REGISTER_KMOD_DEP(net_virtio, "* igb_uio | uio_pci_generic | vfio-pci");
> >
> > What is to conclude?
> > # The In-tree virtio driver uses ""* igb_uio | uio_pci_generic |
> > vfio-pci"" driver as backend and it does not need RTE_KDRV_NONE?
> > OR
> > # The in-tree, legacy virtio(const struct virtio_pci_ops legacy_op)
> > can work without any kernel driver in the backend. So RTE_KDRV_NONE
> > need?
>
> Ping. What is the conclusion? If it is former then this patch is valid.

I am fine with dropping the legacy part, but I wanted to hear from
Maxime at least.


-- 
David Marchand


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-dev] [PATCH] bus/pci: optimize pci device probe
  2020-05-05 16:16             ` David Marchand
@ 2020-05-06  6:34               ` Maxime Coquelin
  2020-05-06  6:43                 ` Jerin Jacob
  0 siblings, 1 reply; 15+ messages in thread
From: Maxime Coquelin @ 2020-05-06  6:34 UTC (permalink / raw)
  To: David Marchand, Jerin Jacob
  Cc: Thomas Monjalon, Jerin Jacob, dpdk-dev, Zhihong Wang, Ye,
	Xiaolong, Harman Kalra, Kevin Traynor, Luca Boccassi,
	Gaetan Rivet

Hi,

On 5/5/20 6:16 PM, David Marchand wrote:
> On Tue, May 5, 2020 at 5:50 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
>>>>
>>>> Removing this special case could break x86 applications running with
>>>> legacy virtio.
>>>>
>>>>
>>>> On the plus side, we have been announcing for some time in virtio:
>>>> RTE_PMD_REGISTER_KMOD_DEP(net_virtio, "* igb_uio | uio_pci_generic | vfio-pci");
>>>
>>> What is to conclude?
>>> # The In-tree virtio driver uses ""* igb_uio | uio_pci_generic |
>>> vfio-pci"" driver as backend and it does not need RTE_KDRV_NONE?
>>> OR
>>> # The in-tree, legacy virtio(const struct virtio_pci_ops legacy_op)
>>> can work without any kernel driver in the backend. So RTE_KDRV_NONE
>>> need?
>>
>> Ping. What is the conclusion? If it is former then this patch is valid.
> 
> I am fine with dropping the legacy part, but I wanted to hear from
> Maxime at least.
> 
> 

IIUC, it means that with Jerin patch, Virtio Legacy devices support will
be dropped as they won't be probed anymore?

Maxime


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-dev] [PATCH] bus/pci: optimize pci device probe
  2020-05-06  6:34               ` Maxime Coquelin
@ 2020-05-06  6:43                 ` Jerin Jacob
  2020-05-06  7:52                   ` Maxime Coquelin
  0 siblings, 1 reply; 15+ messages in thread
From: Jerin Jacob @ 2020-05-06  6:43 UTC (permalink / raw)
  To: Maxime Coquelin
  Cc: David Marchand, Thomas Monjalon, Jerin Jacob, dpdk-dev,
	Zhihong Wang, Ye, Xiaolong, Harman Kalra, Kevin Traynor,
	Luca Boccassi, Gaetan Rivet

On Wed, May 6, 2020 at 12:05 PM Maxime Coquelin
<maxime.coquelin@redhat.com> wrote:
>
> Hi,
>
> On 5/5/20 6:16 PM, David Marchand wrote:
> > On Tue, May 5, 2020 at 5:50 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> >>>>
> >>>> Removing this special case could break x86 applications running with
> >>>> legacy virtio.
> >>>>
> >>>>
> >>>> On the plus side, we have been announcing for some time in virtio:
> >>>> RTE_PMD_REGISTER_KMOD_DEP(net_virtio, "* igb_uio | uio_pci_generic | vfio-pci");
> >>>
> >>> What is to conclude?
> >>> # The In-tree virtio driver uses ""* igb_uio | uio_pci_generic |
> >>> vfio-pci"" driver as backend and it does not need RTE_KDRV_NONE?
> >>> OR
> >>> # The in-tree, legacy virtio(const struct virtio_pci_ops legacy_op)
> >>> can work without any kernel driver in the backend. So RTE_KDRV_NONE
> >>> need?
> >>
> >> Ping. What is the conclusion? If it is former then this patch is valid.
> >
> > I am fine with dropping the legacy part, but I wanted to hear from
> > Maxime at least.
> >
> >
>
> IIUC, it means that with Jerin patch, Virtio Legacy devices support will
> be dropped as they won't be probed anymore?

The device drivers with RTE_KDRV_NONE as the backend will not be probed.
1) Are Virtio Legacy devices are type of RTE_KDRV_NONE?
2) if yes, Would you like to support for virtio legacy device?
3) if yes, Please fix RTE_PMD_REGISTER_KMOD_DEP(net_virtio, "* igb_uio
| uio_pci_generic | vfio-pci");




>
> Maxime
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-dev] [PATCH] bus/pci: optimize pci device probe
  2020-05-06  6:43                 ` Jerin Jacob
@ 2020-05-06  7:52                   ` Maxime Coquelin
  2020-05-06 10:51                     ` Jerin Jacob
  0 siblings, 1 reply; 15+ messages in thread
From: Maxime Coquelin @ 2020-05-06  7:52 UTC (permalink / raw)
  To: Jerin Jacob
  Cc: David Marchand, Thomas Monjalon, Jerin Jacob, dpdk-dev,
	Zhihong Wang, Ye, Xiaolong, Harman Kalra, Kevin Traynor,
	Luca Boccassi, Gaetan Rivet



On 5/6/20 8:43 AM, Jerin Jacob wrote:
> On Wed, May 6, 2020 at 12:05 PM Maxime Coquelin
> <maxime.coquelin@redhat.com> wrote:
>>
>> Hi,
>>
>> On 5/5/20 6:16 PM, David Marchand wrote:
>>> On Tue, May 5, 2020 at 5:50 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
>>>>>>
>>>>>> Removing this special case could break x86 applications running with
>>>>>> legacy virtio.
>>>>>>
>>>>>>
>>>>>> On the plus side, we have been announcing for some time in virtio:
>>>>>> RTE_PMD_REGISTER_KMOD_DEP(net_virtio, "* igb_uio | uio_pci_generic | vfio-pci");
>>>>>
>>>>> What is to conclude?
>>>>> # The In-tree virtio driver uses ""* igb_uio | uio_pci_generic |
>>>>> vfio-pci"" driver as backend and it does not need RTE_KDRV_NONE?
>>>>> OR
>>>>> # The in-tree, legacy virtio(const struct virtio_pci_ops legacy_op)
>>>>> can work without any kernel driver in the backend. So RTE_KDRV_NONE
>>>>> need?
>>>>
>>>> Ping. What is the conclusion? If it is former then this patch is valid.
>>>
>>> I am fine with dropping the legacy part, but I wanted to hear from
>>> Maxime at least.
>>>
>>>
>>
>> IIUC, it means that with Jerin patch, Virtio Legacy devices support will
>> be dropped as they won't be probed anymore?
> 
> The device drivers with RTE_KDRV_NONE as the backend will not be probed.
> 1) Are Virtio Legacy devices are type of RTE_KDRV_NONE?

Virtio Legacy devices can be probed with no kernel driver.

> 2) if yes, Would you like to support for virtio legacy device?

I am OK to remove legacy + RTE_KDRV_NONE case, but I think it needs an
announcement and being done in a later release to let end-users using
that configuration time to do the change.

> 3) if yes, Please fix RTE_PMD_REGISTER_KMOD_DEP(net_virtio, "* igb_uio
> | uio_pci_generic | vfio-pci");

While support gets removed, what about:

RTE_PMD_REGISTER_KMOD_DEP(net_virtio, "* igb_uio | uio_pci_generic |
vfio-pci | none"); ?

Maxime


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-dev] [PATCH] bus/pci: optimize pci device probe
  2020-05-06  7:52                   ` Maxime Coquelin
@ 2020-05-06 10:51                     ` Jerin Jacob
  2020-05-06 11:37                       ` David Marchand
  0 siblings, 1 reply; 15+ messages in thread
From: Jerin Jacob @ 2020-05-06 10:51 UTC (permalink / raw)
  To: Maxime Coquelin
  Cc: David Marchand, Thomas Monjalon, Jerin Jacob, dpdk-dev,
	Zhihong Wang, Ye, Xiaolong, Harman Kalra, Kevin Traynor,
	Luca Boccassi, Gaetan Rivet

On Wed, May 6, 2020 at 1:23 PM Maxime Coquelin
<maxime.coquelin@redhat.com> wrote:
>
>
>
> On 5/6/20 8:43 AM, Jerin Jacob wrote:
> > On Wed, May 6, 2020 at 12:05 PM Maxime Coquelin
> > <maxime.coquelin@redhat.com> wrote:
> >>
> >> Hi,
> >>
> >> On 5/5/20 6:16 PM, David Marchand wrote:
> >>> On Tue, May 5, 2020 at 5:50 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> >>>>>>
> >>>>>> Removing this special case could break x86 applications running with
> >>>>>> legacy virtio.
> >>>>>>
> >>>>>>
> >>>>>> On the plus side, we have been announcing for some time in virtio:
> >>>>>> RTE_PMD_REGISTER_KMOD_DEP(net_virtio, "* igb_uio | uio_pci_generic | vfio-pci");
> >>>>>
> >>>>> What is to conclude?
> >>>>> # The In-tree virtio driver uses ""* igb_uio | uio_pci_generic |
> >>>>> vfio-pci"" driver as backend and it does not need RTE_KDRV_NONE?
> >>>>> OR
> >>>>> # The in-tree, legacy virtio(const struct virtio_pci_ops legacy_op)
> >>>>> can work without any kernel driver in the backend. So RTE_KDRV_NONE
> >>>>> need?
> >>>>
> >>>> Ping. What is the conclusion? If it is former then this patch is valid.
> >>>
> >>> I am fine with dropping the legacy part, but I wanted to hear from
> >>> Maxime at least.
> >>>
> >>>
> >>
> >> IIUC, it means that with Jerin patch, Virtio Legacy devices support will
> >> be dropped as they won't be probed anymore?
> >
> > The device drivers with RTE_KDRV_NONE as the backend will not be probed.
> > 1) Are Virtio Legacy devices are type of RTE_KDRV_NONE?
>
> Virtio Legacy devices can be probed with no kernel driver.
>
> > 2) if yes, Would you like to support for virtio legacy device?
>
> I am OK to remove legacy + RTE_KDRV_NONE case, but I think it needs an
> announcement and being done in a later release to let end-users using
> that configuration time to do the change.

OK

>
> > 3) if yes, Please fix RTE_PMD_REGISTER_KMOD_DEP(net_virtio, "* igb_uio
> > | uio_pci_generic | vfio-pci");
>
> While support gets removed, what about:
>
> RTE_PMD_REGISTER_KMOD_DEP(net_virtio, "* igb_uio | uio_pci_generic |
> vfio-pci | none"); ?

+1

>
> Maxime
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-dev] [PATCH] bus/pci: optimize pci device probe
  2020-05-06 10:51                     ` Jerin Jacob
@ 2020-05-06 11:37                       ` David Marchand
  2020-05-06 11:44                         ` Maxime Coquelin
  0 siblings, 1 reply; 15+ messages in thread
From: David Marchand @ 2020-05-06 11:37 UTC (permalink / raw)
  To: Maxime Coquelin, Jerin Jacob
  Cc: Thomas Monjalon, Jerin Jacob, dpdk-dev, Zhihong Wang, Ye,
	Xiaolong, Harman Kalra, Kevin Traynor, Luca Boccassi,
	Gaetan Rivet, Olivier Matz

On Wed, May 6, 2020 at 12:51 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > 3) if yes, Please fix RTE_PMD_REGISTER_KMOD_DEP(net_virtio, "* igb_uio
> > > | uio_pci_generic | vfio-pci");
> >
> > While support gets removed, what about:
> >
> > RTE_PMD_REGISTER_KMOD_DEP(net_virtio, "* igb_uio | uio_pci_generic |
> > vfio-pci | none"); ?
>
> +1

I know some scripts rely on such info to modprobe and bind the devices
based on such info.
So "none" does not make sense, there is no "none" kmod.

Since we want to deprecate this capability, let's not advertise this.


-- 
David Marchand


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-dev] [PATCH] bus/pci: optimize pci device probe
  2020-05-06 11:37                       ` David Marchand
@ 2020-05-06 11:44                         ` Maxime Coquelin
  0 siblings, 0 replies; 15+ messages in thread
From: Maxime Coquelin @ 2020-05-06 11:44 UTC (permalink / raw)
  To: David Marchand, Jerin Jacob
  Cc: Thomas Monjalon, Jerin Jacob, dpdk-dev, Zhihong Wang, Ye,
	Xiaolong, Harman Kalra, Kevin Traynor, Luca Boccassi,
	Gaetan Rivet, Olivier Matz



On 5/6/20 1:37 PM, David Marchand wrote:
> On Wed, May 6, 2020 at 12:51 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
>>>> 3) if yes, Please fix RTE_PMD_REGISTER_KMOD_DEP(net_virtio, "* igb_uio
>>>> | uio_pci_generic | vfio-pci");
>>>
>>> While support gets removed, what about:
>>>
>>> RTE_PMD_REGISTER_KMOD_DEP(net_virtio, "* igb_uio | uio_pci_generic |
>>> vfio-pci | none"); ?
>>
>> +1
> 
> I know some scripts rely on such info to modprobe and bind the devices
> based on such info.
> So "none" does not make sense, there is no "none" kmod.
> 
> Since we want to deprecate this capability, let's not advertise this.

Makes sense.

Jerin, will add this announcement in the release note? I think we can
remove it in v20.08.

Thanks,
Maxime


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2020-05-06 11:44 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-26 17:38 [dpdk-dev] [PATCH] bus/pci: optimize pci device probe jerinj
2020-04-26 18:08 ` Thomas Monjalon
2020-04-26 18:41   ` Jerin Jacob
2020-04-26 20:06     ` Thomas Monjalon
2020-04-27 17:59       ` Jerin Jacob
2020-04-28  8:50       ` David Marchand
2020-04-28  9:34         ` Jerin Jacob
2020-05-05 15:50           ` Jerin Jacob
2020-05-05 16:16             ` David Marchand
2020-05-06  6:34               ` Maxime Coquelin
2020-05-06  6:43                 ` Jerin Jacob
2020-05-06  7:52                   ` Maxime Coquelin
2020-05-06 10:51                     ` Jerin Jacob
2020-05-06 11:37                       ` David Marchand
2020-05-06 11:44                         ` Maxime Coquelin

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).