-
-
Notifications
You must be signed in to change notification settings - Fork 633
Make TCP keep-alive timeout options configurable #505
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
92f782c
32be38d
6143ca1
e127686
1db818a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,6 +31,9 @@ type Config struct { | |
| // Options are supplement options to apply settings | ||
| // for the internal stack. | ||
| Options []option.Option | ||
|
|
||
| // TCPSocketOptions are TCP socket-level options to apply to each TCP endpoint. | ||
| TCPSocketOptions []option.TCPSocketOption | ||
| } | ||
|
|
||
| // CreateStack creates *stack.Stack with given config. | ||
|
|
@@ -40,6 +43,11 @@ func CreateStack(cfg *Config) (*stack.Stack, error) { | |
| opts = append(opts, cfg.Options...) | ||
| } | ||
|
|
||
| tcpSockOpts := append( | ||
| option.DefaultTCPSocketOptions(), | ||
| cfg.TCPSocketOptions..., | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
| ) | ||
|
|
||
| s := stack.New(stack.Options{ | ||
| NetworkProtocols: []stack.NetworkProtocolFactory{ | ||
| ipv4.NewProtocol, | ||
|
|
@@ -61,7 +69,7 @@ func CreateStack(cfg *Config) (*stack.Stack, error) { | |
| // before creating NIC, otherwise NIC would dispatch packets | ||
| // to stack and cause race condition. | ||
| // Initiate transport protocol (TCP/UDP) with given handler. | ||
| withTCPHandler(cfg.TransportHandler.HandleTCP), | ||
| withTCPHandler(cfg.TransportHandler.HandleTCP, tcpSockOpts), | ||
| withUDPHandler(cfg.TransportHandler.HandleUDP), | ||
|
|
||
| // Create stack NIC and then bind link endpoint to it. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -205,6 +205,7 @@ func netstack(k *Key) (err error) { | |
| } | ||
|
|
||
| var opts []option.Option | ||
| var tcpSocketOpts []option.TCPSocketOption | ||
| if k.TCPModerateReceiveBuffer { | ||
| opts = append(opts, option.WithTCPModerateReceiveBuffer(true)) | ||
| } | ||
|
|
@@ -225,11 +226,27 @@ func netstack(k *Key) (err error) { | |
| opts = append(opts, option.WithTCPReceiveBufferSize(int(size))) | ||
| } | ||
|
|
||
| if k.TCPKeepaliveIdleTime > 0 { | ||
| tcpSocketOpts = append(tcpSocketOpts, option.WithTCPKeepaliveIdleTime(k.TCPKeepaliveIdleTime)) | ||
| log.Infof("[TCP] keepalive idle time: %v", k.TCPKeepaliveIdleTime) | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, logging these options seems good to me. My minor suggestion here is to use |
||
| } | ||
|
|
||
| if k.TCPKeepaliveInterval > 0 { | ||
| tcpSocketOpts = append(tcpSocketOpts, option.WithTCPKeepaliveInterval(k.TCPKeepaliveInterval)) | ||
| log.Infof("[TCP] keepalive interval: %v", k.TCPKeepaliveInterval) | ||
| } | ||
|
|
||
| if k.TCPKeepaliveCount > 0 { | ||
| tcpSocketOpts = append(tcpSocketOpts, option.WithTCPKeepaliveCount(k.TCPKeepaliveCount)) | ||
| log.Infof("[TCP] keepalive count: %d", k.TCPKeepaliveCount) | ||
| } | ||
|
|
||
| if _defaultStack, err = core.CreateStack(&core.Config{ | ||
| LinkEndpoint: _defaultDevice, | ||
| TransportHandler: tunnel.T(), | ||
| MulticastGroups: multicastGroups, | ||
| Options: opts, | ||
| TCPSocketOptions: tcpSocketOpts, | ||
| }); err != nil { | ||
| return err | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
without -s?