[PATCH v2 net-next] net-core: add rx_otherhost_dropped counter

[PATCH v2 net-next] net-core: add rx_otherhost_dropped counter

[Jeffrey Ji]

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.)

changelog:

v2: add kdoc comment

Signed-off-by: jeffreyji <jeffreyji@google.com>
---
 include/linux/netdevice.h    | 3 +++
 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, 12 insertions(+)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index c79ee2296296..e4073c38bd77 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1741,6 +1741,8 @@ enum netdev_ml_priv_type {
  *			do not use this in drivers
  *	@rx_nohandler:	nohandler dropped packets by core network on
  *			inactive devices, do not use this in drivers
+ *	@rx_otherhost_dropped:	Dropped packets due to mismatch in packet dest
+ *				MAC address
  *	@carrier_up_count:	Number of times the carrier has been up
  *	@carrier_down_count:	Number of times the carrier has been down
  *
@@ -2025,6 +2027,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 e315e53125f4..17e74385fca8 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.616.g0bdcbb4464-goog

Re: [PATCH v2 net-next] net-core: add rx_otherhost_dropped counter

[Brian Vazquez]

LGTM, thanks Jeffrey!

Reviewed-by: Brian Vazquez <brianvv@google.com>

On Wed, Mar 2, 2022 at 4:30 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.)
>
> changelog:
>
> v2: add kdoc comment
>
> Signed-off-by: jeffreyji <jeffreyji@google.com>
> ---
>  include/linux/netdevice.h    | 3 +++
>  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, 12 insertions(+)
>
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index c79ee2296296..e4073c38bd77 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -1741,6 +1741,8 @@ enum netdev_ml_priv_type {
>   *                     do not use this in drivers
>   *     @rx_nohandler:  nohandler dropped packets by core network on
>   *                     inactive devices, do not use this in drivers
> + *     @rx_otherhost_dropped:  Dropped packets due to mismatch in packet dest
> + *                             MAC address
>   *     @carrier_up_count:      Number of times the carrier has been up
>   *     @carrier_down_count:    Number of times the carrier has been down
>   *
> @@ -2025,6 +2027,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 e315e53125f4..17e74385fca8 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.616.g0bdcbb4464-goog
>

Re: [PATCH v2 net-next] net-core: add rx_otherhost_dropped counter

[Jakub Kicinski]

On Thu, 3 Mar 2022 13:59:58 -0800 Brian Vazquez wrote:
> LGTM, thanks Jeffrey!
> 
> Reviewed-by: Brian Vazquez <brianvv@google.com>

Please keep Brian's review tag..

> On Wed, Mar 2, 2022 at 4:30 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.)
> >
> > changelog:
> >
> > v2: add kdoc comment

move the changelog under the '---' separator

> > Signed-off-by: jeffreyji <jeffreyji@google.com>
> > ---
> >  include/linux/netdevice.h    | 3 +++
> >  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, 12 insertions(+)

> > diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> > index e315e53125f4..17e74385fca8 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 */

rebase on latest net-next/master and repost. 

Looks like it's conflicting with recently merged 
commit 9309f97aef6d ("net: dev: Add hardware stats support")