netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v2 0/3] net: tunnel: introduce noref xmit flows for tunnels
       [not found] <20250912112420.4394-1-mmietus97.ref@yahoo.com>
@ 2025-09-12 11:24 ` Marek Mietus
  2025-09-12 11:24   ` [PATCH net-next v2 1/3] net: dst_cache: implement RCU variants for dst_cache helpers Marek Mietus
                     ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Marek Mietus @ 2025-09-12 11:24 UTC (permalink / raw)
  To: netdev, antonio, kuba; +Cc: openvpn-devel, Marek Mietus

Currently, all xmit flows use dst_cache in a way that references
its dst_entry for each xmitted packet. These atomic operations
are redundant in some flows.

This patchset introduces new noref xmit helpers, and incorporates
them in the OpenVPN driver.

Changes in v2:
 - Fixed docstrings for dst_cache_get_ip4_rcu and dst_cache_steal_ip4

Link to v1: https://lore.kernel.org/netdev/20250909054333.12572-1-mmietus97@yahoo.com/
-- 
2.51.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH net-next v2 1/3] net: dst_cache: implement RCU variants for dst_cache helpers
  2025-09-12 11:24 ` [PATCH net-next v2 0/3] net: tunnel: introduce noref xmit flows for tunnels Marek Mietus
@ 2025-09-12 11:24   ` Marek Mietus
  2025-09-16  9:28     ` Paolo Abeni
  2025-09-12 11:24   ` [PATCH net-next v2 2/3] net: tunnel: implement noref flows in udp_tunnel_xmit_skb Marek Mietus
  2025-09-12 11:24   ` [PATCH net-next v2 3/3] net: ovpn: use new noref xmit flow in ovpn_udp4_output Marek Mietus
  2 siblings, 1 reply; 8+ messages in thread
From: Marek Mietus @ 2025-09-12 11:24 UTC (permalink / raw)
  To: netdev, antonio, kuba; +Cc: openvpn-devel, Marek Mietus

Implement RCU variants for existing dst_cache helpers interacting with
dst_entry.

The new helpers avoid referencing the dst_entry, sparing some unnecessary
atomic operations. They should only be used in flows that are already
guaranteed to be inside a RCU read-side critical context.

Signed-off-by: Marek Mietus <mmietus97@yahoo.com>
---
 include/net/dst_cache.h | 26 ++++++++++++++
 net/core/dst_cache.c    | 78 +++++++++++++++++++++++++++++++++++------
 2 files changed, 94 insertions(+), 10 deletions(-)

diff --git a/include/net/dst_cache.h b/include/net/dst_cache.h
index 1961699598e2..d3bf616a6e6f 100644
--- a/include/net/dst_cache.h
+++ b/include/net/dst_cache.h
@@ -32,6 +32,21 @@ struct dst_entry *dst_cache_get(struct dst_cache *dst_cache);
  */
 struct rtable *dst_cache_get_ip4(struct dst_cache *dst_cache, __be32 *saddr);
 
+/**
+ * dst_cache_get_ip4_rcu - lookup cache and ipv4 source under RCU
+ * @dst_cache: the cache
+ * @saddr: return value for the retrieved source address
+ *
+ * Perform cache lookup and fetch ipv4 source without taking a
+ * reference on the dst.
+ * Must be called with local BH disabled, and within an rcu read side
+ * critical section.
+ *
+ * Return: Pointer to retrieved rtable if cache is initialized and
+ * cached dst is valid, NULL otherwise.
+ */
+struct rtable *dst_cache_get_ip4_rcu(struct dst_cache *dst_cache, __be32 *saddr);
+
 /**
  *	dst_cache_set_ip4 - store the ipv4 dst into the cache
  *	@dst_cache: the cache
@@ -43,6 +58,17 @@ struct rtable *dst_cache_get_ip4(struct dst_cache *dst_cache, __be32 *saddr);
 void dst_cache_set_ip4(struct dst_cache *dst_cache, struct dst_entry *dst,
 		       __be32 saddr);
 
+/**
+ * dst_cache_steal_ip4 - store the ipv4 dst into the cache and steal its
+ * reference
+ * @dst_cache: the cache
+ * @dst: the entry to be cached whose reference will be stolen
+ * @saddr: the source address to be stored inside the cache
+ *
+ * local BH must be disabled
+ */
+void dst_cache_steal_ip4(struct dst_cache *dst_cache, struct dst_entry *dst,
+			 __be32 saddr);
 #if IS_ENABLED(CONFIG_IPV6)
 
 /**
diff --git a/net/core/dst_cache.c b/net/core/dst_cache.c
index 9ab4902324e1..f1e3992d8171 100644
--- a/net/core/dst_cache.c
+++ b/net/core/dst_cache.c
@@ -25,20 +25,30 @@ struct dst_cache_pcpu {
 	};
 };
 
-static void dst_cache_per_cpu_dst_set(struct dst_cache_pcpu *dst_cache,
-				      struct dst_entry *dst, u32 cookie)
+static void __dst_cache_per_cpu_dst_set(struct dst_cache_pcpu *dst_cache,
+					struct dst_entry *dst, u32 cookie)
 {
 	DEBUG_NET_WARN_ON_ONCE(!in_softirq());
 	dst_release(dst_cache->dst);
-	if (dst)
-		dst_hold(dst);
 
 	dst_cache->cookie = cookie;
 	dst_cache->dst = dst;
 }
 
-static struct dst_entry *dst_cache_per_cpu_get(struct dst_cache *dst_cache,
-					       struct dst_cache_pcpu *idst)
+static void dst_cache_per_cpu_dst_set(struct dst_cache_pcpu *dst_cache,
+				      struct dst_entry *dst, u32 cookie)
+{
+	if (dst == dst_cache->dst && cookie == dst_cache->cookie)
+		return;
+
+	if (dst)
+		dst_hold(dst);
+
+	__dst_cache_per_cpu_dst_set(dst_cache, dst, cookie);
+}
+
+static struct dst_entry *__dst_cache_per_cpu_get(struct dst_cache *dst_cache,
+						 struct dst_cache_pcpu *idst)
 {
 	struct dst_entry *dst;
 
@@ -47,14 +57,10 @@ static struct dst_entry *dst_cache_per_cpu_get(struct dst_cache *dst_cache,
 	if (!dst)
 		goto fail;
 
-	/* the cache already hold a dst reference; it can't go away */
-	dst_hold(dst);
-
 	if (unlikely(!time_after(idst->refresh_ts,
 				 READ_ONCE(dst_cache->reset_ts)) ||
 		     (READ_ONCE(dst->obsolete) && !dst->ops->check(dst, idst->cookie)))) {
 		dst_cache_per_cpu_dst_set(idst, NULL, 0);
-		dst_release(dst);
 		goto fail;
 	}
 	return dst;
@@ -64,6 +70,18 @@ static struct dst_entry *dst_cache_per_cpu_get(struct dst_cache *dst_cache,
 	return NULL;
 }
 
+static struct dst_entry *dst_cache_per_cpu_get(struct dst_cache *dst_cache,
+					       struct dst_cache_pcpu *idst)
+{
+	struct dst_entry *dst;
+
+	dst = __dst_cache_per_cpu_get(dst_cache, idst);
+	if (dst)
+		/* the cache already hold a dst reference; it can't go away */
+		dst_hold(dst);
+	return dst;
+}
+
 struct dst_entry *dst_cache_get(struct dst_cache *dst_cache)
 {
 	struct dst_entry *dst;
@@ -100,6 +118,28 @@ struct rtable *dst_cache_get_ip4(struct dst_cache *dst_cache, __be32 *saddr)
 }
 EXPORT_SYMBOL_GPL(dst_cache_get_ip4);
 
+struct rtable *dst_cache_get_ip4_rcu(struct dst_cache *dst_cache, __be32 *saddr)
+{
+	struct dst_cache_pcpu *idst;
+	struct dst_entry *dst;
+
+	if (!dst_cache->cache)
+		return NULL;
+
+	local_lock_nested_bh(&dst_cache->cache->bh_lock);
+	idst = this_cpu_ptr(dst_cache->cache);
+	dst = __dst_cache_per_cpu_get(dst_cache, idst);
+	if (!dst) {
+		local_unlock_nested_bh(&dst_cache->cache->bh_lock);
+		return NULL;
+	}
+
+	*saddr = idst->in_saddr.s_addr;
+	local_unlock_nested_bh(&dst_cache->cache->bh_lock);
+	return dst_rtable(dst);
+}
+EXPORT_SYMBOL_GPL(dst_cache_get_ip4_rcu);
+
 void dst_cache_set_ip4(struct dst_cache *dst_cache, struct dst_entry *dst,
 		       __be32 saddr)
 {
@@ -116,6 +156,24 @@ void dst_cache_set_ip4(struct dst_cache *dst_cache, struct dst_entry *dst,
 }
 EXPORT_SYMBOL_GPL(dst_cache_set_ip4);
 
+void dst_cache_steal_ip4(struct dst_cache *dst_cache, struct dst_entry *dst,
+			 __be32 saddr)
+{
+	struct dst_cache_pcpu *idst;
+
+	if (!dst_cache->cache) {
+		dst_release(dst);
+		return;
+	}
+
+	local_lock_nested_bh(&dst_cache->cache->bh_lock);
+	idst = this_cpu_ptr(dst_cache->cache);
+	__dst_cache_per_cpu_dst_set(idst, dst, 0);
+	idst->in_saddr.s_addr = saddr;
+	local_unlock_nested_bh(&dst_cache->cache->bh_lock);
+}
+EXPORT_SYMBOL_GPL(dst_cache_steal_ip4);
+
 #if IS_ENABLED(CONFIG_IPV6)
 void dst_cache_set_ip6(struct dst_cache *dst_cache, struct dst_entry *dst,
 		       const struct in6_addr *saddr)
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH net-next v2 2/3] net: tunnel: implement noref flows in udp_tunnel_xmit_skb
  2025-09-12 11:24 ` [PATCH net-next v2 0/3] net: tunnel: introduce noref xmit flows for tunnels Marek Mietus
  2025-09-12 11:24   ` [PATCH net-next v2 1/3] net: dst_cache: implement RCU variants for dst_cache helpers Marek Mietus
@ 2025-09-12 11:24   ` Marek Mietus
  2025-09-12 11:24   ` [PATCH net-next v2 3/3] net: ovpn: use new noref xmit flow in ovpn_udp4_output Marek Mietus
  2 siblings, 0 replies; 8+ messages in thread
From: Marek Mietus @ 2025-09-12 11:24 UTC (permalink / raw)
  To: netdev, antonio, kuba; +Cc: openvpn-devel, Marek Mietus

In cases where udp_tunnel_xmit_skb is called inside a RCU read-side
critical section, we can avoid referencing the dst_entry.

Implement noref variants for udp_tunnel_xmit_skb and iptunnel_xmit to be
used in noref flows.

Signed-off-by: Marek Mietus <mmietus97@yahoo.com>
---
 include/net/ip_tunnels.h   |  3 +++
 include/net/udp_tunnel.h   |  4 ++++
 net/ipv4/ip_tunnel_core.c  | 34 +++++++++++++++++++++++++++-------
 net/ipv4/udp_tunnel_core.c | 29 +++++++++++++++++++++++++----
 4 files changed, 59 insertions(+), 11 deletions(-)

diff --git a/include/net/ip_tunnels.h b/include/net/ip_tunnels.h
index 8cf1380f3656..6dcea237bf21 100644
--- a/include/net/ip_tunnels.h
+++ b/include/net/ip_tunnels.h
@@ -604,6 +604,9 @@ static inline int iptunnel_pull_header(struct sk_buff *skb, int hdr_len,
 void iptunnel_xmit(struct sock *sk, struct rtable *rt, struct sk_buff *skb,
 		   __be32 src, __be32 dst, u8 proto,
 		   u8 tos, u8 ttl, __be16 df, bool xnet, u16 ipcb_flags);
+void iptunnel_xmit_noref(struct sock *sk, struct rtable *rt, struct sk_buff *skb,
+			 __be32 src, __be32 dst, u8 proto,
+			 u8 tos, u8 ttl, __be16 df, bool xnet, u16 ipcb_flags);
 struct metadata_dst *iptunnel_metadata_reply(struct metadata_dst *md,
 					     gfp_t flags);
 int skb_tunnel_check_pmtu(struct sk_buff *skb, struct dst_entry *encap_dst,
diff --git a/include/net/udp_tunnel.h b/include/net/udp_tunnel.h
index 9acef2fbd2fd..033098ebf789 100644
--- a/include/net/udp_tunnel.h
+++ b/include/net/udp_tunnel.h
@@ -135,6 +135,10 @@ void udp_tunnel_xmit_skb(struct rtable *rt, struct sock *sk, struct sk_buff *skb
 			 __be32 src, __be32 dst, __u8 tos, __u8 ttl,
 			 __be16 df, __be16 src_port, __be16 dst_port,
 			 bool xnet, bool nocheck, u16 ipcb_flags);
+void udp_tunnel_xmit_skb_noref(struct rtable *rt, struct sock *sk, struct sk_buff *skb,
+			       __be32 src, __be32 dst, __u8 tos, __u8 ttl,
+			       __be16 df, __be16 src_port, __be16 dst_port,
+			       bool xnet, bool nocheck, u16 ipcb_flags);
 
 void udp_tunnel6_xmit_skb(struct dst_entry *dst, struct sock *sk,
 			  struct sk_buff *skb,
diff --git a/net/ipv4/ip_tunnel_core.c b/net/ipv4/ip_tunnel_core.c
index cc9915543637..8b03fb380397 100644
--- a/net/ipv4/ip_tunnel_core.c
+++ b/net/ipv4/ip_tunnel_core.c
@@ -47,10 +47,10 @@ const struct ip6_tnl_encap_ops __rcu *
 		ip6tun_encaps[MAX_IPTUN_ENCAP_OPS] __read_mostly;
 EXPORT_SYMBOL(ip6tun_encaps);
 
-void iptunnel_xmit(struct sock *sk, struct rtable *rt, struct sk_buff *skb,
-		   __be32 src, __be32 dst, __u8 proto,
-		   __u8 tos, __u8 ttl, __be16 df, bool xnet,
-		   u16 ipcb_flags)
+static void __iptunnel_xmit(struct sock *sk, struct rtable *rt,
+			    struct sk_buff *skb, __be32 src, __be32 dst,
+			    __u8 proto, __u8 tos, __u8 ttl, __be16 df,
+			    u16 ipcb_flags)
 {
 	int pkt_len = skb->len - skb_inner_network_offset(skb);
 	struct net *net = dev_net(rt->dst.dev);
@@ -58,10 +58,7 @@ void iptunnel_xmit(struct sock *sk, struct rtable *rt, struct sk_buff *skb,
 	struct iphdr *iph;
 	int err;
 
-	skb_scrub_packet(skb, xnet);
-
 	skb_clear_hash_if_not_l4(skb);
-	skb_dst_set(skb, &rt->dst);
 	memset(IPCB(skb), 0, sizeof(*IPCB(skb)));
 	IPCB(skb)->flags = ipcb_flags;
 
@@ -89,8 +86,31 @@ void iptunnel_xmit(struct sock *sk, struct rtable *rt, struct sk_buff *skb,
 		iptunnel_xmit_stats(dev, pkt_len);
 	}
 }
+
+void iptunnel_xmit(struct sock *sk, struct rtable *rt, struct sk_buff *skb,
+		   __be32 src, __be32 dst, __u8 proto,
+		   __u8 tos, __u8 ttl, __be16 df, bool xnet,
+		   u16 ipcb_flags)
+{
+	skb_scrub_packet(skb, xnet);
+	skb_dst_set(skb, &rt->dst);
+
+	__iptunnel_xmit(sk, rt, skb, src, dst, proto, tos, ttl, df, ipcb_flags);
+}
 EXPORT_SYMBOL_GPL(iptunnel_xmit);
 
+void iptunnel_xmit_noref(struct sock *sk, struct rtable *rt, struct sk_buff *skb,
+			 __be32 src, __be32 dst, __u8 proto,
+			 __u8 tos, __u8 ttl, __be16 df, bool xnet,
+			 u16 ipcb_flags)
+{
+	skb_scrub_packet(skb, xnet);
+	skb_dst_set_noref(skb, &rt->dst);
+
+	__iptunnel_xmit(sk, rt, skb, src, dst, proto, tos, ttl, df, ipcb_flags);
+}
+EXPORT_SYMBOL_GPL(iptunnel_xmit_noref);
+
 int __iptunnel_pull_header(struct sk_buff *skb, int hdr_len,
 			   __be16 inner_proto, bool raw_proto, bool xnet)
 {
diff --git a/net/ipv4/udp_tunnel_core.c b/net/ipv4/udp_tunnel_core.c
index fce945f23069..c0e9007cf081 100644
--- a/net/ipv4/udp_tunnel_core.c
+++ b/net/ipv4/udp_tunnel_core.c
@@ -170,10 +170,9 @@ void udp_tunnel_notify_del_rx_port(struct socket *sock, unsigned short type)
 }
 EXPORT_SYMBOL_GPL(udp_tunnel_notify_del_rx_port);
 
-void udp_tunnel_xmit_skb(struct rtable *rt, struct sock *sk, struct sk_buff *skb,
-			 __be32 src, __be32 dst, __u8 tos, __u8 ttl,
-			 __be16 df, __be16 src_port, __be16 dst_port,
-			 bool xnet, bool nocheck, u16 ipcb_flags)
+static void udp_tunnel_add_hdr(struct sk_buff *skb, __be32 src,
+			       __be32 dst, __be16 src_port,
+			       __be16 dst_port, bool nocheck)
 {
 	struct udphdr *uh;
 
@@ -188,12 +187,34 @@ void udp_tunnel_xmit_skb(struct rtable *rt, struct sock *sk, struct sk_buff *skb
 	memset(&(IPCB(skb)->opt), 0, sizeof(IPCB(skb)->opt));
 
 	udp_set_csum(nocheck, skb, src, dst, skb->len);
+}
+
+void udp_tunnel_xmit_skb(struct rtable *rt, struct sock *sk,
+			 struct sk_buff *skb, __be32 src, __be32 dst,
+			 __u8 tos, __u8 ttl, __be16 df, __be16 src_port,
+			 __be16 dst_port, bool xnet, bool nocheck,
+			 u16 ipcb_flags)
+{
+	udp_tunnel_add_hdr(skb, src, dst, src_port, dst_port, nocheck);
 
 	iptunnel_xmit(sk, rt, skb, src, dst, IPPROTO_UDP, tos, ttl, df, xnet,
 		      ipcb_flags);
 }
 EXPORT_SYMBOL_GPL(udp_tunnel_xmit_skb);
 
+void udp_tunnel_xmit_skb_noref(struct rtable *rt, struct sock *sk,
+			       struct sk_buff *skb, __be32 src, __be32 dst,
+			       __u8 tos, __u8 ttl, __be16 df, __be16 src_port,
+			       __be16 dst_port, bool xnet, bool nocheck,
+			       u16 ipcb_flags)
+{
+	udp_tunnel_add_hdr(skb, src, dst, src_port, dst_port, nocheck);
+
+	iptunnel_xmit_noref(sk, rt, skb, src, dst, IPPROTO_UDP, tos, ttl, df,
+			    xnet, ipcb_flags);
+}
+EXPORT_SYMBOL_GPL(udp_tunnel_xmit_skb_noref);
+
 void udp_tunnel_sock_release(struct socket *sock)
 {
 	rcu_assign_sk_user_data(sock->sk, NULL);
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH net-next v2 3/3] net: ovpn: use new noref xmit flow in ovpn_udp4_output
  2025-09-12 11:24 ` [PATCH net-next v2 0/3] net: tunnel: introduce noref xmit flows for tunnels Marek Mietus
  2025-09-12 11:24   ` [PATCH net-next v2 1/3] net: dst_cache: implement RCU variants for dst_cache helpers Marek Mietus
  2025-09-12 11:24   ` [PATCH net-next v2 2/3] net: tunnel: implement noref flows in udp_tunnel_xmit_skb Marek Mietus
