Message ID | 1429111219-8789-5-git-send-email-stephen@networkplumber.org (mailing list archive) |
---|---|
State | Superseded, archived |
Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 055EDC384; Wed, 15 Apr 2015 17:20:24 +0200 (CEST) Received: from mail-pd0-f178.google.com (mail-pd0-f178.google.com [209.85.192.178]) by dpdk.org (Postfix) with ESMTP id 88C15C370 for <dev@dpdk.org>; Wed, 15 Apr 2015 17:20:20 +0200 (CEST) Received: by pdbqa5 with SMTP id qa5so55938072pdb.1 for <dev@dpdk.org>; Wed, 15 Apr 2015 08:20:20 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=4mZesJ1ec+1p9HYglUuNFZuPA+CcppCtWTsjnRBXRD4=; b=RACB1X5Bn0/AZ3locO3rZSkEVLUjdQbaFNuUEnCjF/0GIl64+G9ZPnKk59DxrkEC7P Ng7NxJbWjrLqRgvDJrtrhUEWivJWTteBkyCbff+Dc9lf3oP6sk5PO8rfqj5i/oXmchFb u7gEBmN9mIDdfWT6riX+yiIv2t3f5r1lQ6/PXX8IJAu0+5MgFGAkEUm02t0As+Mw7tTj gJIYrPCJf16+UJQxIcye/FIbn4nTzyv/4HKiU3bqWav6JAIgKURgG7AMw10ZvZt29Ugk 4CCQVFsoVAKJyxldtXofxwdN+YW2b30uAtWqCPNKwIBsY49y/QaypFi8CC5rTM00hELy vuQw== X-Gm-Message-State: ALoCoQn640/okbKITySgZXnjnGBabHaSiWMs7wOAOI42dTj/wOPKo4zs7wBkB5XlwiQmPt6LYrqQ X-Received: by 10.70.44.100 with SMTP id d4mr47502638pdm.36.1429111219966; Wed, 15 Apr 2015 08:20:19 -0700 (PDT) Received: from urahara.home.lan (static-50-53-82-155.bvtn.or.frontiernet.net. [50.53.82.155]) by mx.google.com with ESMTPSA id sm7sm4459057pac.45.2015.04.15.08.20.18 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Wed, 15 Apr 2015 08:20:19 -0700 (PDT) From: Stephen Hemminger <stephen@networkplumber.org> To: dev@dpdk.org Date: Wed, 15 Apr 2015 08:20:18 -0700 Message-Id: <1429111219-8789-5-git-send-email-stephen@networkplumber.org> X-Mailer: git-send-email 2.1.4 In-Reply-To: <1429111219-8789-1-git-send-email-stephen@networkplumber.org> References: <1429111219-8789-1-git-send-email-stephen@networkplumber.org> Subject: [dpdk-dev] [PATCH 4/5] virtio: fix ring size negotiation X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK <dev.dpdk.org> List-Unsubscribe: <http://dpdk.org/ml/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://dpdk.org/ml/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <http://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
Commit Message
Stephen Hemminger
April 15, 2015, 3:20 p.m. UTC
This fixes another of the issues with running virtio on non-KVM
envirionments. For example, Google Compute Engine reports a
ring size of 16K.
If guest virtio requests more slots than available then
the queue should just be initialized to the smaller value.
Conversely, if the number of descriptors requested exceeds the virtio
host queue size, then just silently use the smaller host size.
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
lib/librte_pmd_virtio/virtio_ethdev.c | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
Comments
> -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen > Hemminger > Sent: Wednesday, April 15, 2015 11:20 PM > To: dev@dpdk.org > Subject: [dpdk-dev] [PATCH 4/5] virtio: fix ring size negotiation > > This fixes another of the issues with running virtio on non-KVM > envirionments. For example, Google Compute Engine reports a ring size of > 16K. > > If guest virtio requests more slots than available then the queue should just I suspect 'more' here should be 'less'? > be initialized to the smaller value. > > Conversely, if the number of descriptors requested exceeds the virtio host > queue size, then just silently use the smaller host size. > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> > --- > lib/librte_pmd_virtio/virtio_ethdev.c | 18 +++++++++++++----- > 1 file changed, 13 insertions(+), 5 deletions(-) > > diff --git a/lib/librte_pmd_virtio/virtio_ethdev.c > b/lib/librte_pmd_virtio/virtio_ethdev.c > index 3cb9c6a..db0232e 100644 > --- a/lib/librte_pmd_virtio/virtio_ethdev.c > +++ b/lib/librte_pmd_virtio/virtio_ethdev.c > @@ -267,13 +267,21 @@ int virtio_dev_queue_setup(struct rte_eth_dev > *dev, > if (vq_size == 0) { > PMD_INIT_LOG(ERR, "%s: virtqueue does not exist", > __func__); > return -EINVAL; > - } else if (!rte_is_power_of_2(vq_size)) { > + } > + > + if (!rte_is_power_of_2(vq_size)) { > PMD_INIT_LOG(ERR, "%s: virtqueue size is not powerof 2", > __func__); > return -EINVAL; > - } else if (nb_desc != vq_size) { > - PMD_INIT_LOG(ERR, "Warning: nb_desc(%d) is not equal to > vq size (%d), fall to vq size", > - nb_desc, vq_size); > - nb_desc = vq_size; > + } > + > + 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; Don't we need a warning when nb_desc > vq_size? > } > > if (queue_type == VTNET_RQ) { > -- > 2.1.4
No warning is needed, it just works. On Wed, Apr 15, 2015 at 8:39 PM, Ouyang, Changchun < changchun.ouyang@intel.com> wrote: > > > > -----Original Message----- > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen > > Hemminger > > Sent: Wednesday, April 15, 2015 11:20 PM > > To: dev@dpdk.org > > Subject: [dpdk-dev] [PATCH 4/5] virtio: fix ring size negotiation > > > > This fixes another of the issues with running virtio on non-KVM > > envirionments. For example, Google Compute Engine reports a ring size of > > 16K. > > > > If guest virtio requests more slots than available then the queue should > just > > I suspect 'more' here should be 'less'? > > > be initialized to the smaller value. > > > > Conversely, if the number of descriptors requested exceeds the virtio > host > > queue size, then just silently use the smaller host size. > > > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> > > --- > > lib/librte_pmd_virtio/virtio_ethdev.c | 18 +++++++++++++----- > > 1 file changed, 13 insertions(+), 5 deletions(-) > > > > diff --git a/lib/librte_pmd_virtio/virtio_ethdev.c > > b/lib/librte_pmd_virtio/virtio_ethdev.c > > index 3cb9c6a..db0232e 100644 > > --- a/lib/librte_pmd_virtio/virtio_ethdev.c > > +++ b/lib/librte_pmd_virtio/virtio_ethdev.c > > @@ -267,13 +267,21 @@ int virtio_dev_queue_setup(struct rte_eth_dev > > *dev, > > if (vq_size == 0) { > > PMD_INIT_LOG(ERR, "%s: virtqueue does not exist", > > __func__); > > return -EINVAL; > > - } else if (!rte_is_power_of_2(vq_size)) { > > + } > > + > > + if (!rte_is_power_of_2(vq_size)) { > > PMD_INIT_LOG(ERR, "%s: virtqueue size is not powerof 2", > > __func__); > > return -EINVAL; > > - } else if (nb_desc != vq_size) { > > - PMD_INIT_LOG(ERR, "Warning: nb_desc(%d) is not equal to > > vq size (%d), fall to vq size", > > - nb_desc, vq_size); > > - nb_desc = vq_size; > > + } > > + > > + 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; > > Don't we need a warning when nb_desc > vq_size? > > > } > > > > if (queue_type == VTNET_RQ) { > > -- > > 2.1.4 > >
From: Stephen Hemminger [mailto:stephen@networkplumber.org] Sent: Thursday, April 16, 2015 1:48 PM To: Ouyang, Changchun Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH 4/5] virtio: fix ring size negotiation No warning is needed, it just works. I know it works, but the upper user don’t know the descriptor number is reduced. I concern it is not so user-friendly here. On Wed, Apr 15, 2015 at 8:39 PM, Ouyang, Changchun <changchun.ouyang@intel.com<mailto:changchun.ouyang@intel.com>> wrote: > -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org<mailto:dev-bounces@dpdk.org>] On Behalf Of Stephen > Hemminger > Sent: Wednesday, April 15, 2015 11:20 PM > To: dev@dpdk.org<mailto:dev@dpdk.org> > Subject: [dpdk-dev] [PATCH 4/5] virtio: fix ring size negotiation > > This fixes another of the issues with running virtio on non-KVM > envirionments. For example, Google Compute Engine reports a ring size of > 16K. > > If guest virtio requests more slots than available then the queue should just I suspect 'more' here should be 'less'? > be initialized to the smaller value. > > Conversely, if the number of descriptors requested exceeds the virtio host > queue size, then just silently use the smaller host size. > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org<mailto:stephen@networkplumber.org>> > --- > lib/librte_pmd_virtio/virtio_ethdev.c | 18 +++++++++++++----- > 1 file changed, 13 insertions(+), 5 deletions(-) > > diff --git a/lib/librte_pmd_virtio/virtio_ethdev.c > b/lib/librte_pmd_virtio/virtio_ethdev.c > index 3cb9c6a..db0232e 100644 > --- a/lib/librte_pmd_virtio/virtio_ethdev.c > +++ b/lib/librte_pmd_virtio/virtio_ethdev.c > @@ -267,13 +267,21 @@ int virtio_dev_queue_setup(struct rte_eth_dev > *dev, > if (vq_size == 0) { > PMD_INIT_LOG(ERR, "%s: virtqueue does not exist", > __func__); > return -EINVAL; > - } else if (!rte_is_power_of_2(vq_size)) { > + } > + > + if (!rte_is_power_of_2(vq_size)) { > PMD_INIT_LOG(ERR, "%s: virtqueue size is not powerof 2", > __func__); > return -EINVAL; > - } else if (nb_desc != vq_size) { > - PMD_INIT_LOG(ERR, "Warning: nb_desc(%d) is not equal to > vq size (%d), fall to vq size", > - nb_desc, vq_size); > - nb_desc = vq_size; > + } > + > + 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; Don't we need a warning when nb_desc > vq_size? > } > > if (queue_type == VTNET_RQ) { > -- > 2.1.4
Guys, this is an example of what should not be done with emails formatting. 2015-04-16 06:26, Ouyang, Changchun: > > From: Stephen Hemminger [mailto:stephen@networkplumber.org] > Sent: Thursday, April 16, 2015 1:48 PM > To: Ouyang, Changchun > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH 4/5] virtio: fix ring size negotiation This header is partly useless. > > No warning is needed, it just works. Stephen, please do not top post. It makes harder to find what you are commenting. > > I know it works, but the upper user don’t know the descriptor number is reduced. > I concern it is not so user-friendly here. Changchun, please be sure the above text is quoted. How are we supposed to understand who is speaking here? All, please, take care of readers. Thanks > On Wed, Apr 15, 2015 at 8:39 PM, Ouyang, Changchun <changchun.ouyang@intel.com<mailto:changchun.ouyang@intel.com>> wrote: > > > > -----Original Message----- > > From: dev [mailto:dev-bounces@dpdk.org<mailto:dev-bounces@dpdk.org>] On Behalf Of Stephen > > Hemminger > > Sent: Wednesday, April 15, 2015 11:20 PM > > To: dev@dpdk.org<mailto:dev@dpdk.org> > > Subject: [dpdk-dev] [PATCH 4/5] virtio: fix ring size negotiation > > > > This fixes another of the issues with running virtio on non-KVM > > envirionments. For example, Google Compute Engine reports a ring size of > > 16K. > > > > If guest virtio requests more slots than available then the queue should just > > I suspect 'more' here should be 'less'? > > > be initialized to the smaller value. > > > > Conversely, if the number of descriptors requested exceeds the virtio host > > queue size, then just silently use the smaller host size. > > > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org<mailto:stephen@networkplumber.org>> > > --- > > lib/librte_pmd_virtio/virtio_ethdev.c | 18 +++++++++++++----- > > 1 file changed, 13 insertions(+), 5 deletions(-) > > > > diff --git a/lib/librte_pmd_virtio/virtio_ethdev.c > > b/lib/librte_pmd_virtio/virtio_ethdev.c > > index 3cb9c6a..db0232e 100644 > > --- a/lib/librte_pmd_virtio/virtio_ethdev.c > > +++ b/lib/librte_pmd_virtio/virtio_ethdev.c > > @@ -267,13 +267,21 @@ int virtio_dev_queue_setup(struct rte_eth_dev > > *dev, > > if (vq_size == 0) { > > PMD_INIT_LOG(ERR, "%s: virtqueue does not exist", > > __func__); > > return -EINVAL; > > - } else if (!rte_is_power_of_2(vq_size)) { > > + } > > + > > + if (!rte_is_power_of_2(vq_size)) { > > PMD_INIT_LOG(ERR, "%s: virtqueue size is not powerof 2", > > __func__); > > return -EINVAL; > > - } else if (nb_desc != vq_size) { > > - PMD_INIT_LOG(ERR, "Warning: nb_desc(%d) is not equal to > > vq size (%d), fall to vq size", > > - nb_desc, vq_size); > > - nb_desc = vq_size; > > + } > > + > > + 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; > Don't we need a warning when nb_desc > vq_size? > > > } > > > > if (queue_type == VTNET_RQ) { > > -- > > 2.1.4 > >
On Thu, 16 Apr 2015 09:38:46 +0200
Thomas Monjalon <thomas.monjalon@6wind.com> wrote:
> Guys, this is an example of what should not be done with emails formatting.
Sorry, Google mail client for Android seems to encourage
bad formatting.
On Thu, 16 Apr 2015 06:26:02 +0000 "Ouyang, Changchun" <changchun.ouyang@intel.com> wrote: > > > From: Stephen Hemminger [mailto:stephen@networkplumber.org] > Sent: Thursday, April 16, 2015 1:48 PM > To: Ouyang, Changchun > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH 4/5] virtio: fix ring size negotiation > > No warning is needed, it just works. > > I know it works, but the upper user don’t know the descriptor number is reduced. > I concern it is not so user-friendly here. > > > On Wed, Apr 15, 2015 at 8:39 PM, Ouyang, Changchun <changchun.ouyang@intel.com<mailto:changchun.ouyang@intel.com>> wrote: > > > > -----Original Message----- > > From: dev [mailto:dev-bounces@dpdk.org<mailto:dev-bounces@dpdk.org>] On Behalf Of Stephen > > Hemminger > > Sent: Wednesday, April 15, 2015 11:20 PM > > To: dev@dpdk.org<mailto:dev@dpdk.org> > > Subject: [dpdk-dev] [PATCH 4/5] virtio: fix ring size negotiation > > > > This fixes another of the issues with running virtio on non-KVM > > envirionments. For example, Google Compute Engine reports a ring size of > > 16K. > > > > If guest virtio requests more slots than available then the queue should just > > I suspect 'more' here should be 'less'? > > > be initialized to the smaller value. > > > > Conversely, if the number of descriptors requested exceeds the virtio host > > queue size, then just silently use the smaller host size. > > > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org<mailto:stephen@networkplumber.org>> > > --- > > lib/librte_pmd_virtio/virtio_ethdev.c | 18 +++++++++++++----- > > 1 file changed, 13 insertions(+), 5 deletions(-) > > > > diff --git a/lib/librte_pmd_virtio/virtio_ethdev.c > > b/lib/librte_pmd_virtio/virtio_ethdev.c > > index 3cb9c6a..db0232e 100644 > > --- a/lib/librte_pmd_virtio/virtio_ethdev.c > > +++ b/lib/librte_pmd_virtio/virtio_ethdev.c > > @@ -267,13 +267,21 @@ int virtio_dev_queue_setup(struct rte_eth_dev > > *dev, > > if (vq_size == 0) { > > PMD_INIT_LOG(ERR, "%s: virtqueue does not exist", > > __func__); > > return -EINVAL; > > - } else if (!rte_is_power_of_2(vq_size)) { > > + } > > + > > + if (!rte_is_power_of_2(vq_size)) { > > PMD_INIT_LOG(ERR, "%s: virtqueue size is not powerof 2", > > __func__); > > return -EINVAL; > > - } else if (nb_desc != vq_size) { > > - PMD_INIT_LOG(ERR, "Warning: nb_desc(%d) is not equal to > > vq size (%d), fall to vq size", > > - nb_desc, vq_size); > > - nb_desc = vq_size; > > + } > > + > > + 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; > Don't we need a warning when nb_desc > vq_size? No warning is needed. This will actually be a common case for many applications. IMHO application should not have to worry about what type of network device it is running on and therefore would likely pass a reasonably large number of receive descriptors (say 512) and since the default KVM/QEMU ring size is 256, the receive queue would be limited by host not application. The whole idea of application passing number of receive descriptors to DPDK is bogus because each device driver has different timing, and may use different number of receive descriptors per packet.
diff --git a/lib/librte_pmd_virtio/virtio_ethdev.c b/lib/librte_pmd_virtio/virtio_ethdev.c index 3cb9c6a..db0232e 100644 --- a/lib/librte_pmd_virtio/virtio_ethdev.c +++ b/lib/librte_pmd_virtio/virtio_ethdev.c @@ -267,13 +267,21 @@ int virtio_dev_queue_setup(struct rte_eth_dev *dev, if (vq_size == 0) { PMD_INIT_LOG(ERR, "%s: virtqueue does not exist", __func__); return -EINVAL; - } else if (!rte_is_power_of_2(vq_size)) { + } + + if (!rte_is_power_of_2(vq_size)) { PMD_INIT_LOG(ERR, "%s: virtqueue size is not powerof 2", __func__); return -EINVAL; - } else if (nb_desc != vq_size) { - PMD_INIT_LOG(ERR, "Warning: nb_desc(%d) is not equal to vq size (%d), fall to vq size", - nb_desc, vq_size); - nb_desc = vq_size; + } + + 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) {