netdev - Re: [patch iproute2] tc: add -bs option for batch mode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJ3xEMiiXSFf+ai8FmqA+VseOZaivqc_A4WvTRJa6uKYOS41XA@mail.gmail.com>
Date:   Wed, 20 Dec 2017 10:47:01 +0200
From:   Or Gerlitz <gerlitz.or@...il.com>
To:     Stephen Hemminger <stephen@...workplumber.org>
Cc:     Chris Mi <chrism@...lanox.com>,
        Linux Netdev List <netdev@...r.kernel.org>
Subject: Re: [patch iproute2] tc: add -bs option for batch mode

On Tue, Dec 19, 2017 at 5:15 PM, Stephen Hemminger
<stephen@...workplumber.org> wrote:
> On Tue, 19 Dec 2017 15:33:46 +0900
> Chris Mi <chrism@...lanox.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@...lanox.com>
>
> I think this is probably optimizing for an unrealistic use case.
> If there are 1M rules then it should be managed by a dynamic 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.

Standalone tools such as ip and tc are indeed used many ties mostly at staging,
development and performance benchmarking environments (where applications on
production systems (e.g FRR or OVS) would embed the NL code in house
and do direct
calls into the kernel).  As such, I think there's a value to max out
the tc tool ability to add flows.

> Not convinced that the added complexity and error handling issues
> warrant the change. The batch mode already sucks at handling errors.

Lets have Chris fix the patch and see how the revised bits look.

Or.