Skip to content

Commit

Permalink
switch to new sync/atomic helpers in go1.19 (slackhq#728)
Browse files Browse the repository at this point in the history
These new helpers make the code a lot cleaner. I confirmed that the
simple helpers like `atomic.Int64` don't add any extra overhead as they
get inlined by the compiler. `atomic.Pointer` adds an extra method call
as it no longer gets inlined, but we aren't using these on the hot path
so it is probably okay.
  • Loading branch information
wadey authored Oct 31, 2022
1 parent a800a48 commit 9af242d
Show file tree
Hide file tree
Showing 23 changed files with 127 additions and 146 deletions.
8 changes: 4 additions & 4 deletions .github/workflows/gofmt.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ jobs:
runs-on: ubuntu-latest
steps:

- name: Set up Go 1.18
- name: Set up Go 1.19
uses: actions/setup-go@v2
with:
go-version: 1.18
go-version: 1.19
id: go

- name: Check out code into the Go module directory
Expand All @@ -26,9 +26,9 @@ jobs:
- uses: actions/cache@v2
with:
path: ~/go/pkg/mod
key: ${{ runner.os }}-gofmt1.18-${{ hashFiles('**/go.sum') }}
key: ${{ runner.os }}-gofmt1.19-${{ hashFiles('**/go.sum') }}
restore-keys: |
${{ runner.os }}-gofmt1.18-
${{ runner.os }}-gofmt1.19-
- name: Install goimports
run: |
Expand Down
12 changes: 6 additions & 6 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ jobs:
name: Build Linux All
runs-on: ubuntu-latest
steps:
- name: Set up Go 1.18
- name: Set up Go 1.19
uses: actions/setup-go@v2
with:
go-version: 1.18
go-version: 1.19

- name: Checkout code
uses: actions/checkout@v2
Expand All @@ -34,10 +34,10 @@ jobs:
name: Build Windows
runs-on: windows-latest
steps:
- name: Set up Go 1.18
- name: Set up Go 1.19
uses: actions/setup-go@v2
with:
go-version: 1.18
go-version: 1.19

- name: Checkout code
uses: actions/checkout@v2
Expand Down Expand Up @@ -68,10 +68,10 @@ jobs:
HAS_SIGNING_CREDS: ${{ secrets.AC_USERNAME != '' }}
runs-on: macos-11
steps:
- name: Set up Go 1.18
- name: Set up Go 1.19
uses: actions/setup-go@v2
with:
go-version: 1.18
go-version: 1.19

- name: Checkout code
uses: actions/checkout@v2
Expand Down
8 changes: 4 additions & 4 deletions .github/workflows/smoke.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ jobs:
runs-on: ubuntu-latest
steps:

- name: Set up Go 1.18
- name: Set up Go 1.19
uses: actions/setup-go@v2
with:
go-version: 1.18
go-version: 1.19
id: go

- name: Check out code into the Go module directory
Expand All @@ -30,9 +30,9 @@ jobs:
- uses: actions/cache@v2
with:
path: ~/go/pkg/mod
key: ${{ runner.os }}-go1.18-${{ hashFiles('**/go.sum') }}
key: ${{ runner.os }}-go1.19-${{ hashFiles('**/go.sum') }}
restore-keys: |
${{ runner.os }}-go1.18-
${{ runner.os }}-go1.19-
- name: build
run: make bin-docker
Expand Down
16 changes: 8 additions & 8 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ jobs:
runs-on: ubuntu-latest
steps:

- name: Set up Go 1.18
- name: Set up Go 1.19
uses: actions/setup-go@v2
with:
go-version: 1.18
go-version: 1.19
id: go

- name: Check out code into the Go module directory
Expand All @@ -30,9 +30,9 @@ jobs:
- uses: actions/cache@v2
with:
path: ~/go/pkg/mod
key: ${{ runner.os }}-go1.18-${{ hashFiles('**/go.sum') }}
key: ${{ runner.os }}-go1.19-${{ hashFiles('**/go.sum') }}
restore-keys: |
${{ runner.os }}-go1.18-
${{ runner.os }}-go1.19-
- name: Build
run: make all
Expand All @@ -57,10 +57,10 @@ jobs:
os: [windows-latest, macos-11]
steps:

- name: Set up Go 1.18
- name: Set up Go 1.19
uses: actions/setup-go@v2
with:
go-version: 1.18
go-version: 1.19
id: go

- name: Check out code into the Go module directory
Expand All @@ -69,9 +69,9 @@ jobs:
- uses: actions/cache@v2
with:
path: ~/go/pkg/mod
key: ${{ runner.os }}-go1.18-${{ hashFiles('**/go.sum') }}
key: ${{ runner.os }}-go1.19-${{ hashFiles('**/go.sum') }}
restore-keys: |
${{ runner.os }}-go1.18-
${{ runner.os }}-go1.19-
- name: Build nebula
run: go build ./cmd/nebula
Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
GOMINVERSION = 1.18
GOMINVERSION = 1.19
NEBULA_CMD_PATH = "./cmd/nebula"
GO111MODULE = on
export GO111MODULE
Expand Down
2 changes: 1 addition & 1 deletion cmd/nebula-service/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (

// A version string that can be set with
//
// -ldflags "-X main.Build=SOMEVERSION"
// -ldflags "-X main.Build=SOMEVERSION"
//
// at compile-time.
var Build string
Expand Down
2 changes: 1 addition & 1 deletion cmd/nebula/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (

// A version string that can be set with
//
// -ldflags "-X main.Build=SOMEVERSION"
// -ldflags "-X main.Build=SOMEVERSION"
//
// at compile-time.
var Build string
Expand Down
20 changes: 17 additions & 3 deletions connection_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,20 @@ import (

var vpnIp iputil.VpnIp

func newTestLighthouse() *LightHouse {
lh := &LightHouse{
l: test.NewLogger(),
addrMap: map[iputil.VpnIp]*RemoteList{},
}
lighthouses := map[iputil.VpnIp]struct{}{}
staticList := map[iputil.VpnIp]struct{}{}

lh.lighthouses.Store(&lighthouses)
lh.staticList.Store(&staticList)

return lh
}

func Test_NewConnectionManagerTest(t *testing.T) {
l := test.NewLogger()
//_, tuncidr, _ := net.ParseCIDR("1.1.1.1/24")
Expand All @@ -35,7 +49,7 @@ func Test_NewConnectionManagerTest(t *testing.T) {
rawCertificateNoKey: []byte{},
}

lh := &LightHouse{l: l, atomicStaticList: make(map[iputil.VpnIp]struct{}), atomicLighthouses: make(map[iputil.VpnIp]struct{})}
lh := newTestLighthouse()
ifce := &Interface{
hostMap: hostMap,
inside: &test.NoopTun{},
Expand Down Expand Up @@ -104,7 +118,7 @@ func Test_NewConnectionManagerTest2(t *testing.T) {
rawCertificateNoKey: []byte{},
}

lh := &LightHouse{l: l, atomicStaticList: make(map[iputil.VpnIp]struct{}), atomicLighthouses: make(map[iputil.VpnIp]struct{})}
lh := newTestLighthouse()
ifce := &Interface{
hostMap: hostMap,
inside: &test.NoopTun{},
Expand Down Expand Up @@ -213,7 +227,7 @@ func Test_NewConnectionManagerTest_DisconnectInvalid(t *testing.T) {
rawCertificateNoKey: []byte{},
}

lh := &LightHouse{l: l, atomicStaticList: make(map[iputil.VpnIp]struct{}), atomicLighthouses: make(map[iputil.VpnIp]struct{})}
lh := newTestLighthouse()
ifce := &Interface{
hostMap: hostMap,
inside: &test.NoopTun{},
Expand Down
24 changes: 12 additions & 12 deletions connection_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,17 @@ import (
const ReplayWindow = 1024

type ConnectionState struct {
eKey *NebulaCipherState
dKey *NebulaCipherState
H *noise.HandshakeState
certState *CertState
peerCert *cert.NebulaCertificate
initiator bool
atomicMessageCounter uint64
window *Bits
queueLock sync.Mutex
writeLock sync.Mutex
ready bool
eKey *NebulaCipherState
dKey *NebulaCipherState
H *noise.HandshakeState
certState *CertState
peerCert *cert.NebulaCertificate
initiator bool
messageCounter atomic.Uint64
window *Bits
queueLock sync.Mutex
writeLock sync.Mutex
ready bool
}

func (f *Interface) newConnectionState(l *logrus.Logger, initiator bool, pattern noise.HandshakePattern, psk []byte, pskStage int) *ConnectionState {
Expand Down Expand Up @@ -70,7 +70,7 @@ func (cs *ConnectionState) MarshalJSON() ([]byte, error) {
return json.Marshal(m{
"certificate": cs.peerCert,
"initiator": cs.initiator,
"message_counter": atomic.LoadUint64(&cs.atomicMessageCounter),
"message_counter": cs.messageCounter.Load(),
"ready": cs.ready,
})
}
3 changes: 1 addition & 2 deletions control.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"net"
"os"
"os/signal"
"sync/atomic"
"syscall"

"github.com/sirupsen/logrus"
Expand Down Expand Up @@ -219,7 +218,7 @@ func copyHostInfo(h *HostInfo, preferredRanges []*net.IPNet) ControlHostInfo {
}

if h.ConnectionState != nil {
chi.MessageCounter = atomic.LoadUint64(&h.ConnectionState.atomicMessageCounter)
chi.MessageCounter = h.ConnectionState.messageCounter.Load()
}

if c := h.GetCert(); c != nil {
Expand Down
2 changes: 1 addition & 1 deletion firewall.go
Original file line number Diff line number Diff line change
Expand Up @@ -879,7 +879,7 @@ func parsePort(s string) (startPort, endPort int32, err error) {
return
}

//TODO: write tests for these
// TODO: write tests for these
func setTCPRTTTracking(c *conn, p []byte) {
if c.Seq != 0 {
return
Expand Down
6 changes: 3 additions & 3 deletions firewall/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ type ConntrackCache map[Packet]struct{}

type ConntrackCacheTicker struct {
cacheV uint64
cacheTick uint64
cacheTick atomic.Uint64

cache ConntrackCache
}
Expand All @@ -35,7 +35,7 @@ func NewConntrackCacheTicker(d time.Duration) *ConntrackCacheTicker {
func (c *ConntrackCacheTicker) tick(d time.Duration) {
for {
time.Sleep(d)
atomic.AddUint64(&c.cacheTick, 1)
c.cacheTick.Add(1)
}
}

Expand All @@ -45,7 +45,7 @@ func (c *ConntrackCacheTicker) Get(l *logrus.Logger) ConntrackCache {
if c == nil {
return nil
}
if tick := atomic.LoadUint64(&c.cacheTick); tick != c.cacheV {
if tick := c.cacheTick.Load(); tick != c.cacheV {
c.cacheV = tick
if ll := len(c.cache); ll > 0 {
if l.Level == logrus.DebugLevel {
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module github.com/slackhq/nebula

go 1.18
go 1.19

require (
github.com/anmitsu/go-shlex v0.0.0-20200514113438-38f4b401e2be
Expand Down
3 changes: 1 addition & 2 deletions handshake_ix.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package nebula

import (
"sync/atomic"
"time"

"github.com/flynn/noise"
Expand Down Expand Up @@ -51,7 +50,7 @@ func ixHandshakeStage0(f *Interface, vpnIp iputil.VpnIp, hostinfo *HostInfo) {
}

h := header.Encode(make([]byte, header.Len), header.Version, header.Handshake, header.HandshakeIXPSK0, 0, 1)
atomic.AddUint64(&ci.atomicMessageCounter, 1)
ci.messageCounter.Add(1)

msg, _, _, err := ci.H.WriteMessage(h, hsBytes)
if err != nil {
Expand Down
13 changes: 2 additions & 11 deletions handshake_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,7 @@ func Test_NewHandshakeManagerVpnIp(t *testing.T) {
preferredRanges := []*net.IPNet{localrange}
mw := &mockEncWriter{}
mainHM := NewHostMap(l, "test", vpncidr, preferredRanges)
lh := &LightHouse{
atomicStaticList: make(map[iputil.VpnIp]struct{}),
atomicLighthouses: make(map[iputil.VpnIp]struct{}),
addrMap: make(map[iputil.VpnIp]*RemoteList),
}
lh := newTestLighthouse()

blah := NewHandshakeManager(l, tuncidr, preferredRanges, mainHM, lh, &udp.Conn{}, defaultHandshakeConfig)

Expand Down Expand Up @@ -79,12 +75,7 @@ func Test_NewHandshakeManagerTrigger(t *testing.T) {
preferredRanges := []*net.IPNet{localrange}
mw := &mockEncWriter{}
mainHM := NewHostMap(l, "test", vpncidr, preferredRanges)
lh := &LightHouse{
addrMap: make(map[iputil.VpnIp]*RemoteList),
l: l,
atomicStaticList: make(map[iputil.VpnIp]struct{}),
atomicLighthouses: make(map[iputil.VpnIp]struct{}),
}
lh := newTestLighthouse()

blah := NewHandshakeManager(l, tuncidr, preferredRanges, mainHM, lh, &udp.Conn{}, defaultHandshakeConfig)

Expand Down
9 changes: 4 additions & 5 deletions hostmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import (
"github.com/slackhq/nebula/udp"
)

//const ProbeLen = 100
// const ProbeLen = 100
const PromoteEvery = 1000
const ReQueryEvery = 5000
const MaxRemotes = 10
Expand Down Expand Up @@ -153,7 +153,7 @@ type HostInfo struct {

remote *udp.Addr
remotes *RemoteList
promoteCounter uint32
promoteCounter atomic.Uint32
ConnectionState *ConnectionState
handshakeStart time.Time //todo: this an entry in the handshake manager
HandshakeReady bool //todo: being in the manager means you are ready
Expand Down Expand Up @@ -284,7 +284,6 @@ func (hm *HostMap) AddVpnIp(vpnIp iputil.VpnIp, init func(hostinfo *HostInfo)) (
if h, ok := hm.Hosts[vpnIp]; !ok {
hm.RUnlock()
h = &HostInfo{
promoteCounter: 0,
vpnIp: vpnIp,
HandshakePacket: make(map[uint8][]byte, 0),
relayState: RelayState{
Expand Down Expand Up @@ -591,7 +590,7 @@ func (hm *HostMap) Punchy(ctx context.Context, conn *udp.Conn) {
// TryPromoteBest handles re-querying lighthouses and probing for better paths
// NOTE: It is an error to call this if you are a lighthouse since they should not roam clients!
func (i *HostInfo) TryPromoteBest(preferredRanges []*net.IPNet, ifce *Interface) {
c := atomic.AddUint32(&i.promoteCounter, 1)
c := i.promoteCounter.Add(1)
if c%PromoteEvery == 0 {
// The lock here is currently protecting i.remote access
i.RLock()
Expand Down Expand Up @@ -658,7 +657,7 @@ func (i *HostInfo) handshakeComplete(l *logrus.Logger, m *cachedPacketMetrics) {
i.HandshakeComplete = true
//TODO: this should be managed by the handshake state machine to set it based on how many handshake were seen.
// Clamping it to 2 gets us out of the woods for now
atomic.StoreUint64(&i.ConnectionState.atomicMessageCounter, 2)
i.ConnectionState.messageCounter.Store(2)

if l.Level >= logrus.DebugLevel {
i.logger(l).Debugf("Sending %d stored packets", len(i.packetStore))
Expand Down
Loading

0 comments on commit 9af242d

Please sign in to comment.