Merge pull request #14 from dolonet/sync-upstream-keepalive

Sync upstream: improve TCP keepalive and idle timeout for mobile clients

example.config.toml

@@ -226,7 +226,7 @@ proxies = [
 [network.timeout]
 tcp = "5s"
 http = "10s"
-idle = "1m"
+idle = "5m"
 
 # mtg has to mimic real websites. It does not mean domain fronting, it also
 # means that traffic characteristics should be similar to real world traffic.

internal/cli/run_proxy.go

@@ -263,7 +263,7 @@ func runProxy(conf *config.Config, version string) error { //nolint: funlen
 
 		AllowFallbackOnUnknownDC: conf.AllowFallbackOnUnknownDC.Get(false),
 		TolerateTimeSkewness:     conf.TolerateTimeSkewness.Value,
-		IdleTimeout:              conf.Network.Timeout.Idle.Get(time.Minute),
+		IdleTimeout:              conf.Network.Timeout.Idle.Get(mtglib.DefaultIdleTimeout),
 
 		DoppelGangerURLs:    doppelGangerURLs,
 		DoppelGangerPerRaid: conf.Defense.Doppelganger.Repeats.Get(mtglib.DoppelGangerPerRaid),

mtglib/init.go

@@ -77,8 +77,9 @@ const (
 	// DefaultIdleTimeout is a default timeout for closing a connection in case of
 	// idling.
 	//
-	// Deprecated: no longer in use because of changed TCP relay algorithm.
-	DefaultIdleTimeout = time.Minute
+	// Set to 5 minutes to survive typical mobile sleep periods (2-5 min) and
+	// avoid racing with MTProto ping_delay_disconnect (~60s interval).
+	DefaultIdleTimeout = 5 * time.Minute
 
 	// DefaultTolerateTimeSkewness is a default timeout for time skewness on a
 	// faketls timeout verification.

mtglib/proxy_opts.go

@@ -277,7 +277,7 @@ func (p ProxyOpts) getPreferIP() string {
 
 func (p ProxyOpts) getIdleTimeout() time.Duration {
 	if p.IdleTimeout == 0 {
-		return time.Minute
+		return DefaultIdleTimeout
 	}
 
 	return p.IdleTimeout

network/init.go

@@ -36,8 +36,22 @@ const (
 
 	// DefaultTCPKeepAlivePeriod defines a time period between 2 consequitive
 	// probes.
+	//
+	// Deprecated: use DefaultKeepAliveIdle and DefaultKeepAliveInterval instead.
 	DefaultTCPKeepAlivePeriod = 10 * time.Second
 
+	// DefaultKeepAliveIdle is the time a connection must be idle before
+	// the first keepalive probe is sent.
+	DefaultKeepAliveIdle = 30 * time.Second
+
+	// DefaultKeepAliveInterval is the time between consecutive keepalive
+	// probes.
+	DefaultKeepAliveInterval = 10 * time.Second
+
+	// DefaultKeepAliveCount is the number of unacknowledged probes before
+	// the connection is considered dead.
+	DefaultKeepAliveCount = 3
+
 	// ProxyDialerOpenThreshold is used for load balancing SOCKS5 dialer only.
 	//
 	// This dialer uses circuit breaker with of 3 stages: OPEN, HALF_OPEN and

network/sockopts.go

@@ -20,8 +20,13 @@ func SetServerSocketOptions(conn net.Conn, bufferSize int) error {
 }
 
 func setCommonSocketOptions(conn *net.TCPConn) error {
-	if err := conn.SetKeepAlivePeriod(DefaultTCPKeepAlivePeriod); err != nil {
-		return fmt.Errorf("cannot set time period of TCP keepalive probes: %w", err)
+	if err := conn.SetKeepAliveConfig(net.KeepAliveConfig{
+		Enable:   true,
+		Idle:     DefaultKeepAliveIdle,
+		Interval: DefaultKeepAliveInterval,
+		Count:    DefaultKeepAliveCount,
+	}); err != nil {
+		return fmt.Errorf("cannot configure TCP keepalive: %w", err)
 	}
 
 	if err := conn.SetLinger(tcpLingerTimeout); err != nil {

network/sockopts_test.go

@@ -0,0 +1,93 @@
+//go:build linux || darwin
+// +build linux darwin
+
+package network_test
+
+import (
+	"net"
+	"runtime"
+	"syscall"
+	"testing"
+	"time"
+
+	"github.com/dolonet/mtg-multi/network"
+	"github.com/stretchr/testify/require"
+	"golang.org/x/sys/unix"
+)
+
+func tcpKeepIdleOption() int {
+	if runtime.GOOS == "darwin" {
+		return 0x10 // TCP_KEEPALIVE on macOS
+	}
+
+	return 0x4 // TCP_KEEPIDLE on Linux
+}
+
+func TestSetClientSocketOptionsKeepAlive(t *testing.T) {
+	t.Parallel()
+
+	listener, err := net.Listen("tcp", "127.0.0.1:0")
+	require.NoError(t, err)
+	defer func() {
+		err := listener.Close()
+		require.NoError(t, err)
+	}()
+
+	type dialResult struct {
+		conn net.Conn
+		err  error
+	}
+
+	dialDone := make(chan dialResult, 1)
+
+	go func() {
+		c, err := net.Dial("tcp", listener.Addr().String())
+		dialDone <- dialResult{conn: c, err: err}
+	}()
+
+	tcpListener, ok := listener.(*net.TCPListener)
+	require.True(t, ok, "listener must be a *net.TCPListener")
+
+	require.NoError(t, tcpListener.SetDeadline(time.Now().Add(5*time.Second)))
+
+	accepted, err := listener.Accept()
+	require.NoError(t, err)
+	defer func() {
+		err := accepted.Close()
+		require.NoError(t, err)
+	}()
+
+	dr := <-dialDone
+	require.NoError(t, dr.err)
+	defer func() {
+		err := dr.conn.Close()
+		require.NoError(t, err)
+	}()
+
+	err = network.SetClientSocketOptions(accepted, 0)
+	require.NoError(t, err)
+
+	tcpConn := accepted.(*net.TCPConn)
+
+	rawConn, err := tcpConn.SyscallConn()
+	require.NoError(t, err)
+
+	err = rawConn.Control(func(fd uintptr) {
+		val, err := unix.GetsockoptInt(int(fd), syscall.SOL_SOCKET, syscall.SO_KEEPALIVE)
+		require.NoError(t, err)
+		require.NotEqual(t, 0, val, "SO_KEEPALIVE should be enabled")
+
+		idle, err := unix.GetsockoptInt(int(fd), syscall.IPPROTO_TCP, tcpKeepIdleOption())
+		require.NoError(t, err)
+		require.Equal(t, int(network.DefaultKeepAliveIdle.Seconds()), idle)
+
+		interval, err := unix.GetsockoptInt(int(fd), syscall.IPPROTO_TCP, unix.TCP_KEEPINTVL)
+		require.NoError(t, err)
+		require.Equal(t, int(network.DefaultKeepAliveInterval.Seconds()), interval)
+
+		count, err := unix.GetsockoptInt(int(fd), syscall.IPPROTO_TCP, unix.TCP_KEEPCNT)
+		require.NoError(t, err)
+		require.Equal(t, network.DefaultKeepAliveCount, count)
+	})
+	require.NoError(t, err)
+}

network/v2/init.go

@@ -26,8 +26,22 @@ const (
 
 	// DefaultTCPKeepAlivePeriod defines a time period between 2 consecuitive
 	// probes.
+	//
+	// Deprecated: use DefaultKeepAliveIdle and DefaultKeepAliveInterval instead.
 	DefaultTCPKeepAlivePeriod = 10 * time.Second
 
+	// DefaultKeepAliveIdle is the time a connection must be idle before
+	// the first keepalive probe is sent.
+	DefaultKeepAliveIdle = 30 * time.Second
+
+	// DefaultKeepAliveInterval is the time between consecutive keepalive
+	// probes.
+	DefaultKeepAliveInterval = 10 * time.Second
+
+	// DefaultKeepAliveCount is the number of unacknowledged probes before
+	// the connection is considered dead.
+	DefaultKeepAliveCount = 3
+
 	// User Agent to use in HTTP client.
 	UserAgent = "curl/8.5.0"
 

network/v2/sockopts.go

@@ -6,8 +6,13 @@ import (
 )
 
 func setCommonSocketOptions(conn *net.TCPConn) error {
-	if err := conn.SetKeepAlivePeriod(DefaultTCPKeepAlivePeriod); err != nil {
-		return fmt.Errorf("cannot set time period of TCP keepalive probes: %w", err)
+	if err := conn.SetKeepAliveConfig(net.KeepAliveConfig{
+		Enable:   true,
+		Idle:     DefaultKeepAliveIdle,
+		Interval: DefaultKeepAliveInterval,
+		Count:    DefaultKeepAliveCount,
+	}); err != nil {
+		return fmt.Errorf("cannot configure TCP keepalive: %w", err)
 	}
 
 	if err := conn.SetLinger(tcpLingerTimeout); err != nil {

network/v2/sockopts_test.go

@@ -0,0 +1,92 @@
+//go:build linux || darwin
+// +build linux darwin
+
+package network
+
+import (
+	"net"
+	"runtime"
+	"syscall"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+	"golang.org/x/sys/unix"
+)
+
+func tcpKeepIdleOption() int {
+	if runtime.GOOS == "darwin" {
+		return 0x10 // TCP_KEEPALIVE on macOS
+	}
+
+	return 0x4 // TCP_KEEPIDLE on Linux
+}
+
+func TestSetCommonSocketOptionsKeepAlive(t *testing.T) {
+	t.Parallel()
+
+	listener, err := net.Listen("tcp", "127.0.0.1:0")
+	require.NoError(t, err)
+	defer func() {
+		err := listener.Close()
+		require.NoError(t, err)
+	}()
+
+	type dialResult struct {
+		conn net.Conn
+		err  error
+	}
+
+	dialDone := make(chan dialResult, 1)
+
+	go func() {
+		c, err := net.Dial("tcp", listener.Addr().String())
+		dialDone <- dialResult{conn: c, err: err}
+	}()
+
+	tcpListener, ok := listener.(*net.TCPListener)
+	require.True(t, ok, "listener must be a *net.TCPListener")
+
+	require.NoError(t, tcpListener.SetDeadline(time.Now().Add(5*time.Second)))
+
+	accepted, err := listener.Accept()
+	require.NoError(t, err)
+	defer func() {
+		err := accepted.Close()
+		require.NoError(t, err)
+	}()
+
+	dr := <-dialDone
+	require.NoError(t, dr.err)
+	defer func() {
+		err := dr.conn.Close()
+		require.NoError(t, err)
+	}()
+
+	tcpConn := accepted.(*net.TCPConn)
+
+	err = setCommonSocketOptions(tcpConn)
+	require.NoError(t, err)
+
+	rawConn, err := tcpConn.SyscallConn()
+	require.NoError(t, err)
+
+	err = rawConn.Control(func(fd uintptr) {
+		val, err := unix.GetsockoptInt(int(fd), syscall.SOL_SOCKET, syscall.SO_KEEPALIVE)
+		require.NoError(t, err)
+		require.NotEqual(t, 0, val, "SO_KEEPALIVE should be enabled")
+
+		idle, err := unix.GetsockoptInt(int(fd), syscall.IPPROTO_TCP, tcpKeepIdleOption())
+		require.NoError(t, err)
+		require.Equal(t, int(DefaultKeepAliveIdle.Seconds()), idle, "keepalive idle should match DefaultKeepAliveIdle")
+
+		interval, err := unix.GetsockoptInt(int(fd), syscall.IPPROTO_TCP, unix.TCP_KEEPINTVL)
+		require.NoError(t, err)
+		require.Equal(t, int(DefaultKeepAliveInterval.Seconds()), interval, "keepalive interval should match DefaultKeepAliveInterval")
+
+		count, err := unix.GetsockoptInt(int(fd), syscall.IPPROTO_TCP, unix.TCP_KEEPCNT)
+		require.NoError(t, err)
+		require.Equal(t, DefaultKeepAliveCount, count, "keepalive count should match DefaultKeepAliveCount")
+	})
+	require.NoError(t, err)
+}