From 31ccde025c2baceb3a8a81b3b202eb33d37e51b8 Mon Sep 17 00:00:00 2001 From: James Tucker Date: Thu, 8 Sep 2022 14:16:30 -0700 Subject: [PATCH 1/2] device: remove atomic pointer to next and rely on lock MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit name old time/op new time/op delta TrieIPv4Peers100Addresses1000-32 83.5ns ± 0% 89.0ns ± 0% ~ (p=1.000 n=1+1) TrieIPv4Peers10Addresses10-32 33.6ns ± 0% 33.3ns ± 0% ~ (p=1.000 n=1+1) TrieIPv6Peers100Addresses1000-32 83.3ns ± 0% 82.7ns ± 0% ~ (p=1.000 n=1+1) TrieIPv6Peers10Addresses10-32 33.5ns ± 0% 33.4ns ± 0% ~ (p=1.000 n=1+1) Latency-32 216µs ± 0% 211µs ± 0% ~ (p=1.000 n=1+1) Throughput-32 2.31µs ± 0% 2.25µs ± 0% ~ (p=1.000 n=1+1) UAPIGet-32 2.28µs ± 0% 2.12µs ± 0% ~ (p=1.000 n=1+1) WaitPool-32 4.18µs ± 0% 4.06µs ± 0% ~ (p=1.000 n=1+1) name old packet-loss new packet-loss delta Throughput-32 0.00 ± 0% 0.00 ± 0% ~ (p=1.000 n=1+1) name old alloc/op new alloc/op delta UAPIGet-32 224B ± 0% 224B ± 0% ~ (all equal) name old allocs/op new allocs/op delta UAPIGet-32 17.0 ± 0% 17.0 ± 0% ~ (all equal) --- device/keypair.go | 2 +- device/noise-protocol.go | 15 ++++++--------- device/noise_test.go | 2 +- device/peer.go | 8 ++++---- 4 files changed, 12 insertions(+), 15 deletions(-) diff --git a/device/keypair.go b/device/keypair.go index e3540d7d4..93602a699 100644 --- a/device/keypair.go +++ b/device/keypair.go @@ -36,7 +36,7 @@ type Keypairs struct { sync.RWMutex current *Keypair previous *Keypair - next atomic.Pointer[Keypair] + next *Keypair } func (kp *Keypairs) Current() *Keypair { diff --git a/device/noise-protocol.go b/device/noise-protocol.go index 117e960a8..405be7c68 100644 --- a/device/noise-protocol.go +++ b/device/noise-protocol.go @@ -581,12 +581,12 @@ func (peer *Peer) BeginSymmetricSession() error { defer keypairs.Unlock() previous := keypairs.previous - next := keypairs.next.Load() + next := keypairs.next current := keypairs.current if isInitiator { if next != nil { - keypairs.next.Store(nil) + keypairs.next = nil keypairs.previous = next device.DeleteKeypair(current) } else { @@ -595,7 +595,7 @@ func (peer *Peer) BeginSymmetricSession() error { device.DeleteKeypair(previous) keypairs.current = keypair } else { - keypairs.next.Store(keypair) + keypairs.next = keypair device.DeleteKeypair(next) keypairs.previous = nil device.DeleteKeypair(previous) @@ -607,18 +607,15 @@ func (peer *Peer) BeginSymmetricSession() error { func (peer *Peer) ReceivedWithKeypair(receivedKeypair *Keypair) bool { keypairs := &peer.keypairs - if keypairs.next.Load() != receivedKeypair { - return false - } keypairs.Lock() defer keypairs.Unlock() - if keypairs.next.Load() != receivedKeypair { + if keypairs.next != receivedKeypair { return false } old := keypairs.previous keypairs.previous = keypairs.current peer.device.DeleteKeypair(old) - keypairs.current = keypairs.next.Load() - keypairs.next.Store(nil) + keypairs.current = keypairs.next + keypairs.next = nil return true } diff --git a/device/noise_test.go b/device/noise_test.go index 587d1e55d..278b6451d 100644 --- a/device/noise_test.go +++ b/device/noise_test.go @@ -148,7 +148,7 @@ func TestNoiseHandshake(t *testing.T) { t.Fatal("failed to derive keypair for peer 2", err) } - key1 := peer1.keypairs.next.Load() + key1 := peer1.keypairs.next key2 := peer2.keypairs.current // encrypting / decryption test diff --git a/device/peer.go b/device/peer.go index 8266dacc0..904dd083c 100644 --- a/device/peer.go +++ b/device/peer.go @@ -202,10 +202,10 @@ func (peer *Peer) ZeroAndFlushAll() { keypairs.Lock() device.DeleteKeypair(keypairs.previous) device.DeleteKeypair(keypairs.current) - device.DeleteKeypair(keypairs.next.Load()) + device.DeleteKeypair(keypairs.next) keypairs.previous = nil keypairs.current = nil - keypairs.next.Store(nil) + keypairs.next = nil keypairs.Unlock() // clear handshake state @@ -232,8 +232,8 @@ func (peer *Peer) ExpireCurrentKeypairs() { if keypairs.current != nil { keypairs.current.sendNonce.Store(RejectAfterMessages) } - if next := keypairs.next.Load(); next != nil { - next.sendNonce.Store(RejectAfterMessages) + if keypairs.next != nil { + keypairs.next.sendNonce.Store(RejectAfterMessages) } keypairs.Unlock() } From 0cdb15cf031a18e9f31f631d4beb2f439f30c71a Mon Sep 17 00:00:00 2001 From: James Tucker Date: Thu, 8 Sep 2022 14:23:07 -0700 Subject: [PATCH 2/2] device: switch to regular mutex for keypair guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit name old time/op new time/op delta TrieIPv4Peers100Addresses1000-32 83.5ns ± 0% 83.3ns ± 0% ~ (p=1.000 n=1+1) TrieIPv4Peers10Addresses10-32 33.6ns ± 0% 33.4ns ± 0% ~ (p=1.000 n=1+1) TrieIPv6Peers100Addresses1000-32 83.3ns ± 0% 83.2ns ± 0% ~ (p=1.000 n=1+1) TrieIPv6Peers10Addresses10-32 33.5ns ± 0% 33.2ns ± 0% ~ (p=1.000 n=1+1) Latency-32 216µs ± 0% 216µs ± 0% ~ (p=1.000 n=1+1) Throughput-32 2.31µs ± 0% 2.28µs ± 0% ~ (p=1.000 n=1+1) UAPIGet-32 2.28µs ± 0% 2.13µs ± 0% ~ (p=1.000 n=1+1) WaitPool-32 4.18µs ± 0% 4.14µs ± 0% ~ (p=1.000 n=1+1) name old packet-loss new packet-loss delta Throughput-32 0.00 ± 0% 0.01 ± 0% ~ (p=1.000 n=1+1) name old alloc/op new alloc/op delta UAPIGet-32 224B ± 0% 224B ± 0% ~ (all equal) name old allocs/op new allocs/op delta UAPIGet-32 17.0 ± 0% 17.0 ± 0% ~ (all equal) --- device/device.go | 9 ++++----- device/keypair.go | 6 +++--- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/device/device.go b/device/device.go index 8e5572432..7deaa7c5e 100644 --- a/device/device.go +++ b/device/device.go @@ -392,11 +392,10 @@ func (device *Device) SendKeepalivesToPeersWithCurrentKeypair() { device.peers.RLock() for _, peer := range device.peers.keyMap { - peer.keypairs.RLock() - sendKeepalive := peer.keypairs.current != nil && !peer.keypairs.current.created.Add(RejectAfterTime).Before(time.Now()) - peer.keypairs.RUnlock() - if sendKeepalive { - peer.SendKeepalive() + if current := peer.keypairs.Current(); current != nil { + if current.created.Add(RejectAfterTime).Before(time.Now()) { + peer.SendKeepalive() + } } } device.peers.RUnlock() diff --git a/device/keypair.go b/device/keypair.go index 93602a699..eea75c060 100644 --- a/device/keypair.go +++ b/device/keypair.go @@ -33,15 +33,15 @@ type Keypair struct { } type Keypairs struct { - sync.RWMutex + sync.Mutex current *Keypair previous *Keypair next *Keypair } func (kp *Keypairs) Current() *Keypair { - kp.RLock() - defer kp.RUnlock() + kp.Lock() + defer kp.Unlock() return kp.current }