From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id A6BF41E4E1 for ; Tue, 12 Jun 2018 08:35:09 +0200 (CEST) X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 11 Jun 2018 23:35:07 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.51,213,1526367600"; d="scan'208";a="56382278" Received: from debian.sh.intel.com (HELO debian) ([10.67.104.228]) by fmsmga002.fm.intel.com with ESMTP; 11 Jun 2018 23:35:07 -0700 Date: Tue, 12 Jun 2018 14:35:14 +0800 From: Tiwei Bie To: Maxime Coquelin Cc: zhihong.wang@intel.com, dev@dpdk.org Message-ID: <20180612063514.GA14993@debian> References: <20180607092616.27720-1-maxime.coquelin@redhat.com> <20180607092616.27720-2-maxime.coquelin@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20180607092616.27720-2-maxime.coquelin@redhat.com> User-Agent: Mutt/1.9.5 (2018-04-13) Subject: Re: [dpdk-dev] [PATCH v3 1/5] net/virtio: forbid simple Tx path by default X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 12 Jun 2018 06:35:10 -0000 On Thu, Jun 07, 2018 at 11:26:12AM +0200, Maxime Coquelin wrote: > Simple Tx path is not compliant with the Virtio specification, > as it assumes the device will use the descriptors in order. > > VIRTIO_F_IN_ORDER feature has been introduced recently, but the > simple Tx path is not compliant with it as VIRTIO_F_IN_ORDER > requires that chained descriptors are used sequentially, which > is not the case in simple Tx path. > > This patch introduces 'simple_tx_support' devarg to unlock > Tx simple path selection. > > Reported-by: Tiwei Bie > Signed-off-by: Maxime Coquelin > --- > doc/guides/nics/virtio.rst | 9 +++++ > drivers/net/virtio/virtio_ethdev.c | 73 +++++++++++++++++++++++++++++++++++++- > drivers/net/virtio/virtio_pci.h | 1 + > 3 files changed, 82 insertions(+), 1 deletion(-) > > diff --git a/doc/guides/nics/virtio.rst b/doc/guides/nics/virtio.rst > index 8922f9c0b..53ce1c12a 100644 > --- a/doc/guides/nics/virtio.rst > +++ b/doc/guides/nics/virtio.rst > @@ -222,6 +222,9 @@ Tx callbacks: > > #. ``virtio_xmit_pkts_simple``: > Vector version fixes the available ring indexes to optimize performance. > + This implementation does not comply with the Virtio specification, and so > + is not selectable by default. "simple_tx_support=1" devarg must be passed > + to unlock it. > > > By default, the non-vector callbacks are used: > @@ -331,3 +334,9 @@ The user can specify below argument in devargs. > driver, and works as a HW vhost backend. This argument is used to specify > a virtio device needs to work in vDPA mode. > (Default: 0 (disabled)) > + > +#. ``simple_tx_support``: > + > + This argument enables support for the simple Tx path, which is not > + compliant with the Virtio specification. > + (Default: 0 (disabled)) I tried this patch on my server. Virtio-user will fail to probe when simple_tx_support is specified dues to the check in virtio_user_pmd_probe(): PMD: Error parsing device, invalid key virtio_user_pmd_probe(): error when parsing param vdev_probe(): failed to initialize virtio_user0 device EAL: Bus (vdev) probe failed. > diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c > index 5833dad73..052dd056a 100644 > --- a/drivers/net/virtio/virtio_ethdev.c > +++ b/drivers/net/virtio/virtio_ethdev.c > @@ -1331,6 +1331,8 @@ set_rxtx_funcs(struct rte_eth_dev *eth_dev) > if (hw->use_simple_tx) { > PMD_INIT_LOG(INFO, "virtio: using simple Tx path on port %u", > eth_dev->data->port_id); > + PMD_INIT_LOG(WARNING, > + "virtio: simple Tx path does not comply with Virtio spec"); > eth_dev->tx_pkt_burst = virtio_xmit_pkts_simple; > } else { > PMD_INIT_LOG(INFO, "virtio: using standard Tx path on port %u", > @@ -1790,6 +1792,66 @@ rte_virtio_pmd_init(void) > rte_pci_register(&rte_virtio_pmd); > } > > +#define VIRTIO_SIMPLE_TX_SUPPORT "simple_tx_support" > + > +static int virtio_dev_args_check(const char *key, const char *val, > + void *opaque) > +{ > + struct rte_eth_dev *dev = opaque; > + struct virtio_hw *hw = dev->data->dev_private; > + unsigned long tmp; > + int ret = 0; > + > + errno = 0; > + tmp = strtoul(val, NULL, 0); > + if (errno) { > + PMD_INIT_LOG(INFO, > + "%s: \"%s\" is not a valid integer", key, val); > + return errno; > + } > + > + if (strcmp(VIRTIO_SIMPLE_TX_SUPPORT, key) == 0) > + hw->support_simple_tx = !!tmp; > + > + return ret; > +} > + > +static int > +virtio_dev_args(struct rte_eth_dev *dev) > +{ > + struct rte_kvargs *kvlist; > + struct rte_devargs *devargs; > + const char *valid_args[] = { > + VIRTIO_SIMPLE_TX_SUPPORT, > + NULL, > + }; checkpatch is complaining about above definition: WARNING:STATIC_CONST_CHAR_ARRAY: char * array declaration might be better as static const #96: FILE: drivers/net/virtio/virtio_ethdev.c:1824: + const char *valid_args[] = { Best regards, Tiwei Bie