netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next] xen-netfront: always set num queues if possible
@ 2015-09-14 21:28 Charles (Chas) Williams
  2015-09-15 10:59 ` [Xen-devel] " David Vrabel
  0 siblings, 1 reply; 6+ messages in thread
From: Charles (Chas) Williams @ 2015-09-14 21:28 UTC (permalink / raw)
  To: netdev, xen-devel

The xen store preserves this information across module invocations.
If you insmod netfront with two queues and later insmod again with one
queue, the backend will still believe you asked for two queues.

Signed-off-by: Chas Williams <3chas3@gmail.com>
---
 drivers/net/xen-netfront.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index f821a97..b53a681 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -1819,11 +1819,7 @@ again:
 		goto destroy_ring;
 	}
 
-	if (num_queues == 1) {
-		err = write_queue_xenstore_keys(&info->queues[0], &xbt, 0); /* flat */
-		if (err)
-			goto abort_transaction_no_dev_fatal;
-	} else {
+	if (xenbus_exists(xbt, dev->nodename, "multi-queue-num-queues")) {
 		/* Write the number of queues */
 		err = xenbus_printf(xbt, dev->nodename, "multi-queue-num-queues",
 				    "%u", num_queues);
@@ -1831,7 +1827,13 @@ again:
 			message = "writing multi-queue-num-queues";
 			goto abort_transaction_no_dev_fatal;
 		}
+	}
 
+	if (num_queues == 1) {
+		err = write_queue_xenstore_keys(&info->queues[0], &xbt, 0); /* flat */
+		if (err)
+			goto abort_transaction_no_dev_fatal;
+	} else {
 		/* Write the keys for each queue */
 		for (i = 0; i < num_queues; ++i) {
 			queue = &info->queues[i];
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [Xen-devel] [PATCH net-next] xen-netfront: always set num queues if possible
  2015-09-14 21:28 [PATCH net-next] xen-netfront: always set num queues if possible Charles (Chas) Williams
@ 2015-09-15 10:59 ` David Vrabel
  2015-09-15 14:37   ` Charles (Chas) Williams
  2015-09-16 20:28   ` [PATCH net-next v2] " Charles (Chas) Williams
  0 siblings, 2 replies; 6+ messages in thread
From: David Vrabel @ 2015-09-15 10:59 UTC (permalink / raw)
  To: Charles (Chas) Williams, netdev, xen-devel

On 14/09/15 22:28, Charles (Chas) Williams wrote:
> The xen store preserves this information across module invocations.
> If you insmod netfront with two queues and later insmod again with one
> queue, the backend will still believe you asked for two queues.

Can you rewrite the commit message to be clearer?

"If netfront connects with 2 (or more) queues and then reconnects with
only 1 queue it fails to delete or rewrite the multi-queue-num-queues
key and netback will try to use the wrong number of queues.

Always write the num-queues field if the backend has multi-queue support."

