Skip to contentSkip to conversation

ovpn: reduce reference-count contention

PATCH Superseded Ralf Lici 1 participant 3 messages Sep 17

Subsystems (2)
Network drivers Openvpn data channel offload

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Ralf Lici

author reviewer
#0 lore raw

cover ovpn: reduce reference-count contention

Hi,

Currently ovpn holds peer and key-slot references while packets are being processed and until asynchronous crypto completes. With traffic for one peer spread across CPUs, the shared kref cachelines therefore become contended even though both objects are normally long-lived.

This series converts both references to percpu_ref while preserving their existing RCU-delayed destruction. The individual patch messages describe the memory cost and cache-to-cache measurements.

The test used one peer, one AES-128-GCM key and 32 TCP streams on a 32-logical-CPU Ryzen 9 9950X with a ConnectX-5 hairpin. The encrypted RX flow used one source port and one receive queue, so only TX exercised CPU fan-out. perf c2c showed that each shared reference cacheline disappeared after its respective conversion.

Thanks,

Ralf Lici
Mandelbit Srl

---
Ralf Lici (2):
ovpn: use percpu references for peers
ovpn: use percpu references for key slots

 drivers/net/ovpn/crypto.c      | 14 +++++++-------
 drivers/net/ovpn/crypto.h      | 15 +++++++++++----
 drivers/net/ovpn/crypto_aead.c |  7 ++++++-
 drivers/net/ovpn/netlink.c     |  6 +++---
 drivers/net/ovpn/peer.c        | 32 ++++++++++++++++++++------------
 drivers/net/ovpn/peer.h        | 34 ++++++++++++++++++++++++++++++----
 6 files changed, 77 insertions(+), 31 deletions(-)

base-commit: b8e9e7d82e7eefd5d2d528469d94ec20e96b38c3

Signature · 7 lines
2.55.0


_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
author reviewer
#1 lore raw

PATCH 1/2 ovpn: use percpu references for peers

Each packet holds its peer across packet processing and asynchronous crypto completion. When traffic for one peer is spread across several CPUs, the shared kref cache line therefore moves between those CPUs for every packet.

Replace the peer kref with percpu_ref. Use the RCU-specific live-reference helper in lookup paths, and kill the initial reference exactly once after the peer has been removed from its lookup structures and detached from its socket. Keep the existing RCU-delayed destruction and release the percpu_ref storage in the final RCU callback.

percpu_ref_init can fail, so propagate allocation failure from peer creation. This trades a per-CPU counter allocation for a cheaper live data path on these long-lived, heavily shared objects. Specifically, in terms of memory, this costs: 8 bytes per possible CPU for the counters, a 56-byte control object, and 8 additional bytes in struct ovpn_peer.

In a 32-stream test using one peer and one key, perf c2c placed the peer kref cacheline second among system-wide shared cachelines, with 37 sampled HITM loads attributed to the locked reference update. After this change that shared reference line was no longer sampled, while the untouched key-slot kref (addressed by the next commit) remained visible.

RX used a single receive queue, so only TX exercised CPU fan-out in this test.

Signed-off-by Ralf Lici <[email protected]>
---
 drivers/net/ovpn/netlink.c |  6 +++---
 drivers/net/ovpn/peer.c    | 32 ++++++++++++++++++++------------
 drivers/net/ovpn/peer.h    | 34 ++++++++++++++++++++++++++++++----
 3 files changed, 53 insertions(+), 19 deletions(-)
drivers/net/ovpn/netlink.c +3 -3
index 4dad85294198..8cdf46b61142 100644
@@ -463,11 +463,11 @@ int ovpn_nl_peer_new_doit(struct sk_buff *skb, struct genl_info *info)
 	ovpn_socket_release(peer);
 peer_release:
 	/* For UDP, the peer is unreachable until added to the hashtables, so
-	 * dropping the initial reference is enough. For TCP, the peer may be
+	 * killing the initial reference is enough. For TCP, the peer may be
 	 * concurrently reachable via sk_user_data->peer until
-	 * ovpn_socket_release() detaches; rely on the refcount.
+	 * ovpn_socket_release() detaches; rely on the percpu reference.
 	 */
