net: Convert some UDP tunnel drivers to NETDEV_PCPU_STAT_DSTATS.

PATCH Superseded Guillaume Nault 3 participants 9 messages Dec 2, 2024 – Dec 3, 2024

Subsystems (3)

Networking Network drivers VRF

lore

Share & tools

lore

Guillaume Nault

author

2024-12-02 21:48 UTC #0 lore raw

cover net: Convert some UDP tunnel drivers to NETDEV_PCPU_STAT_DSTATS.

VXLAN, Geneve and Bareudp use various device counters for managing RX and TX statistics:

* VXLAN uses the device core_stats for RX and TX drops, tstats for regular RX/TX counters and DEV_STATS_INC() for various types of RX/TX errors.

* Geneve uses tstats for regular RX/TX counters and DEV_STATS_INC() for everything else, include RX/TX drops.

* Bareudp, was recently converted to follow VXLAN behaviour, that is, device core_stats for RX and TX drops, tstats for regular RX/TX counters and DEV_STATS_INC() for other counter types.

Let's consolidate statistics management around the dstats counters instead. This avoids using core_stats in VXLAN and Bareudp, as core_stats is supposed to be used by core networking code only (and not in drivers). This also allows Geneve to avoid using atomic increments when updating RX and TX drop counters, as dstats is per-cpu. Finally, this also simplifies the code as all three modules now handle stats in the same way and with only two different sets of counters (the per-cpu dstats and the atomic DEV_STATS_INC()).

Patch 1 creates dstats helper functions that can be used outside of VRF (before that, dstats was VRF-specific).
Patches 2 to 4 convert VXLAN, Geneve and Bareudp, one by one.

Guillaume Nault (4):
vrf: Make pcpu_dstats update functions available to other modules.
vxlan: Handle stats using NETDEV_PCPU_STAT_DSTATS.
geneve: Handle stats using NETDEV_PCPU_STAT_DSTATS.
bareudp: Handle stats using NETDEV_PCPU_STAT_DSTATS.

 drivers/net/bareudp.c          | 16 ++++++------
 drivers/net/geneve.c           | 12 ++++-----
 drivers/net/vrf.c              | 46 +++++++++-------------------------
 drivers/net/vxlan/vxlan_core.c | 28 ++++++++++-----------
 include/linux/netdevice.h      | 40 +++++++++++++++++++++++++++++
 5 files changed, 80 insertions(+), 62 deletions(-)

Signature · 1 line

2.39.2

Guillaume Nault

author

2024-12-02 21:48 UTC #1 lore raw

PATCH 1/4 vrf: Make pcpu_dstats update functions available to other modules.

Currently vrf is the only module that uses NETDEV_PCPU_STAT_DSTATS. In order to make this kind of statistics available to other modules, we need to define the update functions in netdevice.h.

Therefore, let's define dev_dstats_*() functions for RX and TX packet updates (packets, bytes and drops). Use these new functions in vrf.c instead of vrf_rx_stats() and the other manual counter updates.

While there, update the type of the "len" variables to "unsigned int", so that there're aligned with both skb->len and the new dstats update functions.

Signed-off-by Guillaume Nault <[email protected]>

---
 drivers/net/vrf.c         | 46 ++++++++++-----------------------------
 include/linux/netdevice.h | 40 ++++++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+), 34 deletions(-)

drivers/net/vrf.c +12 ~~-34~~

index 67d25f4f94ef..f0c0b3d4d827 100644
@@ -122,16 +122,6 @@ struct net_vrf {
 	int			ifindex;
 };
 
