netdev - Re: [PATCH nf-next v3 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]

Message-ID: <20200919155405.GA28410@salvia>
Date:   Sat, 19 Sep 2020 17:54:05 +0200
From:   Pablo Neira Ayuso <pablo@...filter.org>
To:     Lukas Wunner <lukas@...ner.de>
Cc:     Jozsef Kadlecsik <kadlec@...filter.org>,
        Florian Westphal <fw@...len.de>,
        netfilter-devel@...r.kernel.org, coreteam@...filter.org,
        netdev@...r.kernel.org, Daniel Borkmann <daniel@...earbox.net>,
        Alexei Starovoitov <ast@...nel.org>,
        Eric Dumazet <edumazet@...gle.com>,
        Thomas Graf <tgraf@...g.ch>, Laura Garcia <nevola@...il.com>,
        David Miller <davem@...emloft.net>
Subject: Re: [PATCH nf-next v3 3/3] netfilter: Introduce egress hook

Hi Lukas,

On Thu, Aug 27, 2020 at 10:55:03AM +0200, Lukas Wunner wrote:
[...]
> Overall, performance improves with this commit if neither netfilter nor
> traffic control is used. However it degrades a little if only traffic
> control is used, due to the "noinline", the additional outer static key
> and the added netfilter code:
> 
> * Before:       4730418pps 2270Mb/sec (2270600640bps)
> * After:        4759206pps 2284Mb/sec (2284418880bps)
> 
> * Before + tc:  4063912pps 1950Mb/sec (1950677760bps)
> * After  + tc:  4007728pps 1923Mb/sec (1923709440bps)
> 
> * After  + nft: 3714546pps 1782Mb/sec (1782982080bps)
[...]
> Commands to enable egress traffic control:
> tc qdisc add dev foo clsact
> tc filter add dev foo egress bpf da bytecode '1,6 0 0 0,'

1,6 0 0 0, means drop. This is a program with one instruction that
says "drop this packet".

> Commands to enable egress netfilter:
> nft add table netdev t
> nft add chain netdev t co \{ type filter hook egress device foo priority 0 \; \}
> nft add rule netdev t co ip daddr 4.3.2.1/32 drop

However, this is actually doing much more than that:

nft --debug=netlink add rule netdev t co ip daddr 4.3.2.1/32 drop
netdev 
  [ meta load protocol => reg 1 ]
  [ cmp eq reg 1 0x00000008 ]
  [ payload load 4b @ network header + 16 => reg 1 ]
  [ bitwise reg 1 = (reg=1 & 0xffffffff ) ^ 0x00000000 ]
  [ cmp eq reg 1 0x01020304 ]
  [ immediate reg 0 drop ]

So this is comparing apples and pears in some way :-)

Then, I'd suggest the Netfilter ruleset to compare it with tc should be:

add table netdev t
add chain netdev t co { type filter hook egress device foo priority 0 ; policy drop; }

Would you redo these numbers using this ruleset to address Daniel's
comments regarding performance?

Moreover, Daniel also suggested dev_direct_xmit() path from AF_PACKET
allows packets to escape from policy, it seems this also needs to be
extended to add a hook there too.

Could you work on this and send a v2?

Thank you.