diff --git a/internal/natpmp/externaladdress_test.go b/internal/natpmp/externaladdress_test.go index cdde3e62..eb64965e 100644 --- a/internal/natpmp/externaladdress_test.go +++ b/internal/natpmp/externaladdress_test.go @@ -18,7 +18,7 @@ func Test_Client_ExternalAddress(t *testing.T) { testCases := map[string]struct { ctx context.Context gateway netip.Addr - initialRetry time.Duration + initialConnDuration time.Duration exchanges []udpExchange durationSinceStartOfEpoch time.Duration externalIPv4Address netip.Addr @@ -26,16 +26,16 @@ func Test_Client_ExternalAddress(t *testing.T) { errMessage string }{ "failure": { - ctx: canceledCtx, - gateway: netip.AddrFrom4([4]byte{127, 0, 0, 1}), - initialRetry: time.Millisecond, - err: context.Canceled, - errMessage: "executing remote procedure call: reading from udp connection: context canceled", + ctx: canceledCtx, + gateway: netip.AddrFrom4([4]byte{127, 0, 0, 1}), + initialConnDuration: time.Millisecond, + err: context.Canceled, + errMessage: "executing remote procedure call: reading from udp connection: context canceled", }, "success": { - ctx: context.Background(), - gateway: netip.AddrFrom4([4]byte{127, 0, 0, 1}), - initialRetry: time.Millisecond, + ctx: context.Background(), + gateway: netip.AddrFrom4([4]byte{127, 0, 0, 1}), + initialConnDuration: time.Millisecond, exchanges: []udpExchange{{ request: []byte{0, 0}, response: []byte{0x0, 0x80, 0x0, 0x0, 0x0, 0x13, 0xf2, 0x4f, 0x49, 0x8c, 0x36, 0x9a}, @@ -53,9 +53,9 @@ func Test_Client_ExternalAddress(t *testing.T) { remoteAddress := launchUDPServer(t, testCase.exchanges) client := Client{ - serverPort: uint16(remoteAddress.Port), - initialRetry: testCase.initialRetry, - maxRetries: 1, + serverPort: uint16(remoteAddress.Port), + initialConnectionDuration: testCase.initialConnDuration, + maxRetries: 1, } durationSinceStartOfEpoch, externalIPv4Address, err := diff --git a/internal/natpmp/natpmp.go b/internal/natpmp/natpmp.go index 6de1c54a..cd83bed7 100644 --- a/internal/natpmp/natpmp.go +++ b/internal/natpmp/natpmp.go @@ -6,9 +6,9 @@ import ( // Client is a NAT-PMP protocol client. type Client struct { - serverPort uint16 - initialRetry time.Duration - maxRetries uint + serverPort uint16 + initialConnectionDuration time.Duration + maxRetries uint } // New creates a new NAT-PMP client. @@ -16,11 +16,11 @@ func New() (client *Client) { const natpmpPort = 5351 // Parameters described in https://www.ietf.org/rfc/rfc6886.html#section-3.1 - const initialRetry = 250 * time.Millisecond + const initialConnectionDuration = 250 * time.Millisecond const maxTries = 9 // 64 seconds return &Client{ - serverPort: natpmpPort, - initialRetry: initialRetry, - maxRetries: maxTries, + serverPort: natpmpPort, + initialConnectionDuration: initialConnectionDuration, + maxRetries: maxTries, } } diff --git a/internal/natpmp/natpmp_test.go b/internal/natpmp/natpmp_test.go index 3ce24d86..414e9fc7 100644 --- a/internal/natpmp/natpmp_test.go +++ b/internal/natpmp/natpmp_test.go @@ -11,9 +11,9 @@ func Test_New(t *testing.T) { t.Parallel() expectedClient := &Client{ - serverPort: 5351, - initialRetry: 250 * time.Millisecond, - maxRetries: 9, + serverPort: 5351, + initialConnectionDuration: 250 * time.Millisecond, + maxRetries: 9, } client := New() assert.Equal(t, expectedClient, client) diff --git a/internal/natpmp/portmapping_test.go b/internal/natpmp/portmapping_test.go index 99721fb2..2905c9cc 100644 --- a/internal/natpmp/portmapping_test.go +++ b/internal/natpmp/portmapping_test.go @@ -19,7 +19,7 @@ func Test_Client_AddPortMapping(t *testing.T) { internalPort uint16 requestedExternalPort uint16 lifetime time.Duration - initialRetry time.Duration + initialConnDuration time.Duration exchanges []udpExchange durationSinceStartOfEpoch time.Duration assignedInternalPort uint16 @@ -46,7 +46,7 @@ func Test_Client_AddPortMapping(t *testing.T) { internalPort: 123, requestedExternalPort: 456, lifetime: 1200 * time.Second, - initialRetry: time.Millisecond, + initialConnDuration: time.Millisecond, exchanges: []udpExchange{{close: true}}, err: ErrConnectionTimeout, errMessage: "executing remote procedure call: connection timeout: after 1ms", @@ -58,7 +58,7 @@ func Test_Client_AddPortMapping(t *testing.T) { internalPort: 123, requestedExternalPort: 456, lifetime: 1200 * time.Second, - initialRetry: time.Second, + initialConnDuration: time.Second, exchanges: []udpExchange{{ request: []byte{0x0, 0x1, 0x0, 0x0, 0x0, 0x7b, 0x1, 0xc8, 0x0, 0x0, 0x4, 0xb0}, response: []byte{0x0, 0x81, 0x0, 0x0, 0x0, 0x13, 0xfe, 0xff, 0x0, 0x7b, 0x1, 0xc8, 0x0, 0x0, 0x4, 0xb0}, @@ -75,7 +75,7 @@ func Test_Client_AddPortMapping(t *testing.T) { internalPort: 123, requestedExternalPort: 456, lifetime: 1200 * time.Second, - initialRetry: time.Second, + initialConnDuration: time.Second, exchanges: []udpExchange{{ request: []byte{0x0, 0x2, 0x0, 0x0, 0x0, 0x7b, 0x1, 0xc8, 0x0, 0x0, 0x4, 0xb0}, response: []byte{0x0, 0x82, 0x0, 0x0, 0x0, 0x14, 0x3, 0x21, 0x0, 0x7b, 0x1, 0xc8, 0x0, 0x0, 0x4, 0xb0}, @@ -86,11 +86,11 @@ func Test_Client_AddPortMapping(t *testing.T) { assignedLifetime: 0x4b0 * time.Second, }, "remove_udp": { - ctx: context.Background(), - gateway: netip.AddrFrom4([4]byte{127, 0, 0, 1}), - protocol: "udp", - internalPort: 123, - initialRetry: time.Second, + ctx: context.Background(), + gateway: netip.AddrFrom4([4]byte{127, 0, 0, 1}), + protocol: "udp", + internalPort: 123, + initialConnDuration: time.Second, exchanges: []udpExchange{{ request: []byte{0x0, 0x1, 0x0, 0x0, 0x0, 0x7b, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, response: []byte{0x0, 0x81, 0x0, 0x0, 0x0, 0x14, 0x3, 0xd5, 0x0, 0x7b, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, @@ -99,11 +99,11 @@ func Test_Client_AddPortMapping(t *testing.T) { assignedInternalPort: 0x7b, }, "remove_tcp": { - ctx: context.Background(), - gateway: netip.AddrFrom4([4]byte{127, 0, 0, 1}), - protocol: "tcp", - internalPort: 123, - initialRetry: time.Second, + ctx: context.Background(), + gateway: netip.AddrFrom4([4]byte{127, 0, 0, 1}), + protocol: "tcp", + internalPort: 123, + initialConnDuration: time.Second, exchanges: []udpExchange{{ request: []byte{0x0, 0x2, 0x0, 0x0, 0x0, 0x7b, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, response: []byte{0x0, 0x82, 0x0, 0x0, 0x0, 0x14, 0x4, 0x96, 0x0, 0x7b, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, @@ -121,9 +121,9 @@ func Test_Client_AddPortMapping(t *testing.T) { remoteAddress := launchUDPServer(t, testCase.exchanges) client := Client{ - serverPort: uint16(remoteAddress.Port), - initialRetry: testCase.initialRetry, - maxRetries: 1, + serverPort: uint16(remoteAddress.Port), + initialConnectionDuration: testCase.initialConnDuration, + maxRetries: 1, } durationSinceStartOfEpoch, assignedInternalPort, diff --git a/internal/natpmp/rpc.go b/internal/natpmp/rpc.go index f1b0be31..88f6e3d8 100644 --- a/internal/natpmp/rpc.go +++ b/internal/natpmp/rpc.go @@ -61,15 +61,15 @@ func (c *Client) rpc(ctx context.Context, gateway netip.Addr, const maxResponseSize = 16 response = make([]byte, maxResponseSize) - // Retry duration doubles on every network error + // Connection duration doubles on every network error // Note it does not double if the source IP mismatches the gateway IP. - retryDuration := c.initialRetry + connectionDuration := c.initialConnectionDuration var totalRetryDuration time.Duration var retryCount uint for retryCount = 0; retryCount < c.maxRetries; retryCount++ { - deadline := time.Now().Add(retryDuration) + deadline := time.Now().Add(connectionDuration) err = connection.SetDeadline(deadline) if err != nil { return nil, fmt.Errorf("setting connection deadline: %w", err) @@ -87,8 +87,8 @@ func (c *Client) rpc(ctx context.Context, gateway netip.Addr, } var netErr net.Error if errors.As(err, &netErr) && netErr.Timeout() { - totalRetryDuration += retryDuration - retryDuration *= 2 + totalRetryDuration += connectionDuration + connectionDuration *= 2 continue } return nil, fmt.Errorf("reading from udp connection: %w", err) diff --git a/internal/natpmp/rpc_test.go b/internal/natpmp/rpc_test.go index 5f56e8f7..4ffbad2e 100644 --- a/internal/natpmp/rpc_test.go +++ b/internal/natpmp/rpc_test.go @@ -13,15 +13,15 @@ func Test_Client_rpc(t *testing.T) { t.Parallel() testCases := map[string]struct { - ctx context.Context - gateway netip.Addr - request []byte - responseSize uint - initialRetry time.Duration - exchanges []udpExchange - expectedResponse []byte - err error - errMessage string + ctx context.Context + gateway netip.Addr + request []byte + responseSize uint + initialConnectionDuration time.Duration + exchanges []udpExchange + expectedResponse []byte + err error + errMessage string }{ "gateway_ip_unspecified": { gateway: netip.IPv6Unspecified(), @@ -30,10 +30,10 @@ func Test_Client_rpc(t *testing.T) { errMessage: "gateway IP is unspecified", }, "request_too_small": { - gateway: netip.AddrFrom4([4]byte{127, 0, 0, 1}), - request: []byte{0}, - initialRetry: time.Second, - err: ErrRequestSizeTooSmall, + gateway: netip.AddrFrom4([4]byte{127, 0, 0, 1}), + request: []byte{0}, + initialConnectionDuration: time.Second, + err: ErrRequestSizeTooSmall, errMessage: `checking request: message size is too small: ` + `need at least 2 bytes and got 1 byte\(s\)`, }, @@ -46,10 +46,10 @@ func Test_Client_rpc(t *testing.T) { `i/o timeout`, }, "call_error": { - ctx: context.Background(), - gateway: netip.AddrFrom4([4]byte{127, 0, 0, 1}), - request: []byte{0, 1}, - initialRetry: time.Millisecond, + ctx: context.Background(), + gateway: netip.AddrFrom4([4]byte{127, 0, 0, 1}), + request: []byte{0, 1}, + initialConnectionDuration: time.Millisecond, exchanges: []udpExchange{ {request: []byte{0, 1}, close: true}, }, @@ -57,10 +57,10 @@ func Test_Client_rpc(t *testing.T) { errMessage: "connection timeout: after 1ms", }, "response_too_small": { - ctx: context.Background(), - gateway: netip.AddrFrom4([4]byte{127, 0, 0, 1}), - request: []byte{0, 0}, - initialRetry: time.Second, + ctx: context.Background(), + gateway: netip.AddrFrom4([4]byte{127, 0, 0, 1}), + request: []byte{0, 0}, + initialConnectionDuration: time.Second, exchanges: []udpExchange{{ request: []byte{0, 0}, response: []byte{1}, @@ -70,11 +70,11 @@ func Test_Client_rpc(t *testing.T) { `need at least 4 bytes and got 1 byte\(s\)`, }, "unexpected_response_size": { - ctx: context.Background(), - gateway: netip.AddrFrom4([4]byte{127, 0, 0, 1}), - request: []byte{0x0, 0x2, 0x0, 0x0, 0x0, 0x7b, 0x1, 0xc8, 0x0, 0x0, 0x4, 0xb0}, - responseSize: 5, - initialRetry: time.Second, + ctx: context.Background(), + gateway: netip.AddrFrom4([4]byte{127, 0, 0, 1}), + request: []byte{0x0, 0x2, 0x0, 0x0, 0x0, 0x7b, 0x1, 0xc8, 0x0, 0x0, 0x4, 0xb0}, + responseSize: 5, + initialConnectionDuration: time.Second, exchanges: []udpExchange{{ request: []byte{0x0, 0x2, 0x0, 0x0, 0x0, 0x7b, 0x1, 0xc8, 0x0, 0x0, 0x4, 0xb0}, response: []byte{0, 1, 2, 3}, // size 4 @@ -84,11 +84,11 @@ func Test_Client_rpc(t *testing.T) { `expected 5 bytes and got 4 byte\(s\)`, }, "unknown_protocol_version": { - ctx: context.Background(), - gateway: netip.AddrFrom4([4]byte{127, 0, 0, 1}), - request: []byte{0x0, 0x2, 0x0, 0x0, 0x0, 0x7b, 0x1, 0xc8, 0x0, 0x0, 0x4, 0xb0}, - responseSize: 16, - initialRetry: time.Second, + ctx: context.Background(), + gateway: netip.AddrFrom4([4]byte{127, 0, 0, 1}), + request: []byte{0x0, 0x2, 0x0, 0x0, 0x0, 0x7b, 0x1, 0xc8, 0x0, 0x0, 0x4, 0xb0}, + responseSize: 16, + initialConnectionDuration: time.Second, exchanges: []udpExchange{{ request: []byte{0x0, 0x2, 0x0, 0x0, 0x0, 0x7b, 0x1, 0xc8, 0x0, 0x0, 0x4, 0xb0}, response: []byte{0x1, 0x82, 0x0, 0x0, 0x0, 0x14, 0x4, 0x96, 0x0, 0x7b, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, @@ -97,11 +97,11 @@ func Test_Client_rpc(t *testing.T) { errMessage: "checking response: protocol version is unknown: 1", }, "unexpected_operation_code": { - ctx: context.Background(), - gateway: netip.AddrFrom4([4]byte{127, 0, 0, 1}), - request: []byte{0x0, 0x2, 0x0, 0x0, 0x0, 0x7b, 0x1, 0xc8, 0x0, 0x0, 0x4, 0xb0}, - responseSize: 16, - initialRetry: time.Second, + ctx: context.Background(), + gateway: netip.AddrFrom4([4]byte{127, 0, 0, 1}), + request: []byte{0x0, 0x2, 0x0, 0x0, 0x0, 0x7b, 0x1, 0xc8, 0x0, 0x0, 0x4, 0xb0}, + responseSize: 16, + initialConnectionDuration: time.Second, exchanges: []udpExchange{{ request: []byte{0x0, 0x2, 0x0, 0x0, 0x0, 0x7b, 0x1, 0xc8, 0x0, 0x0, 0x4, 0xb0}, response: []byte{0x0, 0x88, 0x0, 0x0, 0x0, 0x14, 0x4, 0x96, 0x0, 0x7b, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, @@ -110,11 +110,11 @@ func Test_Client_rpc(t *testing.T) { errMessage: "checking response: operation code is unexpected: expected 0x82 and got 0x88", }, "failure_result_code": { - ctx: context.Background(), - gateway: netip.AddrFrom4([4]byte{127, 0, 0, 1}), - request: []byte{0x0, 0x2, 0x0, 0x0, 0x0, 0x7b, 0x1, 0xc8, 0x0, 0x0, 0x4, 0xb0}, - responseSize: 16, - initialRetry: time.Second, + ctx: context.Background(), + gateway: netip.AddrFrom4([4]byte{127, 0, 0, 1}), + request: []byte{0x0, 0x2, 0x0, 0x0, 0x0, 0x7b, 0x1, 0xc8, 0x0, 0x0, 0x4, 0xb0}, + responseSize: 16, + initialConnectionDuration: time.Second, exchanges: []udpExchange{{ request: []byte{0x0, 0x2, 0x0, 0x0, 0x0, 0x7b, 0x1, 0xc8, 0x0, 0x0, 0x4, 0xb0}, response: []byte{0x0, 0x82, 0x0, 0x11, 0x0, 0x14, 0x4, 0x96, 0x0, 0x7b, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, @@ -123,11 +123,11 @@ func Test_Client_rpc(t *testing.T) { errMessage: "checking response: result code: result code is unknown: 17", }, "success": { - ctx: context.Background(), - gateway: netip.AddrFrom4([4]byte{127, 0, 0, 1}), - request: []byte{0x0, 0x2, 0x0, 0x0, 0x0, 0x7b, 0x1, 0xc8, 0x0, 0x0, 0x4, 0xb0}, - responseSize: 16, - initialRetry: time.Second, + ctx: context.Background(), + gateway: netip.AddrFrom4([4]byte{127, 0, 0, 1}), + request: []byte{0x0, 0x2, 0x0, 0x0, 0x0, 0x7b, 0x1, 0xc8, 0x0, 0x0, 0x4, 0xb0}, + responseSize: 16, + initialConnectionDuration: time.Second, exchanges: []udpExchange{{ request: []byte{0x0, 0x2, 0x0, 0x0, 0x0, 0x7b, 0x1, 0xc8, 0x0, 0x0, 0x4, 0xb0}, response: []byte{0x0, 0x82, 0x0, 0x0, 0x0, 0x0, 0x4, 0x96, 0x0, 0x7b, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, @@ -144,9 +144,9 @@ func Test_Client_rpc(t *testing.T) { remoteAddress := launchUDPServer(t, testCase.exchanges) client := Client{ - serverPort: uint16(remoteAddress.Port), - initialRetry: testCase.initialRetry, - maxRetries: 1, + serverPort: uint16(remoteAddress.Port), + initialConnectionDuration: testCase.initialConnectionDuration, + maxRetries: 1, } response, err := client.rpc(testCase.ctx, testCase.gateway,