> --- a/drivers/net/xen-netfront.c
> +++ b/drivers/net/xen-netfront.c
> @@ -1819,11 +1819,7 @@ again:
>  		goto destroy_ring;
>  	}
>  
> -	if (num_queues == 1) {
> -		err = write_queue_xenstore_keys(&info->queues[0], &xbt, 0); /* flat */
> -		if (err)
> -			goto abort_transaction_no_dev_fatal;
> -	} else {
> +	if (xenbus_exists(xbt, dev->nodename, "multi-queue-num-queues")) {
>  		/* Write the number of queues */
>  		err = xenbus_printf(xbt, dev->nodename, "multi-queue-num-queues",
>  				    "%u", num_queues);

Isn't this broken?  It looks like it won't write the
multi-queue-num-queues key the first time around.

It think this should be conditional on multi-queue-max-queues existing
(which is written by the backend if multi-queue is supported).

David

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Xen-devel] [PATCH net-next] xen-netfront: always set num queues if possible
  2015-09-15 10:59 ` [Xen-devel] " David Vrabel
@ 2015-09-15 14:37   ` Charles (Chas) Williams
  2015-09-16 20:28   ` [PATCH net-next v2] " Charles (Chas) Williams
  1 sibling, 0 replies; 6+ messages in thread
From: Charles (Chas) Williams @ 2015-09-15 14:37 UTC (permalink / raw)
  To: David Vrabel; +Cc: netdev, xen-devel

On Tue, 2015-09-15 at 11:59 +0100, David Vrabel wrote:
> On 14/09/15 22:28, Charles (Chas) Williams wrote:
> > The xen store preserves this information across module invocations.
> > If you insmod netfront with two queues and later insmod again with one
> > queue, the backend will still believe you asked for two queues.
> 
> Can you rewrite the commit message to be clearer?
> 
> "If netfront connects with 2 (or more) queues and then reconnects with
> only 1 queue it fails to delete or rewrite the multi-queue-num-queues
> key and netback will try to use the wrong number of queues.
> 
> Always write the num-queues field if the backend has multi-queue support."

Yes I can do that.

> 
> > --- a/drivers/net/xen-netfront.c
> > +++ b/drivers/net/xen-netfront.c
> > @@ -1819,11 +1819,7 @@ again:
> >  		goto destroy_ring;
> >  	}
> >  
> > -	if (num_queues == 1) {
> > -		err = write_queue_xenstore_keys(&info->queues[0], &xbt, 0); /* flat */
> > -		if (err)
> > -			goto abort_transaction_no_dev_fatal;
> > -	} else {
> > +	if (xenbus_exists(xbt, dev->nodename, "multi-queue-num-queues")) {
> >  		/* Write the number of queues */
> >  		err = xenbus_printf(xbt, dev->nodename, "multi-queue-num-queues",
> >  				    "%u", num_queues);
> 
> Isn't this broken?  It looks like it won't write the
> multi-queue-num-queues key the first time around.
> 
> It think this should be conditional on multi-queue-max-queues existing
> (which is written by the backend if multi-queue is supported).

You are right.  I am testing against the wrong key here.  I should have
tested for multi-queue-max-queues.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH net-next] xen-netfront: always set num queues if possible
@ 2015-09-15 14:49 Charles (Chas) Williams
  0 siblings, 0 replies; 6+ messages in thread
From: Charles (Chas) Williams @ 2015-09-15 14:49 UTC (permalink / raw)
  To: netdev, xen-devel

If netfront connects with two (or more) queues and then reconnects with
only one queue it fails to delete or rewrite the multi-queue-num-queues
key and netback will try to use the wrong number of queues.

Always write the num-queues field if the backend has multi-queue support.

Signed-off-by: Chas Williams <3chas3@gmail.com>
---
 drivers/net/xen-netfront.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index f821a97..7f8b7bd 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -1819,11 +1819,7 @@ again:
 		goto destroy_ring;
 	}
 
-	if (num_queues == 1) {
-		err = write_queue_xenstore_keys(&info->queues[0], &xbt, 0); /* flat */
-		if (err)
-			goto abort_transaction_no_dev_fatal;
-	} else {
+	if (xenbus_exists(xbt, dev->nodename, "multi-queue-max-queues")) {
 		/* Write the number of queues */
 		err = xenbus_printf(xbt, dev->nodename, "multi-queue-num-queues",
 				    "%u", num_queues);
@@ -1831,7 +1827,13 @@ again:
 			message = "writing multi-queue-num-queues";
 			goto abort_transaction_no_dev_fatal;
 		}
+	}
 
+	if (num_queues == 1) {
+		err = write_queue_xenstore_keys(&info->queues[0], &xbt, 0); /* flat */
+		if (err)
+			goto abort_transaction_no_dev_fatal;
+	} else {
 		/* Write the keys for each queue */
 		for (i = 0; i < num_queues; ++i) {
 			queue = &info->queues[i];
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH net-next v2] xen-netfront: always set num queues if possible
  2015-09-15 10:59 ` [Xen-devel] " David Vrabel
  2015-09-15 14:37   ` Charles (Chas) Williams
@ 2015-09-16 20:28   ` Charles (Chas) Williams
  2015-09-21  4:39     ` David Miller
  1 sibling, 1 reply; 6+ messages in thread
From: Charles (Chas) Williams @ 2015-09-16 20:28 UTC (permalink / raw)
  To: David Vrabel; +Cc: netdev, xen-devel

If netfront connects with two (or more) queues and then reconnects with
only one queue it fails to delete or rewrite the multi-queue-num-queues
key and netback will try to use the wrong number of queues.

Always write the num-queues field if the backend has multi-queue support.

Signed-off-by: Chas Williams <3chas3@gmail.com>
---
 drivers/net/xen-netfront.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index f821a97..9bf63c2 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -1819,19 +1819,22 @@ again:
 		goto destroy_ring;
 	}
 
-	if (num_queues == 1) {
-		err = write_queue_xenstore_keys(&info->queues[0], &xbt, 0); /* flat */
-		if (err)
-			goto abort_transaction_no_dev_fatal;
-	} else {
+	if (xenbus_exists(XBT_NIL,
+			  info->xbdev->otherend, "multi-queue-max-queues")) {
 		/* Write the number of queues */
-		err = xenbus_printf(xbt, dev->nodename, "multi-queue-num-queues",
-				    "%u", num_queues);
+		err = xenbus_printf(xbt, dev->nodename,
+				    "multi-queue-num-queues", "%u", num_queues);
 		if (err) {
 			message = "writing multi-queue-num-queues";
 			goto abort_transaction_no_dev_fatal;
 		}
+	}
 
+	if (num_queues == 1) {
+		err = write_queue_xenstore_keys(&info->queues[0], &xbt, 0); /* flat */
+		if (err)
+			goto abort_transaction_no_dev_fatal;
+	} else {
 		/* Write the keys for each queue */
 		for (i = 0; i < num_queues; ++i) {
 			queue = &info->queues[i];
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next v2] xen-netfront: always set num queues if possible
  2015-09-16 20:28   ` [PATCH net-next v2] " Charles (Chas) Williams
@ 2015-09-21  4:39     ` David Miller
  0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2015-09-21  4:39 UTC (permalink / raw)
  To: 3chas3; +Cc: david.vrabel, netdev, xen-devel

From: "Charles (Chas) Williams" <3chas3@gmail.com>
Date: Wed, 16 Sep 2015 16:28:25 -0400

> If netfront connects with two (or more) queues and then reconnects with
> only one queue it fails to delete or rewrite the multi-queue-num-queues
> key and netback will try to use the wrong number of queues.
> 
> Always write the num-queues field if the backend has multi-queue support.
> 
> Signed-off-by: Chas Williams <3chas3@gmail.com>

Applied, thanks Chas.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2015-09-21  4:39 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-14 21:28 [PATCH net-next] xen-netfront: always set num queues if possible Charles (Chas) Williams
2015-09-15 10:59 ` [Xen-devel] " David Vrabel
2015-09-15 14:37   ` Charles (Chas) Williams
2015-09-16 20:28   ` [PATCH net-next v2] " Charles (Chas) Williams
2015-09-21  4:39     ` David Miller
  -- strict thread matches above, loose matches on Subject: below --
2015-09-15 14:49 [PATCH net-next] " Charles (Chas) Williams

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).