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

prototype scion-socket #247

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

amdfxlucas
Copy link

@amdfxlucas amdfxlucas commented Dec 22, 2023

this is required by bindings to languages whose networking libraries main abstraction are berkley sockets, not connections as in Go.It is a prerequisite for i.e. P2P applications where you want to act as both client and server on the same address/socket utilizing QUIC implementations that support it i.e rust-quinn.


This change is Reviewable

this is required by bindings to languages whose networking libraries main abstraction are berkley sockets, not connections as in Go
@matzf
Copy link
Contributor

matzf commented Dec 27, 2023

The background for this change seems somewhat confused; the distinction between client and server side is a peculiarity of pan, not of go the "Conn" concept. Arguably, a "net.Conn"/ "net.PacketConn" is a socket, it's just a different name.

The reason why pan has this separate client/server thing is that we don't want to do a "hidden" path request during a "send" call. Because of this, unconnected sockets are restricted to sending reply packets, as this can be done without an explicit path request. Indeed, the functionality to use a socket for multiple remotes is missing.
The proposed "DefaultCombiSelector" does exactly what we didn't want to do, namely this "hidden" path request. I'm aware that this path request will eventually need to be "implicit" in some shim-layer, but in pan this was intended to be explicit.
I'm not saying the pan approach is perfect; but if we add hidden path requests, perhaps the interface could be entirely restructured, instead of adding a separate new "socket" type. I'm not very fond of this proposal to create an interface that is basically the amalgamation of the two variants, as this seems to bloat the library interface with different ways to create connections, and make the common, simple case more complicated.

An alternative approach could be to export a function that allows a user to initialize a path selector analogous to how DialUDP does it. This would be an "advanced" interface; the user would explicitly need to manage these, and use ListenConn.WriteToVia to send packets with an explicit call to Selector.Path(). This would allow using the ListenConn to also send packets to any "new" destinations.
Effectively, this would be to making the existing type pathRefreshSubscriber and func openPathRefreshSubscriber public -- not sure what would be a good name and if the interface needs more cleaning. Basic usage could look something like this:

listenConn, err := pan.ListenUDP(ctx, local, nil)
pather, err := pan.NewPathSubscriber(ctx, local, dstUDPAddr, policy, selector)
listenConn.WriteToVia(msg, dstUDPAddr, pather.Path())
...
pather.Close()
listenConn.Close()

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.

2 participants