[Openvpn-devel,net-next,v2,2/2] ovpn: use percpu references for key slots
Commit Message
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.
Key destruction needs process context to release the crypto transforms
and an RCU grace period for lockless configuration readers which do not
hold a key reference. With percpu_ref, the release callback may run from
the internal RCU callback which switches the reference to atomic mode.
Queueing rcu_work from there would create a nested callback which the
module cleanup rcu_barrier is not guaranteed to wait for.
Record the RCU state after unpublishing the slot and queue ordinary work
from the release callback instead. The worker conditionally waits for
the recorded grace period before freeing the key. This provides explicit
RCU synchronization without depending on percpu_ref implementation
details or unconditionally starting another grace period.
Peer teardown kills its keys before dropping the final netdevice
reference, so rtnl_link_unregister cannot return before the key
percpu-ref callbacks have been registered. The existing rcu_barrier
drains those callbacks and destroy_workqueue subsequently drains the
ordinary key cleanup work.
Initialize the reference only after the rest of the key slot is ready,
allowing the existing destruction path to handle allocation failure.
The conversion adds an 8-byte counter per possible CPU and a 56-byte
control allocation for each installed key slot; with at most two
long-lived slots per peer, this is a bounded memory tradeoff for
removing the shared reference cacheline from the packet path.
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 of 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 <ralf@mandelbit.com>
---
Changes since v1 https://lore.kernel.org/openvpn-devel/9551c9c842e4ac926089badc81f445becca513e5.1789658051.git.ralf@mandelbit.com/
- Replace the nested rcu_work teardown with ordinary work so module
cleanup cannot destroy the workqueue before the work is queued.
(Sashiko)
- Record the RCU state after unpublishing a key and conditionally wait
for RCU-only readers in the final worker. (Sashiko)
drivers/net/ovpn/crypto.c | 16 ++++++++--------
drivers/net/ovpn/crypto.h | 27 ++++++++++++++++++++++-----
drivers/net/ovpn/crypto_aead.c | 17 +++++++++++++----
3 files changed, 43 insertions(+), 17 deletions(-)
@@ -18,12 +18,12 @@
#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);
- queue_rcu_work(ovpn_wq, &ks->free_work);
+ ks = container_of(ref, struct ovpn_crypto_key_slot, refcount);
+ queue_work(ovpn_wq, &ks->free_work);
}
/* can only be invoked when all peer references have been dropped (i.e. RCU
@@ -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)
@@ -10,6 +10,8 @@
#ifndef _NET_OVPN_OVPNCRYPTO_H_
#define _NET_OVPN_OVPNCRYPTO_H_
+#include <linux/percpu-refcount.h>
+#include <linux/rcupdate.h>
#include <linux/workqueue.h>
#include "pktid.h"
@@ -47,8 +49,9 @@ 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 work_struct free_work;
+ unsigned long rcu_state;
+ struct percpu_ref refcount;
};
struct ovpn_crypto_state {
@@ -61,7 +64,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 +124,25 @@ 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);
+}
+
+/**
+ * ovpn_crypto_key_slot_kill - stop new users and drop the initial reference
+ * @ks: key slot which has already been unpublished
+ *
+ * percpu_ref does not guarantee an RCU grace period before release. Record
+ * the RCU state after unpublishing the key so the final worker can synchronize
+ * with lockless readers without unconditionally starting another grace period.
+ */
+static inline void ovpn_crypto_key_slot_kill(struct ovpn_crypto_key_slot *ks)
+{
+ ks->rcu_state = get_state_synchronize_rcu();
+ percpu_ref_kill(&ks->refcount);
}
int ovpn_crypto_state_reset(struct ovpn_crypto_state *cs,
@@ -391,8 +391,13 @@ static void ovpn_aead_crypto_key_slot_free_work(struct work_struct *work)
{
struct ovpn_crypto_key_slot *ks;
- ks = container_of(to_rcu_work(work), struct ovpn_crypto_key_slot,
- free_work);
+ ks = container_of(work, struct ovpn_crypto_key_slot, free_work);
+ /* Reaching this worker means every reference held by packet processing
+ * and asynchronous crypto has been returned. Separately wait for any
+ * RCU-only reader which observed the slot before it was unpublished.
+ */
+ cond_synchronize_rcu(ks->rcu_state);
+ percpu_ref_exit(&ks->refcount);
ovpn_aead_crypto_key_slot_free(ks);
kfree(ks);
}
@@ -427,8 +432,7 @@ 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);
+ INIT_WORK(&ks->free_work, ovpn_aead_crypto_key_slot_free_work);
ks->key_id = kc->key_id;
ks->encrypt = ovpn_aead_init("encrypt", alg_name,
@@ -458,6 +462,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: