diff mbox

[ovs-dev] netdev: Set the default number of queues at removal from the database

Message ID 1481187331-20575-1-git-send-email-i.maximets@samsung.com
State Accepted
Delegated to: Daniele Di Proietto
Headers show

Commit Message

Ilya Maximets Dec. 8, 2016, 8:55 a.m. UTC
Expected behavior for attribute removal from the database is
resetting it to default value. Currently this doesn't work for
n_rxq/n_txq options of pmd netdevs (last requested value used):

	# ovs-vsctl set interface dpdk0 options:n_rxq=4
	# ovs-vsctl remove interface dpdk0 options n_rxq
	# ovs-appctl dpif/show | grep dpdk0
	  <...>
	  dpdk0 1/1: (dpdk: configured_rx_queues=4, <...> \
	                    requested_rx_queues=4,  <...>)

Fix that by using NR_QUEUE or 1 as a default value for 'smap_get_int'.

Fixes: a14b8947fd13 ("dpif-netdev: Allow different numbers of
                      rx queues for different ports.")
Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
---
 lib/netdev-dpdk.c  | 2 +-
 lib/netdev-dummy.c | 4 ++--
 tests/pmd.at       | 7 +++++++
 3 files changed, 10 insertions(+), 3 deletions(-)

Comments

Stokes, Ian Dec. 8, 2016, 8:18 p.m. UTC | #1
> Expected behavior for attribute removal from the database is resetting it
> to default value. Currently this doesn't work for n_rxq/n_txq options of
> pmd netdevs (last requested value used):
> 
> 	# ovs-vsctl set interface dpdk0 options:n_rxq=4
> 	# ovs-vsctl remove interface dpdk0 options n_rxq
> 	# ovs-appctl dpif/show | grep dpdk0
> 	  <...>
> 	  dpdk0 1/1: (dpdk: configured_rx_queues=4, <...> \
> 	                    requested_rx_queues=4,  <...>)
> 
> Fix that by using NR_QUEUE or 1 as a default value for 'smap_get_int'.
> 
> Fixes: a14b8947fd13 ("dpif-netdev: Allow different numbers of
>                       rx queues for different ports.")
> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
> ---
>  lib/netdev-dpdk.c  | 2 +-
>  lib/netdev-dummy.c | 4 ++--
>  tests/pmd.at       | 7 +++++++
>  3 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 61d7aa3..625f425
> 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -1084,7 +1084,7 @@ dpdk_set_rxq_config(struct netdev_dpdk *dev, const
> struct smap *args)  {
>      int new_n_rxq;
> 
> -    new_n_rxq = MAX(smap_get_int(args, "n_rxq", dev->requested_n_rxq),
> 1);
> +    new_n_rxq = MAX(smap_get_int(args, "n_rxq", NR_QUEUE), 1);
>      if (new_n_rxq != dev->requested_n_rxq) {
>          dev->requested_n_rxq = new_n_rxq;
>          netdev_request_reconfigure(&dev->up);
> diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c index
> dec1a8e..de74846 100644
> --- a/lib/netdev-dummy.c
> +++ b/lib/netdev-dummy.c
> @@ -868,8 +868,8 @@ netdev_dummy_set_config(struct netdev *netdev_, const
> struct smap *args)
>          goto exit;
>      }
> 
> -    new_n_rxq = MAX(smap_get_int(args, "n_rxq", netdev->requested_n_rxq),
> 1);
> -    new_n_txq = MAX(smap_get_int(args, "n_txq", netdev->requested_n_txq),
> 1);
> +    new_n_rxq = MAX(smap_get_int(args, "n_rxq", 1), 1);
> +    new_n_txq = MAX(smap_get_int(args, "n_txq", 1), 1);
>      new_numa_id = smap_get_int(args, "numa_id", 0);
>      if (new_n_rxq != netdev->requested_n_rxq
>          || new_n_txq != netdev->requested_n_txq diff --git a/tests/pmd.at
> b/tests/pmd.at index 8f05d74..7d3fa0d 100644
> --- a/tests/pmd.at
> +++ b/tests/pmd.at
> @@ -259,6 +259,13 @@ NXT_PACKET_IN2 (xid=0x0): cookie=0x0 total_len=42
> in_port=1 (via action) data_le
> 
> icmp,vlan_tci=0x0000,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,nw_
> src=10.0.0.2,nw_dst=10.0.0.1,nw_tos=0,nw_ecn=0,nw_ttl=64,icmp_type=8,icmp_
> code=0 icmp_csum:f7ff
>  ])
> 
> +dnl Check resetting to default number of rx queues after removal from the
> db.
> +AT_CHECK([ovs-vsctl remove interface p1 options n_rxq])
> +
> +AT_CHECK([ovs-appctl dpif/show | grep p1 | sed 's/\(tx_queues=\)[[0-
> 9]]*/\1<cleared>/g'], [0], [dnl
> +		p1 1/1: (dummy-pmd: configured_rx_queues=1,
> +configured_tx_queues=<cleared>, requested_rx_queues=1,
> +requested_tx_queues=<cleared>)
> +])
> +
>  OVS_VSWITCHD_STOP
>  AT_CLEANUP
> 

