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

Add support for SocketOptions #36

Merged
merged 1 commit into from
Dec 18, 2019
Merged

Conversation

spikecurtis
Copy link
Contributor

@spikecurtis spikecurtis commented Dec 12, 2019

Signed-off-by: Spike Curtis spike@tigera.io

I'm finding myself needing to set SO_REUSEADDR and SO_REUSEPORT in my application. WDYT of this way of passing them to the library?

@ishidawataru
Copy link
Owner

ishidawataru commented Dec 13, 2019

How about passing a callback function that will get called before listening/connecting like net pkg?

https://golang.org/pkg/net/#ListenConfig

@spikecurtis
Copy link
Contributor Author

@ishidawataru is this more what you had in mind?

@ishidawataru
Copy link
Owner

ishidawataru commented Dec 13, 2019

@spikecurtis Thank you. It's getting close.

Can we use the same function signature as net.ListenConfig for Control() so that we can use the same callback for other transport protocols that could reduce some code duplication?

For example, it'd be better if we can pass this to the Listen API.

Also I'm wondering if we can even reuse net.ListenConfig in this library.

@spikecurtis
Copy link
Contributor Author

I believe doing that would require me to implement the whole syscall.RawConn interface which is more than I'm really willing to chew off.

@ishidawataru
Copy link
Owner

https://golang.org/pkg/syscall/#RawConn

How about keeping Read() / Write() empty if you don't have time?
Is it hard to implement Control(f func(fd uintptr)) error ?

@spikecurtis
Copy link
Contributor Author

Implementing Control only on the RawConn interface isn't too bad, but I need this to work for Dialed sockets. Are you OK with creating a DialConfig to mirror the function of ListenConfig? I'm not up for trying to implement a whole Dialer API.

@ishidawataru
Copy link
Owner

How about making SocketConfig, put Control(f func(fd uintptr)) error and InitMsg as fields, implement

  • (*SocketConfig) Listen(network string, laddr *SCTPAddr)
  • (*SocketConfig) Dial(network string, laddr, raddr *SCTPAddr)

which uses Control and InitMsg if needed for listening/dialing?

@spikecurtis
Copy link
Contributor Author

@ishidawataru Ok, done.

Note that we want SocketConfig to have func(fd uintpr) not func(f func(fd uintptr)) error. The former is useful for setting socket options, latter is implemented by RawConn.

@ishidawataru
Copy link
Owner

ishidawataru commented Dec 17, 2019

thanks. Can you change like below?

diff --git a/sctp.go b/sctp.go
index 993071c..730b67a 100644
--- a/sctp.go
+++ b/sctp.go
@@ -715,7 +715,7 @@ type SocketConfig struct {
        // If Control is not nil it is called after the socket is created but before
        // it is bound or connected. fd is the file descriptor representing the
        // socket in the OS.
-       Control func(fd uintptr)
+       Control func(network, address string, c syscall.RawConn) error

        // InitMsg is the options to send in the initial SCTP message
        InitMsg InitMsg
diff --git a/sctp_linux.go b/sctp_linux.go
index ce57e0b..47b2263 100644
--- a/sctp_linux.go
+++ b/sctp_linux.go
@@ -172,7 +172,7 @@ func ListenSCTPExt(network string, laddr *SCTPAddr, options InitMsg) (*SCTPListe
 }

 // listenSCTPExtConfig - start listener on specified address/port with given SCTP options and socket configuration
-func listenSCTPExtConfig(network string, laddr *SCTPAddr, options InitMsg, control func(fd uintptr)) (*SCTPListener, error) {
+func listenSCTPExtConfig(network string, laddr *SCTPAddr, options InitMsg, control func(network, address string, c syscall.RawConn) error) (*SCTPListener, error) {
        af, ipv6only := favoriteAddrFamily(network, laddr, nil, "listen")
        sock, err := syscall.Socket(
                af,
@@ -193,7 +193,12 @@ func listenSCTPExtConfig(network string, laddr *SCTPAddr, options InitMsg, contr
                return nil, err
        }
        if control != nil {
-               control(uintptr(sock))
+               c := rawConn{
+                       sockfd: sock,
+               }
+               if err = control(network, laddr.String(), c); err != nil {
+                       return nil, err
+               }
        }
        err = setInitOpts(sock, options)
        if err != nil {
@@ -250,7 +255,7 @@ func DialSCTPExt(network string, laddr, raddr *SCTPAddr, options InitMsg) (*SCTP
 }

 // dialSCTPExtConfig - same as DialSCTP but with given SCTP options and socket configuration
-func dialSCTPExtConfig(network string, laddr, raddr *SCTPAddr, options InitMsg, control func(fd uintptr)) (*SCTPConn, error) {
+func dialSCTPExtConfig(network string, laddr, raddr *SCTPAddr, options InitMsg, control func(network, address string, c syscall.RawConn) error) (*SCTPConn, error) {
        af, ipv6only := favoriteAddrFamily(network, laddr, raddr, "dial")
        sock, err := syscall.Socket(
                af,
@@ -271,7 +276,12 @@ func dialSCTPExtConfig(network string, laddr, raddr *SCTPAddr, options InitMsg,
                return nil, err
        }
        if control != nil {
-               control(uintptr(sock))
+               c := rawConn{
+                       sockfd: sock,
+               }
+               if err = control(network, laddr.String(), c); err != nil {
+                       return nil, err
+               }
        }
        err = setInitOpts(sock, options)
        if err != nil {

Then, we can do like this

// test.go
package main

import (
        "fmt"

        "github.com/ishidawataru/sctp"
        "github.com/libp2p/go-reuseport"
)

func main() {
        c := sctp.SocketConfig{
                Control: reuseport.Control,
        }
        laddr := &sctp.SCTPAddr{}
        ln, err := c.Listen("sctp", laddr)
        fmt.Println(ln, err)
}
$ go build test.go
$ strace ./test 2>&1 | grep SOL_SOCKET
setsockopt(3, SOL_SOCKET, SO_BROADCAST, [1], 4) = 0
setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
setsockopt(3, SOL_SOCKET, SO_REUSEPORT, [1], 4) = 0

@spikecurtis
Copy link
Contributor Author

Nice! I've made those changes

@ishidawataru
Copy link
Owner

Thanks. LGTM. Please squash into one commit.

@spikecurtis
Copy link
Contributor Author

Squashed

Signed-off-by: Spike Curtis <spike@tigera.io>
@ishidawataru
Copy link
Owner

Thanks! merged.

@shanehooker
Copy link

Is there an example or UT of this new usage? Can we modify, extend the ListenSCTPExt() and DialSCTPExt() to include all of the socket options described in RFC6458?
https://tools.ietf.org/html/rfc6458

For an application using SCTP, many of these parameters need to be configurable and I need an API to be able to modify them.

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

3 participants