Merge pull request #1643 from weaveworks/refactor-forwarder-remote-address

Refactor: remove knowledge of overlay connectivity details from Local{Peer,Connection}

router/connection.go

@@ -40,17 +40,16 @@ type RemoteConnection struct {
 type LocalConnection struct {
 	sync.RWMutex
 	RemoteConnection
-	TCPConn       *net.TCPConn
-	version       byte
-	tcpSender     TCPSender
-	remoteUDPAddr *net.UDPAddr
-	SessionKey    *[32]byte
-	heartbeatTCP  *time.Ticker
-	Router        *Router
-	uid           uint64
-	actionChan    chan<- ConnectionAction
-	finished      <-chan struct{} // closed to signal that actorLoop has finished
-	forwarder     OverlayForwarder
+	TCPConn      *net.TCPConn
+	version      byte
+	tcpSender    TCPSender
+	SessionKey   *[32]byte
+	heartbeatTCP *time.Ticker
+	Router       *Router
+	uid          uint64
+	actionChan   chan<- ConnectionAction
+	finished     <-chan struct{} // closed to signal that actorLoop has finished
+	forwarder    OverlayForwarder
 }
 
 type ConnectionAction func() error
@@ -83,7 +82,7 @@ func (conn *RemoteConnection) ErrorLog(args ...interface{}) {
 
 // Does not return anything. If the connection is successful, it will
 // end up in the local peer's connections map.
-func StartLocalConnection(connRemote *RemoteConnection, tcpConn *net.TCPConn, udpAddr *net.UDPAddr, router *Router, acceptNewPeer bool) {
+func StartLocalConnection(connRemote *RemoteConnection, tcpConn *net.TCPConn, router *Router, acceptNewPeer bool) {
 	if connRemote.local != router.Ourself.Peer {
 		log.Fatal("Attempt to create local connection from a peer which is not ourself")
 	}
@@ -94,7 +93,6 @@ func StartLocalConnection(connRemote *RemoteConnection, tcpConn *net.TCPConn, ud
 		RemoteConnection: *connRemote,
 		Router:           router,
 		TCPConn:          tcpConn,
-		remoteUDPAddr:    udpAddr,
 		uid:              randUint64(),
 		actionChan:       actionChan,
 		finished:         finished}
@@ -194,8 +192,9 @@ func (conn *LocalConnection) run(actionChan <-chan ConnectionAction, finished ch
 
 	params := ForwarderParams{
 		RemotePeer:         conn.remote,
-		LocalIP:            conn.TCPConn.LocalAddr().(*net.TCPAddr).IP,
-		RemoteAddr:         conn.remoteUDPAddr,
+		LocalAddr:          conn.TCPConn.LocalAddr().(*net.TCPAddr),
+		RemoteAddr:         conn.TCPConn.RemoteAddr().(*net.TCPAddr),
+		Outbound:           conn.outbound,
 		ConnUID:            conn.uid,
 		Crypto:             conn.forwarderCrypto(),
 		SendControlMessage: conn.sendOverlayControlMessage,

router/fastdp.go

@@ -523,15 +523,16 @@ func (fastdp fastDatapathOverlay) MakeForwarder(params ForwarderParams) (Overlay
 	}
 
 	vxlanVportID := fastdp.mainVxlanVportID
-	remoteAddr := params.RemoteAddr
-	if remoteAddr != nil {
+	var remoteAddr *net.UDPAddr
+
+	if params.Outbound {
+		remoteAddr = makeUDPAddr(params.RemoteAddr)
 		// The provided address contains the main weave port
 		// number to connect to.  We need to derive the vxlan
 		// port number from that.
-		vxlanRemoteAddr := *params.RemoteAddr
+		vxlanRemoteAddr := *remoteAddr
 		vxlanRemoteAddr.Port++
 		remoteAddr = &vxlanRemoteAddr
-
 		var err error
 		vxlanVportID, err = fastdp.getVxlanVportID(remoteAddr.Port)
 		if err != nil {
@@ -539,7 +540,7 @@ func (fastdp fastDatapathOverlay) MakeForwarder(params ForwarderParams) (Overlay
 		}
 	}
 
-	localIP, err := ipv4Bytes(params.LocalIP)
+	localIP, err := ipv4Bytes(params.LocalAddr.IP)
 	if err != nil {
 		return nil, err
 	}

router/local_peer.go

@@ -62,21 +62,16 @@ func (peer *LocalPeer) CreateConnection(peerAddr string, acceptNewPeer bool) err
 	if err := peer.checkConnectionLimit(); err != nil {
 		return err
 	}
-	tcpAddr, tcpErr := net.ResolveTCPAddr("tcp4", peerAddr)
-	udpAddr, udpErr := net.ResolveUDPAddr("udp4", peerAddr)
-	if tcpErr != nil || udpErr != nil {
-		// they really should have the same value, but just in case...
-		if tcpErr == nil {
-			return udpErr
-		}
-		return tcpErr
+	tcpAddr, err := net.ResolveTCPAddr("tcp4", peerAddr)
+	if err != nil {
+		return err
 	}
 	tcpConn, err := net.DialTCP("tcp4", nil, tcpAddr)
 	if err != nil {
 		return err
 	}
 	connRemote := NewRemoteConnection(peer.Peer, nil, tcpConn.RemoteAddr().String(), true, false)
-	StartLocalConnection(connRemote, tcpConn, udpAddr, peer.router, acceptNewPeer)
+	StartLocalConnection(connRemote, tcpConn, peer.router, acceptNewPeer)
 	return nil
 }
 

router/overlay.go

@@ -29,15 +29,19 @@ type Overlay interface {
 type ForwarderParams struct {
 	RemotePeer *Peer
 
-	// The local IP address to use for sending.  Derived from the
-	// local address of the corresponding TCP socket, so may
-	// differ for different forwarders.
-	LocalIP net.IP
+	// The local address of the corresponding TCP connection. Used to
+	// derive the local IP address for sending. May differ for
+	// different forwarders.
+	LocalAddr *net.TCPAddr
 
-	// The remote address to send to.  nil if unknown, i.e. an
-	// incoming connection, in which case the Overlay needs to
-	// discover it (e.g. from incoming datagrams).
-	RemoteAddr *net.UDPAddr
+	// The remote address of the corresponding TCP connection. Used to
+	// determine the address to send to, but only if the TCP
+	// connection is outbound. Otherwise the Overlay needs to discover
+	// it (e.g. from incoming datagrams).
+	RemoteAddr *net.TCPAddr
+
+	// Is the corresponding TCP connection outbound?
+	Outbound bool
 
 	// Unique identifier for this connection
 	ConnUID uint64

router/router.go

@@ -171,7 +171,7 @@ func (router *Router) acceptTCP(tcpConn *net.TCPConn) {
 	remoteAddrStr := tcpConn.RemoteAddr().String()
 	log.Printf("->[%s] connection accepted", remoteAddrStr)
 	connRemote := NewRemoteConnection(router.Ourself.Peer, nil, remoteAddrStr, false, false)
-	StartLocalConnection(connRemote, tcpConn, nil, router, true)
+	StartLocalConnection(connRemote, tcpConn, router, true)
 }
 
 func (router *Router) handleForwardedPacket(key ForwardPacketKey) FlowOp {

router/sleeve.go

@@ -365,6 +365,11 @@ func (sleeve *SleeveOverlay) MakeForwarder(params ForwarderParams) (OverlayForwa
 	confirmedChan := make(chan struct{})
 	finishedChan := make(chan struct{})
 
+	var remoteAddr *net.UDPAddr
+	if params.Outbound {
+		remoteAddr = makeUDPAddr(params.RemoteAddr)
+	}
+
 	fwd := &sleeveForwarder{
 		sleeve:           sleeve,
 		remotePeer:       params.RemotePeer,
@@ -379,13 +384,13 @@ func (sleeve *SleeveOverlay) MakeForwarder(params ForwarderParams) (OverlayForwa
 		finishedChan:     finishedChan,
 		establishedChan:  make(chan struct{}),
 		errorChan:        make(chan error, 1),
-		remoteAddr:       params.RemoteAddr,
+		remoteAddr:       remoteAddr,
 		mtu:              DefaultMTU,
 		crypto:           crypto,
 		maxPayload:       DefaultMTU - UDPOverhead,
 		overheadDF: UDPOverhead + crypto.EncDF.PacketOverhead() +
 			crypto.EncDF.FrameOverhead() + EthernetOverhead,
-		senderDF: newUDPSenderDF(params.LocalIP, sleeve.localPort),
+		senderDF: newUDPSenderDF(params.LocalAddr.IP, sleeve.localPort),
 	}
 
 	go fwd.run(aggChan, aggDFChan, specialChan, controlMsgChan,

router/utils.go

@@ -104,6 +104,10 @@ func intmac(key uint64) (r net.HardwareAddr) {
 	return
 }
 
+func makeUDPAddr(addr *net.TCPAddr) *net.UDPAddr {
+	return &net.UDPAddr{IP: addr.IP, Port: addr.Port, Zone: addr.Zone}
+}
+
 type ListOfPeers []*Peer
 
 func (lop ListOfPeers) Len() int {