LGTM.

I'll wait for Daniele to give the ACK on this but as an FYI I've tested and it worked without issue.

Tested-by: Ian Stokes <ian.stokes@intel.com>

Ian

> --
> 2.7.4
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Daniele Di Proietto Dec. 10, 2016, 2:50 a.m. UTC | #2
On 08/12/2016 12:18, "Stokes, Ian" <ian.stokes@intel.com> wrote:

>> Expected behavior for attribute removal from the database is resetting it
>> to default value. Currently this doesn't work for n_rxq/n_txq options of
>> pmd netdevs (last requested value used):
>> 
>> 	# ovs-vsctl set interface dpdk0 options:n_rxq=4
>> 	# ovs-vsctl remove interface dpdk0 options n_rxq
>> 	# ovs-appctl dpif/show | grep dpdk0
>> 	  <...>
>> 	  dpdk0 1/1: (dpdk: configured_rx_queues=4, <...> \
>> 	                    requested_rx_queues=4,  <...>)
>> 
>> Fix that by using NR_QUEUE or 1 as a default value for 'smap_get_int'.
>> 
>> Fixes: a14b8947fd13 ("dpif-netdev: Allow different numbers of
>>                       rx queues for different ports.")
>> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
>> ---
>>  lib/netdev-dpdk.c  | 2 +-
>>  lib/netdev-dummy.c | 4 ++--
>>  tests/pmd.at       | 7 +++++++
>>  3 files changed, 10 insertions(+), 3 deletions(-)
>> 
>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 61d7aa3..625f425
>> 100644
>> --- a/lib/netdev-dpdk.c
>> +++ b/lib/netdev-dpdk.c
>> @@ -1084,7 +1084,7 @@ dpdk_set_rxq_config(struct netdev_dpdk *dev, const
>> struct smap *args)  {
>>      int new_n_rxq;
>> 
>> -    new_n_rxq = MAX(smap_get_int(args, "n_rxq", dev->requested_n_rxq),
>> 1);
>> +    new_n_rxq = MAX(smap_get_int(args, "n_rxq", NR_QUEUE), 1);
>>      if (new_n_rxq != dev->requested_n_rxq) {
>>          dev->requested_n_rxq = new_n_rxq;
>>          netdev_request_reconfigure(&dev->up);
>> diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c index
>> dec1a8e..de74846 100644
>> --- a/lib/netdev-dummy.c
>> +++ b/lib/netdev-dummy.c
>> @@ -868,8 +868,8 @@ netdev_dummy_set_config(struct netdev *netdev_, const
>> struct smap *args)
>>          goto exit;
>>      }
>> 
>> -    new_n_rxq = MAX(smap_get_int(args, "n_rxq", netdev->requested_n_rxq),
>> 1);
>> -    new_n_txq = MAX(smap_get_int(args, "n_txq", netdev->requested_n_txq),
>> 1);
>> +    new_n_rxq = MAX(smap_get_int(args, "n_rxq", 1), 1);
>> +    new_n_txq = MAX(smap_get_int(args, "n_txq", 1), 1);
>>      new_numa_id = smap_get_int(args, "numa_id", 0);
>>      if (new_n_rxq != netdev->requested_n_rxq
>>          || new_n_txq != netdev->requested_n_txq diff --git a/tests/pmd.at
>> b/tests/pmd.at index 8f05d74..7d3fa0d 100644
>> --- a/tests/pmd.at
>> +++ b/tests/pmd.at
>> @@ -259,6 +259,13 @@ NXT_PACKET_IN2 (xid=0x0): cookie=0x0 total_len=42
>> in_port=1 (via action) data_le
>> 
>> icmp,vlan_tci=0x0000,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,nw_
>> src=10.0.0.2,nw_dst=10.0.0.1,nw_tos=0,nw_ecn=0,nw_ttl=64,icmp_type=8,icmp_
>> code=0 icmp_csum:f7ff
>>  ])
>> 
>> +dnl Check resetting to default number of rx queues after removal from the
>> db.
>> +AT_CHECK([ovs-vsctl remove interface p1 options n_rxq])
>> +
>> +AT_CHECK([ovs-appctl dpif/show | grep p1 | sed 's/\(tx_queues=\)[[0-
>> 9]]*/\1<cleared>/g'], [0], [dnl
>> +		p1 1/1: (dummy-pmd: configured_rx_queues=1,
>> +configured_tx_queues=<cleared>, requested_rx_queues=1,
>> +requested_tx_queues=<cleared>)
>> +])
>> +
>>  OVS_VSWITCHD_STOP
>>  AT_CLEANUP
>> 
>
>LGTM.
>
>I'll wait for Daniele to give the ACK on this but as an FYI I've tested and it worked without issue.
>
>Tested-by: Ian Stokes <ian.stokes@intel.com>

