Possible TCP socket leak and useless keepalive

[lixin9311]

In https://github.com/shadowsocks/go-shadowsocks2/blob/master/tcp.go#L142 ,
the io.Copy will always block until an error occurs, there is no default timeout configuration for TCP sockets, so the socket will be kept open unless the remote server (Google.com) or local application (Chrome) closes the socket. AFAIK, they usually keep the connection open for several minutes, and if the proxy fails to receive the FIN packet, the socket will be kept, until a keepalive packet is lost (there gonna be another issue).

When dealing with a large number of clients and connections, the socket number may be exhausted.
I think we should set the deadline and close the connection if timeout.

I can issue a PR if you agree.

And the default keepalive interval on most systems is 7200s, that means if the proxy fails to receive a FIN packet, hopefully, after 2hr the socket will be closed. It's useless to set SO_KEEPALIVE alone.
Again, set a timeout on the proxy side can solve all these problems.

$ sysctl -n net.ipv4.tcp_keepalive_time
7200

[riobard]

This is intentional. The forced timeout behavior in go-shadowsocks is incorrect. If both sides want to keep a TCP connection open, the proxy should not drop it. You wouldn't like your ISP to randomly drop connections simply because they're idle. Neither should a proxy.

TCP keepalive is to detect dead peers, i.e. peers died without sending FIN packets. If that's a major concern, you should fine-tune kernel TCP keep-alive settings to be more aggressive.

[lixin9311]

that's reasonable, but I still would suggest to add a parameter to override the default timeout in order to manage the resource better in production environment. RST the connect to let the application handle the situation, and it's common behind a large NAT gateway.
And the default timeout might be the same as net.ipv4.netfilter.ip_conntrack_tcp_timeout_established = 432000, which means 5 day, if we need more aggressive timeout, we can change the parameter not the code.

[riobard]

I failed to see what kind of load would stress a properly-configured proxy using go-shadowsocks2.

[lixin9311]

like bittorrent via socks.

[riobard]

Do you have any real world data showing the problem while bittorrenting?

[lixin9311]

Ok, a real scenario. I have a crawler farm, using several servers as proxies. Web crawlers usually have a connection pool, keeping the connection ready to reuse. The problem is when I have multiple crawlers(could be hundreds or thousands) using the same proxy, the number of available ports is likely to be exhausted because of a large number of idle connection (65535 / 100 = 655 ports available for each crawler). I would fix the problem by simply RST the idle connection after a timeout.

Another read scenario, when I use TCP over UDP (or other protocols), the UDP conntrack usually expires much quicker than the TCP connection (minutes vs days), I have to set the timeout to a much smaller number such as 30s, to overcome the issue.

Anyway, it's up to you whether to add this parameter since it's not a concern for major users.

[riobard]

Thanks for the info. Indeed these are interesting and challenging scenarios beyond the intended use case of go-ss2. I do have some questions.

Case 1: I'm wondering why don't you reduce the size of the connection pool when you run lots of crawlers? It seems wasteful to keep many open connections when idle. Ideally the pool should reuse as many connections as possible under load, but keep only maybe a dozen or so when idle. The resource leak happens at the pool.

Case 2: I'm not sure if I understand the setup correctly. How do you run TCP over UDP? With KCP maybe? And what happens to the FIN packets in this case?

[lixin9311]

Although reducing the size of the connection pool is doable, the performance when actively accessing the same website will be affected.

Currently I'm trying to tune the TCP over a modified QUIC, and meanwhile porting this to android platform. Obviously, it's easier to do network experiment with GO than C.

The FIN or DISCONNECT packet can be easily lost when using QUIC, which will create a lot of zombie connections. It can be solved by simply restricting the timeout for idle connections, which should be less than the timeout of UDP

[riobard]

Why is it affected? I think the pool implementation is weird. It should dynamically increase the pool size at load and reduce the pool size when idle. Otherwise it would reconnect even if idle connections are killed.

Have you tried lowering TPC keep alive interval to kill dead connections?

[lixin9311]

That will be another different story to tune the connection pool.

Lowering the system level TCP keepalive interval is not possible on some platforms, like Android.

[riobard]

Hmm, I've never used it on Android. If fine-tuning TCP keepalive isn't an option, I guess you'll have to do it in go-ss2. Maybe an option to set the timeout, which defaults to zero (means no timeout)?