-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Read SO_REUSEPORT on socket before setting #803
Read SO_REUSEPORT on socket before setting #803
Conversation
See if we're able to read the value from the socket. If that fails, skip setting the value on the port.
Might be simpler to always do this, instead of the conditional compilation? Would there be any downsides to that? |
Codecov Report
@@ Coverage Diff @@
## master #803 +/- ##
==========================================
- Coverage 57.62% 57.56% -0.07%
==========================================
Files 42 42
Lines 10856 10859 +3
==========================================
- Hits 6256 6251 -5
- Misses 3511 3517 +6
- Partials 1089 1091 +2
Continue to review full report at Codecov.
|
Based on this comment: #654 (comment) It sounds like this behavior is only supported in go1.11+? |
_, err := unix.GetsockoptInt(int(fd), unix.SOL_SOCKET, unix.SO_REUSEPORT) | ||
if err != nil { | ||
opErr = fmt.Errorf("could not get SO_REUSEPORT: %s", err.Error()) | ||
return |
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.
shouldn't this explicitly return opErr?
[ Quoting <[email protected]> in "Re: [miekg/dns] Read SO_REUSEPORT o..." ]
chantra commented on this pull request.
> @@ -16,6 +17,11 @@ const supportsReusePort = true
func reuseportControl(network, address string, c syscall.RawConn) error {
var opErr error
err := c.Control(func(fd uintptr) {
+ _, err := unix.GetsockoptInt(int(fd), unix.SOL_SOCKET, unix.SO_REUSEPORT)
+ if err != nil {
+ opErr = fmt.Errorf("could not get SO_REUSEPORT: %s", err.Error())
+ return
shouldn't this explicitly return opErr?
this code already has a toggle to allow you to use it, there is no direct reason
to tweak this. In coredns we ignore the error here.
|
Closing, as users are allowed to opt-in to this code. |
See if we're able to read the value from the
socket. If that fails, skip setting the value on
the port.
Note: I don't have any machines with an old enough kernel lying around to test that it fails correctly, but perhaps someone out there could jump in and give it a try.
Given this comment: #802 (comment)
I'm not sure where you want this to live, or if it's on the user to know they can use SO_REUSEPORT or not.