Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: add keep alive request duration to configs #962

Merged
merged 1 commit into from May 4, 2021

Conversation

therealak12
Copy link
Contributor

Trying to solve this.

Copy link
Contributor

@xiaokangwang xiaokangwang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand this correctly, TCP_KEEPINTVL is a TCP specific option. You should not set it without checking the type of the socket.

You can either change the code, let me to fix this and merge, or justify your design.

@therealak12
Copy link
Contributor Author

You're right. I'll try to find a more general approach for setting the keep-alive duration.
If you are going to fix it before me, I would appreciate (solving this problem is by far more important than my contribution).

@xiaokangwang
Copy link
Contributor

I will make necessary modification and merge it.

@ValdikSS
Copy link
Contributor

ValdikSS commented May 7, 2021

Is this modification only for Linux sockets, not for other OS?

@chromer030
Copy link

how we should set it in configs ?

@Loyalsoldier
Copy link
Contributor

Is this modification only for Linux sockets, not for other OS?

Only for Linux.

@Loyalsoldier
Copy link
Contributor

how we should set it in configs ?

https://www.v2fly.org/config/transport.html#sockoptobject

@ValdikSS
Copy link
Contributor

Does not seem to be fixed in v2fly v4.41.1.

Test 1:

  1. Run v2fly v4.41.1 with default config.json (remove geoip:private block first)
  2. start nc -l -p 4141
  3. execute curl --proxy socks5://127.0.0.1:1080 127.0.0.1:4141

Result: 15 second keep-alive from v2fly to socks client (1080 → client source port)

Adding "streamSettings": {"sockopt": {"tcpKeepAliveInterval": 900}}, to both inbounds and outbounds directives did not change anything.

@ValdikSS
Copy link
Contributor

This change is for applyOutboundSocketOptions, however the main issue is with inbound (listening) sockets which are used for client connections and client-v2fly transmission from the server standpoint.

@ValdikSS
Copy link
Contributor

This commit changes the wrong parameter. TCP_KEEPIDLE should be changed, not TCP_KEEPINTVL.

TCP_KEEPIDLE (since Linux 2.4)
The time (in seconds) the connection needs to remain idle before TCP starts sending keepalive probes, if the socket option SO_KEEPALIVE has been set on this socket. This option should not be used in code intended to be portable.

TCP_KEEPINTVL (since Linux 2.4)
The time (in seconds) between individual keepalive probes. This option should not be used in code intended to be portable.

Go sets both TCP_KEEPINTVL and TCP_KEEPIDLE to 15. Leaving TCP_KEEPINTVL to 15 looks fine to me (Linux default is 75). TCP_KEEPCNT is not changed and kept to standard 9, so 15*9 = 135 seconds before connection teardown, after TCP_KEEPIDLE had an effect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants