[dpdk-dev] [PATCH] qat: fix for VFs not getting recognized

[dpdk-dev] [PATCH] qat: fix for VFs not getting recognized

[Jain, Deepak K]

Due to addition of CLASS_ID in EAL, class_id is
amended into the code.

Fixes: 701c8d80c820 ("pci: support class id probing")

Signed-off-by: Deepak Kumar Jain <deepak.k.jain@intel.com>
---
 drivers/crypto/qat/rte_qat_cryptodev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/crypto/qat/rte_qat_cryptodev.c b/drivers/crypto/qat/rte_qat_cryptodev.c
index a7912f5..2bad201 100644
--- a/drivers/crypto/qat/rte_qat_cryptodev.c
+++ b/drivers/crypto/qat/rte_qat_cryptodev.c
@@ -72,7 +72,8 @@ static struct rte_pci_id pci_id_qat_map[] = {
 			.vendor_id = 0x8086,
 			.device_id = 0x0443,
 			.subsystem_vendor_id = PCI_ANY_ID,
-			.subsystem_device_id = PCI_ANY_ID
+			.subsystem_device_id = PCI_ANY_ID,
+			.class_id  = 0xB4000
 		},
 		{.device_id = 0},
 };
-- 
2.5.5

Re: [dpdk-dev] [PATCH] qat: fix for VFs not getting recognized

[Thomas Monjalon]

2016-06-16 16:29, Jain, Deepak K:
> Due to addition of CLASS_ID in EAL, class_id is
> amended into the code.

Why the VF is not recognized?
The class id should not be mandatory.

Re: [dpdk-dev] [PATCH] qat: fix for VFs not getting recognized

[Jain, Deepak K]

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Thursday, June 16, 2016 5:16 PM
> To: Jain, Deepak K <deepak.k.jain@intel.com>
> Cc: dev@dpdk.org; Griffin, John <john.griffin@intel.com>; De Lara Guarch,
> Pablo <pablo.de.lara.guarch@intel.com>; Doherty, Declan
> <declan.doherty@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] qat: fix for VFs not getting recognized
> 
> 2016-06-16 16:29, Jain, Deepak K:
> > Due to addition of CLASS_ID in EAL, class_id is amended into the code.
> 
> Why the VF is not recognized?
> The class id should not be mandatory.

Without the change proposed, QuickAssist Devices were not visible and hence tests were not running.
Seems like changes in EAL especially where class_id is added affected the QuickAssist tests.
With this change, QuickAssist devices are visible during tests and tests working fine.

Re: [dpdk-dev] [PATCH] qat: fix for VFs not getting recognized

[Thomas Monjalon]

2016-06-16 16:25, Jain, Deepak K:
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > 2016-06-16 16:29, Jain, Deepak K:
> > > Due to addition of CLASS_ID in EAL, class_id is amended into the code.
> > 
> > Why the VF is not recognized?
> > The class id should not be mandatory.
> 
> Without the change proposed, QuickAssist Devices were not visible and hence tests were not running.
> Seems like changes in EAL especially where class_id is added affected the QuickAssist tests.
> With this change, QuickAssist devices are visible during tests and tests working fine.

Which tests?
Have you investigated why?

Re: [dpdk-dev] [PATCH] qat: fix for VFs not getting recognized

[Jerin Jacob]

On Fri, Jun 17, 2016 at 10:18:30AM +0200, Thomas Monjalon wrote:
> 2016-06-16 16:25, Jain, Deepak K:
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > 2016-06-16 16:29, Jain, Deepak K:
> > > > Due to addition of CLASS_ID in EAL, class_id is amended into the code.
> > > 
> > > Why the VF is not recognized?
> > > The class id should not be mandatory.
> > 
> > Without the change proposed, QuickAssist Devices were not visible and hence tests were not running.
> > Seems like changes in EAL especially where class_id is added affected the QuickAssist tests.
> > With this change, QuickAssist devices are visible during tests and tests working fine.
> 
> Which tests?
> Have you investigated why?

Thunderx nicvf also got the same problem when I rebased to
dpdk-next-net/rel_16_07.

The root cause for this issue is that, PCI CLASS_ID EAL add
changeset(701c8d80c820461e8255dfb7387a09f0e54399f0)
has taken care only the pci devices where id table is created
with RTE_PCI_DEVICE

For other devices, class_id comes as 0 instread of RTE_CLASS_ANY_ID and
probe failes. To fix it,

one option is to add RTE_CLASS_ANY_ID for the devices where pci id table is not
created with RTE_PCI_DEVICE

or

somewhere in common-code in the initaization set if class_id = 0 then make it as
RTE_CLASS_ANY_ID(Thats would be a hack).

Seems like first option is correct-way to fix the problem? Any thoughts?

looks like following devices does not exhibit this issue

