Re: [dpdk-dev] [PATCH 05/40] net/virtio: move PCI device init in dedicated file

[David Marchand <david.marchand@redhat.com>]

On Thu, Jan 14, 2021 at 5:04 PM Maxime Coquelin
<maxime.coquelin@redhat.com> wrote:
> On 1/5/21 10:19 PM, David Marchand wrote:
> >>  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]

> >> +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.
>
> >
> >> +       .driver = {
> >> +               .name = "net_virtio",
>
> I keept it as is here not to break tools relying on driver name.
> Makes sense?

Yes.


>
> >> +       },
> >> +       .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