On 22.02.2016 19:10, Flavio Leitner wrote:
>> Reviewed-by: Aaron Conole <[email protected]>
>> Acked-by: Flavio Leitner <[email protected]>
>
> If you do small changes to the patch that doesn't alter its logic you
> can preserve the signature from others (e.g.: typos, indentation,
> comments...). However, in this case you changed the logic so you can't
> preserve those anymore.
>
> The procedure would be to take them out and add the previous reviewers
> to the CC in the hope that they will review the new patch again.
Flavio, Aaron,
Really sorry for that copy-pasted header. I'll try to be more careful.
>> @@ -623,6 +628,8 @@ netdev_dpdk_init(struct netdev *netdev_, unsigned int
>> port_no,
>> if (err) {
>> goto unlock;
>> }
>> + } else {
>> + netdev_dpdk_alloc_txq(netdev, VHOST_MAX_QUEUE_PAIRS);
>
> The VHOST_MAX_QUEUE_PAIRS is 0x8000, so we are allocating 32768 queue.
> Also that the struct dpdk_tx_queue has 3096 bytes so in the end it is
> allocating 101MB for each vhost-user port.
>
> Why do we need to pre-allocate all TX queues?
Main reasons:
* First signal from vhost_thread may be received between netdev_open()
and netdev_set_multiq(). This will lead to segfault in previous
version of this patch due to unallocated tx_q[].
* We don't know real number of TX queues before new_device() call, but
vring_state_changed() usually called before new_device().
There are two ways to avoid above problems:
1. Preallocate all TX queues.
I agree that VHOST_MAX_QUEUE_PAIRS is too much. We can use own
constant here:
#define OVS_VHOST_MAX_QUEUE_NUM 1024
1024 will help to reduce memory consumption to ~3 Mb per port.
This constant is from QEMU:
include/net/net.h:#define MAX_QUEUE_NUM 1024
This must be sane to limit number of queues by number supported
by QEMU.
2. Reallocation of tx_q[] each time inside vring_state_changed() and
new_device() with preserving of the previous content and initializing
of newly allocated memory.
First solution was chosen just because it's much simpler to implement.
I'd like to replace VHOST_MAX_QUEUE_PAIRS with OVS_VHOST_MAX_QUEUE_NUM.
What to you think about all this?
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