-	ovpn_peer_put(peer);
+	ovpn_peer_kill(peer);
 
 	return ret;
 }
drivers/net/ovpn/peer.c +20 -12
index c95656ca7c35..3efc3e5c07e2 100644
@@ -33,7 +33,7 @@ static void unlock_ovpn(struct ovpn_priv *ovpn,
 	llist_for_each_entry_safe(peer, next, release_list->first,
 				  release_entry) {
 		ovpn_socket_release(peer);
-		ovpn_peer_put(peer);
+		ovpn_peer_kill(peer);
 	}
 }
 

@@ -113,7 +113,6 @@ struct ovpn_peer *ovpn_peer_new(struct ovpn_priv *ovpn, u32 id)
 	RCU_INIT_POINTER(peer->bind, NULL);
 	ovpn_crypto_state_init(&peer->crypto);
 	spin_lock_init(&peer->lock);
-	kref_init(&peer->refcount);
 	ovpn_peer_stats_init(&peer->vpn_stats);
 	ovpn_peer_stats_init(&peer->link_stats);
 	INIT_WORK(&peer->keepalive_work, ovpn_peer_keepalive_send);

@@ -127,6 +126,14 @@ struct ovpn_peer *ovpn_peer_new(struct ovpn_priv *ovpn, u32 id)
 		return ERR_PTR(ret);
 	}
 
+	ret = percpu_ref_init(&peer->refcount, ovpn_peer_release_ref, 0,
+			      GFP_KERNEL);
+	if (ret < 0) {
+		dst_cache_destroy(&peer->dst_cache);
+		kfree(peer);
+		return ERR_PTR(ret);
+	}
+
 	netdev_hold(ovpn->dev, &peer->dev_tracker, GFP_KERNEL);
 
 	return peer;

@@ -348,6 +355,7 @@ static void ovpn_peer_release_rcu(struct rcu_head *head)
 	 * perform it in the RCU callback, when all contexts are done
 	 */
 	dst_cache_destroy(&peer->dst_cache);
+	percpu_ref_exit(&peer->refcount);
 	kfree(peer);
 }
 

@@ -366,12 +374,12 @@ static void ovpn_peer_release(struct ovpn_peer *peer)
 }
 
 /**
- * ovpn_peer_release_kref - callback for kref_put
- * @kref: the kref object belonging to the peer
+ * ovpn_peer_release_ref - callback for the peer percpu reference
+ * @ref: the percpu_ref object belonging to the peer
  */
-void ovpn_peer_release_kref(struct kref *kref)
+void ovpn_peer_release_ref(struct percpu_ref *ref)
 {
-	struct ovpn_peer *peer = container_of(kref, struct ovpn_peer, refcount);
+	struct ovpn_peer *peer = container_of(ref, struct ovpn_peer, refcount);
 
 	ovpn_peer_release(peer);
 }

@@ -569,7 +577,7 @@ ovpn_peer_get_by_transp_addr_p2p(struct ovpn_priv *ovpn,
 	rcu_read_lock();
 	tmp = rcu_dereference(ovpn->peer);
 	if (likely(tmp && ovpn_peer_transp_match(tmp, ss) &&
-		   ovpn_peer_hold(tmp)))
+		   ovpn_peer_hold_rcu(tmp)))
 		peer = tmp;
 	rcu_read_unlock();
 