[dpdk-thunderx] $ grep -r "RTE_PCI_DEVICE" drivers/
drivers/net/szedata2/rte_eth_szedata2.c:
drivers/net/bnx2x/bnx2x_ethdev.c
drivers/net/vmxnet3/vmxnet3_ethdev.c
drivers/net/enic/enic_ethdev.c
drivers/net/e1000/em_ethdev.c
drivers/net/ena/ena_ethdev.c
drivers/net/qede/qede_ethdev.c
drivers/net/ixgbe/ixgbe_ethdev.c:#define RTE_PCI_DEV_ID_DECL_IXGBE(vend,
drivers/net/virtio/virtio_ethdev.c:#define
drivers/net/i40e/i40e_ethdev.c:vend,

Jerin

Re: [dpdk-dev] [PATCH] qat: fix for VFs not getting recognized

[Thomas Monjalon]

2016-06-17 15:12, Jerin Jacob:
> Thunderx nicvf also got the same problem when I rebased to
> dpdk-next-net/rel_16_07.
> 
> The root cause for this issue is that, PCI CLASS_ID EAL add
> changeset(701c8d80c820461e8255dfb7387a09f0e54399f0)
> has taken care only the pci devices where id table is created
> with RTE_PCI_DEVICE
> 
> For other devices, class_id comes as 0 instread of RTE_CLASS_ANY_ID and
> probe failes. To fix it,
> 
> one option is to add RTE_CLASS_ANY_ID for the devices where pci id table is not
> created with RTE_PCI_DEVICE
> 
> or
> 
> somewhere in common-code in the initaization set if class_id = 0 then make it as
> RTE_CLASS_ANY_ID(Thats would be a hack).
> 
> Seems like first option is correct-way to fix the problem? Any thoughts?

The best fix is to use RTE_PCI_DEVICE.
If we want to set the class id, we can add a new macro
	RTE_PCI_ID(class, vend, dev)
or maybe
	RTE_PCI_NET_ID(vend, dev) and RTE_PCI_CRYPTO_ID(vend, dev)

> looks like following devices does not exhibit this issue
> 
> [dpdk-thunderx] $ grep -r "RTE_PCI_DEVICE" drivers/
> drivers/net/szedata2/rte_eth_szedata2.c:
> drivers/net/bnx2x/bnx2x_ethdev.c
> drivers/net/vmxnet3/vmxnet3_ethdev.c
> drivers/net/enic/enic_ethdev.c
> drivers/net/e1000/em_ethdev.c
> drivers/net/ena/ena_ethdev.c
> drivers/net/qede/qede_ethdev.c
> drivers/net/ixgbe/ixgbe_ethdev.c:#define RTE_PCI_DEV_ID_DECL_IXGBE(vend,
> drivers/net/virtio/virtio_ethdev.c:#define
> drivers/net/i40e/i40e_ethdev.c:vend,

Going further:

pci=$(git grep -l rte_pci drivers | cut -d/ -f3 | sort -u)
ok=$(git grep -l RTE_PCI_DEVICE drivers | cut -d/ -f3 | sort -u)
printf "$pci\n$ok\n$ok" | sort | uniq -u | sed '/^$/d'

bonding
mlx4
mlx5
nfp
qat

It seems we need to fix qat, nfp and mlx.

Re: [dpdk-dev] [PATCH] qat: fix for VFs not getting recognized

[Jain, Deepak K]

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Friday, June 17, 2016 9:19 AM
> To: Jain, Deepak K <deepak.k.jain@intel.com>
> Cc: dev@dpdk.org; Griffin, John <john.griffin@intel.com>; De Lara Guarch,
> Pablo <pablo.de.lara.guarch@intel.com>; Doherty, Declan
> <declan.doherty@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] qat: fix for VFs not getting recognized
> 
> 2016-06-16 16:25, Jain, Deepak K:
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > 2016-06-16 16:29, Jain, Deepak K:
> > > > Due to addition of CLASS_ID in EAL, class_id is amended into the code.
> > >
> > > Why the VF is not recognized?
> > > The class id should not be mandatory.
> >
> > Without the change proposed, QuickAssist Devices were not visible and
> hence tests were not running.
> > Seems like changes in EAL especially where class_id is added affected the
> QuickAssist tests.
> > With this change, QuickAssist devices are visible during tests and tests
> working fine.
> 
> Which tests?
> Have you investigated why?

Hi Thomas,

On investigation, I found that when class_id is not set in the rte_qat_cryptodev.c, the value of id_table->class_id defaults to 0.

Hence the following code snippet always executes and the probing of driver is never done.
if (id_table->class_id != dev->id.class_id &&
                                id_table->class_id != RTE_CLASS_ANY_ID
		continue;	

If value of id_table->class_id is set, as shown in patch which was submitted, the id_table->class_ID is set to RTE_CLASS_ANY_ID and hence its probes the driver and fixes the issues.

Other fix would be to set default value of class_id equal to RTE_CLASS_ANY_ID instead of 0.