2025-03-05, 00:35:09 +0100, Antonio Quartulli wrote:
On 04/03/2025 20:02, Sabrina Dubroca wrote:
2025-03-04, 01:33:39 +0100, Antonio Quartulli wrote: [...]
+static inline struct ovpn_crypto_key_slot * +ovpn_crypto_key_id_to_slot(const struct ovpn_crypto_state *cs, u8 key_id) +{
- struct ovpn_crypto_key_slot *ks;
- u8 idx;
- if (unlikely(!cs))
return NULL;- rcu_read_lock();
- idx = cs->primary_idx;
I'd go with slots[0] and slots[1], since it doesn't really matter whether we check the primary or secondary first. It would avoid a possible reload of cs->primary_idx (which might be updated concurrently by a key swap and cause us to look into the same slot twice) -- a READ_ONCE would also prevent that.
Reason for looking into primary first is that we will most likely need the primary key to decrypt the incoming traffic.
Secondary is used only during a small (if at all) time window where we moved to a new key, but our peer was still sending traffic encrypted with the old (secondary) key.
Therefore optimizing for primary-first may make a non-negligible difference under heavy load.
Code doesn't get more complex due to this logic, therefore I'd keep this version (with READ_ONCE(cs->primary_idx)), unless there is a strong argument against it.
Ok, sounds reasonable.