netdev - Re: [PATCH nf-next 3/3] netfilter: Introduce egress hook

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Thu, 12 Mar 2020 23:40:49 +0100
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Lukas Wunner <lukas@...ner.de>
Cc:     Pablo Neira Ayuso <pablo@...filter.org>,
        Jozsef Kadlecsik <kadlec@...filter.org>,
        Florian Westphal <fw@...len.de>,
        netfilter-devel@...r.kernel.org, coreteam@...filter.org,
        netdev@...r.kernel.org, Martin Mares <mj@....cz>,
        Dmitry Safonov <0x7f454c46@...il.com>,
        Thomas Graf <tgraf@...g.ch>,
        Alexei Starovoitov <ast@...nel.org>,
        David Miller <davem@...emloft.net>
Subject: Re: [PATCH nf-next 3/3] netfilter: Introduce egress hook

On 3/11/20 4:54 PM, Lukas Wunner wrote:
> On Wed, Mar 11, 2020 at 03:05:16PM +0100, Daniel Borkmann wrote:
>> no need to make the fast-path slower for exotic protocols
>> which can be solved through other means.
> 
> As said the fast-path gets faster, not slower.
> 
>>> * Without this commit:
>>>     Result: OK: 34240933(c34238375+d2558) usec, 100000000 (60byte,0frags)
>>>     2920481pps 1401Mb/sec (1401830880bps) errors: 0
>>>
>>> * With this commit:
>>>     Result: OK: 33997299(c33994193+d3106) usec, 100000000 (60byte,0frags)
>>>     2941410pps 1411Mb/sec (1411876800bps) errors: 0
>>
>> So you are suggesting that we've just optimized the stack by adding more
>> hooks to it ...?
> 
> Since I've provided numbers to disprove your allegation, I think the
> onus is now on you to prove that your allegation holds any water.
> Please reproduce the measurements and let's go from there.
> 
> This isn't much work, I've made it really easy by providing all the
> steps necessary in the commit message.

So in terms of micro-benchmarking with pktgen, if I understand you correctly,
you are basically measuring loopback device by pushing packets through the
__dev_queue_xmit() -> loopback_xmit() -> netif_rx() till the stack drops them
in IP layer on ingress side? I wonder how your perf profile looks like ...
Setting a drop point in tc layer and then measuring the effect before/after
this change with CONFIG_NETFILTER_EGRESS enabled, I'm getting a stable degration
from ~4.123Mpps to ~4.082Mpps with pktgen, definitely not seeing a speedup.

# ip link add dev foo type dummy
# ip link set dev foo up
# tc qdisc add dev foo clsact
# tc filter add dev foo egress bpf da bytecode '1,6 0 0 0,'
# modprobe pktgen
# echo "add_device foo" > /proc/net/pktgen/kpktgend_3
# samples/pktgen/pktgen_bench_xmit_mode_queue_xmit.sh -i 'foo' -n 400000000 -m '11:11:11:11:11:11' -d '1.1.1.1'

Also to let pktgen count the skb:

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index acc849df60b5..8920da7a7a67 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -3372,11 +3372,11 @@ static void pktgen_xmit(struct pktgen_dev *pkt_dev)
                 ret = dev_queue_xmit(pkt_dev->skb);
                 switch (ret) {
                 case NET_XMIT_SUCCESS:
+               case NET_XMIT_DROP:
                         pkt_dev->sofar++;
                         pkt_dev->seq_num++;
                         pkt_dev->tx_bytes += pkt_dev->last_pkt_size;
                         break;
-               case NET_XMIT_DROP:
                 case NET_XMIT_CN:
                 /* These are all valid return codes for a qdisc but
                  * indicate packets are being dropped or will likely

(In any case, this whole discussion is moot for out of tree code.)

Thanks,
Daniel