-static void vrf_rx_stats(struct net_device *dev, int len)
-{
-	struct pcpu_dstats *dstats = this_cpu_ptr(dev->dstats);
-
-	u64_stats_update_begin(&dstats->syncp);
-	u64_stats_inc(&dstats->rx_packets);
-	u64_stats_add(&dstats->rx_bytes, len);
-	u64_stats_update_end(&dstats->syncp);
-}
-
 static void vrf_tx_error(struct net_device *vrf_dev, struct sk_buff *skb)
 {
 	vrf_dev->stats.tx_errors++;

@@ -369,7 +359,7 @@ static bool qdisc_tx_is_default(const struct net_device *dev)
 static int vrf_local_xmit(struct sk_buff *skb, struct net_device *dev,
 			  struct dst_entry *dst)
 {
-	int len = skb->len;
+	unsigned int len = skb->len;
 
 	skb_orphan(skb);

@@ -382,15 +372,10 @@ static int vrf_local_xmit(struct sk_buff *skb, struct net_device *dev,
 
 	skb->protocol = eth_type_trans(skb, dev);
 
-	if (likely(__netif_rx(skb) == NET_RX_SUCCESS)) {
-		vrf_rx_stats(dev, len);
-	} else {
-		struct pcpu_dstats *dstats = this_cpu_ptr(dev->dstats);
-
-		u64_stats_update_begin(&dstats->syncp);
-		u64_stats_inc(&dstats->rx_drops);
-		u64_stats_update_end(&dstats->syncp);
-	}
+	if (likely(__netif_rx(skb) == NET_RX_SUCCESS))
+		dev_dstats_rx_add(dev, len);
+	else
+		dev_dstats_rx_dropped(dev);
 
 	return NETDEV_TX_OK;
 }

@@ -578,20 +563,13 @@ static netdev_tx_t is_ip_tx_frame(struct sk_buff *skb, struct net_device *dev)
 
 static netdev_tx_t vrf_xmit(struct sk_buff *skb, struct net_device *dev)
 {
-	struct pcpu_dstats *dstats = this_cpu_ptr(dev->dstats);
-
-	int len = skb->len;
 	netdev_tx_t ret = is_ip_tx_frame(skb, dev);
+	unsigned int len = skb->len;
 
-	u64_stats_update_begin(&dstats->syncp);
-	if (likely(ret == NET_XMIT_SUCCESS || ret == NET_XMIT_CN)) {
-
-		u64_stats_inc(&dstats->tx_packets);
-		u64_stats_add(&dstats->tx_bytes, len);
-	} else {
-		u64_stats_inc(&dstats->tx_drops);
-	}
-	u64_stats_update_end(&dstats->syncp);
+	if (likely(ret == NET_XMIT_SUCCESS || ret == NET_XMIT_CN))
+		dev_dstats_tx_add(dev, len);
+	else
+		dev_dstats_tx_dropped(dev);
 
 	return ret;
 }

@@ -1364,7 +1342,7 @@ static struct sk_buff *vrf_ip6_rcv(struct net_device *vrf_dev,
 	if (!is_ndisc) {
 		struct net_device *orig_dev = skb->dev;
 
-		vrf_rx_stats(vrf_dev, skb->len);
+		dev_dstats_rx_add(vrf_dev, skb->len);
 		skb->dev = vrf_dev;
 		skb->skb_iif = vrf_dev->ifindex;

@@ -1420,7 +1398,7 @@ static struct sk_buff *vrf_ip_rcv(struct net_device *vrf_dev,
 		goto out;
 	}
 
-	vrf_rx_stats(vrf_dev, skb->len);
+	dev_dstats_rx_add(vrf_dev, skb->len);
 
 	if (!list_empty(&vrf_dev->ptype_all)) {
 		int err;

include/linux/netdevice.h +40 -0

index ecc686409161..b49780c724d7 100644
@@ -2854,6 +2854,46 @@ static inline void dev_lstats_add(struct net_device *dev, unsigned int len)
 	u64_stats_update_end(&lstats->syncp);
 }
 
+static inline void dev_dstats_rx_add(struct net_device *dev,
+				     unsigned int len)
+{
+	struct pcpu_dstats *dstats = this_cpu_ptr(dev->dstats);
+
+	u64_stats_update_begin(&dstats->syncp);
+	u64_stats_inc(&dstats->rx_packets);
+	u64_stats_add(&dstats->rx_bytes, len);
+	u64_stats_update_end(&dstats->syncp);
+}
+
+static inline void dev_dstats_rx_dropped(struct net_device *dev)
+{
+	struct pcpu_dstats *dstats = this_cpu_ptr(dev->dstats);
+
+	u64_stats_update_begin(&dstats->syncp);
+	u64_stats_inc(&dstats->rx_drops);
+	u64_stats_update_end(&dstats->syncp);
+}
+
+static inline void dev_dstats_tx_add(struct net_device *dev,
+				     unsigned int len)
+{
+	struct pcpu_dstats *dstats = this_cpu_ptr(dev->dstats);
+
+	u64_stats_update_begin(&dstats->syncp);
+	u64_stats_inc(&dstats->tx_packets);
+	u64_stats_add(&dstats->tx_bytes, len);
+	u64_stats_update_end(&dstats->syncp);
+}
+
+static inline void dev_dstats_tx_dropped(struct net_device *dev)
+{
+	struct pcpu_dstats *dstats = this_cpu_ptr(dev->dstats);
+
+	u64_stats_update_begin(&dstats->syncp);
+	u64_stats_inc(&dstats->tx_drops);
+	u64_stats_update_end(&dstats->syncp);
+}
+
 #define __netdev_alloc_pcpu_stats(type, gfp)				\
 ({									\
 	typeof(type) __percpu *pcpu_stats = alloc_percpu_gfp(type, gfp);\

Signature · 1 line

2.39.2

Jakub Kicinski

maintainer

2024-12-03 03:59 UTC #2 lore raw

Re: [PATCH net-next 1/4] vrf: Make pcpu_dstats update functions available to other modules.

drivers/net/vrf.c static netdev_tx_t is_ip_tx_frame(struct sk_buff *skb, struct net_device *dev) · line 567

Quoted 3 lines · On Mon, 2 Dec 2024 22:48:48 +0100 Guillaume Nault wrote:

drivers/net/vrf.c +1 -1

-	int len = skb->len;
 	netdev_tx_t ret = is_ip_tx_frame(skb, dev);
+	unsigned int len = skb->len;

You can't reorder skb->len init after is_ip_tx_frame(). IDK what is_ stands for but that function xmits / frees the skb.

You're already making this code cleaner, let's take another step forward and move that call out of line. Complex functions should not be called as part of variable init IMHO. It makes the code harder to read at best and error prone at worst..

Signature · 1 line

pw-bot: cr

Guillaume Nault

author

2024-12-03 11:32 UTC #3 lore raw

Re: [PATCH net-next 1/4] vrf: Make pcpu_dstats update functions available to other modules.

Quoted 7 lines · Jakub Kicinski

drivers/net/vrf.c static netdev_tx_t is_ip_tx_frame(struct sk_buff *skb, struct net_device *dev) · line 567
Quoted 3 lines · On Mon, 2 Dec 2024 22:48:48 +0100 Guillaume Nault wrote:
drivers/net/vrf.c +1 -1
-	int len = skb->len;
 	netdev_tx_t ret = is_ip_tx_frame(skb, dev);
+	unsigned int len = skb->len;
You can't reorder skb->len init after is_ip_tx_frame(). IDK what is_ stands for but that function xmits / frees the skb.

Yes, silly mistake. Sorry for not spotting that :/.

Quoted 4 lines

You're already making this code cleaner, let's take another step forward and move that call out of line. Complex functions should not be called as part of variable init IMHO. It makes the code harder to read at best and error prone at worst..

Yes, fully agree. I'll post v2 tomorrow.

Guillaume Nault

author

2024-12-02 21:48 UTC #4 lore raw

PATCH 2/4 vxlan: Handle stats using NETDEV_PCPU_STAT_DSTATS.

In reply to Guillaume Nault

VXLAN uses the TSTATS infrastructure (dev_sw_netstats_*()) for RX and TX packet counters. It also uses the device core stats (dev_core_stats_*()) for RX and TX drops.

Let's consolidate that using the DSTATS infrastructure, which can handle both packet counters and packet drops. Statistics that don't fit DSTATS are still updated atomically with DEV_STATS_INC().

While there, convert the "len" variable of vxlan_encap_bypass() to unsigned int, to respect the types of skb->len and dev_dstats_[rt]x_add().

Signed-off-by Guillaume Nault <[email protected]>

---
 drivers/net/vxlan/vxlan_core.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

drivers/net/vxlan/vxlan_core.c +14 ~~-14~~

index 9ea63059d52d..b46a799bd390 100644
@@ -1818,14 +1818,14 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb)
 
 	if (unlikely(!(vxlan->dev->flags & IFF_UP))) {
 		rcu_read_unlock();
-		dev_core_stats_rx_dropped_inc(vxlan->dev);
+		dev_dstats_rx_dropped(vxlan->dev);
 		vxlan_vnifilter_count(vxlan, vni, vninode,
 				      VXLAN_VNI_STATS_RX_DROPS, 0);
 		reason = SKB_DROP_REASON_DEV_READY;
 		goto drop;
 	}
 
-	dev_sw_netstats_rx_add(vxlan->dev, skb->len);
+	dev_dstats_rx_add(vxlan->dev, skb->len);
 	vxlan_vnifilter_count(vxlan, vni, vninode, VXLAN_VNI_STATS_RX, skb->len);
 	gro_cells_receive(&vxlan->gro_cells, skb);

@@ -1880,7 +1880,7 @@ static int arp_reduce(struct net_device *dev, struct sk_buff *skb, __be32 vni)
 		goto out;
 
 	if (!pskb_may_pull(skb, arp_hdr_len(dev))) {
-		dev_core_stats_tx_dropped_inc(dev);
+		dev_dstats_tx_dropped(dev);
 		vxlan_vnifilter_count(vxlan, vni, NULL,
 				      VXLAN_VNI_STATS_TX_DROPS, 0);
 		goto out;

@@ -1938,7 +1938,7 @@ static int arp_reduce(struct net_device *dev, struct sk_buff *skb, __be32 vni)
 		reply->pkt_type = PACKET_HOST;
 
 		if (netif_rx(reply) == NET_RX_DROP) {
-			dev_core_stats_rx_dropped_inc(dev);
+			dev_dstats_rx_dropped(dev);
 			vxlan_vnifilter_count(vxlan, vni, NULL,
 					      VXLAN_VNI_STATS_RX_DROPS, 0);
 		}

@@ -2097,7 +2097,7 @@ static int neigh_reduce(struct net_device *dev, struct sk_buff *skb, __be32 vni)
 			goto out;
 
 		if (netif_rx(reply) == NET_RX_DROP) {
-			dev_core_stats_rx_dropped_inc(dev);
+			dev_dstats_rx_dropped(dev);
 			vxlan_vnifilter_count(vxlan, vni, NULL,
 					      VXLAN_VNI_STATS_RX_DROPS, 0);
 		}

@@ -2271,8 +2271,8 @@ static void vxlan_encap_bypass(struct sk_buff *skb, struct vxlan_dev *src_vxlan,
 {
 	union vxlan_addr loopback;
 	union vxlan_addr *remote_ip = &dst_vxlan->default_dst.remote_ip;
+	unsigned int len = skb->len;
 	struct net_device *dev;
-	int len = skb->len;
 
 	skb->pkt_type = PACKET_HOST;
 	skb->encapsulation = 0;

@@ -2299,16 +2299,16 @@ static void vxlan_encap_bypass(struct sk_buff *skb, struct vxlan_dev *src_vxlan,
 	if ((dst_vxlan->cfg.flags & VXLAN_F_LEARN) && snoop)
 		vxlan_snoop(dev, &loopback, eth_hdr(skb)->h_source, 0, vni);
 
-	dev_sw_netstats_tx_add(src_vxlan->dev, 1, len);
+	dev_dstats_tx_add(src_vxlan->dev, len);
 	vxlan_vnifilter_count(src_vxlan, vni, NULL, VXLAN_VNI_STATS_TX, len);
 
 	if (__netif_rx(skb) == NET_RX_SUCCESS) {
-		dev_sw_netstats_rx_add(dst_vxlan->dev, len);
+		dev_dstats_rx_add(dst_vxlan->dev, len);
 		vxlan_vnifilter_count(dst_vxlan, vni, NULL, VXLAN_VNI_STATS_RX,
 				      len);
 	} else {
 drop:
-		dev_core_stats_rx_dropped_inc(dev);
+		dev_dstats_rx_dropped(dev);
 		vxlan_vnifilter_count(dst_vxlan, vni, NULL,
 				      VXLAN_VNI_STATS_RX_DROPS, 0);
 	}

@@ -2621,7 +2621,7 @@ void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
 	return;
 
 drop:
-	dev_core_stats_tx_dropped_inc(dev);
+	dev_dstats_tx_dropped(dev);
 	vxlan_vnifilter_count(vxlan, vni, NULL, VXLAN_VNI_STATS_TX_DROPS, 0);
 	kfree_skb_reason(skb, reason);
 	return;

@@ -2666,7 +2666,7 @@ static void vxlan_xmit_nh(struct sk_buff *skb, struct net_device *dev,
 	return;
 
 drop:
-	dev_core_stats_tx_dropped_inc(dev);
+	dev_dstats_tx_dropped(dev);
 	vxlan_vnifilter_count(netdev_priv(dev), vni, NULL,
 			      VXLAN_VNI_STATS_TX_DROPS, 0);
 	dev_kfree_skb(skb);

@@ -2704,7 +2704,7 @@ static netdev_tx_t vxlan_xmit_nhid(struct sk_buff *skb, struct net_device *dev,
 	return NETDEV_TX_OK;
 
 drop:
-	dev_core_stats_tx_dropped_inc(dev);
+	dev_dstats_tx_dropped(dev);
 	vxlan_vnifilter_count(netdev_priv(dev), vni, NULL,
 			      VXLAN_VNI_STATS_TX_DROPS, 0);
 	dev_kfree_skb(skb);

@@ -2801,7 +2801,7 @@ static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev)
 			    !is_multicast_ether_addr(eth->h_dest))
 				vxlan_fdb_miss(vxlan, eth->h_dest);
 
-			dev_core_stats_tx_dropped_inc(dev);
+			dev_dstats_tx_dropped(dev);
 			vxlan_vnifilter_count(vxlan, vni, NULL,
 					      VXLAN_VNI_STATS_TX_DROPS, 0);
 			kfree_skb_reason(skb, SKB_DROP_REASON_VXLAN_NO_REMOTE);

@@ -3371,7 +3371,7 @@ static void vxlan_setup(struct net_device *dev)
 	dev->min_mtu = ETH_MIN_MTU;
 	dev->max_mtu = ETH_MAX_MTU;
 
-	dev->pcpu_stat_type = NETDEV_PCPU_STAT_TSTATS;
+	dev->pcpu_stat_type = NETDEV_PCPU_STAT_DSTATS;
 	INIT_LIST_HEAD(&vxlan->next);
 
 	timer_setup(&vxlan->age_timer, vxlan_cleanup, TIMER_DEFERRABLE);

Signature · 1 line

2.39.2

Guillaume Nault

author

2024-12-02 21:49 UTC #5 lore raw

PATCH 3/4 geneve: Handle stats using NETDEV_PCPU_STAT_DSTATS.

In reply to Guillaume Nault

Geneve uses the TSTATS infrastructure (dev_sw_netstats_*()) for RX packet counters. All other counters are handled using atomic increments with DEV_STATS_INC().

Let's convert packet stats handling to DSTATS, which has a per-cpu counter for packet drops too, to avoid the cost of atomic increments in these cases. Statistics that don't fit DSTATS are still updated atomically with DEV_STATS_INC().

Signed-off-by Guillaume Nault <[email protected]>

---
 drivers/net/geneve.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

drivers/net/geneve.c +6 -6

index 2f29b1386b1c..d927737010cf 100644
@@ -235,7 +235,7 @@ static void geneve_rx(struct geneve_dev *geneve, struct geneve_sock *gs,
 					 vni_to_tunnel_id(gnvh->vni),
 					 gnvh->opt_len * 4);
 		if (!tun_dst) {
-			DEV_STATS_INC(geneve->dev, rx_dropped);
+			dev_dstats_rx_dropped(geneve->dev);
 			goto drop;
 		}
 		/* Update tunnel dst according to Geneve options. */

@@ -322,7 +322,7 @@ static void geneve_rx(struct geneve_dev *geneve, struct geneve_sock *gs,
 	len = skb->len;
 	err = gro_cells_receive(&geneve->gro_cells, skb);
 	if (likely(err == NET_RX_SUCCESS))
-		dev_sw_netstats_rx_add(geneve->dev, len);
+		dev_dstats_rx_add(geneve->dev, len);
 
 	return;
 drop:

@@ -387,14 +387,14 @@ static int geneve_udp_encap_recv(struct sock *sk, struct sk_buff *skb)
 
 	if (unlikely((!geneve->cfg.inner_proto_inherit &&
 		      inner_proto != htons(ETH_P_TEB)))) {
-		DEV_STATS_INC(geneve->dev, rx_dropped);
+		dev_dstats_rx_dropped(geneve->dev);
 		goto drop;
 	}
 
 	opts_len = geneveh->opt_len * 4;
 	if (iptunnel_pull_header(skb, GENEVE_BASE_HLEN + opts_len, inner_proto,
 				 !net_eq(geneve->net, dev_net(geneve->dev)))) {
-		DEV_STATS_INC(geneve->dev, rx_dropped);
+		dev_dstats_rx_dropped(geneve->dev);
 		goto drop;
 	}

@@ -1023,7 +1023,7 @@ static netdev_tx_t geneve_xmit(struct sk_buff *skb, struct net_device *dev)
 		if (unlikely(!info || !(info->mode & IP_TUNNEL_INFO_TX))) {
 			netdev_dbg(dev, "no tunnel metadata\n");
 			dev_kfree_skb(skb);
-			DEV_STATS_INC(dev, tx_dropped);
+			dev_dstats_tx_dropped(dev);
 			return NETDEV_TX_OK;
 		}
 	} else {

@@ -1202,7 +1202,7 @@ static void geneve_setup(struct net_device *dev)
 	dev->hw_features |= NETIF_F_RXCSUM;
 	dev->hw_features |= NETIF_F_GSO_SOFTWARE;
 
-	dev->pcpu_stat_type = NETDEV_PCPU_STAT_TSTATS;
+	dev->pcpu_stat_type = NETDEV_PCPU_STAT_DSTATS;
 	/* MTU range: 68 - (something less than 65535) */
 	dev->min_mtu = ETH_MIN_MTU;
 	/* The max_mtu calculation does not take account of GENEVE

Signature · 1 line

2.39.2

Guillaume Nault

author

2024-12-02 21:49 UTC #6 lore raw

PATCH 4/4 bareudp: Handle stats using NETDEV_PCPU_STAT_DSTATS.

In reply to Guillaume Nault

Bareudp uses the TSTATS infrastructure (dev_sw_netstats_*()) for RX packet counters. It was also recently converted to use the device core stats (dev_core_stats_*()) for RX and TX drops (see commit 788d5d655bc9 ("bareudp: Use pcpu stats to update rx_dropped counter.")).

Since core stats are to be avoided in drivers, and for consistency with VXLAN and Geneve, let's convert packet stats handling to DSTATS, which can handle RX/TX stats and packet drops. Statistics that don't fit DSTATS are still updated atomically with DEV_STATS_INC().

Signed-off-by Guillaume Nault <[email protected]>

---
 drivers/net/bareudp.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

drivers/net/bareudp.c +8 -8

index a2abfade82dd..70814303aab8 100644
@@ -84,7 +84,7 @@ static int bareudp_udp_encap_recv(struct sock *sk, struct sk_buff *skb)
 
 		if (skb_copy_bits(skb, BAREUDP_BASE_HLEN, &ipversion,
 				  sizeof(ipversion))) {
-			dev_core_stats_rx_dropped_inc(bareudp->dev);
+			dev_dstats_rx_dropped(bareudp->dev);
 			goto drop;
 		}
 		ipversion >>= 4;

@@ -94,7 +94,7 @@ static int bareudp_udp_encap_recv(struct sock *sk, struct sk_buff *skb)
 		} else if (ipversion == 6 && bareudp->multi_proto_mode) {
 			proto = htons(ETH_P_IPV6);
 		} else {
-			dev_core_stats_rx_dropped_inc(bareudp->dev);
+			dev_dstats_rx_dropped(bareudp->dev);
 			goto drop;
 		}
 	} else if (bareudp->ethertype == htons(ETH_P_MPLS_UC)) {

@@ -108,7 +108,7 @@ static int bareudp_udp_encap_recv(struct sock *sk, struct sk_buff *skb)
 				   ipv4_is_multicast(tunnel_hdr->daddr)) {
 				proto = htons(ETH_P_MPLS_MC);
 			} else {
-				dev_core_stats_rx_dropped_inc(bareudp->dev);
+				dev_dstats_rx_dropped(bareudp->dev);
 				goto drop;
 			}
 		} else {

@@ -124,7 +124,7 @@ static int bareudp_udp_encap_recv(struct sock *sk, struct sk_buff *skb)
 				   (addr_type & IPV6_ADDR_MULTICAST)) {
 				proto = htons(ETH_P_MPLS_MC);
 			} else {
-				dev_core_stats_rx_dropped_inc(bareudp->dev);
+				dev_dstats_rx_dropped(bareudp->dev);
 				goto drop;
 			}
 		}

@@ -136,7 +136,7 @@ static int bareudp_udp_encap_recv(struct sock *sk, struct sk_buff *skb)
 				 proto,
 				 !net_eq(bareudp->net,
 				 dev_net(bareudp->dev)))) {
-		dev_core_stats_rx_dropped_inc(bareudp->dev);
+		dev_dstats_rx_dropped(bareudp->dev);
 		goto drop;
 	}

@@ -144,7 +144,7 @@ static int bareudp_udp_encap_recv(struct sock *sk, struct sk_buff *skb)
 
 	tun_dst = udp_tun_rx_dst(skb, family, key, 0, 0);
 	if (!tun_dst) {
-		dev_core_stats_rx_dropped_inc(bareudp->dev);
+		dev_dstats_rx_dropped(bareudp->dev);
 		goto drop;
 	}
 	skb_dst_set(skb, &tun_dst->dst);

@@ -194,7 +194,7 @@ static int bareudp_udp_encap_recv(struct sock *sk, struct sk_buff *skb)
 	len = skb->len;
 	err = gro_cells_receive(&bareudp->gro_cells, skb);
 	if (likely(err == NET_RX_SUCCESS))
-		dev_sw_netstats_rx_add(bareudp->dev, len);
+		dev_dstats_rx_add(bareudp->dev, len);
 
 	return 0;
 drop:

@@ -589,7 +589,7 @@ static void bareudp_setup(struct net_device *dev)
 	dev->priv_flags |= IFF_NO_QUEUE;
 	dev->lltx = true;
 	dev->flags = IFF_POINTOPOINT | IFF_NOARP | IFF_MULTICAST;
-	dev->pcpu_stat_type = NETDEV_PCPU_STAT_TSTATS;
+	dev->pcpu_stat_type = NETDEV_PCPU_STAT_DSTATS;
 }
 
 static int bareudp_validate(struct nlattr *tb[], struct nlattr *data[],

Signature · 1 line

2.39.2

James Chapman

2024-12-03 08:20 UTC #7 lore raw

In reply to Guillaume Nault

Hi Guillaume,

I can work on similar changes to l2tp if you haven't already started work on it.

James

Quoted 40 lines · Guillaume Nault

VXLAN, Geneve and Bareudp use various device counters for managing RX and TX statistics:
* VXLAN uses the device core_stats for RX and TX drops, tstats for regular RX/TX counters and DEV_STATS_INC() for various types of RX/TX errors.
* Geneve uses tstats for regular RX/TX counters and DEV_STATS_INC() for everything else, include RX/TX drops.
* Bareudp, was recently converted to follow VXLAN behaviour, that is, device core_stats for RX and TX drops, tstats for regular RX/TX counters and DEV_STATS_INC() for other counter types.
Let's consolidate statistics management around the dstats counters instead. This avoids using core_stats in VXLAN and Bareudp, as core_stats is supposed to be used by core networking code only (and not in drivers). This also allows Geneve to avoid using atomic increments when updating RX and TX drop counters, as dstats is per-cpu. Finally, this also simplifies the code as all three modules now handle stats in the same way and with only two different sets of counters (the per-cpu dstats and the atomic DEV_STATS_INC()).
Patch 1 creates dstats helper functions that can be used outside of VRF (before that, dstats was VRF-specific).
Patches 2 to 4 convert VXLAN, Geneve and Bareudp, one by one.
Guillaume Nault (4):
vrf: Make pcpu_dstats update functions available to other modules.
vxlan: Handle stats using NETDEV_PCPU_STAT_DSTATS.
geneve: Handle stats using NETDEV_PCPU_STAT_DSTATS.
bareudp: Handle stats using NETDEV_PCPU_STAT_DSTATS.
  drivers/net/bareudp.c          | 16 ++++++------
  drivers/net/geneve.c           | 12 ++++-----
  drivers/net/vrf.c              | 46 +++++++++-------------------------
  drivers/net/vxlan/vxlan_core.c | 28 ++++++++++-----------
  include/linux/netdevice.h      | 40 +++++++++++++++++++++++++++++
  5 files changed, 80 insertions(+), 62 deletions(-)

Signature · 4 lines

James Chapman
Katalix Systems Ltd
https://katalix.com
Catalysts for your Embedded Linux software development

Guillaume Nault

author

2024-12-03 11:39 UTC #8 lore raw

Quoted 4 lines · James Chapman

Hi Guillaume,
I can work on similar changes to l2tp if you haven't already started work on it.

I haven't, so yes, please do. You can Cc me when submitting the patch, so that I can ack it in time.

Quoted 50 lines

James
Quoted 40 lines · Guillaume Nault
VXLAN, Geneve and Bareudp use various device counters for managing RX and TX statistics:
* VXLAN uses the device core_stats for RX and TX drops, tstats for regular RX/TX counters and DEV_STATS_INC() for various types of RX/TX errors.
* Geneve uses tstats for regular RX/TX counters and DEV_STATS_INC() for everything else, include RX/TX drops.
* Bareudp, was recently converted to follow VXLAN behaviour, that is, device core_stats for RX and TX drops, tstats for regular RX/TX counters and DEV_STATS_INC() for other counter types.
Let's consolidate statistics management around the dstats counters instead. This avoids using core_stats in VXLAN and Bareudp, as core_stats is supposed to be used by core networking code only (and not in drivers). This also allows Geneve to avoid using atomic increments when updating RX and TX drop counters, as dstats is per-cpu. Finally, this also simplifies the code as all three modules now handle stats in the same way and with only two different sets of counters (the per-cpu dstats and the atomic DEV_STATS_INC()).
Patch 1 creates dstats helper functions that can be used outside of VRF (before that, dstats was VRF-specific).
Patches 2 to 4 convert VXLAN, Geneve and Bareudp, one by one.
Guillaume Nault (4):
vrf: Make pcpu_dstats update functions available to other modules.
vxlan: Handle stats using NETDEV_PCPU_STAT_DSTATS.
geneve: Handle stats using NETDEV_PCPU_STAT_DSTATS.
bareudp: Handle stats using NETDEV_PCPU_STAT_DSTATS.
  drivers/net/bareudp.c          | 16 ++++++------
  drivers/net/geneve.c           | 12 ++++-----
  drivers/net/vrf.c              | 46 +++++++++-------------------------
  drivers/net/vxlan/vxlan_core.c | 28 ++++++++++-----------
  include/linux/netdevice.h      | 40 +++++++++++++++++++++++++++++
  5 files changed, 80 insertions(+), 62 deletions(-)
Signature · 4 lines
James Chapman
Katalix Systems Ltd
https://katalix.com
Catalysts for your Embedded Linux software development