[ovs-dev] [PATCH v4] dpif-netdev: Move setting of queue number to netdev layer.

Ilya Maximets i.maximets at samsung.com
Fri Jul 8 13:58:09 UTC 2016


Hi,
Thanks for review.

On 08.07.2016 05:19, Daniele Di Proietto wrote:
> Thanks for the patch, I think this moves in the right direction.
> 
> I like how this patch removes "real_n_txq", as you pointed out
> it was confusing and, as proven here, unnecessary.
> 
> I don't like very much that the netdev implementation decides
> to create max_tx_queue_id + 1 queues.  I still think the
> request should come from the datapath with netdev_dpdk_set_tx_multiq().
> 
> How about this?
> 
> * For phy devices netdev_dpdk_set_tx_multiq() stays as it is.
>   requested_n_txq is coming only from the datapath
> * For vhost devices netdev_dpdk_set_tx_multiq() becomes a no-op, because
>   it doesn't matter how many queues the datapath requests, we're locking
>   on every transmission anyway.  requested_n_txq is coming only from the
>   new_device() callback.

I've sent v5 with this changes.

> 
> Other than this I tested the patch and it appears to work, at least the
> automatic assignment of the number of queues from qemu, so thanks!
> 
> 
> 
> On 07/07/2016 06:58, "Ilya Maximets" <i.maximets at samsung.com> wrote:
> 
>> Currently, there are few inconsistencies between dpif-netdev
>> and netdev layers:
>>
>> 	* dpif-netdev can't know about exact number of tx queues
>> 	  allocated inside netdev.
>> 	  This leads to constant mapping of queue-ids to 'real' ones.
> 
> Now n_txq is always the real number of transmission queues in the device.
> It think this is an improvement.
> 
>>
>> 	* dpif-netdev is able to change number of tx queues while
>> 	  it knows nothing about real hardware or number of queues
>> 	  allocated in VM.
>> 	  This leads to complications in reconfiguration of vhost-user
>> 	  ports, because setting of 'n_txq' from different sources
>> 	  (dpif-netdev and 'new_device()' call) requires additional
>> 	  sychronization between this two layers.
> 
> I suggested above a way to avoid this synchronization problem while
> maintaining the netdev_dpdk_set_tx_multiq() call.
> 
>>
>> Also: We are able to configure 'n_rxq' for vhost-user devices, but
>>      there is only one sane number of rx queues which must be used and
>>      configured manually (number of queues that allocated in QEMU).
>>
>> This patch moves all configuration of queues to netdev layer and disables
>> configuration of 'n_rxq' for vhost devices.
>>
>> Configuration of rx and tx queues now automatically applied from
>> connected virtio device. Standard reconfiguration mechanism was used to
>> apply this changes.
>>
>> Number of tx queues by default set to 'n_cores + 1' for physical ports
>> and old 'needs_locking' logic preserved.
>>
>> For dummy-pmd ports new undocumented option 'n_txq' introduced to
>> configure number of tx queues.
>>
>> Ex.:
>> 	ovs-vsctl set interface dummy-pmd0 options:n_txq=32
>>
>> Signed-off-by: Ilya Maximets <i.maximets at samsung.com>
>> ---
>> INSTALL.DPDK-ADVANCED.md |  26 +++-----
>> NEWS                     |   2 +
>> lib/dpif-netdev.c        |  31 ++-------
>> lib/netdev-bsd.c         |   1 -
>> lib/netdev-dpdk.c        | 162 +++++++++++++++++++----------------------------
>> lib/netdev-dummy.c       |  31 ++-------
>> lib/netdev-linux.c       |   1 -
>> lib/netdev-provider.h    |  16 -----
>> lib/netdev-vport.c       |   1 -
>> lib/netdev.c             |  30 ---------
>> lib/netdev.h             |   1 -
>> vswitchd/vswitch.xml     |   3 +-
>> 12 files changed, 90 insertions(+), 215 deletions(-)
> 
> [...]
> 
>>
>> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
>> index 072fef4..a32f4ef 100644
>> --- a/vswitchd/vswitch.xml
>> +++ b/vswitchd/vswitch.xml
>> @@ -2346,12 +2346,13 @@
>>         Only PMD netdevs support these options.
>>       </p>
>>
>> -      <column name="options" key="n_rxqs"
>> +      <column name="options" key="n_rxq"
> 
> I'm embarrassed, I didn't notice this pretty obvious documentation mistake
> for a long time.  Thanks for fixing it!
> 
>>               type='{"type": "integer", "minInteger": 1}'>
>>         <p>
>>           Specifies the maximum number of rx queues to be created for PMD
>>           netdev.  If not specified or specified to 0, one rx queue will
>>           be created by default.
>> +          Not supported by vHost interfaces.
> 
> maybe "DPDK vHost", instead of "vHost"?
> 
>>         </p>
>>       </column>
>>     </group>
>> -- 
>> 2.7.4
>>



More information about the dev mailing list