@ 2025-09-12 11:24   ` Marek Mietus
  2025-09-16 10:49     ` Sabrina Dubroca
  2 siblings, 1 reply; 8+ messages in thread
From: Marek Mietus @ 2025-09-12 11:24 UTC (permalink / raw)
  To: netdev, antonio, kuba; +Cc: openvpn-devel, Marek Mietus

ovpn_udp4_output unnecessarily references the dst_entry from the
dst_cache.

Reduce this overhead by using the newly implemented
udp_tunnel_xmit_skb_noref function and dst_cache helpers.

Signed-off-by: Marek Mietus <mmietus97@yahoo.com>
---
 drivers/net/ovpn/udp.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ovpn/udp.c b/drivers/net/ovpn/udp.c
index d6a0f7a0b75d..c5d289c23d2b 100644
--- a/drivers/net/ovpn/udp.c
+++ b/drivers/net/ovpn/udp.c
@@ -158,7 +158,7 @@ static int ovpn_udp4_output(struct ovpn_peer *peer, struct ovpn_bind *bind,
 	int ret;
 
 	local_bh_disable();
-	rt = dst_cache_get_ip4(cache, &fl.saddr);
+	rt = dst_cache_get_ip4_rcu(cache, &fl.saddr);
 	if (rt)
 		goto transmit;
 
@@ -194,12 +194,12 @@ static int ovpn_udp4_output(struct ovpn_peer *peer, struct ovpn_bind *bind,
 				    ret);
 		goto err;
 	}
