diff --git a/internal/pmtud/pmtud.go b/internal/pmtud/pmtud.go index 9ad90e35..56138f55 100644 --- a/internal/pmtud/pmtud.go +++ b/internal/pmtud/pmtud.go @@ -12,12 +12,13 @@ import ( "golang.org/x/net/icmp" ) -var ErrICMPAllMTUsFailed = errors.New("all MTU sizes tested failed") +var ErrMTUNotFound = errors.New("path MTU discovery failed to find MTU") // PathMTUDiscover discovers the maximum MTU for the path to the given ip address. // If the physicalLinkMTU is zero, it defaults to 1500 which is the ethernet standard MTU. // If the pingTimeout is zero, it defaults to 1 second. // If the logger is nil, a no-op logger is used. +// It returns [ErrMTUNotFound] if the MTU could not be determined. func PathMTUDiscover(ctx context.Context, ip netip.Addr, physicalLinkMTU int, pingTimeout time.Duration, logger Logger) ( mtu int, err error, @@ -90,7 +91,7 @@ func pmtudMultiSizes(ctx context.Context, ip netip.Addr, } mtusToTest := makeMTUsToTest(minMTU, maxPossibleMTU) - if len(mtusToTest) == 0 { + if len(mtusToTest) == 1 { // only minMTU because minMTU == maxPossibleMTU return minMTU, nil } logger.Debugf("testing the following MTUs: %v", mtusToTest) @@ -139,13 +140,8 @@ func pmtudMultiSizes(ctx context.Context, ip netip.Addr, } } - if tests[0].mtu == minMTU+1 { // All MTUs failed. - return 0, fmt.Errorf("%w", ErrICMPAllMTUsFailed) - } - // Re-test with MTUs between the minimum MTU - // and the smallest next MTU we tested. - return pmtudMultiSizes(ctx, ip, minMTU, tests[0].mtu-1, - pingTimeout, logger) + // All MTUs failed. + return 0, fmt.Errorf("%w: ICMP might be blocked", ErrMTUNotFound) case err != nil: return 0, fmt.Errorf("collecting ICMP echo replies: %w", err) default: @@ -154,12 +150,11 @@ func pmtudMultiSizes(ctx context.Context, ip netip.Addr, } // Create the MTU slice of length 11 such that: -// - the first element is the minMTU plus the step +// - the first element is the minMTU // - the last element is the maxMTU // - elements in-between are separated as close to each other -// - Don't make the minMTU part of the MTUs to test since -// it's assumed it's already working. -// The number 11 is chosen to find the final MTU in 3 searches; +// The number 11 is chosen to find the final MTU in 3 searches, +// with a total search space of 1728 MTUs which is enough; // to find it in 2 searches requires 37 parallel queries which // could be blocked by firewalls. func makeMTUsToTest(minMTU, maxMTU int) (mtus []int) { @@ -170,21 +165,14 @@ func makeMTUsToTest(minMTU, maxMTU int) (mtus []int) { panic("minMTU > maxMTU") case diff <= mtusLength: mtus = make([]int, 0, diff) - for mtu := minMTU + 1; mtu <= maxMTU; mtu++ { + for mtu := minMTU; mtu <= maxMTU; mtu++ { mtus = append(mtus, mtu) } - case diff < mtusLength*2: - step := float64(diff) / float64(mtusLength) - mtus = make([]int, 0, mtusLength) - for mtu := float64(minMTU) + step; len(mtus) < mtusLength-1; mtu += step { - mtus = append(mtus, int(math.Round(mtu))) - } - mtus = append(mtus, maxMTU) // last element is the maxMTU default: - step := diff / mtusLength + step := float64(diff) / float64(mtusLength-1) mtus = make([]int, 0, mtusLength) - for mtu := minMTU + step; len(mtus) < mtusLength-1; mtu += step { - mtus = append(mtus, mtu) + for mtu := float64(minMTU); len(mtus) < mtusLength-1; mtu += step { + mtus = append(mtus, int(math.Round(mtu))) } mtus = append(mtus, maxMTU) // last element is the maxMTU } diff --git a/internal/pmtud/pmtud_test.go b/internal/pmtud/pmtud_test.go index e6513395..db10d924 100644 --- a/internal/pmtud/pmtud_test.go +++ b/internal/pmtud/pmtud_test.go @@ -15,32 +15,32 @@ func Test_makeMTUsToTest(t *testing.T) { mtus []int }{ "0_0": { - mtus: []int{}, + mtus: []int{0}, }, "0_1": { maxMTU: 1, - mtus: []int{1}, + mtus: []int{0, 1}, }, "0_8": { maxMTU: 8, - mtus: []int{1, 2, 3, 4, 5, 6, 7, 8}, + mtus: []int{0, 1, 2, 3, 4, 5, 6, 7, 8}, }, "0_12": { maxMTU: 12, - mtus: []int{1, 2, 3, 4, 5, 7, 8, 9, 10, 11, 12}, + mtus: []int{0, 1, 2, 4, 5, 6, 7, 8, 10, 11, 12}, }, "0_80": { maxMTU: 80, - mtus: []int{7, 14, 21, 28, 35, 42, 49, 56, 63, 70, 80}, + mtus: []int{0, 8, 16, 24, 32, 40, 48, 56, 64, 72, 80}, }, "0_100": { maxMTU: 100, - mtus: []int{9, 18, 27, 36, 45, 54, 63, 72, 81, 90, 100}, + mtus: []int{0, 10, 20, 30, 40, 50, 60, 70, 80, 90, 100}, }, "1280_1500": { minMTU: 1280, maxMTU: 1500, - mtus: []int{1300, 1320, 1340, 1360, 1380, 1400, 1420, 1440, 1460, 1480, 1500}, + mtus: []int{1280, 1302, 1324, 1346, 1368, 1390, 1412, 1434, 1456, 1478, 1500}, }, } diff --git a/internal/vpn/tunnelup.go b/internal/vpn/tunnelup.go index 8ef1fb5e..5813bfa3 100644 --- a/internal/vpn/tunnelup.go +++ b/internal/vpn/tunnelup.go @@ -120,7 +120,13 @@ func updateToMaxMTU(ctx context.Context, vpnInterface string, const pingTimeout = time.Second vpnLinkMTU, err = pmtud.PathMTUDiscover(ctx, serverIP, vpnLinkMTU, pingTimeout, logger) if err != nil { - return fmt.Errorf("path MTU discovering: %w", err) + if errors.Is(err, pmtud.ErrMTUNotFound) { + const conservativeMTU = 1300 + vpnLinkMTU = conservativeMTU + logger.Debugf("using a conservative MTU of %d (%s)", conservativeMTU, err) + } else { + return fmt.Errorf("path MTU discovering: %w", err) + } } err = netlinker.LinkSetMTU(link, vpnLinkMTU)