[RFC PATCH net] net/core: don't increment rx_dropped on inactive slaves

From: Jarod Wilson
Date: Fri Jan 22 2016 - 14:12:13 EST


The network core tries to keep track of dropped packets, but some packets
you wouldn't really call dropped, so much as intentionally ignored, under
certain circumstances. One such case is that of bonding and team device
slaves that are currently inactive. Their respective rx_handler functions
return RX_HANDLER_EXACT (the only places in the kernel that return that),
which ends up tracking into the network core's __netif_receive_skb_core()
function's drop path, with no pt_prev set. On a noisy network, this can
result in a very rapidly incrementing rx_dropped counter, not only on the
inactive slave(s), but also on the master device, such as the following:

Inter-| Receive | Transmit
face |bytes packets errs drop fifo frame compressed multicast|bytes packets errs drop fifo colls carrier compressed
p7p1: 14783346 140430 0 140428 0 0 0 2040 680 8 0 0 0 0 0 0
p7p2: 14805198 140648 0 0 0 0 0 2034 0 0 0 0 0 0 0 0
bond0: 53365248 532798 0 421160 0 0 0 115151 2040 24 0 0 0 0 0 0
lo: 5420 54 0 0 0 0 0 0 5420 54 0 0 0 0 0 0
p5p1: 19292195 196197 0 140368 0 0 0 56564 680 8 0 0 0 0 0 0
p5p2: 19289707 196171 0 140364 0 0 0 56547 680 8 0 0 0 0 0 0
em3: 20996626 158214 0 0 0 0 0 383 0 0 0 0 0 0 0 0
em2: 14065122 138462 0 0 0 0 0 310 0 0 0 0 0 0 0 0
em1: 14063162 138440 0 0 0 0 0 308 0 0 0 0 0 0 0 0
em4: 21050830 158729 0 0 0 0 0 385 71662 469 0 0 0 0 0 0
ib0: 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0

In this scenario, p5p1, p5p2 and p7p1 are all inactive slaves in an
active-backup bond0, and you can see that all three have high drop counts,
with the master bond0 showing a tally of all three.

I know that this was previously discussed some here:

http://www.spinics.net/lists/netdev/msg226341.html

It seems additional counters never came to fruition, but honestly, for
this particular case, I'm not even sure they're warranted, I'd be inclined
to say just silently drop these packets without incrementing a counter. At
least, that's probably what would make someone who has complained loudly
about this issue happy, as they have monitoring tools that are squaking
loudly at any increments to rx_dropped.

CC: "David S. Miller" <davem@xxxxxxxxxxxxx>
CC: Eric Dumazet <edumazet@xxxxxxxxxx>
CC: Jiri Pirko <jiri@xxxxxxxxxxxx>
CC: Daniel Borkmann <daniel@xxxxxxxxxxxxx>
CC: Tom Herbert <tom@xxxxxxxxxxxxxxx>
CC: Jay Vosburgh <j.vosburgh@xxxxxxxxx>
CC: Veaceslav Falico <vfalico@xxxxxxxxx>
CC: Andy Gospodarek <gospo@xxxxxxxxxxxxxxxxxxx>
CC: netdev@xxxxxxxxxxxxxxx
Signed-off-by: Jarod Wilson <jarod@xxxxxxxxxx>
---
net/core/dev.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/net/core/dev.c b/net/core/dev.c
index 8cba3d8..1354c7b 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4153,8 +4153,11 @@ ncls:
else
ret = pt_prev->func(skb, skb->dev, pt_prev, orig_dev);
} else {
+ if (deliver_exact)
+ goto inactive; /* bond or team inactive slave */
drop:
atomic_long_inc(&skb->dev->rx_dropped);
+inactive:
kfree_skb(skb);
/* Jamal, now you will not able to escape explaining
* me how you were going to use this. :-)
--
1.8.3.1