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

From: jeffreyji <redacted>

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 <redacted>
Reviewed-by: Brian Vazquez <redacted>
---
changelog:

v3: rebase onto net-next/master, fix comments

v2: add kdoc comment

 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 29a850a8d460..43af5012b39c 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
  *

@@ -2026,6 +2028,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 ddca20357e7e..a9681908617b 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;
 };
 
 /* Subset of link stats useful for in-HW collection. Meaning of the fields is as

diff --git a/net/core/dev.c b/net/core/dev.c
index ba69ddf85af6..fd7ec8ce972b 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c

@@ -10308,6 +10308,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 95f7bb052784..8b87ea99904b 100644
--- a/net/ipv4/ip_input.c
+++ b/net/ipv4/ip_input.c

@@ -451,6 +451,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 5b5ea35635f9..5624c937f87f 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

On 3/8/22 2:25 PM, Jeffrey Ji wrote:

that's an expensive packet counter for a common path (e.g., hosting
environments).

On Tue, Mar 8, 2022 at 2:51 PM David Ahern [off-list ref] wrote:

On 3/8/22 2:25 PM, Jeffrey Ji wrote:

quoted

diff --git a/net/ipv4/ip_input.c b/net/ipv4/ip_input.c
index 95f7bb052784..8b87ea99904b 100644
--- a/net/ipv4/ip_input.c
+++ b/net/ipv4/ip_input.c

@@ -451,6 +451,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 5b5ea35635f9..5624c937f87f 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;
      }

that's an expensive packet counter for a common path (e.g., hosting
environments).

This was the reason for the initial patch, using SNMP stat, being per cpu.

Adding per-device per-cpu data for this counter will increase cost of
netdevice dismantle phase,
and increase time for ndo_get_stats64(), especially on hosts with 256
or 512 cpus.

On Tue, 8 Mar 2022 15:18:25 -0800 Eric Dumazet wrote:

quoted

that's an expensive packet counter for a common path (e.g., hosting
environments).  

This was the reason for the initial patch, using SNMP stat, being per cpu.

Adding per-device per-cpu data for this counter will increase cost of
netdevice dismantle phase,
and increase time for ndo_get_stats64(), especially on hosts with 256
or 512 cpus.

Two ways to solve this:
 - make dev->pcpu_refcnt point to a structure which holds both
   refcnt and whatever stats
 - combine these stats into lstats, assuming the netdevs we care
   about spawning / destroying fast are sw devices anyway;
   struct rtnl_link_ops can indicate to the core if the driver
   wants lstats (or just put how many bytes it wants), otherwise 
   we'd only allocate enough mem for core's stats

Option three - both.