add darwin #1
add darwin #1
Conversation
There was a problem hiding this comment.
@viphak thanks for your PR.
I put some comments since I'm not sure that copying the current golang implementation is right since we have to fine grain control the 3 tcp keepalive options and looks like darwin permits this. Though I don't use any other os than Linux so I can't verify this.
tcpkeepalive_darwin.go
Outdated
| // d += (time.Second - time.Nanosecond) | ||
| // secs := int(d.Seconds()) | ||
| // return os.NewSyscallError("setsockopt", syscall.SetsockoptInt(int(fd), syscall.IPPROTO_TCP, sysTCP_KEEPINTVL, secs)) | ||
| // The kernel expects seconds so round to next highest second. |
There was a problem hiding this comment.
I'll keep this comment as it helps explaining the expected time unit.
tcpkeepalive_darwin.go
Outdated
| return os.NewSyscallError("setsockopt", err) | ||
| } | ||
| err := syscall.SetsockoptInt(int(fd), syscall.IPPROTO_TCP, syscall.TCP_KEEPALIVE, secs) | ||
| runtime.KeepAlive(fd) |
There was a problem hiding this comment.
This isn't needed since the rawConn interface alreafy guarantees that the file descriptor will remain valid https://golang.org/pkg/syscall/#RawConn
tcpkeepalive_darwin.go
Outdated
| default: | ||
| return os.NewSyscallError("setsockopt", err) | ||
| } | ||
| err := syscall.SetsockoptInt(int(fd), syscall.IPPROTO_TCP, syscall.TCP_KEEPALIVE, secs) |
There was a problem hiding this comment.
From the man https://github.com/apple/darwin-xnu/blob/5394bb038891708cd4ba748da79b90a33b19f82e/bsd/man/man4/tcp.4#L172 TCP_KEEPALIVE should set the idle and not the interval so it doesn't belong here but should be put in the setIdle function.
tcpkeepalive_darwin.go
Outdated
| } | ||
|
|
||
| func setCount(fd uintptr, n int) error { | ||
| // not possible with darwin |
There was a problem hiding this comment.
Looking at the source code https://github.com/apple/darwin-xnu/blob/0a798f6738bc1db01281fc08ae024145e84df927/bsd/netinet/tcp.h looks like also count is implemented on darwin (not sure from which version)
In this repo that uses the hackish way of calling setsockopt they are using it:
https://github.com/felixge/tcpkeepalive/blob/master/keepalive_darwin.go
|
@viphak Thanks! I have some cosmetic changes in mind but I will do them in some followup PR. Did you manage to try if it works? |
|
I'll create some unit tests for macOS tonight |
|
@viphak gentle ping 😄 |
attempt to make it work on darwin