Make TCP keep-alive timeout options configurable#505
Make TCP keep-alive timeout options configurable#505twelsh-aw wants to merge 5 commits intoxjasonlyu:mainfrom
Conversation
xjasonlyu
left a comment
xjasonlyu
left a comment
There was a problem hiding this comment.
Thank you for your pull request!
I think it's nice to make some stack/tcp options configurable. Just got some comments.
core/option/option.go
Outdated
|
|
||
| // tcpKeepAlivesEnabled is the value used to enable or disable keepalives on | ||
| // the socket. | ||
| tcpKeepAlivesEnabled = true |
There was a problem hiding this comment.
| tcpKeepAlivesEnabled = true | |
| tcpKeepAliveEnabled = true |
without -s?
core/stack.go
Outdated
|
|
||
| tcpSockOpts := append( | ||
| option.DefaultTCPSocketOptions(), | ||
| cfg.TCPSocketOptions..., |
There was a problem hiding this comment.
I'm a bit concerned about the potential for duplicate calls; the current way to call this option chain might not be ideal/efficient because they might be called for the same option with different values for every TCP endpoint.
I'm not sure what the best way to do it is, but I'd prefer to avoid calling an option multiple times.
There was a problem hiding this comment.
ya good callout. this is doing duplicate options every TCP conn. I can refactor a bit so we always register the Options, preferring user supplied values if present but using defaults otherwise, but never both
engine/engine.go
Outdated
|
|
||
| if k.TCPKeepaliveIdleTime > 0 { | ||
| tcpSocketOpts = append(tcpSocketOpts, option.WithTCPKeepaliveIdleTime(k.TCPKeepaliveIdleTime)) | ||
| log.Infof("[TCP] keepalive idle time: %v", k.TCPKeepaliveIdleTime) |
There was a problem hiding this comment.
Yeah, logging these options seems good to me. My minor suggestion here is to use [STACK] instead of [TCP] since these are part of netstack settings.
Can just explicitly pass these for a simpler interface
twelsh-aw
force-pushed
the
tcp-keepalive-opts
branch
from
1477366 to
1db818a
Compare
2 participants
First off, thanks for this entire codebase! I spent a lot of time trying to setup something similar and then stumbled on this which is exactly what I was after.
I have a use case where tun2socks sits behind a load balancer which currently does sticky flow state tracking to route flows always to the same instance behind it (based on source/dest ips, ports, proto). For reasons, this load balancer must have short timeouts around inactive TCP flows; once timeout is hit, it will stop being sticky and start being able to route traffic to different instances registered to it.
I want to be able to control tun2socks TCP keepalive settings to align with this LB timeout, which requires the ability to configure values that are currently hardcoded.
I tried to set things up similar to how we do other network stack options, and just mimic that pattern for socket options. Open to different ways to open this up!
Thank you once again!