netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Miller <davem@davemloft.net>
To: fubar@us.ibm.com
Cc: netdev@vger.kernel.org, eric.dumazet@gmail.com, andy@greyhouse.net
Subject: Re: bonding inactive slaves vs rx_dropped
Date: Thu, 14 Feb 2013 17:18:21 -0500 (EST)	[thread overview]
Message-ID: <20130214.171821.186191007478674738.davem@davemloft.net> (raw)
In-Reply-To: <4606.1360878661@death.nxdomain>

From: Jay Vosburgh <fubar@us.ibm.com>
Date: Thu, 14 Feb 2013 13:51:01 -0800

> David Miller <davem@davemloft.net> wrote:
> 
>>People are starting to notice that rx_dropped now increments on every
>>packet received on an bond's inactive slave.
>>
>>I'm actually fine with rx_dropped incrementing in this situation.
>>
>>The problem I want to address is that rx_dropped is encompassing
>>several unrelated situations and thus has become less useful for
>>diagnosis.
>>
>>I think we should add some new RX stats such that we can get at
>>least a small amount of granularity for rx_dropped.
>>
>>This way team, bond, etc. can increment a new netdev_stats->rx_foo in
>>this situation, and then someone doing diagnosis can see that
>>rx_dropped and rx_foo are incrementing at similar rates.
> 
> 	This drop isn't really happening in bonding, though.  From
> looking at the code, it comes about because, for the inactive slave, the
> rx_handler call returns EXACT, and there aren't any exact match ptype
> bindings, so __netif_receive_skb throws it away.  This isn't always the
> case; sometimes there is an exact match, for things like iSCSI or FCoE
> that are really determined to get the packet.

This isn't even the whole story, it won't return 'exact' if the packet
from the inactive slave is broadcast or multicast.

My general rule is that every special case increments a special
'absurdity' statistic counter for the code :-)

> 	We could probably add an, oh, rx_dropped_inactive, or some
> variation on that theme, that is incremented at the end of
> __netif_receive_skb if deliver_exact is set, e.g., something like:

Yes, that looks fine to me.

> 	There's the separate questions of whether there should be more
> counters (e.g., drops in dev_skb_forward or enqueue_to_backlog), and how
> to deliver the counter(s) to user space.

Since there is some pain in adding counters, I think we should try to
find a nice (very small) set of cases to cover all at once.

  reply	other threads:[~2013-02-14 22:18 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-02-14 21:06 bonding inactive slaves vs rx_dropped David Miller
2013-02-14 21:51 ` Jay Vosburgh
2013-02-14 22:18   ` David Miller [this message]
2013-02-14 23:43     ` Jay Vosburgh
2013-02-15 17:47       ` Ben Hutchings

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=20130214.171821.186191007478674738.davem@davemloft.net \
    --to=davem@davemloft.net \
    --cc=andy@greyhouse.net \
    --cc=eric.dumazet@gmail.com \
    --cc=fubar@us.ibm.com \
    --cc=netdev@vger.kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).