@@ -610,7 +618,7 @@ struct ovpn_peer *ovpn_peer_get_by_transp_addr(struct ovpn_priv *ovpn,
 		if (!ovpn_peer_transp_match(tmp, &ss))
 			continue;
 
-		if (!ovpn_peer_hold(tmp))
+		if (!ovpn_peer_hold_rcu(tmp))
 			continue;
 
 		peer = tmp;

@@ -641,7 +649,7 @@ static struct ovpn_peer *ovpn_peer_get_by_id_p2p(struct ovpn_priv *ovpn,
 
 	rcu_read_lock();
 	tmp = rcu_dereference(ovpn->peer);
-	if (likely(tmp && tmp->id == peer_id && ovpn_peer_hold(tmp)))
+	if (likely(tmp && tmp->id == peer_id && ovpn_peer_hold_rcu(tmp)))
 		peer = tmp;
 	rcu_read_unlock();
 

@@ -671,7 +679,7 @@ struct ovpn_peer *ovpn_peer_get_by_id(struct ovpn_priv *ovpn, u32 peer_id)
 		if (tmp->id != peer_id)
 			continue;
 
-		if (!ovpn_peer_hold(tmp))
+		if (!ovpn_peer_hold_rcu(tmp))
 			continue;
 
 		peer = tmp;

@@ -745,7 +753,7 @@ struct ovpn_peer *ovpn_peer_get_by_dst(struct ovpn_priv *ovpn,
 	if (ovpn->mode == OVPN_MODE_P2P) {
 		rcu_read_lock();
 		peer = rcu_dereference(ovpn->peer);
-		if (unlikely(peer && !ovpn_peer_hold(peer)))
+		if (unlikely(peer && !ovpn_peer_hold_rcu(peer)))
 			peer = NULL;
 		rcu_read_unlock();
 		return peer;

@@ -763,7 +771,7 @@ struct ovpn_peer *ovpn_peer_get_by_dst(struct ovpn_priv *ovpn,
 		break;
 	}
 
-	if (unlikely(peer && !ovpn_peer_hold(peer)))
+	if (unlikely(peer && !ovpn_peer_hold_rcu(peer)))
 		peer = NULL;
 	rcu_read_unlock();
drivers/net/ovpn/peer.h +30 -4
index dfa5c0037e02..dd6127976ac9 100644
@@ -10,6 +10,7 @@
 #ifndef _NET_OVPN_OVPNPEER_H_
 #define _NET_OVPN_OVPNPEER_H_
 
+#include <linux/percpu-refcount.h>
 #include <net/dst_cache.h>
 #include <net/strparser.h>
 

@@ -110,7 +111,7 @@ struct ovpn_peer {
 	struct ovpn_peer_stats link_stats;
 	enum ovpn_del_peer_reason delete_reason;
 	spinlock_t lock; /* protects bind  and keepalive* */
-	struct kref refcount;
+	struct percpu_ref refcount;
 	struct rcu_head rcu;
 	struct llist_node release_entry;
 	struct work_struct keepalive_work;

@@ -124,10 +125,23 @@ struct ovpn_peer {
  */
 static inline bool ovpn_peer_hold(struct ovpn_peer *peer)
 {
-	return kref_get_unless_zero(&peer->refcount);
+	return percpu_ref_tryget_live(&peer->refcount);
 }
 
-void ovpn_peer_release_kref(struct kref *kref);
+/**
+ * ovpn_peer_hold_rcu - acquire a live peer reference under RCU
+ * @peer: peer to acquire
+ *
+ * The caller must hold rcu_read_lock.
+ *
+ * Return: true if the reference was acquired, false if teardown has started
+ */
+static inline bool ovpn_peer_hold_rcu(struct ovpn_peer *peer)
+{
+	return percpu_ref_tryget_live_rcu(&peer->refcount);
+}
+
+void ovpn_peer_release_ref(struct percpu_ref *ref);
 
 /**
  * ovpn_peer_put - decrease reference counter

@@ -135,7 +149,19 @@ void ovpn_peer_release_kref(struct kref *kref);
  */
 static inline void ovpn_peer_put(struct ovpn_peer *peer)
 {
-	kref_put(&peer->refcount, ovpn_peer_release_kref);
+	percpu_ref_put(&peer->refcount);
+}
+
+/**
+ * ovpn_peer_kill - drop the peer's initial reference
+ * @peer: peer which has been unpublished and is being destroyed
+ *
+ * This must be called exactly once, after the peer is no longer reachable
+ * through its owning ovpn instance.
+ */
+static inline void ovpn_peer_kill(struct ovpn_peer *peer)
+{
+	percpu_ref_kill(&peer->refcount);
 }
 
 struct ovpn_peer *ovpn_peer_new(struct ovpn_priv *ovpn, u32 id);
Signature · 8 lines
2.55.0



_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
author reviewer
#2 lore raw

PATCH 2/2 ovpn: use percpu references for key slots

In reply to Ralf Lici

Packet processing holds a key slot until asynchronous crypto completion. Parallel traffic using one key consequently updates the same kref cache line for every packet, even though key slots are normally long-lived.

Replace the key-slot kref with percpu_ref. Kill the initial reference after atomically unpublishing a slot, while ordinary packet completions only put their live references. The release callback retains the existing rcu_work teardown so lockless readers receive an RCU grace period and crypto transforms are freed from workqueue context. Release the percpu_ref storage from that final worker.

Initialize the reference only after the rest of the key slot is ready, allowing the existing destruction path to handle allocation failure.

After the peer conversion, perf c2c still identified the key-slot kref cacheline, with 15 sampled HITM loads attributed to its locked reference update. This change removed that line from the shared-cacheline profile as well. In a set interleaved 32-flow iperf3 runs, the combined peer and key conversion moved throughput to 8.812 Gbit/s, against 7.734 Gbit/s for the baseline, a +13.95% improvement. Single-stream control found no reproducible regression.

Signed-off-by Ralf Lici <[email protected]>
---
 drivers/net/ovpn/crypto.c      | 14 +++++++-------
 drivers/net/ovpn/crypto.h      | 15 +++++++++++----
 drivers/net/ovpn/crypto_aead.c |  7 ++++++-
 3 files changed, 24 insertions(+), 12 deletions(-)
drivers/net/ovpn/crypto.c +7 -7
index 7e545428900a..23502aa125db 100644
@@ -18,11 +18,11 @@
 #include "crypto_aead.h"
 #include "crypto.h"
 
-void ovpn_crypto_key_slot_release(struct kref *kref)
+void ovpn_crypto_key_slot_release(struct percpu_ref *ref)
 {
 	struct ovpn_crypto_key_slot *ks;
 
-	ks = container_of(kref, struct ovpn_crypto_key_slot, refcount);
+	ks = container_of(ref, struct ovpn_crypto_key_slot, refcount);
 	queue_rcu_work(ovpn_wq, &ks->free_work);
 }
 

@@ -36,13 +36,13 @@ void ovpn_crypto_state_release(struct ovpn_crypto_state *cs)
 	ks = rcu_access_pointer(cs->slots[0]);
 	if (ks) {
 		RCU_INIT_POINTER(cs->slots[0], NULL);
-		ovpn_crypto_key_slot_put(ks);
+		ovpn_crypto_key_slot_kill(ks);
 	}
 
 	ks = rcu_access_pointer(cs->slots[1]);
 	if (ks) {
 		RCU_INIT_POINTER(cs->slots[1], NULL);
-		ovpn_crypto_key_slot_put(ks);
+		ovpn_crypto_key_slot_kill(ks);
 	}
 }
 

@@ -66,7 +66,7 @@ bool ovpn_crypto_kill_key(struct ovpn_crypto_state *cs, u8 key_id)
 	spin_unlock_bh(&cs->lock);
 
 	if (ks)
-		ovpn_crypto_key_slot_put(ks);
+		ovpn_crypto_key_slot_kill(ks);
 
 	/* let the caller know if a key was actually killed */
 	return ks;

@@ -104,7 +104,7 @@ int ovpn_crypto_state_reset(struct ovpn_crypto_state *cs,
 	spin_unlock_bh(&cs->lock);
 
 	if (old)
-		ovpn_crypto_key_slot_put(old);
+		ovpn_crypto_key_slot_kill(old);
 
 	return 0;
 }

@@ -141,7 +141,7 @@ void ovpn_crypto_key_slot_delete(struct ovpn_crypto_state *cs,
 	}
 
 	pr_debug("deleting key slot %u, key_id=%u\n", slot, ks->key_id);
-	ovpn_crypto_key_slot_put(ks);
+	ovpn_crypto_key_slot_kill(ks);
 }
 
 void ovpn_crypto_key_slots_swap(struct ovpn_crypto_state *cs)
drivers/net/ovpn/crypto.h +11 -4
index e3feb16d5498..bd4056570d54 100644
@@ -10,6 +10,7 @@
 #ifndef _NET_OVPN_OVPNCRYPTO_H_
 #define _NET_OVPN_OVPNCRYPTO_H_
 
+#include <linux/percpu-refcount.h>
 #include <linux/workqueue.h>
 
 #include "pktid.h"

@@ -48,7 +49,7 @@ struct ovpn_crypto_key_slot {
 	struct ovpn_pktid_recv pid_recv ____cacheline_aligned_in_smp;
 	struct ovpn_pktid_xmit pid_xmit ____cacheline_aligned_in_smp;
 	struct rcu_work free_work;
-	struct kref refcount;
+	struct percpu_ref refcount;
 };
 
 struct ovpn_crypto_state {

@@ -61,7 +62,7 @@ struct ovpn_crypto_state {
 
 static inline bool ovpn_crypto_key_slot_hold(struct ovpn_crypto_key_slot *ks)
 {
-	return kref_get_unless_zero(&ks->refcount);
+	return percpu_ref_tryget_live_rcu(&ks->refcount);
 }
 
 static inline void ovpn_crypto_state_init(struct ovpn_crypto_state *cs)

@@ -121,11 +122,17 @@ ovpn_crypto_key_slot_primary(const struct ovpn_crypto_state *cs)
 	return ks;
 }
 
-void ovpn_crypto_key_slot_release(struct kref *kref);
+void ovpn_crypto_key_slot_release(struct percpu_ref *ref);
 
 static inline void ovpn_crypto_key_slot_put(struct ovpn_crypto_key_slot *ks)
 {
-	kref_put(&ks->refcount, ovpn_crypto_key_slot_release);
+	percpu_ref_put(&ks->refcount);
+}
+
+static inline void
+ovpn_crypto_key_slot_kill(struct ovpn_crypto_key_slot *ks)
+{
+	percpu_ref_kill(&ks->refcount);
 }
 
 int ovpn_crypto_state_reset(struct ovpn_crypto_state *cs,
drivers/net/ovpn/crypto_aead.c +6 -1
index 74eaf6fac2f5..414e5d7d32bf 100644
@@ -393,6 +393,7 @@ static void ovpn_aead_crypto_key_slot_free_work(struct work_struct *work)
 
 	ks = container_of(to_rcu_work(work), struct ovpn_crypto_key_slot,
 			  free_work);
+	percpu_ref_exit(&ks->refcount);
 	ovpn_aead_crypto_key_slot_free(ks);
 	kfree(ks);
 }

@@ -428,7 +429,6 @@ ovpn_aead_crypto_key_slot_new(const struct ovpn_key_config *kc)
 	ks->encrypt = NULL;
 	ks->decrypt = NULL;
 	INIT_RCU_WORK(&ks->free_work, ovpn_aead_crypto_key_slot_free_work);
-	kref_init(&ks->refcount);
 	ks->key_id = kc->key_id;
 
 	ks->encrypt = ovpn_aead_init("encrypt", alg_name,

@@ -458,6 +458,11 @@ ovpn_aead_crypto_key_slot_new(const struct ovpn_key_config *kc)
 	ovpn_pktid_xmit_init(&ks->pid_xmit);
 	ovpn_pktid_recv_init(&ks->pid_recv);
 
+	ret = percpu_ref_init(&ks->refcount, ovpn_crypto_key_slot_release, 0,
+			      GFP_KERNEL);
+	if (ret < 0)
+		goto destroy_ks;
+
 	return ks;
 
 destroy_ks:
Signature · 8 lines
2.55.0



_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel