client-stats.go | 7 +++++++ client.go | 60 +++++++++++++++++++++++++++++++++++++++++++---------- internal/panicif/panicif.go | 12 ++++++++++++ torrent.go | 14 ++++++++------ ut-holepunching_test.go | 13 ++++++++++++- diff --git a/client-stats.go b/client-stats.go index d799bae6bb86f12816d923c7e11809b8d72f1e90..3513f8f1c250c98ffff3c0a4b33b0af5be8f737a 100644 --- a/client-stats.go +++ b/client-stats.go @@ -7,4 +7,11 @@ // Ongoing outgoing dial attempts. There may be more than one dial going on per peer address due // to hole-punch connect requests. The total may not match the sum of attempts for all Torrents // if a Torrent is dropped while there are outstanding dials. ActiveHalfOpenAttempts int + + // Number of unique peer addresses that were dialed after receiving a holepunch connect message, + // that have previously been undialable without any hole-punching attempts. + NumPeersUndialableWithoutHolepunchDialedAfterHolepunchConnect int + // Number of unique peer addresses that were successfully dialed and connected after a holepunch + // connect message and previously failing to connect without holepunching. + NumPeersDialableOnlyAfterHolepunch int } diff --git a/client.go b/client.go index 77f5587c6cd7174f4b2bda17600fa49f4ede1c37..3c42fda4bc489fa5afa361f246da8d6bbe82e109 100644 --- a/client.go +++ b/client.go @@ -18,6 +18,8 @@ "sort" "strconv" "time" + "github.com/anacrolix/torrent/internal/panicif" + "github.com/anacrolix/chansync" "github.com/anacrolix/chansync/events" "github.com/anacrolix/dht/v2" @@ -89,6 +91,10 @@ websocketTrackers websocketTrackers activeAnnounceLimiter limiter.Instance httpClient *http.Client + + undialableWithoutHolepunch map[netip.AddrPort]struct{} + undialableWithoutHolepunchDialAttemptedAfterHolepunchConnect map[netip.AddrPort]struct{} + dialableOnlyAfterHolepunch map[netip.AddrPort]struct{} } type ipStr string @@ -748,12 +754,8 @@ } // Returns nil connection and nil error if no connection could be established for valid reasons. func (cl *Client) initiateRendezvousConnect( - t *Torrent, addr PeerRemoteAddr, + t *Torrent, holepunchAddr netip.AddrPort, ) (ok bool, err error) { - holepunchAddr, err := addrPortFromPeerRemoteAddr(addr) - if err != nil { - return - } cl.lock() defer cl.unlock() rz, err := t.startHolepunchRendezvous(holepunchAddr) @@ -783,14 +785,18 @@ _ *PeerConn, err error, ) { t := opts.t addr := opts.addr - var rzOk bool - if !opts.skipHolepunchRendezvous { - rzOk, err = cl.initiateRendezvousConnect(t, addr) - if err != nil { - err = fmt.Errorf("initiating rendezvous connect: %w", err) + holepunchAddr, err := addrPortFromPeerRemoteAddr(addr) + var sentRendezvous bool + if err == nil { + if !opts.skipHolepunchRendezvous { + sentRendezvous, err = cl.initiateRendezvousConnect(t, holepunchAddr) + if err != nil { + err = fmt.Errorf("initiating rendezvous connect: %w", err) + } } } - if opts.requireRendezvous && !rzOk { + gotHolepunchConnect := (err == nil && sentRendezvous) || opts.receivedHolepunchConnect + if opts.requireRendezvous && !sentRendezvous { return nil, err } if err != nil { @@ -804,11 +810,37 @@ }()) defer cancel() dr := cl.dialFirst(dialCtx, addr.String()) nc := dr.Conn + cl.lock() + if gotHolepunchConnect && g.MapContains(cl.undialableWithoutHolepunch, holepunchAddr) { + g.MakeMapIfNilAndSet( + &cl.undialableWithoutHolepunchDialAttemptedAfterHolepunchConnect, + holepunchAddr, + struct{}{}, + ) + } + cl.unlock() if nc == nil { + if !sentRendezvous && !gotHolepunchConnect { + cl.lock() + g.MakeMapIfNilAndSet(&cl.undialableWithoutHolepunch, holepunchAddr, struct{}{}) + cl.unlock() + } if dialCtx.Err() != nil { return nil, fmt.Errorf("dialing: %w", dialCtx.Err()) } return nil, errors.New("dial failed") + } + if gotHolepunchConnect { + panicif.False(holepunchAddr.IsValid()) + cl.lock() + if g.MapContains(cl.undialableWithoutHolepunchDialAttemptedAfterHolepunchConnect, holepunchAddr) { + g.MakeMapIfNilAndSet( + &cl.dialableOnlyAfterHolepunch, + holepunchAddr, + struct{}{}, + ) + } + cl.unlock() } addrIpPort, _ := tryIpPortFromNetAddr(addr) c, err := cl.initiateProtocolHandshakes( @@ -859,6 +891,8 @@ // Don't attempt to connect unless a connect message is received after initiating a rendezvous. requireRendezvous bool // Don't send rendezvous requests to eligible relays. skipHolepunchRendezvous bool + // Outgoing connection attempt is in response to holepunch connect message. + receivedHolepunchConnect bool } // Called to dial out and run a connection. The addr we're given is already @@ -1795,5 +1829,9 @@ func (cl *Client) statsLocked() (stats ClientStats) { stats.ConnStats = cl.connStats.Copy() stats.ActiveHalfOpenAttempts = cl.numHalfOpen + stats.NumPeersUndialableWithoutHolepunchDialedAfterHolepunchConnect = + len(cl.undialableWithoutHolepunchDialAttemptedAfterHolepunchConnect) + stats.NumPeersDialableOnlyAfterHolepunch = + len(cl.dialableOnlyAfterHolepunch) return } diff --git a/internal/panicif/panicif.go b/internal/panicif/panicif.go index 8cfb0ef9bceb8986207b249a196b83e47f82886c..c12d09c0fa959712fc0455d468adff6c1ba421cd 100644 --- a/internal/panicif/panicif.go +++ b/internal/panicif/panicif.go @@ -7,3 +7,15 @@ if a != b { panic(fmt.Sprintf("%v != %v", a, b)) } } + +func False(b bool) { + if !b { + panic("is false") + } +} + +func True(b bool) { + if b { + panic("is true") + } +} diff --git a/torrent.go b/torrent.go index 7ad67b2b4f92025a9aaf04136421213f62fe629f..c5af556def2a2248503a03c7450b4d9f3c937a84 100644 --- a/torrent.go +++ b/torrent.go @@ -1383,7 +1383,7 @@ if t.cl.numHalfOpen >= t.cl.config.TotalHalfOpenConns { return } p := t.peers.PopMax() - t.initiateConn(p, false, false, false) + t.initiateConn(p, false, false, false, false) initiated++ } return @@ -2403,6 +2403,7 @@ peer PeerInfo, requireRendezvous bool, skipHolepunchRendezvous bool, ignoreLimits bool, + receivedHolepunchConnect bool, ) { if peer.Id == t.cl.peerID { return @@ -2424,10 +2425,11 @@ attemptKey := &peer t.addHalfOpen(addrStr, attemptKey) go t.cl.outgoingConnection( outgoingConnOpts{ - t: t, - addr: peer.Addr, - requireRendezvous: requireRendezvous, - skipHolepunchRendezvous: skipHolepunchRendezvous, + t: t, + addr: peer.Addr, + requireRendezvous: requireRendezvous, + skipHolepunchRendezvous: skipHolepunchRendezvous, + receivedHolepunchConnect: receivedHolepunchConnect, }, peer.Source, peer.Trusted, @@ -2814,7 +2816,7 @@ // it doesn't hurt to try again. t.initiateConn(PeerInfo{ Addr: msg.AddrPort, Source: PeerSourceUtHolepunch, - }, false, true, true) + }, false, true, true, true) } return nil case utHolepunch.Error: diff --git a/ut-holepunching_test.go b/ut-holepunching_test.go index 5742362905e67746a5f82bb056173f32d5cf71ec..a45eb8e591d7154bdf211493ecbac7c6538b2d58 100644 --- a/ut-holepunching_test.go +++ b/ut-holepunching_test.go @@ -94,9 +94,20 @@ targetAddr := seeder.ListenAddrs()[1] log.Printf("trying to initiate to %v", targetAddr) llg.initiateConn(PeerInfo{ Addr: targetAddr, - }, true, false, false) + }, true, false, false, false) llg.cl.unlock() wg.Wait() + // These checks would require that the leecher leecher first attempt to connect without + // holepunching. + + //llClientStats := leecherLeecher.Stats() + //c := qt.New(t) + //c.Check(llClientStats.NumPeersDialedRequiringHolepunch, qt.Not(qt.Equals), 0) + //c.Check( + // llClientStats.NumPeersDialedRequiringHolepunch, + // qt.Equals, + // llClientStats.NumPeersUndiableWithoutHolepunch, + //) } func waitForConns(t *Torrent) {