All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brian Vazquez <brianvv@google.com>
To: Jakub Kicinski <kuba@kernel.org>
Cc: Jeffrey Ji <jeffreyjilinux@gmail.com>,
	Eric Dumazet <edumazet@google.com>,
	"David S . Miller" <davem@davemloft.net>,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	jeffreyji <jeffreyji@google.com>
Subject: Re: [PATCH v6 net-next] net-core: add InMacErrors counter
Date: Thu, 3 Feb 2022 10:05:17 -0800	[thread overview]
Message-ID: <CAMzD94SNEPACnp+uniXVgRDWr9oukj-xnAoqyQQCE77GH_kqdg@mail.gmail.com> (raw)
In-Reply-To: <20220202205916.58f4a592@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>

On Wed, Feb 2, 2022 at 8:59 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Tue,  1 Feb 2022 22:28:45 +0000 Jeffrey Ji wrote:
> > From: jeffreyji <jeffreyji@google.com>
> >
> > Increment InMacErrors counter when packet dropped due to incorrect dest
> > MAC addr.
> >
> > An example when this drop can occur is when manually crafting raw
> > packets that will be consumed by a user space application via a tap
> > device. For testing purposes local traffic was generated using trafgen
> > for the client and netcat to start a server
> >
> > example output from nstat:
> > \~# nstat -a | grep InMac
> > Ip6InMacErrors                  0                  0.0
> > IpExtInMacErrors                1                  0.0
>
> I had another thing and this still doesn't sit completely well
> with me :(
>
> Shouldn't we count those drops as skb->dev->rx_dropped?
> Commonly NICs will do such filtering and if I got it right
> in struct rtnl_link_stats64 kdoc - report them as rx_dropped.
> It'd be inconsistent if on a physical interface we count
> these as rx_dropped and on SW interface (or with promisc enabled
> etc.) in the SNMP counters.
> Or we can add a new link stat that NICs can use as well.
>
> In fact I'm not sure this is really a IP AKA L3 statistic,
> it's the L2 address that doesn't match.
>
>
> If everyone disagrees - should we at least rename the MIB counter
> similarly to the drop reason? Experience shows that users call for
> help when they see counters with Error in their name, I'd vote for
> IpExtInDropOtherhost or some such. The statistic should also be
> documented in Documentation/networking/snmp_counter.rst

Changing the Name to IpExtInDropOtherhost and adding the documentation
makes sense to me. What do others think? I'd like to get more feedback
before Jeffrey sends another version.

  reply	other threads:[~2022-02-03 18:05 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-01 22:28 [PATCH v6 net-next] net-core: add InMacErrors counter Jeffrey Ji
2022-02-03  4:59 ` Jakub Kicinski
2022-02-03 18:05   ` Brian Vazquez [this message]
2022-02-03 19:35     ` Jakub Kicinski
2022-02-03 18:13   ` Eric Dumazet
2022-02-03 19:34     ` Jakub Kicinski

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=CAMzD94SNEPACnp+uniXVgRDWr9oukj-xnAoqyQQCE77GH_kqdg@mail.gmail.com \
    --to=brianvv@google.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=jeffreyji@google.com \
    --cc=jeffreyjilinux@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --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 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.