Skip to content

Commit

Permalink
Remove unused variables from peer conn (netbirdio#2074)
Browse files Browse the repository at this point in the history 
Remove unused variables from peer conn
  • Loading branch information
@pappz
pappz authored Jun 4, 2024
1 parent 4da2945 commit 983d7ba
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 119 deletions.
5 changes: 0 additions & 5 deletions client/internal/engine.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -983,7 +983,6 @@ func (e *Engine) createPeerConn(pubKey string, allowedIPs string) (*peer.Conn, e
WgConfig: wgConfig,
LocalWgPort: e.config.WgPort,
NATExternalIPs: e.parseNATExternalIPMappings(),
UserspaceBind: e.wgInterface.IsUserspaceBind(),
RosenpassPubKey: e.getRosenpassPubKey(),
RosenpassAddr: e.getRosenpassAddr(),
}
Expand Down Expand Up @@ -1046,8 +1045,6 @@ func (e *Engine) receiveSignalEvents() {
return err
}

conn.RegisterProtoSupportMeta(msg.Body.GetFeaturesSupported())

var rosenpassPubKey []byte
rosenpassAddr := ""
if msg.GetBody().GetRosenpassConfig() != nil {
Expand All @@ -1070,8 +1067,6 @@ func (e *Engine) receiveSignalEvents() {
return err
}

conn.RegisterProtoSupportMeta(msg.GetBody().GetFeaturesSupported())

var rosenpassPubKey []byte
rosenpassAddr := ""
if msg.GetBody().GetRosenpassConfig() != nil {
Expand Down
109 changes: 48 additions & 61 deletions client/internal/peer/conn.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"github.com/netbirdio/netbird/iface"
"github.com/netbirdio/netbird/iface/bind"
"github.com/netbirdio/netbird/route"
signal "github.com/netbirdio/netbird/signal/client"
sProto "github.com/netbirdio/netbird/signal/proto"
nbnet "github.com/netbirdio/netbird/util/net"
"github.com/netbirdio/netbird/version"
Expand Down Expand Up @@ -70,9 +69,6 @@ type ConnConfig struct {

NATExternalIPs []string

// UsesBind indicates whether the WireGuard interface is userspace and uses bind.ICEBind
UserspaceBind bool

// RosenpassPubKey is this peer's Rosenpass public key
RosenpassPubKey []byte
// RosenpassPubKey is this peer's RosenpassAddr server address (IP:port)
Expand Down Expand Up @@ -135,32 +131,15 @@ type Conn struct {
wgProxyFactory *wgproxy.Factory
wgProxy wgproxy.Proxy

remoteModeCh chan ModeMessage
meta meta

adapter iface.TunAdapter
iFaceDiscover stdnet.ExternalIFaceDiscover
sentExtraSrflx bool

remoteEndpoint *net.UDPAddr
remoteConn *ice.Conn

connID nbnet.ConnectionID
beforeAddPeerHooks []BeforeAddPeerHookFunc
afterRemovePeerHooks []AfterRemovePeerHookFunc
}

// meta holds meta information about a connection
type meta struct {
protoSupport signal.FeaturesSupport
}

// ModeMessage represents a connection mode chosen by the peer
type ModeMessage struct {
// Direct indicates that it decided to use a direct connection
Direct bool
}

// GetConf returns the connection config
func (conn *Conn) GetConf() ConnConfig {
return conn.config
Expand All @@ -187,7 +166,6 @@ func NewConn(config ConnConfig, statusRecorder *Status, wgProxyFactory *wgproxy.
remoteOffersCh: make(chan OfferAnswer),
remoteAnswerCh: make(chan OfferAnswer),
statusRecorder: statusRecorder,
remoteModeCh: make(chan ModeMessage, 1),
wgProxyFactory: wgProxyFactory,
adapter: adapter,
iFaceDiscover: iFaceDiscover,
Expand Down Expand Up @@ -378,8 +356,6 @@ func (conn *Conn) Open(ctx context.Context) error {
remoteWgPort = remoteOfferAnswer.WgListenPort
}

conn.remoteConn = remoteConn

// the ice connection has been established successfully so we are ready to start the proxy
remoteAddr, err := conn.configureConnection(remoteConn, remoteWgPort, remoteOfferAnswer.RosenpassPubKey,
remoteOfferAnswer.RosenpassAddr)
Expand Down Expand Up @@ -437,7 +413,6 @@ func (conn *Conn) configureConnection(remoteConn net.Conn, remoteWgPort int, rem
}

endpointUdpAddr, _ := net.ResolveUDPAddr(endpoint.Network(), endpoint.String())
conn.remoteEndpoint = endpointUdpAddr
log.Debugf("Conn resolved IP for %s: %s", endpoint, endpointUdpAddr.IP)

conn.connID = nbnet.GenerateConnID()
Expand Down Expand Up @@ -623,40 +598,39 @@ func (conn *Conn) SetSendSignalMessage(handler func(message *sProto.Message) err
// onICECandidate is a callback attached to an ICE Agent to receive new local connection candidates
// and then signals them to the remote peer
func (conn *Conn) onICECandidate(candidate ice.Candidate) {
if candidate != nil {
// TODO: reported port is incorrect for CandidateTypeHost, makes understanding ICE use via logs confusing as port is ignored
log.Debugf("discovered local candidate %s", candidate.String())
go func() {
err := conn.signalCandidate(candidate)
if err != nil {
log.Errorf("failed signaling candidate to the remote peer %s %s", conn.config.Key, err)
}
// nil means candidate gathering has been ended
if candidate == nil {
return
}

// sends an extra server reflexive candidate to the remote peer with our related port (usually the wireguard port)
// this is useful when network has an existing port forwarding rule for the wireguard port and this peer
if !conn.sentExtraSrflx && candidate.Type() == ice.CandidateTypeServerReflexive && candidate.Port() != candidate.RelatedAddress().Port {
relatedAdd := candidate.RelatedAddress()
extraSrflx, err := ice.NewCandidateServerReflexive(&ice.CandidateServerReflexiveConfig{
Network: candidate.NetworkType().String(),
Address: candidate.Address(),
Port: relatedAdd.Port,
Component: candidate.Component(),
RelAddr: relatedAdd.Address,
RelPort: relatedAdd.Port,
})
if err != nil {
log.Errorf("failed creating extra server reflexive candidate %s", err)
return
}
err = conn.signalCandidate(extraSrflx)
if err != nil {
log.Errorf("failed signaling the extra server reflexive candidate to the remote peer %s: %s", conn.config.Key, err)
return
}
conn.sentExtraSrflx = true
}
}()
// TODO: reported port is incorrect for CandidateTypeHost, makes understanding ICE use via logs confusing as port is ignored
log.Debugf("discovered local candidate %s", candidate.String())
go func() {
err := conn.signalCandidate(candidate)
if err != nil {
log.Errorf("failed signaling candidate to the remote peer %s %s", conn.config.Key, err)
}
}()

if !conn.shouldSendExtraSrflxCandidate(candidate) {
return
}

// sends an extra server reflexive candidate to the remote peer with our related port (usually the wireguard port)
// this is useful when network has an existing port forwarding rule for the wireguard port and this peer
extraSrflx, err := extraSrflxCandidate(candidate)
if err != nil {
log.Errorf("failed creating extra server reflexive candidate %s", err)
return
}
conn.sentExtraSrflx = true

go func() {
err = conn.signalCandidate(extraSrflx)
if err != nil {
log.Errorf("failed signaling the extra server reflexive candidate to the remote peer %s: %s", conn.config.Key, err)
}
}()
}

func (conn *Conn) onICESelectedCandidatePair(c1 ice.Candidate, c2 ice.Candidate) {
Expand Down Expand Up @@ -807,10 +781,23 @@ func (conn *Conn) GetKey() string {
return conn.config.Key
}

// RegisterProtoSupportMeta register supported proto message in the connection metadata
func (conn *Conn) RegisterProtoSupportMeta(support []uint32) {
protoSupport := signal.ParseFeaturesSupported(support)
conn.meta.protoSupport = protoSupport
func (conn *Conn) shouldSendExtraSrflxCandidate(candidate ice.Candidate) bool {
if !conn.sentExtraSrflx && candidate.Type() == ice.CandidateTypeServerReflexive && candidate.Port() != candidate.RelatedAddress().Port {
return true
}
return false
}

func extraSrflxCandidate(candidate ice.Candidate) (*ice.CandidateServerReflexive, error) {
relatedAdd := candidate.RelatedAddress()
return ice.NewCandidateServerReflexive(&ice.CandidateServerReflexiveConfig{
Network: candidate.NetworkType().String(),
Address: candidate.Address(),
Port: relatedAdd.Port,
Component: candidate.Component(),
RelAddr: relatedAdd.Address,
RelPort: relatedAdd.Port,
})
}

func candidateViaRoutes(candidate ice.Candidate, clientRoutes route.HAMap) bool {
Expand Down
17 changes: 0 additions & 17 deletions signal/client/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,6 @@ const (
DirectCheck uint32 = 1
)

// FeaturesSupport register protocol supported features
type FeaturesSupport struct {
DirectCheck bool
}

type Client interface {
io.Closer
StreamConnected() bool
Expand Down Expand Up @@ -79,15 +74,3 @@ type Credential struct {
UFrag string
Pwd string
}

// ParseFeaturesSupported parses a slice of supported features into FeaturesSupport
func ParseFeaturesSupported(featuresMessage []uint32) FeaturesSupport {
var protoSupport FeaturesSupport
for _, feature := range featuresMessage {
if feature == DirectCheck {
protoSupport.DirectCheck = true
return protoSupport
}
}
return protoSupport
}
36 changes: 0 additions & 36 deletions signal/client/client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"net"
"sync"
"testing"
"time"

. "github.com/onsi/ginkgo"
Expand Down Expand Up @@ -168,41 +167,6 @@ var _ = Describe("GrpcClient", func() {

})

func TestParseFeaturesSupported(t *testing.T) {
expectedOnEmptyOrUnsupported := FeaturesSupport{DirectCheck: false}
expectedWithDirectCheck := FeaturesSupport{DirectCheck: true}
testCases := []struct {
name string
input []uint32
expected FeaturesSupport
}{
{
name: "Should Return DirectCheck Supported",
input: []uint32{DirectCheck},
expected: expectedWithDirectCheck,
},
{
name: "Should Return DirectCheck Unsupported When Nil",
input: nil,
expected: expectedOnEmptyOrUnsupported,
},
{
name: "Should Return DirectCheck Unsupported When Not Known Feature",
input: []uint32{9999},
expected: expectedOnEmptyOrUnsupported,
},
}

for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
result := ParseFeaturesSupported(testCase.input)
if result.DirectCheck != testCase.expected.DirectCheck {
t.Errorf("Direct check feature should match: Expected: %t, Got: %t", testCase.expected.DirectCheck, result.DirectCheck)
}
})
}
}

func createSignalClient(addr string, key wgtypes.Key) *GrpcClient {
var sigTLSEnabled = false
client, err := NewClient(context.Background(), addr, key, sigTLSEnabled)
Expand Down

0 comments on commit 983d7ba

Please sign in to comment.