-
Notifications
You must be signed in to change notification settings - Fork 16
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
Multicast bikeshedding #457
Comments
In a perfect world, we'd have:
This would require a language revision, plus gating. But then we could throw a better error of having bad arguments / unsupport values (+ extra bikeshedding, as @timo suggested equally cromulent |
We could just throw an error when detecting
This method would be backwards compatible with current code. |
We could do nothing. Any of the VMs should instead catch it or error internally, and pass it back up. |
The type argument should not be a string. That's what Enums are for. Otherwise we'd again have to validate that argument against an allowlist manually and there would not be any tool support for allowed values. |
Perhaps better an |
Could do it like with the Endianness then: IO::Socket::Async.bind-udp: $host, UDP-Unicast;
IO::Socket::Async.bind-udp: $host, UDP-Multicast;
IO::Socket::Async.bind-udp: $host, UDP-Broadcast; |
As I've continued to refine the implementation, a few other wrinkles to consider:
With regards to the interface, NodeJS says the OS will choose a default interface if not specified for IPv6, but can be specified (and virtually all IPv6 examples do include this, with some users reporting issues unless they do). NodeJS's manner is passing In other words, it seems like it's a really good idea for us to expose the ability to specify an interface, and probably also leaving it to module land to decide optimal defaults (and/or even saying attempts will be made without it specified, but no guarantees). Regardless, we need to have a way to specify one or more interfaces. Both NodeJS and Julia specify the broadcast/multicast status after creating the socket, akin to the following in Raku: my $socket = IO::Socket::Async.bind-udp: '::', $port;
$socket.join-multicast-group: $multicast-addr, $interface; I don't necessarily think we need to go that route, since to me it's not immediately intuitive that you should bind to my $socket = IO::Socket::Async.bind-udp: $multicast-addr, $port, :multicast($interface [, ...]); Seems much cleaner and intuitive. However, there are even more potential complexities that NodeJS and Julia support. Potential rendering in a single call as Raku currently does, versus the multicall way they do: # Single call
my $socket = IO::Socket::Async.bind-udp:
$multicast-addr,
$port,
:multicast(
$interface1,
$interface2,
$interface3,
:filter(<source1 source2>)
)
# Multi call (a la NodeJS / Julia)
my $socket = IO::Socket::Async.bind-udp: '::', $port;
$socket.join-source-specific-multicast: $source1, $multicast-addr, $interface1;
$socket.join-source-specific-multicast: $source2, $multicast-addr, $interface1;
$socket.join-source-specific-multicast: $source1, $multicast-addr, $interface2;
$socket.join-source-specific-multicast: $source2, $multicast-addr, $interface2;
$socket.join-source-specific-multicast: $source1, $multicast-addr, $interface3;
$socket.join-source-specific-multicast: $source2, $multicast-addr, $interface3; |
Actually, @timo had an even better idea. Multidispatch, such that the signatures would be: multi method bind-udp(IO::Socket::Async:U: Str() $host, Int() $port)
multi method bind-udp(IO::Socket::Async:U: Str() $host, Int() $port, :$broadcast!)
multi method bind-udp(IO::Socket::Async:U: Str() $host, Int() $port, :$multicast!,
Int() :$interface = 0, # the interface ID (generally 0, but on Mac, `en0` might be >10)
Str() :$only-from, # source specific host, if specified, only get packets from here
Bool() :$loop-back = False, # receive our own messages?
) The broadcast option doesn't really need a host, it's only available on IPv4 sockets and will always be |
When
IO::Socket::Async
was designed, itsbind-udp
was developed just with unicast and broadcast modes in mind, hence it has the option:broadcast
, defaulting to:unicast
. MoarVM similarly was designed only to handle this option, though I've since added support for multicast.Adding a
:multicast
option would, given the current interface, feel nice, but provide two incompatible named arguments. That isThe above makes no sense and will/should ultimately create an error somewhere further down the line at the VM level, which will provide a LTA error.
The text was updated successfully, but these errors were encountered: