On Thu, Jan 14, 2021 at 5:04 PM Maxime Coquelin
[off-list ref] wrote:

On 1/5/21 10:19 PM, David Marchand wrote:

quoted

quoted

 static bool
 rx_offload_enabled(struct virtio_hw *hw)

@@ -2521,7 +2419,7 @@ static void virtio_dev_free_mbufs(struct rte_eth_dev *dev)
 /*
  * Stop device: disable interrupt and mark link down
  */
-static int
+int
 virtio_dev_stop(struct rte_eth_dev *dev)
 {
        struct virtio_hw *hw = dev->data->dev_private;

@@ -2673,7 +2571,5 @@ __rte_unused uint8_t is_rx)
 }

 RTE_PMD_EXPORT_NAME(net_virtio, __COUNTER__);

This belongs with the rest of the pci driver declarations.

$ ./usertools/dpdk-pmdinfo.py
$HOME/builds/build-gcc-shared/drivers/librte_net_virtio.so
PMD NAME: net_virtio_user
PMD PARAMETERS: path=<path> mac=<mac addr> cq=<int> queue_size=<int>
queues=<int> iface=<string> server=<0|1> mrg_rxbuf=<0|1>
in_order=<0|1> packed_vq=<0|1> speed=<int> vectorized=<0|1>

PMD NAME: net_virtio

^^^
Here, I would expect the pci table and the kmod dependencies.


This makes me realise that the net_virtio_user driver is not exported
(but it seems to work without it.. interesting).

As discussed offlist, I have moved RTE_PMD_EXPORT_NAME to
virtio_pci_ethdev.c, and I now get expected output:

Some details, if someone is interested:

In the pci driver case, we can't rely on the pci bus register macro
because of the custom constructor, hence the manual call to
RTE_PMD_EXPORT_NAME.
For the virtio_user driver, the vdev bus register macro actually
exports the driver and this is why it works fine.

$ ./usertools/dpdk-pmdinfo.py build/drivers/librte_net_virtio.so
PMD NAME: net_virtio_user
PMD PARAMETERS: path=<path> mac=<mac addr> cq=<int> queue_size=<int>
queues=<int> iface=<string> server=<0|1> mrg_rxbuf=<0|1> in_order=<0|1>
packed_vq=<0|1> speed=<int> vectorized=<0|1>

PMD NAME: net_virtio
PMD KMOD DEPENDENCIES: * igb_uio | uio_pci_generic | vfio-pci
PMD HW SUPPORT:
 Red Hat, Inc. (1af4) : Virtio network device (1000) (All Subdevices)
 Red Hat, Inc. (1af4) : Virtio network device (1041) (All Subdevices)

[snip]

quoted

quoted

+static int eth_virtio_pci_remove(struct rte_pci_device *pci_dev)
+{
+       int ret;
+
+       ret = rte_eth_dev_pci_generic_remove(pci_dev, eth_virtio_pci_uninit);
+       /* Port has already been released by close. */
+       if (ret == -ENODEV)
+               ret = 0;
+       return ret;
+}
+
+static struct rte_pci_driver rte_virtio_pmd = {

This seems too generic.

virtio_pci_pmd ?

virtio_net_pci_pmd is even better.

quoted

quoted

+       .driver = {
+               .name = "net_virtio",

I keept it as is here not to break tools relying on driver name.
Makes sense?

Yes.

quoted

quoted

+       },
+       .id_table = pci_id_virtio_map,
+       .drv_flags = 0,
+       .probe = eth_virtio_pci_probe,
+       .remove = eth_virtio_pci_remove,
+};
+
+RTE_INIT(rte_virtio_pmd_init)

virtio_pci_pmd_init ?

rte_virtio_net_pci_pmd_init

Ok for me.
Thanks Maxime.


-- 
David Marchand