| Message ID | 20220308212531.752215-1-jeffreyjilinux@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v3,net-next] net-core: add rx_otherhost_dropped counter | expand |
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 <dsahern@kernel.org> 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.
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; }