| Message ID | 20201220211405.313012-6-maxime.coquelin@redhat.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Maxime Coquelin |
| Headers | show |
| Series | net/virtio: Virtio PMD rework | expand |
| Context | Check | Description |
|---|---|---|
| ci/checkpatch | warning | coding style issues |
Hi Maxime, > -----Original Message----- > From: Maxime Coquelin <maxime.coquelin@redhat.com> > Sent: Monday, December 21, 2020 5:14 AM > To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; olivier.matz@6wind.com; > amorenoz@redhat.com; david.marchand@redhat.com > Cc: Maxime Coquelin <maxime.coquelin@redhat.com> > Subject: [PATCH 05/40] net/virtio: move PCI device init in dedicated file > > Thi patch moves the PCI Ethernet device registration bits s/Thi/This > in a dedicated patch. In following patches, more code will > be moved there, with the goal of making virtio_ethdev.c > truely bus-agnostic. s/truely/truly With above fixes: Reviewed-by: Chenbo Xia <chenbo.xia@intel.com> > > Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com> > --- > drivers/net/virtio/meson.build | 1 + > drivers/net/virtio/virtio_ethdev.c | 114 +------------------ > drivers/net/virtio/virtio_ethdev.h | 2 + > drivers/net/virtio/virtio_pci_ethdev.c | 148 +++++++++++++++++++++++++ > 4 files changed, 156 insertions(+), 109 deletions(-) > create mode 100644 drivers/net/virtio/virtio_pci_ethdev.c > > diff --git a/drivers/net/virtio/meson.build b/drivers/net/virtio/meson.build > index eaed46373d..8e0f1a9951 100644 > --- a/drivers/net/virtio/meson.build > +++ b/drivers/net/virtio/meson.build > @@ -2,6 +2,7 @@ > # Copyright(c) 2018 Intel Corporation > > sources += files('virtio_ethdev.c', > + 'virtio_pci_ethdev.c', > 'virtio_pci.c', > 'virtio_rxtx.c', > 'virtio_rxtx_simple.c', > diff --git a/drivers/net/virtio/virtio_ethdev.c > b/drivers/net/virtio/virtio_ethdev.c > index 99a5a1bb88..ca21a019e5 100644 > --- a/drivers/net/virtio/virtio_ethdev.c > +++ b/drivers/net/virtio/virtio_ethdev.c > @@ -38,17 +38,14 @@ > #include "virtio_rxtx.h" > #include "virtio_user/virtio_user_dev.h" > > -static int eth_virtio_dev_uninit(struct rte_eth_dev *eth_dev); > static int virtio_dev_configure(struct rte_eth_dev *dev); > static int virtio_dev_start(struct rte_eth_dev *dev); > -static int virtio_dev_stop(struct rte_eth_dev *dev); > static int virtio_dev_promiscuous_enable(struct rte_eth_dev *dev); > static int virtio_dev_promiscuous_disable(struct rte_eth_dev *dev); > static int virtio_dev_allmulticast_enable(struct rte_eth_dev *dev); > static int virtio_dev_allmulticast_disable(struct rte_eth_dev *dev); > static uint32_t virtio_dev_speed_capa_get(uint32_t speed); > static int virtio_dev_devargs_parse(struct rte_devargs *devargs, > - int *vdpa, > uint32_t *speed, > int *vectorized); > static int virtio_dev_info_get(struct rte_eth_dev *dev, > @@ -89,15 +86,6 @@ static int virtio_dev_queue_stats_mapping_set( > static void virtio_notify_peers(struct rte_eth_dev *dev); > static void virtio_ack_link_announce(struct rte_eth_dev *dev); > > -/* > - * The set of PCI devices this driver supports > - */ > -static const struct rte_pci_id pci_id_virtio_map[] = { > - { RTE_PCI_DEVICE(VIRTIO_PCI_VENDORID, VIRTIO_PCI_LEGACY_DEVICEID_NET) }, > - { RTE_PCI_DEVICE(VIRTIO_PCI_VENDORID, VIRTIO_PCI_MODERN_DEVICEID_NET) }, > - { .vendor_id = 0, /* sentinel */ }, > -}; > - > struct rte_virtio_xstats_name_off { > char name[RTE_ETH_XSTATS_NAME_SIZE]; > unsigned offset; > @@ -714,7 +702,7 @@ virtio_alloc_queues(struct rte_eth_dev *dev) > > static void virtio_queues_unbind_intr(struct rte_eth_dev *dev); > > -static int > +int > virtio_dev_close(struct rte_eth_dev *dev) > { > struct virtio_hw *hw = dev->data->dev_private; > @@ -1929,8 +1917,7 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev) > > return 0; > } > - ret = virtio_dev_devargs_parse(eth_dev->device->devargs, > - NULL, &speed, &vectorized); > + ret = virtio_dev_devargs_parse(eth_dev->device->devargs, &speed, > &vectorized); > if (ret < 0) > return ret; > hw->speed = speed; > @@ -1992,36 +1979,6 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev) > return ret; > } > > -static int > -eth_virtio_dev_uninit(struct rte_eth_dev *eth_dev) > -{ > - int ret; > - PMD_INIT_FUNC_TRACE(); > - > - if (rte_eal_process_type() == RTE_PROC_SECONDARY) > - return 0; > - > - ret = virtio_dev_stop(eth_dev); > - virtio_dev_close(eth_dev); > - > - PMD_INIT_LOG(DEBUG, "dev_uninit completed"); > - > - return ret; > -} > - > - > -static int vdpa_check_handler(__rte_unused const char *key, > - const char *value, void *ret_val) > -{ > - if (strcmp(value, "1") == 0) > - *(int *)ret_val = 1; > - else > - *(int *)ret_val = 0; > - > - return 0; > -} > - > - > static uint32_t > virtio_dev_speed_capa_get(uint32_t speed) > { > @@ -2059,10 +2016,8 @@ static int vectorized_check_handler(__rte_unused const > char *key, > } > > #define VIRTIO_ARG_SPEED "speed" > -#define VIRTIO_ARG_VDPA "vdpa" > #define VIRTIO_ARG_VECTORIZED "vectorized" > > - > static int > link_speed_handler(const char *key __rte_unused, > const char *value, void *ret_val) > @@ -2081,8 +2036,7 @@ link_speed_handler(const char *key __rte_unused, > > > static int > -virtio_dev_devargs_parse(struct rte_devargs *devargs, int *vdpa, > - uint32_t *speed, int *vectorized) > +virtio_dev_devargs_parse(struct rte_devargs *devargs, uint32_t *speed, int > *vectorized) > { > struct rte_kvargs *kvlist; > int ret = 0; > @@ -2095,18 +2049,7 @@ virtio_dev_devargs_parse(struct rte_devargs *devargs, > int *vdpa, > PMD_INIT_LOG(ERR, "error when parsing param"); > return 0; > } > - if (vdpa && rte_kvargs_count(kvlist, VIRTIO_ARG_VDPA) == 1) { > - /* vdpa mode selected when there's a key-value pair: > - * vdpa=1 > - */ > - ret = rte_kvargs_process(kvlist, VIRTIO_ARG_VDPA, > - vdpa_check_handler, vdpa); > - if (ret < 0) { > - PMD_INIT_LOG(ERR, "Failed to parse %s", > - VIRTIO_ARG_VDPA); > - goto exit; > - } > - } > + > if (speed && rte_kvargs_count(kvlist, VIRTIO_ARG_SPEED) == 1) { > ret = rte_kvargs_process(kvlist, > VIRTIO_ARG_SPEED, > @@ -2135,52 +2078,7 @@ virtio_dev_devargs_parse(struct rte_devargs *devargs, > int *vdpa, > return ret; > } > > -static int eth_virtio_pci_probe(struct rte_pci_driver *pci_drv __rte_unused, > - struct rte_pci_device *pci_dev) > -{ > - int vdpa = 0; > - int ret = 0; > - > - ret = virtio_dev_devargs_parse(pci_dev->device.devargs, &vdpa, NULL, > - NULL); > - if (ret < 0) { > - PMD_INIT_LOG(ERR, "devargs parsing is failed"); > - return ret; > - } > - /* virtio pmd skips probe if device needs to work in vdpa mode */ > - if (vdpa == 1) > - return 1; > - > - return rte_eth_dev_pci_generic_probe(pci_dev, sizeof(struct > virtio_pci_dev), > - eth_virtio_dev_init); > -} > - > -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_dev_uninit); > - /* Port has already been released by close. */ > - if (ret == -ENODEV) > - ret = 0; > - return ret; > -} > - > -static struct rte_pci_driver rte_virtio_pmd = { > - .driver = { > - .name = "net_virtio", > - }, > - .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) > -{ > - rte_eal_iopl_init(); > - rte_pci_register(&rte_virtio_pmd); > -} > > 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__); > -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"); > RTE_LOG_REGISTER(virtio_logtype_init, pmd.net.virtio.init, NOTICE); > RTE_LOG_REGISTER(virtio_logtype_driver, pmd.net.virtio.driver, NOTICE); > diff --git a/drivers/net/virtio/virtio_ethdev.h > b/drivers/net/virtio/virtio_ethdev.h > index b7d52d497f..13395937c8 100644 > --- a/drivers/net/virtio/virtio_ethdev.h > +++ b/drivers/net/virtio/virtio_ethdev.h > @@ -117,6 +117,8 @@ void virtio_interrupt_handler(void *param); > > int virtio_dev_pause(struct rte_eth_dev *dev); > void virtio_dev_resume(struct rte_eth_dev *dev); > +int virtio_dev_stop(struct rte_eth_dev *dev); > +int virtio_dev_close(struct rte_eth_dev *dev); > int virtio_inject_pkts(struct rte_eth_dev *dev, struct rte_mbuf **tx_pkts, > int nb_pkts); > > diff --git a/drivers/net/virtio/virtio_pci_ethdev.c > b/drivers/net/virtio/virtio_pci_ethdev.c > new file mode 100644 > index 0000000000..9c0935068e > --- /dev/null > +++ b/drivers/net/virtio/virtio_pci_ethdev.c > @@ -0,0 +1,148 @@ > +/* SPDX-License-Identifier: BSD-3-Clause > + * Copyright(c) 2010-2016 Intel Corporation > + */ > + > +#include <stdint.h> > +#include <string.h> > +#include <stdio.h> > +#include <errno.h> > +#include <unistd.h> > + > +#include <rte_ethdev_driver.h> > +#include <rte_ethdev_pci.h> > +#include <rte_pci.h> > +#include <rte_bus_pci.h> > +#include <rte_errno.h> > + > +#include <rte_memory.h> > +#include <rte_eal.h> > +#include <rte_dev.h> > +#include <rte_kvargs.h> > + > +#include "virtio_ethdev.h" > +#include "virtio_pci.h" > +#include "virtio_logs.h" > + > +/* > + * The set of PCI devices this driver supports > + */ > +static const struct rte_pci_id pci_id_virtio_map[] = { > + { RTE_PCI_DEVICE(VIRTIO_PCI_VENDORID, VIRTIO_PCI_LEGACY_DEVICEID_NET) }, > + { RTE_PCI_DEVICE(VIRTIO_PCI_VENDORID, VIRTIO_PCI_MODERN_DEVICEID_NET) }, > + { .vendor_id = 0, /* sentinel */ }, > +}; > + > +static int > +eth_virtio_pci_init(struct rte_eth_dev *eth_dev) > +{ > + return eth_virtio_dev_init(eth_dev); > +} > + > +static int > +eth_virtio_pci_uninit(struct rte_eth_dev *eth_dev) > +{ > + int ret; > + PMD_INIT_FUNC_TRACE(); > + > + if (rte_eal_process_type() == RTE_PROC_SECONDARY) > + return 0; > + > + ret = virtio_dev_stop(eth_dev); > + virtio_dev_close(eth_dev); > + > + PMD_INIT_LOG(DEBUG, "dev_uninit completed"); > + > + return ret; > +} > + > +static int vdpa_check_handler(__rte_unused const char *key, > + const char *value, void *ret_val) > +{ > + if (strcmp(value, "1") == 0) > + *(int *)ret_val = 1; > + else > + *(int *)ret_val = 0; > + > + return 0; > +} > + > +#define VIRTIO_ARG_VDPA "vdpa" > + > +static int > +virtio_pci_devargs_parse(struct rte_devargs *devargs, int *vdpa) > +{ > + struct rte_kvargs *kvlist; > + int ret = 0; > + > + if (devargs == NULL) > + return 0; > + > + kvlist = rte_kvargs_parse(devargs->args, NULL); > + if (kvlist == NULL) { > + PMD_INIT_LOG(ERR, "error when parsing param"); > + return 0; > + } > + > + if (vdpa && rte_kvargs_count(kvlist, VIRTIO_ARG_VDPA) == 1) { > + /* vdpa mode selected when there's a key-value pair: > + * vdpa=1 > + */ > + ret = rte_kvargs_process(kvlist, VIRTIO_ARG_VDPA, > + vdpa_check_handler, vdpa); > + if (ret < 0) > + PMD_INIT_LOG(ERR, "Failed to parse %s", VIRTIO_ARG_VDPA); > + } > + > + rte_kvargs_free(kvlist); > + > + return ret; > +} > + > +static int eth_virtio_pci_probe(struct rte_pci_driver *pci_drv __rte_unused, > + struct rte_pci_device *pci_dev) > +{ > + int vdpa = 0; > + int ret = 0; > + > + ret = virtio_pci_devargs_parse(pci_dev->device.devargs, &vdpa); > + if (ret < 0) { > + PMD_INIT_LOG(ERR, "devargs parsing is failed"); > + return ret; > + } > + /* virtio pmd skips probe if device needs to work in vdpa mode */ > + if (vdpa == 1) > + return 1; > + > + return rte_eth_dev_pci_generic_probe(pci_dev, sizeof(struct > virtio_pci_dev), > + eth_virtio_pci_init); > +} > + > +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 = { > + .driver = { > + .name = "net_virtio", > + }, > + .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) > +{ > + rte_eal_iopl_init(); > + rte_pci_register(&rte_virtio_pmd); > +} > + > +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"); > -- > 2.29.2
On Sun, Dec 20, 2020 at 10:14 PM Maxime Coquelin <maxime.coquelin@redhat.com> wrote: > diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c > index 99a5a1bb88..ca21a019e5 100644 > --- a/drivers/net/virtio/virtio_ethdev.c > +++ b/drivers/net/virtio/virtio_ethdev.c [...] > @@ -2135,52 +2078,7 @@ virtio_dev_devargs_parse(struct rte_devargs *devargs, int *vdpa, > return ret; > } > > -static int eth_virtio_pci_probe(struct rte_pci_driver *pci_drv __rte_unused, > - struct rte_pci_device *pci_dev) > -{ > - int vdpa = 0; > - int ret = 0; > - > - ret = virtio_dev_devargs_parse(pci_dev->device.devargs, &vdpa, NULL, > - NULL); > - if (ret < 0) { > - PMD_INIT_LOG(ERR, "devargs parsing is failed"); > - return ret; > - } > - /* virtio pmd skips probe if device needs to work in vdpa mode */ > - if (vdpa == 1) > - return 1; > - > - return rte_eth_dev_pci_generic_probe(pci_dev, sizeof(struct virtio_pci_dev), > - eth_virtio_dev_init); > -} > - > -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_dev_uninit); > - /* Port has already been released by close. */ > - if (ret == -ENODEV) > - ret = 0; > - return ret; > -} > - > -static struct rte_pci_driver rte_virtio_pmd = { > - .driver = { > - .name = "net_virtio", > - }, > - .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) > -{ > - rte_eal_iopl_init(); > - rte_pci_register(&rte_virtio_pmd); > -} > Leaving three empty lines if I count correctly. > 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). > -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"); > RTE_LOG_REGISTER(virtio_logtype_init, pmd.net.virtio.init, NOTICE); > RTE_LOG_REGISTER(virtio_logtype_driver, pmd.net.virtio.driver, NOTICE); > diff --git a/drivers/net/virtio/virtio_ethdev.h b/drivers/net/virtio/virtio_ethdev.h > index b7d52d497f..13395937c8 100644 > --- a/drivers/net/virtio/virtio_ethdev.h > +++ b/drivers/net/virtio/virtio_ethdev.h > @@ -117,6 +117,8 @@ void virtio_interrupt_handler(void *param); > > int virtio_dev_pause(struct rte_eth_dev *dev); > void virtio_dev_resume(struct rte_eth_dev *dev); > +int virtio_dev_stop(struct rte_eth_dev *dev); > +int virtio_dev_close(struct rte_eth_dev *dev); > int virtio_inject_pkts(struct rte_eth_dev *dev, struct rte_mbuf **tx_pkts, > int nb_pkts); > > diff --git a/drivers/net/virtio/virtio_pci_ethdev.c b/drivers/net/virtio/virtio_pci_ethdev.c > new file mode 100644 > index 0000000000..9c0935068e > --- /dev/null > +++ b/drivers/net/virtio/virtio_pci_ethdev.c > @@ -0,0 +1,148 @@ > +/* SPDX-License-Identifier: BSD-3-Clause > + * Copyright(c) 2010-2016 Intel Corporation > + */ > + > +#include <stdint.h> > +#include <string.h> > +#include <stdio.h> > +#include <errno.h> > +#include <unistd.h> > + > +#include <rte_ethdev_driver.h> > +#include <rte_ethdev_pci.h> > +#include <rte_pci.h> > +#include <rte_bus_pci.h> > +#include <rte_errno.h> > + > +#include <rte_memory.h> > +#include <rte_eal.h> > +#include <rte_dev.h> > +#include <rte_kvargs.h> > + > +#include "virtio_ethdev.h" > +#include "virtio_pci.h" > +#include "virtio_logs.h" > + > +/* > + * The set of PCI devices this driver supports > + */ > +static const struct rte_pci_id pci_id_virtio_map[] = { > + { RTE_PCI_DEVICE(VIRTIO_PCI_VENDORID, VIRTIO_PCI_LEGACY_DEVICEID_NET) }, > + { RTE_PCI_DEVICE(VIRTIO_PCI_VENDORID, VIRTIO_PCI_MODERN_DEVICEID_NET) }, > + { .vendor_id = 0, /* sentinel */ }, > +}; > + > +static int > +eth_virtio_pci_init(struct rte_eth_dev *eth_dev) > +{ > + return eth_virtio_dev_init(eth_dev); > +} > + > +static int > +eth_virtio_pci_uninit(struct rte_eth_dev *eth_dev) > +{ > + int ret; > + PMD_INIT_FUNC_TRACE(); > + > + if (rte_eal_process_type() == RTE_PROC_SECONDARY) > + return 0; > + > + ret = virtio_dev_stop(eth_dev); > + virtio_dev_close(eth_dev); > + > + PMD_INIT_LOG(DEBUG, "dev_uninit completed"); > + > + return ret; > +} > + > +static int vdpa_check_handler(__rte_unused const char *key, > + const char *value, void *ret_val) > +{ > + if (strcmp(value, "1") == 0) > + *(int *)ret_val = 1; > + else > + *(int *)ret_val = 0; > + > + return 0; > +} > + > +#define VIRTIO_ARG_VDPA "vdpa" > + > +static int > +virtio_pci_devargs_parse(struct rte_devargs *devargs, int *vdpa) > +{ > + struct rte_kvargs *kvlist; > + int ret = 0; > + > + if (devargs == NULL) > + return 0; > + > + kvlist = rte_kvargs_parse(devargs->args, NULL); > + if (kvlist == NULL) { > + PMD_INIT_LOG(ERR, "error when parsing param"); > + return 0; > + } > + > + if (vdpa && rte_kvargs_count(kvlist, VIRTIO_ARG_VDPA) == 1) { I would remove this check on vdpa pointer being NULL, as it gets passed on the stack by a single caller in this file. > + /* vdpa mode selected when there's a key-value pair: > + * vdpa=1 > + */ > + ret = rte_kvargs_process(kvlist, VIRTIO_ARG_VDPA, > + vdpa_check_handler, vdpa); > + if (ret < 0) > + PMD_INIT_LOG(ERR, "Failed to parse %s", VIRTIO_ARG_VDPA); > + } > + > + rte_kvargs_free(kvlist); > + > + return ret; > +} > + > +static int eth_virtio_pci_probe(struct rte_pci_driver *pci_drv __rte_unused, > + struct rte_pci_device *pci_dev) > +{ > + int vdpa = 0; > + int ret = 0; > + > + ret = virtio_pci_devargs_parse(pci_dev->device.devargs, &vdpa); > + if (ret < 0) { > + PMD_INIT_LOG(ERR, "devargs parsing is failed"); > + return ret; > + } > + /* virtio pmd skips probe if device needs to work in vdpa mode */ > + if (vdpa == 1) > + return 1; > + > + return rte_eth_dev_pci_generic_probe(pci_dev, sizeof(struct virtio_pci_dev), > + eth_virtio_pci_init); > +} > + > +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 ? > + .driver = { > + .name = "net_virtio", > + }, > + .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_eal_iopl_init(); > + rte_pci_register(&rte_virtio_pmd); > +} > + > +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"); -- David Marchand
On 1/5/21 10:19 PM, David Marchand wrote: > On Sun, Dec 20, 2020 at 10:14 PM Maxime Coquelin > <maxime.coquelin@redhat.com> wrote: >> diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c >> index 99a5a1bb88..ca21a019e5 100644 >> --- a/drivers/net/virtio/virtio_ethdev.c >> +++ b/drivers/net/virtio/virtio_ethdev.c > > [...] > >> @@ -2135,52 +2078,7 @@ virtio_dev_devargs_parse(struct rte_devargs *devargs, int *vdpa, >> return ret; >> } >> >> -static int eth_virtio_pci_probe(struct rte_pci_driver *pci_drv __rte_unused, >> - struct rte_pci_device *pci_dev) >> -{ >> - int vdpa = 0; >> - int ret = 0; >> - >> - ret = virtio_dev_devargs_parse(pci_dev->device.devargs, &vdpa, NULL, >> - NULL); >> - if (ret < 0) { >> - PMD_INIT_LOG(ERR, "devargs parsing is failed"); >> - return ret; >> - } >> - /* virtio pmd skips probe if device needs to work in vdpa mode */ >> - if (vdpa == 1) >> - return 1; >> - >> - return rte_eth_dev_pci_generic_probe(pci_dev, sizeof(struct virtio_pci_dev), >> - eth_virtio_dev_init); >> -} >> - >> -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_dev_uninit); >> - /* Port has already been released by close. */ >> - if (ret == -ENODEV) >> - ret = 0; >> - return ret; >> -} >> - >> -static struct rte_pci_driver rte_virtio_pmd = { >> - .driver = { >> - .name = "net_virtio", >> - }, >> - .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) >> -{ >> - rte_eal_iopl_init(); >> - rte_pci_register(&rte_virtio_pmd); >> -} >> > > Leaving three empty lines if I count correctly. > > >> 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: $ ./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) > >> -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"); >> RTE_LOG_REGISTER(virtio_logtype_init, pmd.net.virtio.init, NOTICE); >> RTE_LOG_REGISTER(virtio_logtype_driver, pmd.net.virtio.driver, NOTICE); >> diff --git a/drivers/net/virtio/virtio_ethdev.h b/drivers/net/virtio/virtio_ethdev.h >> index b7d52d497f..13395937c8 100644 >> --- a/drivers/net/virtio/virtio_ethdev.h >> +++ b/drivers/net/virtio/virtio_ethdev.h >> @@ -117,6 +117,8 @@ void virtio_interrupt_handler(void *param); >> >> int virtio_dev_pause(struct rte_eth_dev *dev); >> void virtio_dev_resume(struct rte_eth_dev *dev); >> +int virtio_dev_stop(struct rte_eth_dev *dev); >> +int virtio_dev_close(struct rte_eth_dev *dev); >> int virtio_inject_pkts(struct rte_eth_dev *dev, struct rte_mbuf **tx_pkts, >> int nb_pkts); >> >> diff --git a/drivers/net/virtio/virtio_pci_ethdev.c b/drivers/net/virtio/virtio_pci_ethdev.c >> new file mode 100644 >> index 0000000000..9c0935068e >> --- /dev/null >> +++ b/drivers/net/virtio/virtio_pci_ethdev.c >> @@ -0,0 +1,148 @@ >> +/* SPDX-License-Identifier: BSD-3-Clause >> + * Copyright(c) 2010-2016 Intel Corporation >> + */ >> + >> +#include <stdint.h> >> +#include <string.h> >> +#include <stdio.h> >> +#include <errno.h> >> +#include <unistd.h> >> + >> +#include <rte_ethdev_driver.h> >> +#include <rte_ethdev_pci.h> >> +#include <rte_pci.h> >> +#include <rte_bus_pci.h> >> +#include <rte_errno.h> >> + >> +#include <rte_memory.h> >> +#include <rte_eal.h> >> +#include <rte_dev.h> >> +#include <rte_kvargs.h> >> + >> +#include "virtio_ethdev.h" >> +#include "virtio_pci.h" >> +#include "virtio_logs.h" >> + >> +/* >> + * The set of PCI devices this driver supports >> + */ >> +static const struct rte_pci_id pci_id_virtio_map[] = { >> + { RTE_PCI_DEVICE(VIRTIO_PCI_VENDORID, VIRTIO_PCI_LEGACY_DEVICEID_NET) }, >> + { RTE_PCI_DEVICE(VIRTIO_PCI_VENDORID, VIRTIO_PCI_MODERN_DEVICEID_NET) }, >> + { .vendor_id = 0, /* sentinel */ }, >> +}; >> + >> +static int >> +eth_virtio_pci_init(struct rte_eth_dev *eth_dev) >> +{ >> + return eth_virtio_dev_init(eth_dev); >> +} >> + >> +static int >> +eth_virtio_pci_uninit(struct rte_eth_dev *eth_dev) >> +{ >> + int ret; >> + PMD_INIT_FUNC_TRACE(); >> + >> + if (rte_eal_process_type() == RTE_PROC_SECONDARY) >> + return 0; >> + >> + ret = virtio_dev_stop(eth_dev); >> + virtio_dev_close(eth_dev); >> + >> + PMD_INIT_LOG(DEBUG, "dev_uninit completed"); >> + >> + return ret; >> +} >> + >> +static int vdpa_check_handler(__rte_unused const char *key, >> + const char *value, void *ret_val) >> +{ >> + if (strcmp(value, "1") == 0) >> + *(int *)ret_val = 1; >> + else >> + *(int *)ret_val = 0; >> + >> + return 0; >> +} >> + >> +#define VIRTIO_ARG_VDPA "vdpa" >> + >> +static int >> +virtio_pci_devargs_parse(struct rte_devargs *devargs, int *vdpa) >> +{ >> + struct rte_kvargs *kvlist; >> + int ret = 0; >> + >> + if (devargs == NULL) >> + return 0; >> + >> + kvlist = rte_kvargs_parse(devargs->args, NULL); >> + if (kvlist == NULL) { >> + PMD_INIT_LOG(ERR, "error when parsing param"); >> + return 0; >> + } >> + >> + if (vdpa && rte_kvargs_count(kvlist, VIRTIO_ARG_VDPA) == 1) { > > I would remove this check on vdpa pointer being NULL, as it gets > passed on the stack by a single caller in this file. I agree this is not necessary. > >> + /* vdpa mode selected when there's a key-value pair: >> + * vdpa=1 >> + */ >> + ret = rte_kvargs_process(kvlist, VIRTIO_ARG_VDPA, >> + vdpa_check_handler, vdpa); >> + if (ret < 0) >> + PMD_INIT_LOG(ERR, "Failed to parse %s", VIRTIO_ARG_VDPA); >> + } >> + >> + rte_kvargs_free(kvlist); >> + >> + return ret; >> +} >> + >> +static int eth_virtio_pci_probe(struct rte_pci_driver *pci_drv __rte_unused, >> + struct rte_pci_device *pci_dev) >> +{ >> + int vdpa = 0; >> + int ret = 0; >> + >> + ret = virtio_pci_devargs_parse(pci_dev->device.devargs, &vdpa); >> + if (ret < 0) { >> + PMD_INIT_LOG(ERR, "devargs parsing is failed"); >> + return ret; >> + } >> + /* virtio pmd skips probe if device needs to work in vdpa mode */ >> + if (vdpa == 1) >> + return 1; >> + >> + return rte_eth_dev_pci_generic_probe(pci_dev, sizeof(struct virtio_pci_dev), >> + eth_virtio_pci_init); >> +} >> + >> +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? >> + }, >> + .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 > >> +{ >> + rte_eal_iopl_init(); >> + rte_pci_register(&rte_virtio_pmd); >> +} >> + >> +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"); > > > -- > David Marchand >
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.
diff --git a/drivers/net/virtio/meson.build b/drivers/net/virtio/meson.build index eaed46373d..8e0f1a9951 100644 --- a/drivers/net/virtio/meson.build +++ b/drivers/net/virtio/meson.build @@ -2,6 +2,7 @@ # Copyright(c) 2018 Intel Corporation sources += files('virtio_ethdev.c', + 'virtio_pci_ethdev.c', 'virtio_pci.c', 'virtio_rxtx.c', 'virtio_rxtx_simple.c', diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c index 99a5a1bb88..ca21a019e5 100644 --- a/drivers/net/virtio/virtio_ethdev.c +++ b/drivers/net/virtio/virtio_ethdev.c @@ -38,17 +38,14 @@ #include "virtio_rxtx.h" #include "virtio_user/virtio_user_dev.h" -static int eth_virtio_dev_uninit(struct rte_eth_dev *eth_dev); static int virtio_dev_configure(struct rte_eth_dev *dev); static int virtio_dev_start(struct rte_eth_dev *dev); -static int virtio_dev_stop(struct rte_eth_dev *dev); static int virtio_dev_promiscuous_enable(struct rte_eth_dev *dev); static int virtio_dev_promiscuous_disable(struct rte_eth_dev *dev); static int virtio_dev_allmulticast_enable(struct rte_eth_dev *dev); static int virtio_dev_allmulticast_disable(struct rte_eth_dev *dev); static uint32_t virtio_dev_speed_capa_get(uint32_t speed); static int virtio_dev_devargs_parse(struct rte_devargs *devargs, - int *vdpa, uint32_t *speed, int *vectorized); static int virtio_dev_info_get(struct rte_eth_dev *dev, @@ -89,15 +86,6 @@ static int virtio_dev_queue_stats_mapping_set( static void virtio_notify_peers(struct rte_eth_dev *dev); static void virtio_ack_link_announce(struct rte_eth_dev *dev); -/* - * The set of PCI devices this driver supports - */ -static const struct rte_pci_id pci_id_virtio_map[] = { - { RTE_PCI_DEVICE(VIRTIO_PCI_VENDORID, VIRTIO_PCI_LEGACY_DEVICEID_NET) }, - { RTE_PCI_DEVICE(VIRTIO_PCI_VENDORID, VIRTIO_PCI_MODERN_DEVICEID_NET) }, - { .vendor_id = 0, /* sentinel */ }, -}; - struct rte_virtio_xstats_name_off { char name[RTE_ETH_XSTATS_NAME_SIZE]; unsigned offset; @@ -714,7 +702,7 @@ virtio_alloc_queues(struct rte_eth_dev *dev) static void virtio_queues_unbind_intr(struct rte_eth_dev *dev); -static int +int virtio_dev_close(struct rte_eth_dev *dev) { struct virtio_hw *hw = dev->data->dev_private; @@ -1929,8 +1917,7 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev) return 0; } - ret = virtio_dev_devargs_parse(eth_dev->device->devargs, - NULL, &speed, &vectorized); + ret = virtio_dev_devargs_parse(eth_dev->device->devargs, &speed, &vectorized); if (ret < 0) return ret; hw->speed = speed; @@ -1992,36 +1979,6 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev) return ret; } -static int -eth_virtio_dev_uninit(struct rte_eth_dev *eth_dev) -{ - int ret; - PMD_INIT_FUNC_TRACE(); - - if (rte_eal_process_type() == RTE_PROC_SECONDARY) - return 0; - - ret = virtio_dev_stop(eth_dev); - virtio_dev_close(eth_dev); - - PMD_INIT_LOG(DEBUG, "dev_uninit completed"); - - return ret; -} - - -static int vdpa_check_handler(__rte_unused const char *key, - const char *value, void *ret_val) -{ - if (strcmp(value, "1") == 0) - *(int *)ret_val = 1; - else - *(int *)ret_val = 0; - - return 0; -} - - static uint32_t virtio_dev_speed_capa_get(uint32_t speed) { @@ -2059,10 +2016,8 @@ static int vectorized_check_handler(__rte_unused const char *key, } #define VIRTIO_ARG_SPEED "speed" -#define VIRTIO_ARG_VDPA "vdpa" #define VIRTIO_ARG_VECTORIZED "vectorized" - static int link_speed_handler(const char *key __rte_unused, const char *value, void *ret_val) @@ -2081,8 +2036,7 @@ link_speed_handler(const char *key __rte_unused, static int -virtio_dev_devargs_parse(struct rte_devargs *devargs, int *vdpa, - uint32_t *speed, int *vectorized) +virtio_dev_devargs_parse(struct rte_devargs *devargs, uint32_t *speed, int *vectorized) { struct rte_kvargs *kvlist; int ret = 0; @@ -2095,18 +2049,7 @@ virtio_dev_devargs_parse(struct rte_devargs *devargs, int *vdpa, PMD_INIT_LOG(ERR, "error when parsing param"); return 0; } - if (vdpa && rte_kvargs_count(kvlist, VIRTIO_ARG_VDPA) == 1) { - /* vdpa mode selected when there's a key-value pair: - * vdpa=1 - */ - ret = rte_kvargs_process(kvlist, VIRTIO_ARG_VDPA, - vdpa_check_handler, vdpa); - if (ret < 0) { - PMD_INIT_LOG(ERR, "Failed to parse %s", - VIRTIO_ARG_VDPA); - goto exit; - } - } + if (speed && rte_kvargs_count(kvlist, VIRTIO_ARG_SPEED) == 1) { ret = rte_kvargs_process(kvlist, VIRTIO_ARG_SPEED, @@ -2135,52 +2078,7 @@ virtio_dev_devargs_parse(struct rte_devargs *devargs, int *vdpa, return ret; } -static int eth_virtio_pci_probe(struct rte_pci_driver *pci_drv __rte_unused, - struct rte_pci_device *pci_dev) -{ - int vdpa = 0; - int ret = 0; - - ret = virtio_dev_devargs_parse(pci_dev->device.devargs, &vdpa, NULL, - NULL); - if (ret < 0) { - PMD_INIT_LOG(ERR, "devargs parsing is failed"); - return ret; - } - /* virtio pmd skips probe if device needs to work in vdpa mode */ - if (vdpa == 1) - return 1; - - return rte_eth_dev_pci_generic_probe(pci_dev, sizeof(struct virtio_pci_dev), - eth_virtio_dev_init); -} - -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_dev_uninit); - /* Port has already been released by close. */ - if (ret == -ENODEV) - ret = 0; - return ret; -} - -static struct rte_pci_driver rte_virtio_pmd = { - .driver = { - .name = "net_virtio", - }, - .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) -{ - rte_eal_iopl_init(); - rte_pci_register(&rte_virtio_pmd); -} 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__); -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"); RTE_LOG_REGISTER(virtio_logtype_init, pmd.net.virtio.init, NOTICE); RTE_LOG_REGISTER(virtio_logtype_driver, pmd.net.virtio.driver, NOTICE); diff --git a/drivers/net/virtio/virtio_ethdev.h b/drivers/net/virtio/virtio_ethdev.h index b7d52d497f..13395937c8 100644 --- a/drivers/net/virtio/virtio_ethdev.h +++ b/drivers/net/virtio/virtio_ethdev.h @@ -117,6 +117,8 @@ void virtio_interrupt_handler(void *param); int virtio_dev_pause(struct rte_eth_dev *dev); void virtio_dev_resume(struct rte_eth_dev *dev); +int virtio_dev_stop(struct rte_eth_dev *dev); +int virtio_dev_close(struct rte_eth_dev *dev); int virtio_inject_pkts(struct rte_eth_dev *dev, struct rte_mbuf **tx_pkts, int nb_pkts); diff --git a/drivers/net/virtio/virtio_pci_ethdev.c b/drivers/net/virtio/virtio_pci_ethdev.c new file mode 100644 index 0000000000..9c0935068e --- /dev/null +++ b/drivers/net/virtio/virtio_pci_ethdev.c @@ -0,0 +1,148 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * Copyright(c) 2010-2016 Intel Corporation + */ + +#include <stdint.h> +#include <string.h> +#include <stdio.h> +#include <errno.h> +#include <unistd.h> + +#include <rte_ethdev_driver.h> +#include <rte_ethdev_pci.h> +#include <rte_pci.h> +#include <rte_bus_pci.h> +#include <rte_errno.h> + +#include <rte_memory.h> +#include <rte_eal.h> +#include <rte_dev.h> +#include <rte_kvargs.h> + +#include "virtio_ethdev.h" +#include "virtio_pci.h" +#include "virtio_logs.h" + +/* + * The set of PCI devices this driver supports + */ +static const struct rte_pci_id pci_id_virtio_map[] = { + { RTE_PCI_DEVICE(VIRTIO_PCI_VENDORID, VIRTIO_PCI_LEGACY_DEVICEID_NET) }, + { RTE_PCI_DEVICE(VIRTIO_PCI_VENDORID, VIRTIO_PCI_MODERN_DEVICEID_NET) }, + { .vendor_id = 0, /* sentinel */ }, +}; + +static int +eth_virtio_pci_init(struct rte_eth_dev *eth_dev) +{ + return eth_virtio_dev_init(eth_dev); +} + +static int +eth_virtio_pci_uninit(struct rte_eth_dev *eth_dev) +{ + int ret; + PMD_INIT_FUNC_TRACE(); + + if (rte_eal_process_type() == RTE_PROC_SECONDARY) + return 0; + + ret = virtio_dev_stop(eth_dev); + virtio_dev_close(eth_dev); + + PMD_INIT_LOG(DEBUG, "dev_uninit completed"); + + return ret; +} + +static int vdpa_check_handler(__rte_unused const char *key, + const char *value, void *ret_val) +{ + if (strcmp(value, "1") == 0) + *(int *)ret_val = 1; + else + *(int *)ret_val = 0; + + return 0; +} + +#define VIRTIO_ARG_VDPA "vdpa" + +static int +virtio_pci_devargs_parse(struct rte_devargs *devargs, int *vdpa) +{ + struct rte_kvargs *kvlist; + int ret = 0; + + if (devargs == NULL) + return 0; + + kvlist = rte_kvargs_parse(devargs->args, NULL); + if (kvlist == NULL) { + PMD_INIT_LOG(ERR, "error when parsing param"); + return 0; + } + + if (vdpa && rte_kvargs_count(kvlist, VIRTIO_ARG_VDPA) == 1) { + /* vdpa mode selected when there's a key-value pair: + * vdpa=1 + */ + ret = rte_kvargs_process(kvlist, VIRTIO_ARG_VDPA, + vdpa_check_handler, vdpa); + if (ret < 0) + PMD_INIT_LOG(ERR, "Failed to parse %s", VIRTIO_ARG_VDPA); + } + + rte_kvargs_free(kvlist); + + return ret; +} + +static int eth_virtio_pci_probe(struct rte_pci_driver *pci_drv __rte_unused, + struct rte_pci_device *pci_dev) +{ + int vdpa = 0; + int ret = 0; + + ret = virtio_pci_devargs_parse(pci_dev->device.devargs, &vdpa); + if (ret < 0) { + PMD_INIT_LOG(ERR, "devargs parsing is failed"); + return ret; + } + /* virtio pmd skips probe if device needs to work in vdpa mode */ + if (vdpa == 1) + return 1; + + return rte_eth_dev_pci_generic_probe(pci_dev, sizeof(struct virtio_pci_dev), + eth_virtio_pci_init); +} + +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 = { + .driver = { + .name = "net_virtio", + }, + .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) +{ + rte_eal_iopl_init(); + rte_pci_register(&rte_virtio_pmd); +} + +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");
Thi patch moves the PCI Ethernet device registration bits in a dedicated patch. In following patches, more code will be moved there, with the goal of making virtio_ethdev.c truely bus-agnostic. Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com> --- drivers/net/virtio/meson.build | 1 + drivers/net/virtio/virtio_ethdev.c | 114 +------------------ drivers/net/virtio/virtio_ethdev.h | 2 + drivers/net/virtio/virtio_pci_ethdev.c | 148 +++++++++++++++++++++++++ 4 files changed, 156 insertions(+), 109 deletions(-) create mode 100644 drivers/net/virtio/virtio_pci_ethdev.c