Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1315273

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

Show all headers | View raw

Jarod Wilson <jarod@redhat.com> wrote:

>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:
[...]
>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.

	I don't think the kernel should silently drop packets; there
should be a counter somewhere.  If a packet is being thrown away
deliberately, it should not just vanish into the screaming void of
space.  Someday someone will try and track down where that packet is
being dropped.

	I've had that same conversation with customers who insist on
accounting for every packet drop (from the "any drop is an error"
mindset), so I understand the issue.

	Thinking about the prior discussion, the rx_drop_inactive is
still a good idea, but I'd actually today get good use from a
"rx_drop_unforwardable" (or an equivalent but shorter name) counter that
counts every time a packet is dropped due to is_skb_forwardable()
returning false.  __dev_forward_skb does this (and hits rx_dropped), as
does the bridge (and does not count it).

	-J

>CC: "David S. Miller" <davem@davemloft.net>
>CC: Eric Dumazet <edumazet@google.com>
>CC: Jiri Pirko <jiri@mellanox.com>
>CC: Daniel Borkmann <daniel@iogearbox.net>
>CC: Tom Herbert <tom@herbertland.com>
>CC: Jay Vosburgh <j.vosburgh@gmail.com>
>CC: Veaceslav Falico <vfalico@gmail.com>
>CC: Andy Gospodarek <gospo@cumulusnetworks.com>
>CC: netdev@vger.kernel.org
>Signed-off-by: Jarod Wilson <jarod@redhat.com>
>---
> 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
>

---
	-Jay Vosburgh, jay.vosburgh@canonical.com

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[RFC PATCH net] net/core: don't increment rx_dropped on inactive slaves Jarod Wilson <jarod@redhat.com> - 2016-01-22 20:20 +0100
  Re: [RFC PATCH net] net/core: don't increment rx_dropped on inactive slaves Jay Vosburgh <jay.vosburgh@canonical.com> - 2016-01-22 22:00 +0100
    Re: [RFC PATCH net] net/core: don't increment rx_dropped on inactive  slaves Jiri Pirko <jiri@resnulli.us> - 2016-01-23 09:30 +0100
  Re: [RFC PATCH net] net/core: don't increment rx_dropped on inactive  slaves Jiri Pirko <jiri@resnulli.us> - 2016-01-23 09:10 +0100
  Re: [RFC PATCH net] net/core: don't increment rx_dropped on inactive  slaves Andy Gospodarek <gospo@cumulusnetworks.com> - 2016-01-23 15:30 +0100
  Re: [RFC PATCH net] net/core: don't increment rx_dropped on  inactive slaves Eric Dumazet <eric.dumazet@gmail.com> - 2016-01-23 16:30 +0100
    Re: [RFC PATCH net] net/core: don't increment rx_dropped on inactive  slaves Jarod Wilson <jarod@redhat.com> - 2016-01-26 22:20 +0100
      Re: [RFC PATCH net] net/core: don't increment rx_dropped on inactive slaves Eric Dumazet <edumazet@google.com> - 2016-01-26 22:30 +0100
        Re: [RFC PATCH net] net/core: don't increment rx_dropped on inactive  slaves Jarod Wilson <jarod@redhat.com> - 2016-01-26 22:40 +0100
      Re: [RFC PATCH net] net/core: don't increment rx_dropped on  inactive slaves David Miller <davem@davemloft.net> - 2016-01-26 22:30 +0100
        Re: [RFC PATCH net] net/core: don't increment rx_dropped on inactive  slaves Jarod Wilson <jarod@redhat.com> - 2016-01-26 22:40 +0100
  Re: [RFC PATCH net] net/core: don't increment rx_dropped on  inactive slaves David Miller <davem@davemloft.net> - 2016-01-25 07:50 +0100
    Re: [RFC PATCH net] net/core: don't increment rx_dropped on inactive  slaves Jarod Wilson <jarod@redhat.com> - 2016-01-25 15:30 +0100
      Re: [RFC PATCH net] net/core: don't increment rx_dropped on inactive  slaves Jarod Wilson <jarod@redhat.com> - 2016-01-26 05:50 +0100
  [PATCH net 0/4] net: add rx_unhandled stat counter Jarod Wilson <jarod@redhat.com> - 2016-01-27 21:30 +0100
    [PATCH net 1/4] net: add rx_unhandled stat counter Jarod Wilson <jarod@redhat.com> - 2016-01-27 21:30 +0100
    [PATCH net 2/4] net-procfs: show rx_unhandled counters Jarod Wilson <jarod@redhat.com> - 2016-01-27 21:30 +0100
    [PATCH net 4/4] bond: track sum of rx_unhandled for all slaves Jarod Wilson <jarod@redhat.com> - 2016-01-27 21:30 +0100
    [PATCH net 3/4] team: track sum of rx_unhandled for all slaves Jarod Wilson <jarod@redhat.com> - 2016-01-27 21:30 +0100
    Re: [PATCH net 0/4] net: add rx_unhandled stat counter Eric Dumazet <eric.dumazet@gmail.com> - 2016-01-27 22:10 +0100

csiph-web