Thanks!

I've pushed this to master and branch-2.6

>
>Ian
>
>> --
>> 2.7.4
>> 
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff mbox

Patch

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 61d7aa3..625f425 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -1084,7 +1084,7 @@  dpdk_set_rxq_config(struct netdev_dpdk *dev, const struct smap *args)
 {
     int new_n_rxq;
 
-    new_n_rxq = MAX(smap_get_int(args, "n_rxq", dev->requested_n_rxq), 1);
+    new_n_rxq = MAX(smap_get_int(args, "n_rxq", NR_QUEUE), 1);
     if (new_n_rxq != dev->requested_n_rxq) {
         dev->requested_n_rxq = new_n_rxq;
         netdev_request_reconfigure(&dev->up);
diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c
index dec1a8e..de74846 100644
--- a/lib/netdev-dummy.c
+++ b/lib/netdev-dummy.c
@@ -868,8 +868,8 @@  netdev_dummy_set_config(struct netdev *netdev_, const struct smap *args)
         goto exit;
     }
 
-    new_n_rxq = MAX(smap_get_int(args, "n_rxq", netdev->requested_n_rxq), 1);
-    new_n_txq = MAX(smap_get_int(args, "n_txq", netdev->requested_n_txq), 1);
+    new_n_rxq = MAX(smap_get_int(args, "n_rxq", 1), 1);
+    new_n_txq = MAX(smap_get_int(args, "n_txq", 1), 1);
     new_numa_id = smap_get_int(args, "numa_id", 0);
     if (new_n_rxq != netdev->requested_n_rxq
         || new_n_txq != netdev->requested_n_txq
diff --git a/tests/pmd.at b/tests/pmd.at
index 8f05d74..7d3fa0d 100644
--- a/tests/pmd.at
+++ b/tests/pmd.at
@@ -259,6 +259,13 @@  NXT_PACKET_IN2 (xid=0x0): cookie=0x0 total_len=42 in_port=1 (via action) data_le
 icmp,vlan_tci=0x0000,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,nw_src=10.0.0.2,nw_dst=10.0.0.1,nw_tos=0,nw_ecn=0,nw_ttl=64,icmp_type=8,icmp_code=0 icmp_csum:f7ff
 ])
 
+dnl Check resetting to default number of rx queues after removal from the db.
+AT_CHECK([ovs-vsctl remove interface p1 options n_rxq])
+
+AT_CHECK([ovs-appctl dpif/show | grep p1 | sed 's/\(tx_queues=\)[[0-9]]*/\1<cleared>/g'], [0], [dnl
+		p1 1/1: (dummy-pmd: configured_rx_queues=1, configured_tx_queues=<cleared>, requested_rx_queues=1, requested_tx_queues=<cleared>)
+])
+
 OVS_VSWITCHD_STOP
 AT_CLEANUP