From: jeffreyji <[email protected]>
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 <[email protected]>
Reviewed-by: Brian Vazquez <[email protected]>
---
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:
> 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).
On Tue, Mar 8, 2022 at 2:51 PM David Ahern <[email protected]> wrote:
>
> On 3/8/22 2:25 PM, Jeffrey Ji wrote:
> > 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:
> > 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.