refactor(nat): make changes to internal nat library

2025-09-26 20:21:26 +08:00 · 2025-01-23 19:01:50 -08:00
parent d5b0d8f8ac
commit 3c079e6363
22 changed files with 257 additions and 511 deletions
--- a/go.mod
+++ b/go.mod
@@ -15,19 +15,21 @@ require (
 	github.com/gorilla/websocket v1.5.3
 	github.com/hashicorp/golang-lru/arc/v2 v2.0.7
 	github.com/hashicorp/golang-lru/v2 v2.0.7
+	github.com/huin/goupnp v1.3.0
 	github.com/ipfs/go-cid v0.4.1
 	github.com/ipfs/go-datastore v0.6.0
 	github.com/ipfs/go-ds-badger v0.3.0
 	github.com/ipfs/go-ds-leveldb v0.5.0
 	github.com/ipfs/go-log/v2 v2.5.1
+	github.com/jackpal/go-nat-pmp v1.0.2
 	github.com/jbenet/go-temp-err-catcher v0.1.0
 	github.com/klauspost/compress v1.17.11
+	github.com/koron/go-ssdp v0.0.4
 	github.com/libp2p/go-buffer-pool v0.1.0
 	github.com/libp2p/go-flow-metrics v0.2.0
 	github.com/libp2p/go-libp2p-asn-util v0.4.1
 	github.com/libp2p/go-libp2p-testing v0.12.0
 	github.com/libp2p/go-msgio v0.3.0
-	github.com/libp2p/go-nat v0.2.0
 	github.com/libp2p/go-netroute v0.2.2
 	github.com/libp2p/go-reuseport v0.4.0
 	github.com/libp2p/go-yamux/v4 v4.0.1
@@ -90,11 +92,8 @@ require (
 	github.com/golang/snappy v0.0.0-20180518054509-2e65f85255db // indirect
 	github.com/google/pprof v0.0.0-20241210010833-40e02aabc2ad // indirect
 	github.com/google/uuid v1.6.0 // indirect
-	github.com/huin/goupnp v1.3.0 // indirect
-	github.com/jackpal/go-nat-pmp v1.0.2 // indirect
 	github.com/jbenet/goprocess v0.1.4 // indirect
 	github.com/klauspost/cpuid/v2 v2.2.9 // indirect
-	github.com/koron/go-ssdp v0.0.4 // indirect
 	github.com/mattn/go-isatty v0.0.20 // indirect
 	github.com/miekg/dns v1.1.62 // indirect
 	github.com/mikioh/tcpopt v0.0.0-20190314235656-172688c1accc // indirect
--- a/go.sum
+++ b/go.sum
@@ -188,8 +188,6 @@ github.com/libp2p/go-libp2p-testing v0.12.0 h1:EPvBb4kKMWO29qP4mZGyhVzUyR25dvfUI
 github.com/libp2p/go-libp2p-testing v0.12.0/go.mod h1:KcGDRXyN7sQCllucn1cOOS+Dmm7ujhfEyXQL5lvkcPg=
 github.com/libp2p/go-msgio v0.3.0 h1:mf3Z8B1xcFN314sWX+2vOTShIE0Mmn2TXn3YCUQGNj0=
 github.com/libp2p/go-msgio v0.3.0/go.mod h1:nyRM819GmVaF9LX3l03RMh10QdOroF++NBbxAb0mmDM=
-github.com/libp2p/go-nat v0.2.0 h1:Tyz+bUFAYqGyJ/ppPPymMGbIgNRH+WqC5QrT5fKrrGk=
-github.com/libp2p/go-nat v0.2.0/go.mod h1:3MJr+GRpRkyT65EpVPBstXLvOlAPzUVlG6Pwg9ohLJk=
 github.com/libp2p/go-netroute v0.2.2 h1:Dejd8cQ47Qx2kRABg6lPwknU7+nBnFRpko45/fFPuZ8=
 github.com/libp2p/go-netroute v0.2.2/go.mod h1:Rntq6jUAH0l9Gg17w5bFGhcC9a+vk4KNXs6s7IljKYE=
 github.com/libp2p/go-reuseport v0.4.0 h1:nR5KU7hD0WxXCJbmw7r2rhRYruNRl2koHw8fQscQm2s=
--- a/p2p/net/nat/internal/nat/.github/workflows/go-check.yml
+++ b/p2p/net/nat/internal/nat/.github/workflows/go-check.yml
@@ -1,18 +0,0 @@
-name: Go Checks
-
-on:
-  pull_request:
-  push:
-    branches: ["master"]
-  workflow_dispatch:
-
-permissions:
-  contents: read
-
-concurrency:
-  group: ${{ github.workflow }}-${{ github.event_name }}-${{ github.event_name == 'push' && github.sha || github.ref }}
-  cancel-in-progress: true
-
-jobs:
-  go-check:
-    uses: ipdxco/unified-github-workflows/.github/workflows/go-check.yml@v1.0
--- a/p2p/net/nat/internal/nat/.github/workflows/go-test.yml
+++ b/p2p/net/nat/internal/nat/.github/workflows/go-test.yml
@@ -1,20 +0,0 @@
-name: Go Test
-
-on:
-  pull_request:
-  push:
-    branches: ["master"]
-  workflow_dispatch:
-
-permissions:
-  contents: read
-
-concurrency:
-  group: ${{ github.workflow }}-${{ github.event_name }}-${{ github.event_name == 'push' && github.sha || github.ref }}
-  cancel-in-progress: true
-
-jobs:
-  go-test:
-    uses: ipdxco/unified-github-workflows/.github/workflows/go-test.yml@v1.0
-    secrets:
-      CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
--- a/p2p/net/nat/internal/nat/.github/workflows/release-check.yml
+++ b/p2p/net/nat/internal/nat/.github/workflows/release-check.yml
@@ -1,19 +0,0 @@
-name: Release Checker
-
-on:
-  pull_request_target:
-    paths: [ 'version.json' ]
-    types: [ opened, synchronize, reopened, labeled, unlabeled ]
-  workflow_dispatch:
-
-permissions:
-  contents: write
-  pull-requests: write
-
-concurrency:
-  group: ${{ github.workflow }}-${{ github.ref }}
-  cancel-in-progress: true
-
-jobs:
-  release-check:
-    uses: ipdxco/unified-github-workflows/.github/workflows/release-check.yml@v1.0
--- a/p2p/net/nat/internal/nat/.github/workflows/releaser.yml
+++ b/p2p/net/nat/internal/nat/.github/workflows/releaser.yml
@@ -1,17 +0,0 @@
-name: Releaser
-
-on:
-  push:
-    paths: [ 'version.json' ]
-  workflow_dispatch:
-
-permissions:
-  contents: write
-
-concurrency:
-  group: ${{ github.workflow }}-${{ github.sha }}
-  cancel-in-progress: true
-
-jobs:
-  releaser:
-    uses: ipdxco/unified-github-workflows/.github/workflows/releaser.yml@v1.0
--- a/p2p/net/nat/internal/nat/.github/workflows/stale.yml
+++ b/p2p/net/nat/internal/nat/.github/workflows/stale.yml
@@ -1,13 +0,0 @@
-name: Close and mark stale issue
-
-on:
-  schedule:
-    - cron: '0 0 * * *'
-
-permissions:
-  issues: write
-  pull-requests: write
-
-jobs:
-  stale:
-    uses: pl-strflt/.github/.github/workflows/reusable-stale-issue.yml@v0.3
--- a/p2p/net/nat/internal/nat/.github/workflows/tagpush.yml
+++ b/p2p/net/nat/internal/nat/.github/workflows/tagpush.yml
@@ -1,18 +0,0 @@
-name: Tag Push Checker
-
-on:
-  push:
-    tags:
-      - v*
-
-permissions:
-  contents: read
-  issues: write
-
-concurrency:
-  group: ${{ github.workflow }}-${{ github.ref }}
-  cancel-in-progress: true
-
-jobs:
-  releaser:
-    uses: ipdxco/unified-github-workflows/.github/workflows/tagpush.yml@v1.0
--- a/p2p/net/nat/internal/nat/README.md
+++ b/p2p/net/nat/internal/nat/README.md
@@ -1,9 +1 @@
-# go-nat
-
-[![GoDoc](https://godoc.org/github.com/libp2p/go-nat?status.svg)](https://godoc.org/github.com/libp2p/go-nat) [![status](https://sourcegraph.com/api/repos/github.com/libp2p/go-nat/.badges/status.png)](https://sourcegraph.com/github.com/libp2p/go-nat)
-
-Forked from: [fd/go-nat](https://github.com/fd/go-nat).
-
---
-
-The last gx published version of this module was: 1.0.3: QmdwkZHamNNrj7k3G29rnurmW3mFzsDhnyXppNcgYsiBVz
+Originally forked from: [fd/go-nat](https://github.com/fd/go-nat).
--- a/p2p/net/nat/internal/nat/_examples/nat-tester.go
+++ b/p2p/net/nat/internal/nat/_examples/nat-tester.go
@@ -1,67 +0,0 @@
-package main
-
-import (
-	"fmt"
-	"log"
-	"net/http"
-	"time"
-
-	"github.com/libp2p/go-nat"
-)
-
-func main() {
-	nat, err := nat.DiscoverGateway()
-	if err != nil {
-		log.Fatalf("error: %s", err)
-	}
-	log.Printf("nat type: %s", nat.Type())
-
-	daddr, err := nat.GetDeviceAddress()
-	if err != nil {
-		log.Fatalf("error: %s", err)
-	}
-	log.Printf("device address: %s", daddr)
-
-	iaddr, err := nat.GetInternalAddress()
-	if err != nil {
-		log.Fatalf("error: %s", err)
-	}
-	log.Printf("internal address: %s", iaddr)
-
-	eaddr, err := nat.GetExternalAddress()
-	if err != nil {
-		log.Fatalf("error: %s", err)
-	}
-	log.Printf("external address: %s", eaddr)
-
-	eport, err := nat.AddPortMapping("tcp", 3080, "http", 60)
-	if err != nil {
-		log.Fatalf("error: %s", err)
-	}
-
-	log.Printf("test-page: http://%s:%d/", eaddr, eport)
-
-	go func() {
-		for {
-			time.Sleep(30 * time.Second)
-
-			_, err = nat.AddPortMapping("tcp", 3080, "http", 60)
-			if err != nil {
-				log.Fatalf("error: %s", err)
-			}
-		}
-	}()
-
-	defer nat.DeletePortMapping("tcp", 3080)
-
-	http.ListenAndServe(":3080", http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
-		rw.Header().Set("Content-Type", "text/plain")
-		rw.WriteHeader(200)
-		fmt.Fprintf(rw, "Hello there!\n")
-		fmt.Fprintf(rw, "nat type: %s\n", nat.Type())
-		fmt.Fprintf(rw, "device address: %s\n", daddr)
-		fmt.Fprintf(rw, "internal address: %s\n", iaddr)
-		fmt.Fprintf(rw, "external address: %s\n", eaddr)
-		fmt.Fprintf(rw, "test-page: http://%s:%d/\n", eaddr, eport)
-	}))
-}
--- a/p2p/net/nat/internal/nat/gateway.go
+++ b/p2p/net/nat/internal/nat/gateway.go
@@ -1,17 +0,0 @@
-package nat
-
-import (
-	"net"
-
-	"github.com/libp2p/go-netroute"
-)
-
-func getDefaultGateway() (net.IP, error) {
-	router, err := netroute.New()
-	if err != nil {
-		return nil, err
-	}
-
-	_, ip, _, err := router.Route(net.IPv4zero)
-	return ip, err
-}
--- a/p2p/net/nat/internal/nat/go.mod
+++ b/p2p/net/nat/internal/nat/go.mod
@@ -1,17 +0,0 @@
-module github.com/libp2p/go-nat
-
-require (
-	github.com/huin/goupnp v1.2.0
-	github.com/jackpal/go-nat-pmp v1.0.2
-	github.com/koron/go-ssdp v0.0.4
-	github.com/libp2p/go-netroute v0.2.1
-)
-
-require (
-	github.com/google/gopacket v1.1.19 // indirect
-	golang.org/x/net v0.10.0 // indirect
-	golang.org/x/sync v0.2.0 // indirect
-	golang.org/x/sys v0.8.0 // indirect
-)
-
-go 1.22
--- a/p2p/net/nat/internal/nat/go.sum
+++ b/p2p/net/nat/internal/nat/go.sum
@@ -1,29 +0,0 @@
-github.com/google/gopacket v1.1.19 h1:ves8RnFZPGiFnTS0uPQStjwru6uO6h+nlr9j6fL7kF8=
-github.com/google/gopacket v1.1.19/go.mod h1:iJ8V8n6KS+z2U1A8pUwu8bW5SyEMkXJB8Yo/Vo+TKTo=
-github.com/huin/goupnp v1.2.0 h1:uOKW26NG1hsSSbXIZ1IR7XP9Gjd1U8pnLaCMgntmkmY=
-github.com/huin/goupnp v1.2.0/go.mod h1:gnGPsThkYa7bFi/KWmEysQRf48l2dvR5bxr2OFckNX8=
-github.com/jackpal/go-nat-pmp v1.0.2 h1:KzKSgb7qkJvOUTqYl9/Hg/me3pWgBmERKrTGD7BdWus=
-github.com/jackpal/go-nat-pmp v1.0.2/go.mod h1:QPH045xvCAeXUZOxsnwmrtiCoxIr9eob+4orBN1SBKc=
-github.com/koron/go-ssdp v0.0.4 h1:1IDwrghSKYM7yLf7XCzbByg2sJ/JcNOZRXS2jczTwz0=
-github.com/koron/go-ssdp v0.0.4/go.mod h1:oDXq+E5IL5q0U8uSBcoAXzTzInwy5lEgC91HoKtbmZk=
-github.com/libp2p/go-netroute v0.2.1 h1:V8kVrpD8GK0Riv15/7VN6RbUQ3URNZVosw7H2v9tksU=
-github.com/libp2p/go-netroute v0.2.1/go.mod h1:hraioZr0fhBjG0ZRXJJ6Zj2IVEVNx6tDTFQfSmcq7mQ=
-golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
-golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
-golang.org/x/lint v0.0.0-20200302205851-738671d3881b/go.mod h1:3xt1FjdF8hUf6vQPIChWIBhFzV8gjjsPE/fR3IyQdNY=
-golang.org/x/mod v0.1.1-0.20191105210325-c90efee705ee/go.mod h1:QqPTAvyqsEbceGzBzNggFXnrqF1CaUcvgkdR5Ot7KZg=
-golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
-golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
-golang.org/x/net v0.10.0 h1:X2//UzNDwYmtCLn7To6G58Wr6f5ahEAQgKNzv9Y951M=
-golang.org/x/net v0.10.0/go.mod h1:0qNGK6F8kojg2nk9dLZ2mShWaEBan6FAoqfSigmmuDg=
-golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
-golang.org/x/sync v0.0.0-20210220032951-036812b2e83c/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
-golang.org/x/sync v0.2.0 h1:PUR+T4wwASmuSTYdKjYHI5TD22Wy5ogLU5qZCOLxBrI=
-golang.org/x/sync v0.2.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
-golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
-golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
-golang.org/x/sys v0.8.0 h1:EBmGv8NaZBZTWvrbjNoL6HVt+IVy3QDQpJs7VRIw3tU=
-golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
-golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
-golang.org/x/tools v0.0.0-20200130002326-2f3ba24bd6e7/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28=
-golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
--- a/p2p/net/nat/internal/nat/nat.go
+++ b/p2p/net/nat/internal/nat/nat.go
@@ -4,15 +4,37 @@ package nat
 import (
 	"context"
 	"errors"
+	"fmt"
 	"math"
 	"math/rand"
 	"net"
+	"strings"
 	"time"
+
+	logging "github.com/ipfs/go-log/v2"
+	"github.com/libp2p/go-netroute"
 )

+var log = logging.Logger("internal/nat")
+
 var ErrNoExternalAddress = errors.New("no external address")
 var ErrNoInternalAddress = errors.New("no internal address")
-var ErrNoNATFound = errors.New("no NAT found")
+
+type ErrNoNATFound struct {
+	Errs []error
+}
+
+func (e ErrNoNATFound) Unwrap() []error {
+	return e.Errs
+}
+
+func (e ErrNoNATFound) Error() string {
+	var errStrs []string
+	for _, err := range e.Errs {
+		errStrs = append(errStrs, err.Error())
+	}
+	return fmt.Sprintf("no NAT found: [%s]", strings.Join(errStrs, "; "))
+}

 // protocol is either "udp" or "tcp"
 type NAT interface {
@@ -35,65 +57,91 @@ type NAT interface {
 	DeletePortMapping(ctx context.Context, protocol string, internalPort int) (err error)
 }

-// DiscoverNATs returns all NATs discovered in the network.
-func DiscoverNATs(ctx context.Context) <-chan NAT {
-	nats := make(chan NAT)
+// discoverNATs returns all NATs discovered in the network.
+func discoverNATs(ctx context.Context) ([]NAT, []error) {
+	type natsAndErrs struct {
+		nats []NAT
+		errs []error
+	}
+	upnpCh := make(chan natsAndErrs)
+	pmpCh := make(chan natsAndErrs)

 	go func() {
-		defer close(nats)
+		defer close(upnpCh)

-		upnpIg1 := discoverUPNP_IG1(ctx)
-		upnpIg2 := discoverUPNP_IG2(ctx)
-		natpmp := discoverNATPMP(ctx)
-		upnpGenIGDev := discoverUPNP_GenIGDev(ctx)
-		for upnpIg1 != nil || upnpIg2 != nil || natpmp != nil || upnpGenIGDev != nil {
-			var (
-				nat NAT
-				ok  bool
-			)
-			select {
-			case nat, ok = <-upnpIg1:
-				if !ok {
-					upnpIg1 = nil
-				}
-			case nat, ok = <-upnpIg2:
-				if !ok {
-					upnpIg2 = nil
-				}
-			case nat, ok = <-upnpGenIGDev:
-				if !ok {
-					upnpGenIGDev = nil
-				}
-			case nat, ok = <-natpmp:
-				if !ok {
-					natpmp = nil
-				}
-			case <-ctx.Done():
-				// timeout.
-				return
-			}
-			if ok {
-				select {
-				case nats <- nat:
-				case <-ctx.Done():
-					return
-				}
-			}
+		// We do these UPNP queries sequentially because some routers will fail to handle parallel requests.
+		nats, errs := discoverUPNP_IG1(ctx)
+
+		// Do IG2 after IG1 so that its NAT devices will appear as "better" when we
+		// find the best NAT to return below.
+		n, e := discoverUPNP_IG2(ctx)
+		nats = append(nats, n...)
+		errs = append(errs, e...)
+
+		if len(nats) == 0 {
+			// We don't have a NAT. We should try querying all devices over
+			// SSDP to find a InternetGatewayDevice. This shouldn't be necessary for
+			// a well behaved router.
+			n, e = discoverUPNP_GenIGDev(ctx)
+			nats = append(nats, n...)
+			errs = append(errs, e...)
+		}
+
+		select {
+		case upnpCh <- natsAndErrs{nats, errs}:
+		case <-ctx.Done():
 		}
 	}()
-	return nats
+
+	go func() {
+		defer close(pmpCh)
+		nat, err := discoverNATPMP(ctx)
+		var nats []NAT
+		var errs []error
+		if err != nil {
+			errs = append(errs, err)
+		} else {
+			nats = append(nats, nat)
+		}
+		select {
+		case pmpCh <- natsAndErrs{nats, errs}:
+		case <-ctx.Done():
+		}
+	}()
+
+	var nats []NAT
+	var errs []error
+
+	for upnpCh != nil || pmpCh != nil {
+		select {
+		case res := <-pmpCh:
+			pmpCh = nil
+			nats = append(nats, res.nats...)
+			errs = append(errs, res.errs...)
+		case res := <-upnpCh:
+			upnpCh = nil
+			nats = append(nats, res.nats...)
+			errs = append(errs, res.errs...)
+		case <-ctx.Done():
+			errs = append(errs, ctx.Err())
+			return nats, errs
+		}
+	}
+	return nats, errs
 }

 // DiscoverGateway attempts to find a gateway device.
 func DiscoverGateway(ctx context.Context) (NAT, error) {
-	var nats []NAT
-	for nat := range DiscoverNATs(ctx) {
-		nats = append(nats, nat)
-	}
+	nats, errs := discoverNATs(ctx)
+
 	switch len(nats) {
 	case 0:
-		return nil, ErrNoNATFound
+		return nil, ErrNoNATFound{Errs: errs}
 	case 1:
+		if len(errs) > 0 {
+			log.Debugf("NAT found, but some potentially unrelated errors occurred: %v", errs)
+		}
+
 		return nats[0], nil
 	}
 	gw, _ := getDefaultGateway()
@@ -115,6 +163,10 @@ func DiscoverGateway(ctx context.Context) (NAT, error) {
 		bestNATIsGw = natIsGw
 		bestNAT = nat
 	}
+
+	if len(errs) > 0 {
+		log.Debugf("NAT found, but some potentially unrelated errors occurred: %v", errs)
+	}
 	return bestNAT, nil
 }

@@ -123,3 +175,13 @@ var random = rand.New(rand.NewSource(time.Now().UnixNano()))
 func randomPort() int {
 	return random.Intn(math.MaxUint16-10000) + 10000
 }
+
+func getDefaultGateway() (net.IP, error) {
+	router, err := netroute.New()
+	if err != nil {
+		return nil, err
+	}
+
+	_, ip, _, err := router.Route(net.IPv4zero)
+	return ip, err
+}
--- a/p2p/net/nat/internal/nat/natpmp.go
+++ b/p2p/net/nat/internal/nat/natpmp.go
@@ -12,42 +12,48 @@ var (
 	_ NAT = (*natpmpNAT)(nil)
 )

-func discoverNATPMP(ctx context.Context) <-chan NAT {
-	res := make(chan NAT, 1)
-
+func discoverNATPMP(ctx context.Context) (NAT, error) {
 	ip, err := getDefaultGateway()
 	if err != nil {
-		return nil
+		return nil, err
 	}

-	go func() {
-		defer close(res)
-		// Unfortunately, we can't actually _stop_ the natpmp
-		// library. However, we can at least close _our_ channel
-		// and walk away.
-		select {
-		case client, ok := <-discoverNATPMPWithAddr(ip):
-			if ok {
-				res <- &natpmpNAT{client, ip, make(map[int]int)}
-			}
-		case <-ctx.Done():
-		}
-	}()
-	return res
-}
+	clientCh := make(chan *natpmp.Client, 1)
+	errCh := make(chan error, 1)

-func discoverNATPMPWithAddr(ip net.IP) <-chan *natpmp.Client {
-	res := make(chan *natpmp.Client, 1)
+	// We can't cancel the natpmp library, but we can at least still return
+	// on context cancellation by putting this in a goroutine
 	go func() {
-		defer close(res)
-		client := natpmp.NewClient(ip)
-		_, err := client.GetExternalAddress()
+		client, err := discoverNATPMPWithAddr(ctx, ip)
 		if err != nil {
+			errCh <- err
 			return
 		}
-		res <- client
+		clientCh <- client
 	}()
-	return res
+
+	select {
+	case client := <-clientCh:
+		return &natpmpNAT{client, ip, make(map[int]int)}, nil
+	case err := <-errCh:
+		return nil, err
+	case <-ctx.Done():
+		return nil, ctx.Err()
+	}
+}
+
+func discoverNATPMPWithAddr(ctx context.Context, ip net.IP) (*natpmp.Client, error) {
+	var client *natpmp.Client
+	if deadline, ok := ctx.Deadline(); ok {
+		client = natpmp.NewClientWithTimeout(ip, time.Until(deadline))
+	} else {
+		client = natpmp.NewClient(ip)
+	}
+	_, err := client.GetExternalAddress()
+	if err != nil {
+		return nil, err
+	}
+	return client, nil
 }

 type natpmpNAT struct {
--- a/p2p/net/nat/internal/nat/upnp.go
+++ b/p2p/net/nat/internal/nat/upnp.go
@@ -2,6 +2,7 @@ package nat

 import (
 	"context"
+	"fmt"
 	"net"
 	"net/url"
 	"strings"
@@ -16,197 +17,127 @@ import (

 var _ NAT = (*upnp_NAT)(nil)

-func discoverUPNP_IG1(ctx context.Context) <-chan NAT {
-	res := make(chan NAT)
-	go func() {
-		defer close(res)
-
-		// find devices
-		devs, err := goupnp.DiscoverDevicesCtx(ctx, internetgateway1.URN_WANConnectionDevice_1)
-		if err != nil {
-			return
-		}
-
-		for _, dev := range devs {
-			if dev.Root == nil {
-				continue
-			}
-
-			dev.Root.Device.VisitServices(func(srv *goupnp.Service) {
-				if ctx.Err() != nil {
-					return
-				}
-				switch srv.ServiceType {
-				case internetgateway1.URN_WANIPConnection_1:
-					client := &internetgateway1.WANIPConnection1{ServiceClient: goupnp.ServiceClient{
-						SOAPClient: srv.NewSOAPClient(),
-						RootDevice: dev.Root,
-						Service:    srv,
-					}}
-					_, isNat, err := client.GetNATRSIPStatusCtx(ctx)
-					if err == nil && isNat {
-						select {
-						case res <- &upnp_NAT{client, make(map[int]int), "UPNP (IG1-IP1)", dev.Root}:
-						case <-ctx.Done():
-						}
-					}
-
-				case internetgateway1.URN_WANPPPConnection_1:
-					client := &internetgateway1.WANPPPConnection1{ServiceClient: goupnp.ServiceClient{
-						SOAPClient: srv.NewSOAPClient(),
-						RootDevice: dev.Root,
-						Service:    srv,
-					}}
-					_, isNat, err := client.GetNATRSIPStatusCtx(ctx)
-					if err == nil && isNat {
-						select {
-						case res <- &upnp_NAT{client, make(map[int]int), "UPNP (IG1-PPP1)", dev.Root}:
-						case <-ctx.Done():
-						}
-					}
-
-				}
-			})
-		}
-
-	}()
-	return res
+func discoverUPNP_IG1(ctx context.Context) ([]NAT, []error) {
+	return discoverSearchTarget(ctx, internetgateway1.URN_WANConnectionDevice_1)
 }

-func discoverUPNP_IG2(ctx context.Context) <-chan NAT {
-	res := make(chan NAT)
-	go func() {
-		defer close(res)
-
-		// find devices
-		devs, err := goupnp.DiscoverDevicesCtx(ctx, internetgateway2.URN_WANConnectionDevice_2)
-		if err != nil {
-			return
-		}
-
-		for _, dev := range devs {
-			if dev.Root == nil {
-				continue
-			}
-
-			dev.Root.Device.VisitServices(func(srv *goupnp.Service) {
-				if ctx.Err() != nil {
-					return
-				}
-				switch srv.ServiceType {
-				case internetgateway2.URN_WANIPConnection_1:
-					client := &internetgateway2.WANIPConnection1{ServiceClient: goupnp.ServiceClient{
-						SOAPClient: srv.NewSOAPClient(),
-						RootDevice: dev.Root,
-						Service:    srv,
-					}}
-					_, isNat, err := client.GetNATRSIPStatusCtx(ctx)
-					if err == nil && isNat {
-						select {
-						case res <- &upnp_NAT{client, make(map[int]int), "UPNP (IG2-IP1)", dev.Root}:
-						case <-ctx.Done():
-						}
-					}
-
-				case internetgateway2.URN_WANIPConnection_2:
-					client := &internetgateway2.WANIPConnection2{ServiceClient: goupnp.ServiceClient{
-						SOAPClient: srv.NewSOAPClient(),
-						RootDevice: dev.Root,
-						Service:    srv,
-					}}
-					_, isNat, err := client.GetNATRSIPStatusCtx(ctx)
-					if err == nil && isNat {
-						select {
-						case res <- &upnp_NAT{client, make(map[int]int), "UPNP (IG2-IP2)", dev.Root}:
-						case <-ctx.Done():
-						}
-					}
-
-				case internetgateway2.URN_WANPPPConnection_1:
-					client := &internetgateway2.WANPPPConnection1{ServiceClient: goupnp.ServiceClient{
-						SOAPClient: srv.NewSOAPClient(),
-						RootDevice: dev.Root,
-						Service:    srv,
-					}}
-					_, isNat, err := client.GetNATRSIPStatusCtx(ctx)
-					if err == nil && isNat {
-						select {
-						case res <- &upnp_NAT{client, make(map[int]int), "UPNP (IG2-PPP1)", dev.Root}:
-						case <-ctx.Done():
-						}
-					}
-
-				}
-			})
-		}
-
-	}()
-	return res
+func discoverUPNP_IG2(ctx context.Context) ([]NAT, []error) {
+	return discoverSearchTarget(ctx, internetgateway2.URN_WANConnectionDevice_2)
 }

-func discoverUPNP_GenIGDev(ctx context.Context) <-chan NAT {
-	res := make(chan NAT, 1)
-	go func() {
-		defer close(res)
+func discoverSearchTarget(ctx context.Context, target string) (nats []NAT, errs []error) {
+	// find devices
+	devs, err := goupnp.DiscoverDevicesCtx(ctx, target)
+	if err != nil {
+		errs = append(errs, err)
+		return
+	}

-		DeviceList, err := ssdp.Search(ssdp.All, 5, "")
-		if err != nil {
-			return
-		}
-		var gw ssdp.Service
-		for _, Service := range DeviceList {
-			if strings.Contains(Service.Type, "InternetGatewayDevice") {
-				gw = Service
-				break
-			}
+	for _, dev := range devs {
+		if dev.Err != nil {
+			errs = append(errs, dev.Err)
+			continue
 		}
+		dev.Root.Device.VisitServices(serviceVisitor(ctx, dev.Root, &nats, &errs))
+	}
+	return
+}

-		DeviceURL, err := url.Parse(gw.Location)
+// discoverUPNP_GenIGDev is a fallback for routers that fail to respond to our
+// targetted SSDP queries. It will query all devices and try to find any
+// InternetGatewayDevice.
+func discoverUPNP_GenIGDev(ctx context.Context) (nats []NAT, errs []error) {
+	DeviceList, err := ssdp.Search(ssdp.All, 5, "")
+	if err != nil {
+		errs = append(errs, err)
+		return
+	}
+
+	// Limit the number of InternetGateways we'll query. Normally we'd only
+	// expect 1 or 2, but in case of a weird network we also don't want to do
+	// too much work.
+	const maxIGDevs = 3
+	foundIGDevs := 0
+	for _, Service := range DeviceList {
+		if !strings.Contains(Service.Type, "InternetGatewayDevice") {
+			continue
+		}
+		if foundIGDevs >= maxIGDevs {
+			log.Debug("found more than maxIGDevs UPnP devices, stopping search")
+			break
+		}
+		foundIGDevs++
+
+		DeviceURL, err := url.Parse(Service.Location)
 		if err != nil {
-			return
+			errs = append(errs, err)
+			continue
 		}
 		RootDevice, err := goupnp.DeviceByURLCtx(ctx, DeviceURL)
 		if err != nil {
-			return
+			errs = append(errs, err)
+			continue
 		}

-		RootDevice.Device.VisitServices(func(srv *goupnp.Service) {
-			if ctx.Err() != nil {
+		RootDevice.Device.VisitServices(serviceVisitor(ctx, RootDevice, &nats, &errs))
+	}
+	return
+}
+
+// serviceVisitor is a vistor function that visits all services of a root
+// device and collects NATs.
+//
+// It works on InternetGateway V1 and V2 devices. For V1 devices, V2 services should not be encountered, and the visitor will collect an error in that case.
+func serviceVisitor(ctx context.Context, rootDevice *goupnp.RootDevice, outNats *[]NAT, outErrs *[]error) func(srv *goupnp.Service) {
+	return func(srv *goupnp.Service) {
+		if ctx.Err() != nil {
+			return
+		}
+		switch srv.ServiceType {
+		case internetgateway2.URN_WANIPConnection_1:
+			client := &internetgateway2.WANIPConnection1{ServiceClient: goupnp.ServiceClient{
+				SOAPClient: srv.NewSOAPClient(),
+				RootDevice: rootDevice,
+				Service:    srv,
+			}}
+			_, isNat, err := client.GetNATRSIPStatusCtx(ctx)
+			if err != nil {
+				*outErrs = append(*outErrs, err)
+			} else if isNat {
+				*outNats = append(*outNats, &upnp_NAT{client, make(map[int]int), "UPNP (IP1)", rootDevice})
+			}
+
+		case internetgateway2.URN_WANIPConnection_2:
+			if rootDevice.Device.DeviceType == internetgateway2.URN_WANConnectionDevice_1 {
+				*outErrs = append(*outErrs, fmt.Errorf("found V2 service on V1 device"))
 				return
 			}
-			switch srv.ServiceType {
-			case internetgateway1.URN_WANIPConnection_1:
-				client := &internetgateway1.WANIPConnection1{ServiceClient: goupnp.ServiceClient{
-					SOAPClient: srv.NewSOAPClient(),
-					RootDevice: RootDevice,
-					Service:    srv,
-				}}
-				_, isNat, err := client.GetNATRSIPStatusCtx(ctx)
-				if err == nil && isNat {
-					select {
-					case res <- &upnp_NAT{client, make(map[int]int), "UPNP (IG1-IP1)", RootDevice}:
-					case <-ctx.Done():
-					}
-				}
-
-			case internetgateway1.URN_WANPPPConnection_1:
-				client := &internetgateway1.WANPPPConnection1{ServiceClient: goupnp.ServiceClient{
-					SOAPClient: srv.NewSOAPClient(),
-					RootDevice: RootDevice,
-					Service:    srv,
-				}}
-				_, isNat, err := client.GetNATRSIPStatusCtx(ctx)
-				if err == nil && isNat {
-					select {
-					case res <- &upnp_NAT{client, make(map[int]int), "UPNP (IG1-PPP1)", RootDevice}:
-					case <-ctx.Done():
-					}
-				}
-
+			client := &internetgateway2.WANIPConnection2{ServiceClient: goupnp.ServiceClient{
+				SOAPClient: srv.NewSOAPClient(),
+				RootDevice: rootDevice,
+				Service:    srv,
+			}}
+			_, isNat, err := client.GetNATRSIPStatusCtx(ctx)
+			if err != nil {
+				*outErrs = append(*outErrs, err)
+			} else if isNat {
+				*outNats = append(*outNats, &upnp_NAT{client, make(map[int]int), "UPNP (IP2)", rootDevice})
 			}
-		})
-	}()
-	return res
+
+		case internetgateway2.URN_WANPPPConnection_1:
+			client := &internetgateway2.WANPPPConnection1{ServiceClient: goupnp.ServiceClient{
+				SOAPClient: srv.NewSOAPClient(),
+				RootDevice: rootDevice,
+				Service:    srv,
+			}}
+			_, isNat, err := client.GetNATRSIPStatusCtx(ctx)
+			if err != nil {
+				*outErrs = append(*outErrs, err)
+			} else if isNat {
+				*outNats = append(*outNats, &upnp_NAT{client, make(map[int]int), "UPNP (PPP1)", rootDevice})
+			}
+		}
+	}
 }

 type upnp_NAT_Client interface {
--- a/p2p/net/nat/internal/nat/version.json
+++ b/p2p/net/nat/internal/nat/version.json
@@ -1,3 +0,0 @@
-{
-  "version": "v0.2.0"
-}
--- a/p2p/net/nat/mock_nat_test.go
+++ b/p2p/net/nat/mock_nat_test.go
@@ -1,9 +1,9 @@
 // Code generated by MockGen. DO NOT EDIT.
-// Source: github.com/libp2p/go-nat (interfaces: NAT)
+// Source: github.com/libp2p/go-libp2p/p2p/net/nat/internal/nat (interfaces: NAT)
 //
 // Generated by this command:
 //
-//	mockgen -package nat -destination mock_nat_test.go github.com/libp2p/go-nat NAT
+//	mockgen -package nat -destination mock_nat_test.go github.com/libp2p/go-libp2p/p2p/net/nat/internal/nat NAT
 //

 // Package nat is a generated GoMock package.
--- a/p2p/net/nat/nat.go
+++ b/p2p/net/nat/nat.go
@@ -11,7 +11,7 @@ import (

 	logging "github.com/ipfs/go-log/v2"

-	"github.com/libp2p/go-nat"
+	"github.com/libp2p/go-libp2p/p2p/net/nat/internal/nat"
 )

 // ErrNoMapping signals no mapping exists for an address
--- a/p2p/net/nat/nat_test.go
+++ b/p2p/net/nat/nat_test.go
@@ -7,13 +7,12 @@ import (
 	"net/netip"
 	"testing"

-	"github.com/libp2p/go-nat"
-
+	"github.com/libp2p/go-libp2p/p2p/net/nat/internal/nat"
 	"github.com/stretchr/testify/require"
 	"go.uber.org/mock/gomock"
 )

-//go:generate sh -c "go run go.uber.org/mock/mockgen -package nat -destination mock_nat_test.go github.com/libp2p/go-nat NAT"
+//go:generate sh -c "go run go.uber.org/mock/mockgen -package nat -destination mock_nat_test.go github.com/libp2p/go-libp2p/p2p/net/nat/internal/nat NAT"

 func setupMockNAT(t *testing.T) (mockNAT *MockNAT, reset func()) {
 	t.Helper()
--- a/test-plans/go.mod
+++ b/test-plans/go.mod
@@ -41,7 +41,6 @@ require (
 	github.com/libp2p/go-flow-metrics v0.2.0 // indirect
 	github.com/libp2p/go-libp2p-asn-util v0.4.1 // indirect
 	github.com/libp2p/go-msgio v0.3.0 // indirect
-	github.com/libp2p/go-nat v0.2.0 // indirect
 	github.com/libp2p/go-netroute v0.2.2 // indirect
 	github.com/libp2p/go-reuseport v0.4.0 // indirect
 	github.com/libp2p/go-yamux/v4 v4.0.1 // indirect
--- a/test-plans/go.sum
+++ b/test-plans/go.sum
@@ -144,8 +144,6 @@ github.com/libp2p/go-libp2p-testing v0.12.0 h1:EPvBb4kKMWO29qP4mZGyhVzUyR25dvfUI
 github.com/libp2p/go-libp2p-testing v0.12.0/go.mod h1:KcGDRXyN7sQCllucn1cOOS+Dmm7ujhfEyXQL5lvkcPg=
 github.com/libp2p/go-msgio v0.3.0 h1:mf3Z8B1xcFN314sWX+2vOTShIE0Mmn2TXn3YCUQGNj0=
 github.com/libp2p/go-msgio v0.3.0/go.mod h1:nyRM819GmVaF9LX3l03RMh10QdOroF++NBbxAb0mmDM=
-github.com/libp2p/go-nat v0.2.0 h1:Tyz+bUFAYqGyJ/ppPPymMGbIgNRH+WqC5QrT5fKrrGk=
-github.com/libp2p/go-nat v0.2.0/go.mod h1:3MJr+GRpRkyT65EpVPBstXLvOlAPzUVlG6Pwg9ohLJk=
 github.com/libp2p/go-netroute v0.2.2 h1:Dejd8cQ47Qx2kRABg6lPwknU7+nBnFRpko45/fFPuZ8=
 github.com/libp2p/go-netroute v0.2.2/go.mod h1:Rntq6jUAH0l9Gg17w5bFGhcC9a+vk4KNXs6s7IljKYE=
 github.com/libp2p/go-reuseport v0.4.0 h1:nR5KU7hD0WxXCJbmw7r2rhRYruNRl2koHw8fQscQm2s=