All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jiri Pirko <jiri@resnulli.us>
To: Jarod Wilson <jarod@redhat.com>
Cc: linux-kernel@vger.kernel.org,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jiri Pirko <jiri@mellanox.com>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Tom Herbert <tom@herbertland.com>,
	Jay Vosburgh <j.vosburgh@gmail.com>,
	Veaceslav Falico <vfalico@gmail.com>,
	Andy Gospodarek <gospo@cumulusnetworks.com>,
	netdev@vger.kernel.org
Subject: Re: [RFC PATCH net] net/core: don't increment rx_dropped on inactive slaves
Date: Sat, 23 Jan 2016 09:07:41 +0100	[thread overview]
Message-ID: <20160123080741.GB2193@nanopsycho.orion> (raw)
In-Reply-To: <1453489882-57948-1-git-send-email-jarod@redhat.com>

Fri, Jan 22, 2016 at 08:11:22PM CET, 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:
>
>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@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>

Acked-by: Jiri Pirko <jiri@mellanox.com>

I think this should be considered as a bug and go to -net.

  parent reply	other threads:[~2016-01-23  8:07 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-22 19:11 [RFC PATCH net] net/core: don't increment rx_dropped on inactive slaves Jarod Wilson
2016-01-22 20:59 ` Jay Vosburgh
2016-01-23  8:26   ` Jiri Pirko
2016-01-23  8:07 ` Jiri Pirko [this message]
2016-01-23 14:19 ` Andy Gospodarek
2016-01-23 15:23 ` Eric Dumazet
2016-01-26 21:14   ` Jarod Wilson
2016-01-26 21:21     ` David Miller
2016-01-26 21:36       ` Jarod Wilson
2016-01-26 21:24     ` Eric Dumazet
2016-01-26 21:35       ` Jarod Wilson
2016-01-25  6:42 ` David Miller
2016-01-25 14:27   ` Jarod Wilson
2016-01-26  4:45     ` Jarod Wilson
2016-01-27 20:21 ` [PATCH net 0/4] net: add rx_unhandled stat counter Jarod Wilson
2016-01-27 20:21   ` [PATCH net 1/4] " Jarod Wilson
2016-01-27 20:21   ` [PATCH net 2/4] net-procfs: show rx_unhandled counters Jarod Wilson
2016-01-27 20:21   ` [PATCH net 3/4] team: track sum of rx_unhandled for all slaves Jarod Wilson
2016-01-27 20:21   ` [PATCH net 4/4] bond: " Jarod Wilson
2016-01-27 21:09   ` [PATCH net 0/4] net: add rx_unhandled stat counter Eric Dumazet
2016-01-28  6:02     ` Jarod Wilson
2016-01-28  6:10       ` Jarod Wilson
2016-01-28  6:18       ` Jarod Wilson
2016-01-28 13:00         ` Eric Dumazet
2016-01-28 14:38           ` Jarod Wilson
2016-01-28 14:42             ` Eric Dumazet
2016-01-28 14:44               ` Eric Dumazet
2016-01-28 14:46                 ` Eric Dumazet
2016-01-28 15:11                   ` Jarod Wilson
2016-01-28 13:00       ` Eric Dumazet
2016-01-28 15:49 ` [PATCH net v2 0/4] net: add and use rx_nohandler " Jarod Wilson
2016-01-28 15:49   ` [PATCH net v2 1/4] net/core: relax BUILD_BUG_ON in netdev_stats_to_stats64 Jarod Wilson
2016-01-28 15:49   ` [PATCH net v2 2/4] net: add rx_nohandler stat counter Jarod Wilson
2016-01-28 15:49   ` [PATCH net v2 3/4] team: track sum of rx_nohandler for all slaves Jarod Wilson
2016-01-28 15:49   ` [PATCH net v2 4/4] bond: " Jarod Wilson
2016-01-30  3:37   ` [PATCH net v2 0/4] net: add and use rx_nohandler stat counter David Miller
2016-01-30 18:16     ` Jarod Wilson
2016-01-30 18:19   ` [PATCH net v2 1/4] net/core: relax BUILD_BUG_ON in netdev_stats_to_stats64 Jarod Wilson
2016-01-30 18:34     ` Eric Dumazet
2016-01-30 20:39       ` Jarod Wilson
2016-01-30 20:53         ` Jarod Wilson
2016-01-30 23:26           ` David Miller
2016-01-31 18:07             ` Jarod Wilson
2016-02-01 23:51   ` [PATCH net v3 0/4] net: add and use rx_nohandler stat counter Jarod Wilson
2016-02-01 23:51     ` [PATCH net v3 1/4] net/core: relax BUILD_BUG_ON in netdev_stats_to_stats64 Jarod Wilson
2016-02-01 23:51     ` [PATCH net v3 2/4] net: add rx_nohandler stat counter Jarod Wilson
2016-02-07 19:37       ` Stephen Hemminger
2016-02-07 19:46         ` David Miller
2016-02-07 20:19           ` Eric Dumazet
2016-02-08 18:32             ` Jarod Wilson
2016-02-08 19:38               ` Stephen Hemminger
2016-02-08 22:57                 ` Eric Dumazet
2016-02-09  8:40                   ` David Miller
2016-02-09 10:56                     ` Jamal Hadi Salim
2016-02-09 19:17                       ` [PATCH net-next iproute2] iplink: display rx nohandler stats Stephen Hemminger
2016-02-09 23:51                         ` Jarod Wilson
2016-02-10  1:41                           ` Stephen Hemminger
2016-02-10  4:52                             ` Eric Dumazet
2016-02-10 13:20                               ` Jarod Wilson
2016-02-10 15:06                                 ` Andy Gospodarek
2016-02-01 23:51     ` [PATCH net v3 3/4] team: track sum of rx_nohandler for all slaves Jarod Wilson
2016-02-01 23:51     ` [PATCH net v3 4/4] bond: " Jarod Wilson
2016-02-06  8:00     ` [PATCH net v3 0/4] net: add and use rx_nohandler stat counter David Miller
2016-01-28 16:22 ` [PATCH net v3 1/4] net/core: relax BUILD_BUG_ON in netdev_stats_to_stats64 Jarod Wilson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160123080741.GB2193@nanopsycho.orion \
    --to=jiri@resnulli.us \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=gospo@cumulusnetworks.com \
    --cc=j.vosburgh@gmail.com \
    --cc=jarod@redhat.com \
    --cc=jiri@mellanox.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=tom@herbertland.com \
    --cc=vfalico@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.