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

[Daniel Borkmann <daniel@iogearbox.net>]

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