-	dst_cache_set_ip4(cache, &rt->dst, fl.saddr);
+	dst_cache_steal_ip4(cache, &rt->dst, fl.saddr);
 
 transmit:
-	udp_tunnel_xmit_skb(rt, sk, skb, fl.saddr, fl.daddr, 0,
-			    ip4_dst_hoplimit(&rt->dst), 0, fl.fl4_sport,
-			    fl.fl4_dport, false, sk->sk_no_check_tx, 0);
+	udp_tunnel_xmit_skb_noref(rt, sk, skb, fl.saddr, fl.daddr, 0,
+				  ip4_dst_hoplimit(&rt->dst), 0, fl.fl4_sport,
+				  fl.fl4_dport, false, sk->sk_no_check_tx, 0);
 	ret = 0;
 err:
 	local_bh_enable();
@@ -269,7 +269,7 @@ static int ovpn_udp6_output(struct ovpn_peer *peer, struct ovpn_bind *bind,
 	 * fragment packets if needed.
 	 *
 	 * NOTE: this is not needed for IPv4 because we pass df=0 to
-	 * udp_tunnel_xmit_skb()
+	 * udp_tunnel_xmit_skb_noref()
 	 */
 	skb->ignore_df = 1;
 	udp_tunnel6_xmit_skb(dst, sk, skb, skb->dev, &fl.saddr, &fl.daddr, 0,
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next v2 1/3] net: dst_cache: implement RCU variants for dst_cache helpers
  2025-09-12 11:24   ` [PATCH net-next v2 1/3] net: dst_cache: implement RCU variants for dst_cache helpers Marek Mietus
@ 2025-09-16  9:28     ` Paolo Abeni
  0 siblings, 0 replies; 8+ messages in thread
From: Paolo Abeni @ 2025-09-16  9:28 UTC (permalink / raw)
  To: Marek Mietus, netdev, antonio, kuba; +Cc: openvpn-devel

On 9/12/25 1:24 PM, Marek Mietus wrote:
> -static struct dst_entry *dst_cache_per_cpu_get(struct dst_cache *dst_cache,
> -					       struct dst_cache_pcpu *idst)
> +static void dst_cache_per_cpu_dst_set(struct dst_cache_pcpu *dst_cache,
> +				      struct dst_entry *dst, u32 cookie)
> +{
> +	if (dst == dst_cache->dst && cookie == dst_cache->cookie)
> +		return;

The additional checks above could possibly make sense, but should never
trigger as the _set operation should follow a failing lookup.

Also are unrelated with the other changes here, they should land in a
separate patch - or be dropped.

Thanks,

Paolo


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next v2 3/3] net: ovpn: use new noref xmit flow in ovpn_udp4_output
  2025-09-12 11:24   ` [PATCH net-next v2 3/3] net: ovpn: use new noref xmit flow in ovpn_udp4_output Marek Mietus
@ 2025-09-16 10:49     ` Sabrina Dubroca
  2025-09-18 16:29       ` Marek Mietus
  0 siblings, 1 reply; 8+ messages in thread
From: Sabrina Dubroca @ 2025-09-16 10:49 UTC (permalink / raw)
  To: Marek Mietus; +Cc: netdev, antonio, kuba, openvpn-devel

2025-09-12, 13:24:20 +0200, Marek Mietus wrote:
> ovpn_udp4_output unnecessarily references the dst_entry from the
> dst_cache.

This should probably include a description of why it's safe to skip
the reference in this context.

> Reduce this overhead by using the newly implemented
> udp_tunnel_xmit_skb_noref function and dst_cache helpers.
> 
> Signed-off-by: Marek Mietus <mmietus97@yahoo.com>
> ---
>  drivers/net/ovpn/udp.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/ovpn/udp.c b/drivers/net/ovpn/udp.c
> index d6a0f7a0b75d..c5d289c23d2b 100644
> --- a/drivers/net/ovpn/udp.c
> +++ b/drivers/net/ovpn/udp.c
> @@ -158,7 +158,7 @@ static int ovpn_udp4_output(struct ovpn_peer *peer, struct ovpn_bind *bind,
>  	int ret;
>  
>  	local_bh_disable();
> -	rt = dst_cache_get_ip4(cache, &fl.saddr);
> +	rt = dst_cache_get_ip4_rcu(cache, &fl.saddr);
>  	if (rt)
>  		goto transmit;
>  
> @@ -194,12 +194,12 @@ static int ovpn_udp4_output(struct ovpn_peer *peer, struct ovpn_bind *bind,
>  				    ret);
>  		goto err;
>  	}
> -	dst_cache_set_ip4(cache, &rt->dst, fl.saddr);
> +	dst_cache_steal_ip4(cache, &rt->dst, fl.saddr);
>  
>  transmit:
> -	udp_tunnel_xmit_skb(rt, sk, skb, fl.saddr, fl.daddr, 0,
> -			    ip4_dst_hoplimit(&rt->dst), 0, fl.fl4_sport,
> -			    fl.fl4_dport, false, sk->sk_no_check_tx, 0);
> +	udp_tunnel_xmit_skb_noref(rt, sk, skb, fl.saddr, fl.daddr, 0,
> +				  ip4_dst_hoplimit(&rt->dst), 0, fl.fl4_sport,
> +				  fl.fl4_dport, false, sk->sk_no_check_tx, 0);
>  	ret = 0;
>  err:
>  	local_bh_enable();
> @@ -269,7 +269,7 @@ static int ovpn_udp6_output(struct ovpn_peer *peer, struct ovpn_bind *bind,
>  	 * fragment packets if needed.
>  	 *
>  	 * NOTE: this is not needed for IPv4 because we pass df=0 to
> -	 * udp_tunnel_xmit_skb()
> +	 * udp_tunnel_xmit_skb_noref()
>  	 */
>  	skb->ignore_df = 1;
>  	udp_tunnel6_xmit_skb(dst, sk, skb, skb->dev, &fl.saddr, &fl.daddr, 0,

Why are you changing only ipv4? Is there something in the ipv6 code
that prevents this?

-- 
Sabrina

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next v2 3/3] net: ovpn: use new noref xmit flow in ovpn_udp4_output
  2025-09-16 10:49     ` Sabrina Dubroca
@ 2025-09-18 16:29       ` Marek Mietus
  2025-09-19 15:03         ` Sabrina Dubroca
  0 siblings, 1 reply; 8+ messages in thread
From: Marek Mietus @ 2025-09-18 16:29 UTC (permalink / raw)
  To: Sabrina Dubroca; +Cc: netdev, antonio, kuba, openvpn-devel

W dniu 9/16/25 o 12:49, Sabrina Dubroca pisze:
> 2025-09-12, 13:24:20 +0200, Marek Mietus wrote:
>> ovpn_udp4_output unnecessarily references the dst_entry from the
>> dst_cache.
> 
> This should probably include a description of why it's safe to skip
> the reference in this context.
> 

Noted, will do.

>> Reduce this overhead by using the newly implemented
>> udp_tunnel_xmit_skb_noref function and dst_cache helpers.
>>
>> Signed-off-by: Marek Mietus <mmietus97@yahoo.com>
>> ---
>>  drivers/net/ovpn/udp.c | 12 ++++++------
>>  1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/net/ovpn/udp.c b/drivers/net/ovpn/udp.c
>> index d6a0f7a0b75d..c5d289c23d2b 100644
>> --- a/drivers/net/ovpn/udp.c
>> +++ b/drivers/net/ovpn/udp.c
>> @@ -158,7 +158,7 @@ static int ovpn_udp4_output(struct ovpn_peer *peer, struct ovpn_bind *bind,
>>  	int ret;
>>  
>>  	local_bh_disable();
>> -	rt = dst_cache_get_ip4(cache, &fl.saddr);
>> +	rt = dst_cache_get_ip4_rcu(cache, &fl.saddr);
>>  	if (rt)
>>  		goto transmit;
>>  
>> @@ -194,12 +194,12 @@ static int ovpn_udp4_output(struct ovpn_peer *peer, struct ovpn_bind *bind,
>>  				    ret);
>>  		goto err;
>>  	}
>> -	dst_cache_set_ip4(cache, &rt->dst, fl.saddr);
>> +	dst_cache_steal_ip4(cache, &rt->dst, fl.saddr);
>>  
>>  transmit:
>> -	udp_tunnel_xmit_skb(rt, sk, skb, fl.saddr, fl.daddr, 0,
>> -			    ip4_dst_hoplimit(&rt->dst), 0, fl.fl4_sport,
>> -			    fl.fl4_dport, false, sk->sk_no_check_tx, 0);
>> +	udp_tunnel_xmit_skb_noref(rt, sk, skb, fl.saddr, fl.daddr, 0,
>> +				  ip4_dst_hoplimit(&rt->dst), 0, fl.fl4_sport,
>> +				  fl.fl4_dport, false, sk->sk_no_check_tx, 0);
>>  	ret = 0;
>>  err:
>>  	local_bh_enable();
>> @@ -269,7 +269,7 @@ static int ovpn_udp6_output(struct ovpn_peer *peer, struct ovpn_bind *bind,
>>  	 * fragment packets if needed.
>>  	 *
>>  	 * NOTE: this is not needed for IPv4 because we pass df=0 to
>> -	 * udp_tunnel_xmit_skb()
>> +	 * udp_tunnel_xmit_skb_noref()
>>  	 */
>>  	skb->ignore_df = 1;
>>  	udp_tunnel6_xmit_skb(dst, sk, skb, skb->dev, &fl.saddr, &fl.daddr, 0,
> 
> Why are you changing only ipv4? Is there something in the ipv6 code
> that prevents this?
> 

I'm not sure. I'm not as acquainted with IPv6 as I am with IPv4. (and thought I'd hold off
until I got a positive response about the series)
IPv4 already has some noref xmit optimizations, so it just felt like the right place to start.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next v2 3/3] net: ovpn: use new noref xmit flow in ovpn_udp4_output
  2025-09-18 16:29       ` Marek Mietus
@ 2025-09-19 15:03         ` Sabrina Dubroca
  0 siblings, 0 replies; 8+ messages in thread
From: Sabrina Dubroca @ 2025-09-19 15:03 UTC (permalink / raw)
  To: Marek Mietus; +Cc: netdev, antonio, kuba, openvpn-devel

2025-09-18, 18:29:08 +0200, Marek Mietus wrote:
> W dniu 9/16/25 o 12:49, Sabrina Dubroca pisze:
> > 2025-09-12, 13:24:20 +0200, Marek Mietus wrote:
> > Why are you changing only ipv4? Is there something in the ipv6 code
> > that prevents this?
> > 
> 
> I'm not sure. I'm not as acquainted with IPv6 as I am with IPv4. (and thought I'd hold off
> until I got a positive response about the series)

Ok, understood.

> IPv4 already has some noref xmit optimizations, so it just felt like the right place to start.

There's also some in IPv6, see d14730b8e911 ("ipv6: use RCU in inet6_csk_xmit()").

-- 
Sabrina

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2025-09-19 15:03 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20250912112420.4394-1-mmietus97.ref@yahoo.com>
2025-09-12 11:24 ` [PATCH net-next v2 0/3] net: tunnel: introduce noref xmit flows for tunnels Marek Mietus
2025-09-12 11:24   ` [PATCH net-next v2 1/3] net: dst_cache: implement RCU variants for dst_cache helpers Marek Mietus
2025-09-16  9:28     ` Paolo Abeni
2025-09-12 11:24   ` [PATCH net-next v2 2/3] net: tunnel: implement noref flows in udp_tunnel_xmit_skb Marek Mietus
2025-09-12 11:24   ` [PATCH net-next v2 3/3] net: ovpn: use new noref xmit flow in ovpn_udp4_output Marek Mietus
2025-09-16 10:49     ` Sabrina Dubroca
2025-09-18 16:29       ` Marek Mietus
2025-09-19 15:03         ` Sabrina Dubroca

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).