From: Stephen Hemminger <stephen@networkplumber.org>
To: Chris Mi <chrism@mellanox.com>
Cc: netdev@vger.kernel.org, gerlitz.or@gmail.com
Subject: Re: [patch iproute2] tc: add -bs option for batch mode
Date: Tue, 19 Dec 2017 07:15:47 -0800 [thread overview]
Message-ID: <20171219071547.35b002c8@xeon-e3> (raw)
In-Reply-To: <20171219063346.19300-1-chrism@mellanox.com>
On Tue, 19 Dec 2017 15:33:46 +0900
Chris Mi <chrism@mellanox.com> wrote:
> Currently in tc batch mode, only one command is read from the batch
> file and sent to kernel to process. With this patch, we can accumulate
> several commands before sending to kernel. The batch size is specified
> using option -bs or -batchsize.
>
> To accumulate the commands in tc, we allocate an array of struct iovec.
> If batchsize is bigger than 1 and we haven't accumulated enough
> commands, rtnl_talk() will return without actually sending the message.
> One exception is that there is no more command in the batch file.
>
> But please note that kernel still processes the requests one by one.
> To process the requests in parallel in kernel is another effort.
> The time we're saving in this patch is the user mode and kernel mode
> context switch. So this patch works on top of the current kernel.
>
> Using the following script in kernel, we can generate 1,000,000 rules.
> tools/testing/selftests/tc-testing/tdc_batch.py
>
> Without this patch, 'tc -b $file' exection time is:
>
> real 0m14.916s
> user 0m6.808s
> sys 0m8.046s
>
> With this patch, 'tc -b $file -bs 10' exection time is:
>
> real 0m12.286s
> user 0m5.903s
> sys 0m6.312s
>
> The insertion rate is improved more than 10%.
>
> Signed-off-by: Chris Mi <chrism@mellanox.com>
I think this is probably optmizing for an unrealistic use case.
If there are 1M rules then it should be managed by a dynmaic process
like a routing daemon with a real database and API. Such a daemon
would talk to kernel directly. Plus at scale netlink gets overwhelmed
and the daemon has to handle resync.
Not convinced that the added complexity and error handling issues
warrant the change. The batch mode already sucks at handling
errors.
next prev parent reply other threads:[~2017-12-19 15:15 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-12-19 6:33 [patch iproute2] tc: add -bs option for batch mode Chris Mi
2017-12-19 15:15 ` Stephen Hemminger [this message]
2017-12-20 8:47 ` Or Gerlitz
2017-12-19 15:22 ` Stephen Hemminger
2017-12-20 9:23 ` Chris Mi
2017-12-20 15:17 ` Stephen Hemminger
2017-12-21 0:13 ` David Ahern
2017-12-21 1:30 ` Chris Mi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20171219071547.35b002c8@xeon-e3 \
--to=stephen@networkplumber.org \
--cc=chrism@mellanox.com \
--cc=gerlitz.or@gmail.com \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).