From b63049f4040c893c2a3aa1081a7a79e4bc702de6 Mon Sep 17 00:00:00 2001 From: risetechlab <79727391+risetechlab@users.noreply.github.com> Date: Wed, 31 Mar 2021 00:42:02 +0800 Subject: [PATCH] Fix: TFO AsIs bug (#452) --- infra/conf/transport_internet.go | 9 +- infra/conf/transport_test.go | 115 ++++++++++++++++++++++---- transport/internet/sockopt.go | 11 +++ transport/internet/sockopt_darwin.go | 8 +- transport/internet/sockopt_freebsd.go | 9 +- transport/internet/sockopt_linux.go | 9 +- transport/internet/sockopt_windows.go | 8 +- 7 files changed, 133 insertions(+), 36 deletions(-) diff --git a/infra/conf/transport_internet.go b/infra/conf/transport_internet.go index d0f448e0..62c3f1a8 100644 --- a/infra/conf/transport_internet.go +++ b/infra/conf/transport_internet.go @@ -478,22 +478,19 @@ type SocketConfig struct { // Build implements Buildable. func (c *SocketConfig) Build() (*internet.SocketConfig, error) { - tfo := int32(-1) + tfo := int32(0) // don't invoke setsockopt() for TFO if c.TFO != nil { switch v := c.TFO.(type) { case bool: if v { tfo = 256 } else { - tfo = 0 + tfo = -1 // TFO need to be disabled } case float64: - if v < 0 { - return nil, newError("tcpFastOpen: only boolean and non-negative integer value is acceptable") - } tfo = int32(math.Min(v, math.MaxInt32)) default: - return nil, newError("tcpFastOpen: only boolean and non-negative integer value is acceptable") + return nil, newError("tcpFastOpen: only boolean and integer value is acceptable") } } var tproxy internet.SocketConfig_TProxyMode diff --git a/infra/conf/transport_test.go b/infra/conf/transport_test.go index a3e9cc74..3afe9729 100644 --- a/infra/conf/transport_test.go +++ b/infra/conf/transport_test.go @@ -31,6 +31,13 @@ func TestSocketConfig(t *testing.T) { } } + // test "tcpFastOpen": true, queue length 256 is expected. other parameters are tested here too + expectedOutput := &internet.SocketConfig{ + Mark: 1, + Tfo: 256, + DomainStrategy: internet.DomainStrategy_USE_IP, + DialerProxy: "tag", + } runMultiTestCase(t, []TestCase{ { Input: `{ @@ -40,38 +47,118 @@ func TestSocketConfig(t *testing.T) { "dialerProxy": "tag" }`, Parser: createParser(), - Output: &internet.SocketConfig{ - Mark: 1, - Tfo: 256, - DomainStrategy: internet.DomainStrategy_USE_IP, - DialerProxy: "tag", - }, + Output: expectedOutput, }, }) + if expectedOutput.ParseTFOValue() != 256 { + t.Fatalf("unexpected parsed TFO value, which should be 256") + } + + // test "tcpFastOpen": false, disabled TFO is expected + expectedOutput = &internet.SocketConfig{ + Mark: 0, + Tfo: -1, + } runMultiTestCase(t, []TestCase{ { Input: `{ "tcpFastOpen": false }`, Parser: createParser(), - Output: &internet.SocketConfig{ - Mark: 0, - Tfo: 0, - }, + Output: expectedOutput, }, }) + if expectedOutput.ParseTFOValue() != 0 { + t.Fatalf("unexpected parsed TFO value, which should be 0") + } + + // test "tcpFastOpen": 65535, queue length 65535 is expected + expectedOutput = &internet.SocketConfig{ + Mark: 0, + Tfo: 65535, + } runMultiTestCase(t, []TestCase{ { Input: `{ "tcpFastOpen": 65535 }`, Parser: createParser(), - Output: &internet.SocketConfig{ - Mark: 0, - Tfo: 65535, - }, + Output: expectedOutput, }, }) + if expectedOutput.ParseTFOValue() != 65535 { + t.Fatalf("unexpected parsed TFO value, which should be 65535") + } + + // test "tcpFastOpen": -65535, disable TFO is expected + expectedOutput = &internet.SocketConfig{ + Mark: 0, + Tfo: -65535, + } + runMultiTestCase(t, []TestCase{ + { + Input: `{ + "tcpFastOpen": -65535 + }`, + Parser: createParser(), + Output: expectedOutput, + }, + }) + if expectedOutput.ParseTFOValue() != 0 { + t.Fatalf("unexpected parsed TFO value, which should be 0") + } + + // test "tcpFastOpen": 0, no operation is expected + expectedOutput = &internet.SocketConfig{ + Mark: 0, + Tfo: 0, + } + runMultiTestCase(t, []TestCase{ + { + Input: `{ + "tcpFastOpen": 0 + }`, + Parser: createParser(), + Output: expectedOutput, + }, + }) + if expectedOutput.ParseTFOValue() != -1 { + t.Fatalf("unexpected parsed TFO value, which should be -1") + } + + // test omit "tcpFastOpen", no operation is expected + expectedOutput = &internet.SocketConfig{ + Mark: 0, + Tfo: 0, + } + runMultiTestCase(t, []TestCase{ + { + Input: `{}`, + Parser: createParser(), + Output: expectedOutput, + }, + }) + if expectedOutput.ParseTFOValue() != -1 { + t.Fatalf("unexpected parsed TFO value, which should be -1") + } + + // test "tcpFastOpen": null, no operation is expected + expectedOutput = &internet.SocketConfig{ + Mark: 0, + Tfo: 0, + } + runMultiTestCase(t, []TestCase{ + { + Input: `{ + "tcpFastOpen": null + }`, + Parser: createParser(), + Output: expectedOutput, + }, + }) + if expectedOutput.ParseTFOValue() != -1 { + t.Fatalf("unexpected parsed TFO value, which should be -1") + } } func TestTransportConfig(t *testing.T) { diff --git a/transport/internet/sockopt.go b/transport/internet/sockopt.go index 7facf30f..d79191ab 100644 --- a/transport/internet/sockopt.go +++ b/transport/internet/sockopt.go @@ -17,3 +17,14 @@ func isUDPSocket(network string) bool { return false } } + +func (v *SocketConfig) ParseTFOValue() int { + if v.Tfo == 0 { + return -1 + } + tfo := int(v.Tfo) + if tfo < 0 { + tfo = 0 + } + return tfo +} diff --git a/transport/internet/sockopt_darwin.go b/transport/internet/sockopt_darwin.go index bdbd1b33..399cc88f 100644 --- a/transport/internet/sockopt_darwin.go +++ b/transport/internet/sockopt_darwin.go @@ -15,12 +15,12 @@ const ( func applyOutboundSocketOptions(network string, address string, fd uintptr, config *SocketConfig) error { if isTCPSocket(network) { - tfo := config.Tfo + tfo := config.ParseTFOValue() if tfo > 0 { tfo = TCP_FASTOPEN_CLIENT } if tfo >= 0 { - if err := syscall.SetsockoptInt(int(fd), syscall.IPPROTO_TCP, TCP_FASTOPEN, int(tfo)); err != nil { + if err := syscall.SetsockoptInt(int(fd), syscall.IPPROTO_TCP, TCP_FASTOPEN, tfo); err != nil { return err } } @@ -31,12 +31,12 @@ func applyOutboundSocketOptions(network string, address string, fd uintptr, conf func applyInboundSocketOptions(network string, fd uintptr, config *SocketConfig) error { if isTCPSocket(network) { - tfo := config.Tfo + tfo := config.ParseTFOValue() if tfo > 0 { tfo = TCP_FASTOPEN_SERVER } if tfo >= 0 { - if err := syscall.SetsockoptInt(int(fd), syscall.IPPROTO_TCP, TCP_FASTOPEN, int(tfo)); err != nil { + if err := syscall.SetsockoptInt(int(fd), syscall.IPPROTO_TCP, TCP_FASTOPEN, tfo); err != nil { return err } } diff --git a/transport/internet/sockopt_freebsd.go b/transport/internet/sockopt_freebsd.go index 5ffbeea7..29c19de5 100644 --- a/transport/internet/sockopt_freebsd.go +++ b/transport/internet/sockopt_freebsd.go @@ -130,7 +130,7 @@ func applyOutboundSocketOptions(network string, address string, fd uintptr, conf } if isTCPSocket(network) { - tfo := int(config.Tfo) + tfo := config.ParseTFOValue() if tfo > 0 { tfo = 1 } @@ -163,9 +163,10 @@ func applyInboundSocketOptions(network string, fd uintptr, config *SocketConfig) } } if isTCPSocket(network) { - if config.Tfo >= 0 { - if err := syscall.SetsockoptInt(int(fd), syscall.IPPROTO_TCP, unix.TCP_FASTOPEN, int(config.Tfo)); err != nil { - return newError("failed to set TCP_FASTOPEN=", config.Tfo).Base(err) + tfo := config.ParseTFOValue() + if tfo >= 0 { + if err := syscall.SetsockoptInt(int(fd), syscall.IPPROTO_TCP, unix.TCP_FASTOPEN, tfo); err != nil { + return newError("failed to set TCP_FASTOPEN=", tfo).Base(err) } } } diff --git a/transport/internet/sockopt_linux.go b/transport/internet/sockopt_linux.go index 75b7a5e4..13c284bc 100644 --- a/transport/internet/sockopt_linux.go +++ b/transport/internet/sockopt_linux.go @@ -48,7 +48,7 @@ func applyOutboundSocketOptions(network string, address string, fd uintptr, conf } if isTCPSocket(network) { - tfo := int(config.Tfo) + tfo := config.ParseTFOValue() if tfo > 0 { tfo = 1 } @@ -75,9 +75,10 @@ func applyInboundSocketOptions(network string, fd uintptr, config *SocketConfig) } } if isTCPSocket(network) { - if config.Tfo >= 0 { - if err := syscall.SetsockoptInt(int(fd), syscall.SOL_TCP, TCP_FASTOPEN, int(config.Tfo)); err != nil { - return newError("failed to set TCP_FASTOPEN=", config.Tfo).Base(err) + tfo := config.ParseTFOValue() + if tfo >= 0 { + if err := syscall.SetsockoptInt(int(fd), syscall.SOL_TCP, TCP_FASTOPEN, tfo); err != nil { + return newError("failed to set TCP_FASTOPEN=", tfo).Base(err) } } } diff --git a/transport/internet/sockopt_windows.go b/transport/internet/sockopt_windows.go index 50e3a14c..840834b6 100644 --- a/transport/internet/sockopt_windows.go +++ b/transport/internet/sockopt_windows.go @@ -8,12 +8,12 @@ const ( TCP_FASTOPEN = 15 ) -func setTFO(fd syscall.Handle, tfo int32) error { +func setTFO(fd syscall.Handle, tfo int) error { if tfo > 0 { tfo = 1 } if tfo >= 0 { - if err := syscall.SetsockoptInt(fd, syscall.IPPROTO_TCP, TCP_FASTOPEN, int(tfo)); err != nil { + if err := syscall.SetsockoptInt(fd, syscall.IPPROTO_TCP, TCP_FASTOPEN, tfo); err != nil { return err } } @@ -22,7 +22,7 @@ func setTFO(fd syscall.Handle, tfo int32) error { func applyOutboundSocketOptions(network string, address string, fd uintptr, config *SocketConfig) error { if isTCPSocket(network) { - if err := setTFO(syscall.Handle(fd), config.Tfo); err != nil { + if err := setTFO(syscall.Handle(fd), config.ParseTFOValue()); err != nil { return err } @@ -33,7 +33,7 @@ func applyOutboundSocketOptions(network string, address string, fd uintptr, conf func applyInboundSocketOptions(network string, fd uintptr, config *SocketConfig) error { if isTCPSocket(network) { - if err := setTFO(syscall.Handle(fd), config.Tfo); err != nil { + if err := setTFO(syscall.Handle(fd), config.ParseTFOValue()); err != nil { return err } }