From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (xvm-189-124.dc0.ghst.net [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 8138EA09FF; Tue, 5 Jan 2021 22:19:24 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 11C601607D8; Tue, 5 Jan 2021 22:19:24 +0100 (CET) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by mails.dpdk.org (Postfix) with ESMTP id 3FEBE1607A1 for ; Tue, 5 Jan 2021 22:19:23 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1609881562; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=trtZlUxt8lF9aiqiEYMDUAP9hPIMugkxy3xL7IHnZnY=; b=dNIeRn0bFOwzbe/QLWGICMlJe0J+SeHT/1NzL/F5Jx/25QctXeFkazobXV1cPRbl7keuEP i3xV5O5b9D3+PQVgO9zoMkMCvy7Y1d8k1J7Cbns3/GwgWf7UEYyhf6gfilxseoXtcTuwrz aQNJnF1TXDh/5rimOgNTHYzWeiXrvsM= Received: from mail-vk1-f199.google.com (mail-vk1-f199.google.com [209.85.221.199]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-547-d9D0JCX6OQqlANtEf2dilg-1; Tue, 05 Jan 2021 16:19:20 -0500 X-MC-Unique: d9D0JCX6OQqlANtEf2dilg-1 Received: by mail-vk1-f199.google.com with SMTP id c69so504494vke.14 for ; Tue, 05 Jan 2021 13:19:20 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=trtZlUxt8lF9aiqiEYMDUAP9hPIMugkxy3xL7IHnZnY=; b=BDkEIumW87S8gJxcnHgc7a14oUyqkrv7kXhuiqAcdwie9g1UfnPoOhgpV0c9CPn1V+ K17isBKlwH8JFVYUwgz/8NCTP6o/Sl5lLaaXgUaTBXqwfvErq3I6OcDL9KDnJziBka36 STgd7VVDITCjXkIG6x3KgcLELfhQcACcon7x6iPPt+CbTnDxjW5UxMT1eEtDl1WrwenQ Owp6OX2Zx/601QrYD1C4k8gC1rDe4Y5hlw0TMFEPJStT5adqWsb2lGn6ZXzdVzogmIad +OFIuiPU9Ted4pIqGbK1mOKtWHY+KSRrnkKoWcMsCAGAcs52NQOm83WKT0aFNf42J+7n 5P/Q== X-Gm-Message-State: AOAM5330iPBmk2bl4eWTGugXLgglwHZOl9whWQLCnr4x18KsUozb9qRD 9SIX2M/07hkvAnarwuXbzan0/ZBLrx8g2S3zcSkUogw4Izo75usMWqZNalKtdeyBsKp+LUwLws0 UwBV4IFMqQ4F0y/NziUA= X-Received: by 2002:a9f:2628:: with SMTP id 37mr1251850uag.87.1609881560139; Tue, 05 Jan 2021 13:19:20 -0800 (PST) X-Google-Smtp-Source: ABdhPJwm6QQAD3/LfMxN5GZb8odtP94titRitr/HewWbaTFsIPnB5PT9ostgGsuZ3kXEOcyOst7QL0daT5swuc5nCfM= X-Received: by 2002:a9f:2628:: with SMTP id 37mr1251838uag.87.1609881559903; Tue, 05 Jan 2021 13:19:19 -0800 (PST) MIME-Version: 1.0 References: <20201220211405.313012-1-maxime.coquelin@redhat.com> <20201220211405.313012-6-maxime.coquelin@redhat.com> In-Reply-To: <20201220211405.313012-6-maxime.coquelin@redhat.com> From: David Marchand Date: Tue, 5 Jan 2021 22:19:09 +0100 Message-ID: To: Maxime Coquelin Cc: dev , "Xia, Chenbo" , Olivier Matz , Adrian Moreno Zapata Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=dmarchan@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Subject: Re: [dpdk-dev] [PATCH 05/40] net/virtio: move PCI device init in dedicated file X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Sun, Dec 20, 2020 at 10:14 PM Maxime Coquelin 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= mac= cq= queue_size= queues= iface= server=<0|1> mrg_rxbuf=<0|1> in_order=<0|1> packed_vq=<0|1> speed= 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 > +#include > +#include > +#include > +#include > + > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > +#include > +#include > + > +#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