diff --git a/client-stats.go b/client-stats.go index d799bae6bb..3513f8f1c2 100644 --- a/client-stats.go +++ b/client-stats.go @@ -7,4 +7,11 @@ type ClientStats struct { // 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 77f5587c6c..3c42fda4bc 100644 --- a/client.go +++ b/client.go @@ -18,6 +18,8 @@ import ( "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 @@ type Client struct { 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 @@ func (cl *Client) waitForRendezvousConnect(ctx context.Context, rz *utHolepunchR // 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 @@ func (cl *Client) establishOutgoingConnEx( ) { 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,12 +810,38 @@ func (cl *Client) establishOutgoingConnEx( 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( context.Background(), nc, t, obfuscatedHeader, @@ -859,6 +891,8 @@ type outgoingConnOpts struct { 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) Stats() ClientStats { 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 8cfb0ef9bc..c12d09c0fa 100644 --- a/internal/panicif/panicif.go +++ b/internal/panicif/panicif.go @@ -7,3 +7,15 @@ func NotEqual[T comparable](a, b T) { 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 7ad67b2b4f..c5af556def 100644 --- a/torrent.go +++ b/torrent.go @@ -1383,7 +1383,7 @@ func (t *Torrent) openNewConns() (initiated int) { return } p := t.peers.PopMax() - t.initiateConn(p, false, false, false) + t.initiateConn(p, false, false, false, false) initiated++ } return @@ -2403,6 +2403,7 @@ func (t *Torrent) initiateConn( requireRendezvous bool, skipHolepunchRendezvous bool, ignoreLimits bool, + receivedHolepunchConnect bool, ) { if peer.Id == t.cl.peerID { return @@ -2424,10 +2425,11 @@ func (t *Torrent) initiateConn( 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 @@ func (t *Torrent) handleReceivedUtHolepunchMsg(msg utHolepunch.Msg, sender *Peer 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 5742362905..a45eb8e591 100644 --- a/ut-holepunching_test.go +++ b/ut-holepunching_test.go @@ -94,9 +94,20 @@ func TestHolepunchConnect(t *testing.T) { 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) {