Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220301001153.1608374-1-jeffreyjilinux@gmail.com>
In-Reply-To: <20220301001153.1608374-1-jeffreyjilinux@gmail.com>
From:   Brian Vazquez <brianvv@google.com>
Date:   Tue, 1 Mar 2022 07:28:28 -0800
Message-ID: <CAMzD94Re5jakJ=hMNgiPBh-B-a8DGrCmNMA98r8-Gw8413FjKQ@mail.gmail.com>
Subject: Re: [PATCH net-next] net-core: add rx_otherhost_dropped counter
To:     Jeffrey Ji <jeffreyjilinux@gmail.com>
Cc:     Jakub Kicinski <kuba@kernel.org>,
        "David S . Miller" <davem@davemloft.net>,
        Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>,
        David Ahern <dsahern@kernel.org>,
        Eric Dumazet <edumazet@google.com>,
        Antoine Tenart <atenart@kernel.org>,
        Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
        Alexander Lobakin <alexandr.lobakin@intel.com>,
        Kumar Kartikeya Dwivedi <memxor@gmail.com>,
        Arnd Bergmann <arnd@arndb.de>, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org, jeffreyji <jeffreyji@google.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hey Jeffrey,

Looking at it in patchwork
(https://patchwork.kernel.org/project/netdevbpf/patch/20220301001153.1608374-1-jeffreyjilinux@gmail.com/),
it is complaining about kdoc.

Seems that you're missing the kdoc comment about rx_otherhost_dropped
in the include/linux/netdevice.h file, other than that LGTM.

On Mon, Feb 28, 2022 at 4:12 PM Jeffrey Ji <jeffreyjilinux@gmail.com> wrote:
>
> From: jeffreyji <jeffreyji@google.com>
>
> Increment rx_otherhost_dropped counter when packet dropped due to
> mismatched 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
>
> Tested: Created 2 netns, sent 1 packet using trafgen from 1 to the other
> with "{eth(daddr=$INCORRECT_MAC...}", verified that iproute2 showed the
> counter was incremented. (Also had to modify iproute2 to show the stat,
> additional patch for that coming next.)
>
> Signed-off-by: jeffreyji <jeffreyji@google.com>
> ---
>  include/linux/netdevice.h    | 1 +
>  include/uapi/linux/if_link.h | 5 +++++
>  net/core/dev.c               | 2 ++
>  net/ipv4/ip_input.c          | 1 +
>  net/ipv6/ip6_input.c         | 1 +
>  5 files changed, 10 insertions(+)
>
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index c79ee2296296..96c2030f4c1f 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -2025,6 +2025,7 @@ struct net_device {
>         atomic_long_t           rx_dropped;
>         atomic_long_t           tx_dropped;
>         atomic_long_t           rx_nohandler;
> +       atomic_long_t           rx_otherhost_dropped;
>
>         /* Stats to monitor link on/off, flapping */
>         atomic_t                carrier_up_count;
> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> index be09d2ad4b5d..834382317889 100644
> --- a/include/uapi/linux/if_link.h
> +++ b/include/uapi/linux/if_link.h
> @@ -211,6 +211,9 @@ struct rtnl_link_stats {
>   * @rx_nohandler: Number of packets received on the interface
>   *   but dropped by the networking stack because the device is
>   *   not designated to receive packets (e.g. backup link in a bond).
> + *
> + * @rx_otherhost_dropped: Number of packets dropped due to mismatch in
> + * packet's destination MAC address.
>   */
>  struct rtnl_link_stats64 {
>         __u64   rx_packets;
> @@ -243,6 +246,8 @@ struct rtnl_link_stats64 {
>         __u64   rx_compressed;
>         __u64   tx_compressed;
>         __u64   rx_nohandler;
> +
> +       __u64   rx_otherhost_dropped;
>  };
>
>  /* The struct should be in sync with struct ifmap */
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 2d6771075720..d039d8fdc16a 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -10037,6 +10037,8 @@ struct rtnl_link_stats64 *dev_get_stats(struct net_device *dev,
>         storage->rx_dropped += (unsigned long)atomic_long_read(&dev->rx_dropped);
>         storage->tx_dropped += (unsigned long)atomic_long_read(&dev->tx_dropped);
>         storage->rx_nohandler += (unsigned long)atomic_long_read(&dev->rx_nohandler);
> +       storage->rx_otherhost_dropped +=
> +               (unsigned long)atomic_long_read(&dev->rx_otherhost_dropped);
>         return storage;
>  }
>  EXPORT_SYMBOL(dev_get_stats);
> diff --git a/net/ipv4/ip_input.c b/net/ipv4/ip_input.c
> index d94f9f7e60c3..ef97b0a4c77f 100644
> --- a/net/ipv4/ip_input.c
> +++ b/net/ipv4/ip_input.c
> @@ -450,6 +450,7 @@ static struct sk_buff *ip_rcv_core(struct sk_buff *skb, struct net *net)
>          * that it receives, do not try to analyse it.
>          */
>         if (skb->pkt_type == PACKET_OTHERHOST) {
> +               atomic_long_inc(&skb->dev->rx_otherhost_dropped);
>                 drop_reason = SKB_DROP_REASON_OTHERHOST;
>                 goto drop;
>         }
> diff --git a/net/ipv6/ip6_input.c b/net/ipv6/ip6_input.c
> index d4b1e2c5aa76..3f0cbe126d82 100644
> --- a/net/ipv6/ip6_input.c
> +++ b/net/ipv6/ip6_input.c
> @@ -150,6 +150,7 @@ static struct sk_buff *ip6_rcv_core(struct sk_buff *skb, struct net_device *dev,
>         struct inet6_dev *idev;
>
>         if (skb->pkt_type == PACKET_OTHERHOST) {
> +               atomic_long_inc(&skb->dev->rx_otherhost_dropped);
>                 kfree_skb(skb);
>                 return NULL;
>         }
> --
> 2.35.1.574.g5d30c73bfb-goog
>