Re: [Xen-devel] [PATCH net-next] xen-netfront: always set num queues if possible

2015-09-15 Thread David Vrabel
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(>queues[0], , 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

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Xen-devel] [PATCH net-next] xen-netfront: always set num queues if possible

2015-09-15 Thread Charles (Chas) Williams
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(>queues[0], , 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.


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH net-next] xen-netfront: always set num queues if possible

2015-09-15 Thread Charles (Chas) Williams
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 <3ch...@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(>queues[0], , 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(>queues[0], , 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 = >queues[i];
-- 
2.1.0



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH net-next] xen-netfront: always set num queues if possible

2015-09-14 Thread Charles (Chas) Williams
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 <3ch...@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(>queues[0], , 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(>queues[0], , 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 = >queues[i];
-- 
2.1.0



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html