[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87fswnea9g.fsf@toke.dk>
Date: Fri, 09 Jul 2021 18:12:59 +0200
From: Toke Høiland-Jørgensen <toke@...hat.com>
To: Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org
Cc: "David S. Miller" <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>
Subject: Re: [RFC PATCH 2/3] veth: make queues nr configurable via kernel
module params
Paolo Abeni <pabeni@...hat.com> writes:
> On Fri, 2021-07-09 at 12:24 +0200, Toke Høiland-Jørgensen wrote:
>> Paolo Abeni <pabeni@...hat.com> writes:
>>
>> > This allows configuring the number of tx and rx queues at
>> > module load time. A single module parameter controls
>> > both the default number of RX and TX queues created
>> > at device registration time.
>> >
>> > Signed-off-by: Paolo Abeni <pabeni@...hat.com>
>> > ---
>> > drivers/net/veth.c | 21 +++++++++++++++++++++
>> > 1 file changed, 21 insertions(+)
>> >
>> > diff --git a/drivers/net/veth.c b/drivers/net/veth.c
>> > index 10360228a06a..787b4ad2cc87 100644
>> > --- a/drivers/net/veth.c
>> > +++ b/drivers/net/veth.c
>> > @@ -27,6 +27,11 @@
>> > #include <linux/bpf_trace.h>
>> > #include <linux/net_tstamp.h>
>> >
>> > +static int queues_nr = 1;
>> > +
>> > +module_param(queues_nr, int, 0644);
>> > +MODULE_PARM_DESC(queues_nr, "Max number of RX and TX queues (default = 1)");
>>
>> Adding new module parameters is generally discouraged. Also, it's sort
>> of a cumbersome API that you'll have to set this first, then re-create
>> the device, and then use channels to get the number you want.
>>
>> So why not just default to allocating num_possible_cpus() number of
>> queues? Arguably that is the value that makes the most sense from a
>> scalability point of view anyway, but if we're concerned about behaviour
>> change (are we?), we could just default real_num_*_queues to 1, so that
>> the extra queues have to be explicitly enabled by ethtool?
>
> I was concerned by the amount of memory wasted memory (should be ~256
> bytes per rx queue, ~320 per tx, plus the sysfs entries).
I'm not too worried by that since it's per CPU; systems with a lot of
CPUs should hopefully also have plenty of memory. Or at least I think
the user friendliness outweighs the cost in memory.
> real_num_tx_queue > 1 will makes the xmit path slower, so we likely
> want to keep that to 1 by default - unless the userspace explicitly set
> numtxqueues via netlink.
Right, that's fine by me :)
> Finally, a default large num_tx_queue slows down device creation:
>
> cat << ENDL > run.sh
> #!/bin/sh
> MAX=$1
> for I in `seq 1 $MAX`; do
> ip link add name v$I type veth peer name pv$I
> done
> for I in `seq 1 $MAX`; do
> ip link del dev v$I
> done
> ENDL
> chmod a+x run.sh
>
> # with num_tx_queue == 1
> time ./run.sh 100
> real 0m2.276s
> user 0m0.107s
> sys 0m0.162s
>
> # with num_tx_queue == 128
> time ./run.sh 100 1
> real 0m4.199s
> user 0m0.091s
> sys 0m1.419s
>
> # with num_tx_queue == 4096
> time ./run.sh 100
> real 0m24.519s
> user 0m0.089s
> sys 0m21.711s
So ~42 ms to create a device if there are 128 CPUs? And ~245 when
there's 4k CPUs? Doesn't seem too onerous to me...
> Still, if there is agreement I can switch to num_possible_cpus default,
> plus some trickery to keep real_num_{r,t}x_queue unchanged.
>
> WDYT?
SGTM :)
-Toke
Powered by blists - more mailing lists