[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