On Wed, 24 Feb 2016 10:54:20 +0300
Ilya Maximets <i.maxim...@samsung.com> wrote:
> On 22.02.2016 19:10, Flavio Leitner wrote:
> >> Reviewed-by: Aaron Conole <acon...@redhat.com>
> >> Acked-by: Flavio Leitner <f...@sysclose.org>
> >
> > 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?
OK, I think we can have the new macro using less queues for now and
then optimize the code to allocate on demand later.
Could you please spin a new version?
--
fbl
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
