* [dpdk-dev] [PATCH v2 0/2] virtio: fixes for 2.1-rc1
@ 2015-07-20 18:40 Stephen Hemminger
2015-07-20 18:40 ` [dpdk-dev] [PATCH v2 1/2] virtio: fix queue size and number of descriptors Stephen Hemminger
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Stephen Hemminger @ 2015-07-20 18:40 UTC (permalink / raw)
To: changchun.ouyang; +Cc: dev, Stephen Hemminger
From: Stephen Hemminger <shemming@brocade.com>
This integrates my change and earlier change by Ouyang Changchun
into one fix. And second patch is minor stuff found while reviewing.
Stephen Hemminger (2):
virtio: fix queue size and number of descriptors
virtio: small cleanups
drivers/net/virtio/virtio_ethdev.c | 24 +++++++-----------------
drivers/net/virtio/virtio_ethdev.h | 2 +-
drivers/net/virtio/virtio_rxtx.c | 2 +-
3 files changed, 9 insertions(+), 19 deletions(-)
--
2.1.4
^ permalink raw reply [flat|nested] 8+ messages in thread* [dpdk-dev] [PATCH v2 1/2] virtio: fix queue size and number of descriptors 2015-07-20 18:40 [dpdk-dev] [PATCH v2 0/2] virtio: fixes for 2.1-rc1 Stephen Hemminger @ 2015-07-20 18:40 ` Stephen Hemminger 2015-07-21 5:06 ` Ouyang, Changchun 2015-07-22 8:55 ` Thomas Monjalon 2015-07-20 18:40 ` [dpdk-dev] [PATCH v2 2/2] virtio: small cleanups Stephen Hemminger ` (2 subsequent siblings) 3 siblings, 2 replies; 8+ messages in thread From: Stephen Hemminger @ 2015-07-20 18:40 UTC (permalink / raw) To: changchun.ouyang; +Cc: dev The virtual queue ring size and the number of slots actually usable are separate parameters. In the most common environment (QEMU) the virtual queue ring size is 256, but some environments the ring maybe much larger. The ring size comes from the host and the driver must use the actual size passed. The number of descriptors can be either zero to use the whole available ring, or some value smaller. This is used to limit the number of mbufs allocated for the receive ring. If more descriptors are requested than available the size is silently truncated. Note: the ring size (from host) must be a power of two, but the number of descriptors used can be any size from 1 to the size of the virtual ring. Reported-by: Ouyang Changchun <changchun.ouyang@intel.com> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- drivers/net/virtio/virtio_ethdev.c | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c index 9ca9bb2..d460d89 100644 --- a/drivers/net/virtio/virtio_ethdev.c +++ b/drivers/net/virtio/virtio_ethdev.c @@ -276,8 +276,6 @@ int virtio_dev_queue_setup(struct rte_eth_dev *dev, */ vq_size = VIRTIO_READ_REG_2(hw, VIRTIO_PCI_QUEUE_NUM); PMD_INIT_LOG(DEBUG, "vq_size: %d nb_desc:%d", vq_size, nb_desc); - if (nb_desc == 0) - nb_desc = vq_size; if (vq_size == 0) { PMD_INIT_LOG(ERR, "%s: virtqueue does not exist", __func__); return -EINVAL; @@ -288,16 +286,6 @@ int virtio_dev_queue_setup(struct rte_eth_dev *dev, return -EINVAL; } - if (nb_desc < vq_size) { - if (!rte_is_power_of_2(nb_desc)) { - PMD_INIT_LOG(ERR, - "nb_desc(%u) size is not powerof 2", - nb_desc); - return -EINVAL; - } - vq_size = nb_desc; - } - if (queue_type == VTNET_RQ) { snprintf(vq_name, sizeof(vq_name), "port%d_rvq%d", dev->data->port_id, queue_idx); @@ -325,7 +313,10 @@ int virtio_dev_queue_setup(struct rte_eth_dev *dev, vq->queue_id = queue_idx; vq->vq_queue_index = vtpci_queue_idx; vq->vq_nentries = vq_size; - vq->vq_free_cnt = vq_size; + + if (nb_desc == 0 || nb_desc > vq_size) + nb_desc = vq_size; + vq->vq_free_cnt = nb_desc; /* * Reserve a memzone for vring elements -- 2.1.4 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH v2 1/2] virtio: fix queue size and number of descriptors 2015-07-20 18:40 ` [dpdk-dev] [PATCH v2 1/2] virtio: fix queue size and number of descriptors Stephen Hemminger @ 2015-07-21 5:06 ` Ouyang, Changchun 2015-07-22 8:55 ` Thomas Monjalon 1 sibling, 0 replies; 8+ messages in thread From: Ouyang, Changchun @ 2015-07-21 5:06 UTC (permalink / raw) To: Stephen Hemminger; +Cc: dev > -----Original Message----- > From: Stephen Hemminger [mailto:stephen@networkplumber.org] > Sent: Tuesday, July 21, 2015 2:41 AM > To: Ouyang, Changchun > Cc: dev@dpdk.org; Stephen Hemminger > Subject: [PATCH v2 1/2] virtio: fix queue size and number of descriptors > > The virtual queue ring size and the number of slots actually usable are > separate parameters. In the most common environment (QEMU) the virtual > queue ring size is 256, but some environments the ring maybe much larger. > > The ring size comes from the host and the driver must use the actual size > passed. > > The number of descriptors can be either zero to use the whole available ring, > or some value smaller. This is used to limit the number of mbufs allocated for > the receive ring. If more descriptors are requested than available the size is > silently truncated. > > Note: the ring size (from host) must be a power of two, but the number of > descriptors used can be any size from 1 to the size of the virtual ring. > > Reported-by: Ouyang Changchun <changchun.ouyang@intel.com> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> Basically ok for this change, so Acked-by: Changchun Ouyang <changchun.ouyang@intel.com> > --- > drivers/net/virtio/virtio_ethdev.c | 17 ++++------------- > 1 file changed, 4 insertions(+), 13 deletions(-) > > diff --git a/drivers/net/virtio/virtio_ethdev.c > b/drivers/net/virtio/virtio_ethdev.c > index 9ca9bb2..d460d89 100644 > --- a/drivers/net/virtio/virtio_ethdev.c > +++ b/drivers/net/virtio/virtio_ethdev.c > @@ -276,8 +276,6 @@ int virtio_dev_queue_setup(struct rte_eth_dev *dev, > */ > vq_size = VIRTIO_READ_REG_2(hw, VIRTIO_PCI_QUEUE_NUM); > PMD_INIT_LOG(DEBUG, "vq_size: %d nb_desc:%d", vq_size, > nb_desc); > - if (nb_desc == 0) > - nb_desc = vq_size; > if (vq_size == 0) { > PMD_INIT_LOG(ERR, "%s: virtqueue does not exist", > __func__); > return -EINVAL; > @@ -288,16 +286,6 @@ int virtio_dev_queue_setup(struct rte_eth_dev > *dev, > return -EINVAL; > } > > - if (nb_desc < vq_size) { > - if (!rte_is_power_of_2(nb_desc)) { > - PMD_INIT_LOG(ERR, > - "nb_desc(%u) size is not powerof 2", > - nb_desc); > - return -EINVAL; > - } > - vq_size = nb_desc; > - } > - > if (queue_type == VTNET_RQ) { > snprintf(vq_name, sizeof(vq_name), "port%d_rvq%d", > dev->data->port_id, queue_idx); > @@ -325,7 +313,10 @@ int virtio_dev_queue_setup(struct rte_eth_dev > *dev, > vq->queue_id = queue_idx; > vq->vq_queue_index = vtpci_queue_idx; > vq->vq_nentries = vq_size; > - vq->vq_free_cnt = vq_size; > + > + if (nb_desc == 0 || nb_desc > vq_size) > + nb_desc = vq_size; > + vq->vq_free_cnt = nb_desc; > > /* > * Reserve a memzone for vring elements > -- > 2.1.4 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH v2 1/2] virtio: fix queue size and number of descriptors 2015-07-20 18:40 ` [dpdk-dev] [PATCH v2 1/2] virtio: fix queue size and number of descriptors Stephen Hemminger 2015-07-21 5:06 ` Ouyang, Changchun @ 2015-07-22 8:55 ` Thomas Monjalon 1 sibling, 0 replies; 8+ messages in thread From: Thomas Monjalon @ 2015-07-22 8:55 UTC (permalink / raw) To: Stephen Hemminger; +Cc: dev 2015-07-20 11:40, Stephen Hemminger: > The number of descriptors can be either zero to use the whole > available ring, or some value smaller. This is used to limit > the number of mbufs allocated for the receive ring. If more > descriptors are requested than available the size is silently > truncated. [...] > + if (nb_desc == 0 || nb_desc > vq_size) > + nb_desc = vq_size; This behaviour should be common to every drivers and explained in the API doxygen: http://dpdk.org/browse/dpdk/tree/lib/librte_ether/rte_ethdev.h?id=v2.1.0-rc1#n1843 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [dpdk-dev] [PATCH v2 2/2] virtio: small cleanups 2015-07-20 18:40 [dpdk-dev] [PATCH v2 0/2] virtio: fixes for 2.1-rc1 Stephen Hemminger 2015-07-20 18:40 ` [dpdk-dev] [PATCH v2 1/2] virtio: fix queue size and number of descriptors Stephen Hemminger @ 2015-07-20 18:40 ` Stephen Hemminger 2015-07-21 4:57 ` Ouyang, Changchun 2015-07-21 5:00 ` [dpdk-dev] [PATCH v2 0/2] virtio: fixes for 2.1-rc1 Ouyang, Changchun 2015-07-22 8:59 ` Thomas Monjalon 3 siblings, 1 reply; 8+ messages in thread From: Stephen Hemminger @ 2015-07-20 18:40 UTC (permalink / raw) To: changchun.ouyang; +Cc: dev Some minor cleanups. * pass constant to virtio_dev_queue_setup * fix message on rx_queue_setup * get rid of extra double spaces Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- drivers/net/virtio/virtio_ethdev.c | 7 +++---- drivers/net/virtio/virtio_ethdev.h | 2 +- drivers/net/virtio/virtio_rxtx.c | 2 +- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c index d460d89..465d3cd 100644 --- a/drivers/net/virtio/virtio_ethdev.c +++ b/drivers/net/virtio/virtio_ethdev.c @@ -254,7 +254,7 @@ virtio_dev_queue_release(struct virtqueue *vq) { int virtio_dev_queue_setup(struct rte_eth_dev *dev, int queue_type, uint16_t queue_idx, - uint16_t vtpci_queue_idx, + uint16_t vtpci_queue_idx, uint16_t nb_desc, unsigned int socket_id, struct virtqueue **pvq) @@ -264,7 +264,7 @@ int virtio_dev_queue_setup(struct rte_eth_dev *dev, uint16_t vq_size; int size; struct virtio_hw *hw = dev->data->dev_private; - struct virtqueue *vq = NULL; + struct virtqueue *vq = NULL; /* Write the virtqueue index to the Queue Select Field */ VIRTIO_WRITE_REG_2(hw, VIRTIO_PCI_QUEUE_SEL, vtpci_queue_idx); @@ -413,13 +413,12 @@ virtio_dev_cq_queue_setup(struct rte_eth_dev *dev, uint16_t vtpci_queue_idx, uint32_t socket_id) { struct virtqueue *vq; - uint16_t nb_desc = 0; int ret; struct virtio_hw *hw = dev->data->dev_private; PMD_INIT_FUNC_TRACE(); ret = virtio_dev_queue_setup(dev, VTNET_CQ, VTNET_SQ_CQ_QUEUE_IDX, - vtpci_queue_idx, nb_desc, socket_id, &vq); + vtpci_queue_idx, 0, socket_id, &vq); if (ret < 0) { PMD_INIT_LOG(ERR, "control vq initialization failed"); return ret; diff --git a/drivers/net/virtio/virtio_ethdev.h b/drivers/net/virtio/virtio_ethdev.h index 3858b00..9026d42 100644 --- a/drivers/net/virtio/virtio_ethdev.h +++ b/drivers/net/virtio/virtio_ethdev.h @@ -79,7 +79,7 @@ void virtio_dev_rxtx_start(struct rte_eth_dev *dev); int virtio_dev_queue_setup(struct rte_eth_dev *dev, int queue_type, uint16_t queue_idx, - uint16_t vtpci_queue_idx, + uint16_t vtpci_queue_idx, uint16_t nb_desc, unsigned int socket_id, struct virtqueue **pvq); diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c index 5388caa..c5b53bb 100644 --- a/drivers/net/virtio/virtio_rxtx.c +++ b/drivers/net/virtio/virtio_rxtx.c @@ -390,7 +390,7 @@ virtio_dev_rx_queue_setup(struct rte_eth_dev *dev, ret = virtio_dev_queue_setup(dev, VTNET_RQ, queue_idx, vtpci_queue_idx, nb_desc, socket_id, &vq); if (ret < 0) { - PMD_INIT_LOG(ERR, "tvq initialization failed"); + PMD_INIT_LOG(ERR, "rvq initialization failed"); return ret; } -- 2.1.4 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH v2 2/2] virtio: small cleanups 2015-07-20 18:40 ` [dpdk-dev] [PATCH v2 2/2] virtio: small cleanups Stephen Hemminger @ 2015-07-21 4:57 ` Ouyang, Changchun 0 siblings, 0 replies; 8+ messages in thread From: Ouyang, Changchun @ 2015-07-21 4:57 UTC (permalink / raw) To: Stephen Hemminger; +Cc: dev > -----Original Message----- > From: Stephen Hemminger [mailto:stephen@networkplumber.org] > Sent: Tuesday, July 21, 2015 2:41 AM > To: Ouyang, Changchun > Cc: dev@dpdk.org; Stephen Hemminger > Subject: [PATCH v2 2/2] virtio: small cleanups > > Some minor cleanups. > * pass constant to virtio_dev_queue_setup > * fix message on rx_queue_setup > * get rid of extra double spaces > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> Acked-by: Changchun Ouyang <changchun.ouyang@intel.com> > --- > drivers/net/virtio/virtio_ethdev.c | 7 +++---- > drivers/net/virtio/virtio_ethdev.h | 2 +- > drivers/net/virtio/virtio_rxtx.c | 2 +- > 3 files changed, 5 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/virtio/virtio_ethdev.c > b/drivers/net/virtio/virtio_ethdev.c > index d460d89..465d3cd 100644 > --- a/drivers/net/virtio/virtio_ethdev.c > +++ b/drivers/net/virtio/virtio_ethdev.c > @@ -254,7 +254,7 @@ virtio_dev_queue_release(struct virtqueue *vq) { int > virtio_dev_queue_setup(struct rte_eth_dev *dev, > int queue_type, > uint16_t queue_idx, > - uint16_t vtpci_queue_idx, > + uint16_t vtpci_queue_idx, > uint16_t nb_desc, > unsigned int socket_id, > struct virtqueue **pvq) > @@ -264,7 +264,7 @@ int virtio_dev_queue_setup(struct rte_eth_dev *dev, > uint16_t vq_size; > int size; > struct virtio_hw *hw = dev->data->dev_private; > - struct virtqueue *vq = NULL; > + struct virtqueue *vq = NULL; > > /* Write the virtqueue index to the Queue Select Field */ > VIRTIO_WRITE_REG_2(hw, VIRTIO_PCI_QUEUE_SEL, > vtpci_queue_idx); @@ -413,13 +413,12 @@ > virtio_dev_cq_queue_setup(struct rte_eth_dev *dev, uint16_t > vtpci_queue_idx, > uint32_t socket_id) > { > struct virtqueue *vq; > - uint16_t nb_desc = 0; > int ret; > struct virtio_hw *hw = dev->data->dev_private; > > PMD_INIT_FUNC_TRACE(); > ret = virtio_dev_queue_setup(dev, VTNET_CQ, > VTNET_SQ_CQ_QUEUE_IDX, > - vtpci_queue_idx, nb_desc, socket_id, &vq); > + vtpci_queue_idx, 0, socket_id, &vq); > if (ret < 0) { > PMD_INIT_LOG(ERR, "control vq initialization failed"); > return ret; > diff --git a/drivers/net/virtio/virtio_ethdev.h > b/drivers/net/virtio/virtio_ethdev.h > index 3858b00..9026d42 100644 > --- a/drivers/net/virtio/virtio_ethdev.h > +++ b/drivers/net/virtio/virtio_ethdev.h > @@ -79,7 +79,7 @@ void virtio_dev_rxtx_start(struct rte_eth_dev *dev); > int virtio_dev_queue_setup(struct rte_eth_dev *dev, > int queue_type, > uint16_t queue_idx, > - uint16_t vtpci_queue_idx, > + uint16_t vtpci_queue_idx, > uint16_t nb_desc, > unsigned int socket_id, > struct virtqueue **pvq); > diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c > index 5388caa..c5b53bb 100644 > --- a/drivers/net/virtio/virtio_rxtx.c > +++ b/drivers/net/virtio/virtio_rxtx.c > @@ -390,7 +390,7 @@ virtio_dev_rx_queue_setup(struct rte_eth_dev *dev, > ret = virtio_dev_queue_setup(dev, VTNET_RQ, queue_idx, > vtpci_queue_idx, > nb_desc, socket_id, &vq); > if (ret < 0) { > - PMD_INIT_LOG(ERR, "tvq initialization failed"); > + PMD_INIT_LOG(ERR, "rvq initialization failed"); > return ret; > } > > -- > 2.1.4 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH v2 0/2] virtio: fixes for 2.1-rc1 2015-07-20 18:40 [dpdk-dev] [PATCH v2 0/2] virtio: fixes for 2.1-rc1 Stephen Hemminger 2015-07-20 18:40 ` [dpdk-dev] [PATCH v2 1/2] virtio: fix queue size and number of descriptors Stephen Hemminger 2015-07-20 18:40 ` [dpdk-dev] [PATCH v2 2/2] virtio: small cleanups Stephen Hemminger @ 2015-07-21 5:00 ` Ouyang, Changchun 2015-07-22 8:59 ` Thomas Monjalon 3 siblings, 0 replies; 8+ messages in thread From: Ouyang, Changchun @ 2015-07-21 5:00 UTC (permalink / raw) To: Stephen Hemminger; +Cc: dev, Stephen Hemminger > -----Original Message----- > From: Stephen Hemminger [mailto:stephen@networkplumber.org] > Sent: Tuesday, July 21, 2015 2:41 AM > To: Ouyang, Changchun > Cc: dev@dpdk.org; Stephen Hemminger > Subject: [PATCH v2 0/2] virtio: fixes for 2.1-rc1 I think v2 here should be v3, as you have already a v2. > > From: Stephen Hemminger <shemming@brocade.com> > > This integrates my change and earlier change by Ouyang Changchun into one > fix. And second patch is minor stuff found while reviewing. > > Stephen Hemminger (2): > virtio: fix queue size and number of descriptors > virtio: small cleanups > > drivers/net/virtio/virtio_ethdev.c | 24 +++++++----------------- > drivers/net/virtio/virtio_ethdev.h | 2 +- > drivers/net/virtio/virtio_rxtx.c | 2 +- > 3 files changed, 9 insertions(+), 19 deletions(-) > > -- > 2.1.4 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH v2 0/2] virtio: fixes for 2.1-rc1 2015-07-20 18:40 [dpdk-dev] [PATCH v2 0/2] virtio: fixes for 2.1-rc1 Stephen Hemminger ` (2 preceding siblings ...) 2015-07-21 5:00 ` [dpdk-dev] [PATCH v2 0/2] virtio: fixes for 2.1-rc1 Ouyang, Changchun @ 2015-07-22 8:59 ` Thomas Monjalon 3 siblings, 0 replies; 8+ messages in thread From: Thomas Monjalon @ 2015-07-22 8:59 UTC (permalink / raw) To: Stephen Hemminger; +Cc: dev, Stephen Hemminger > This integrates my change and earlier change by Ouyang Changchun > into one fix. And second patch is minor stuff found while reviewing. > > Stephen Hemminger (2): > virtio: fix queue size and number of descriptors > virtio: small cleanups Applied, thanks Please take care of patch numbering to ease patch management. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-07-22 9:00 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-07-20 18:40 [dpdk-dev] [PATCH v2 0/2] virtio: fixes for 2.1-rc1 Stephen Hemminger 2015-07-20 18:40 ` [dpdk-dev] [PATCH v2 1/2] virtio: fix queue size and number of descriptors Stephen Hemminger 2015-07-21 5:06 ` Ouyang, Changchun 2015-07-22 8:55 ` Thomas Monjalon 2015-07-20 18:40 ` [dpdk-dev] [PATCH v2 2/2] virtio: small cleanups Stephen Hemminger 2015-07-21 4:57 ` Ouyang, Changchun 2015-07-21 5:00 ` [dpdk-dev] [PATCH v2 0/2] virtio: fixes for 2.1-rc1 Ouyang, Changchun 2015-07-22 8:59 ` Thomas Monjalon
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).