-
Notifications
You must be signed in to change notification settings - Fork 30.3k
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
dgram: setMulticastInterface() outgoing interface selection #7855
Changes from all commits
cdf1f68
2446b57
90a160d
d3a7c2e
539bb23
1f4576d
e557572
ec4881e
0a274c1
50b30fb
4a2b811
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -370,6 +370,84 @@ added: v0.6.9 | |
Sets or clears the `SO_BROADCAST` socket option. When set to `true`, UDP | ||
packets may be sent to a local interface's broadcast address. | ||
|
||
### socket.setMulticastInterface(multicastInterface) | ||
<!-- YAML | ||
added: REPLACEME | ||
--> | ||
|
||
* `multicastInterface` {String} | ||
|
||
*Note: All references to scope in this section are refering to | ||
[IPv6 Zone Indices][], which are defined by [RFC 4007][]. In string form, an IP | ||
with a scope index is written as `'IP%scope'` where scope is an interface name or | ||
interface number.* | ||
|
||
Sets the default outgoing multicast interface of the socket to a chosen | ||
interface or back to system interface selection. The `multicastInterface` must | ||
be a valid string representation of an IP from the socket's family. | ||
|
||
For IPv4 sockets, this should be the IP configured for the desired physical | ||
interface. All packets sent to multicast on the socket will be sent on the | ||
interface determined by the most recent successful use of this call. | ||
|
||
For IPv6 sockets, `multicastInterface` should include a scope to indicate the | ||
interface as in the examples that follow. In IPv6, individual `send` calls can | ||
also use explicit scope in addresses, so only packets sent to a multicast | ||
address without specifying an explicit scope are affected by the most recent | ||
successful use of this call. | ||
|
||
#### Examples: IPv6 Outgoing Multicast Interface | ||
|
||
On most systems, where scope format uses the interface name: | ||
|
||
```js | ||
const socket = dgram.createSocket('udp6'); | ||
|
||
socket.bind(1234, () => { | ||
socket.setMulticastInterface('::%eth1'); | ||
}); | ||
``` | ||
|
||
On Windows, where scope format uses an interface number: | ||
|
||
```js | ||
const socket = dgram.createSocket('udp6'); | ||
|
||
socket.bind(1234, () => { | ||
socket.setMulticastInterface('::%2'); | ||
}); | ||
``` | ||
|
||
#### Example: IPv4 Outgoing Multicast Interface | ||
All systems use an IP of the host on the desired physical interface: | ||
```js | ||
const socket = dgram.createSocket('udp4'); | ||
|
||
socket.bind(1234, () => { | ||
socket.setMulticastInterface('10.0.0.2'); | ||
}); | ||
``` | ||
|
||
#### Call Results | ||
|
||
A call on a socket that is not ready to send or no longer open may throw a *Not | ||
running* [`Error`][]. | ||
|
||
If `multicastInterface` can not be parsed into an IP then an *EINVAL* | ||
[`System Error`][] is thrown. | ||
|
||
On IPv4, if `multicastInterface` is a valid address but does not match any | ||
interface, or if the address does not match the family then | ||
a [`System Error`][] such as `EADDRNOTAVAIL` or `EPROTONOSUP` is thrown. | ||
|
||
On IPv6, most errors with specifying or omiting scope will result in the socket | ||
continuing to use (or returning to) the system's default interface selection. | ||
|
||
A socket's address family's ANY address (IPv4 `'0.0.0.0'` or IPv6 `'::'`) can be | ||
used to return control of the sockets default outgoing interface to the system | ||
for future multicast packets. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This whole section is not in harmony with the rest of the document. I would try to sum it up a bit. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added this level of detail in response to @jasnell 's request and then some requests for clarification of terms. Given that combinations like windows, IPv6 and multicast are extreme edge cases but need the most detail to implement correctly, I don't think it is possible to create a harmony where more common cases get the most (or at least equal) attention. I also don't see a clear way to restructure multicast related methods to a separate doc? |
||
|
||
|
||
### socket.setMulticastLoopback(flag) | ||
<!-- YAML | ||
added: v0.3.8 | ||
|
@@ -512,4 +590,7 @@ and `udp6` sockets). The bound address and port can be retrieved using | |
[`socket.address().address`]: #dgram_socket_address | ||
[`socket.address().port`]: #dgram_socket_address | ||
[`socket.bind()`]: #dgram_socket_bind_port_address_callback | ||
[`System Error`]: errors.html#errors_class_system_error | ||
[byte length]: buffer.html#buffer_class_method_buffer_bytelength_string_encoding | ||
[IPv6 Zone Indices]: https://en.wikipedia.org/wiki/IPv6_address#Link-local_addresses_and_zone_indices | ||
[RFC 4007]: https://tools.ietf.org/html/rfc4007 |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -563,6 +563,21 @@ Socket.prototype.setMulticastLoopback = function(arg) { | |||||
}; | ||||||
|
||||||
|
||||||
Socket.prototype.setMulticastInterface = function(interfaceAddress) { | ||||||
this._healthCheck(); | ||||||
|
||||||
if (typeof interfaceAddress !== 'string') { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would we be able to get away with checking for IP addresses only here ( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. net.isIP() makes it look like no checks are necessary either in JS or the wrapper: Its existing tests don't cover Scope, so I added some: Line 1079 in f8d3f6f
yet, this doesn't need any wrapper: Line 1581 in 863952e
and still passes: assert.equal(net.isIP(), 0); If net.isIP() is correct then no check on interfaceAddress in JS and removing the argument checks should be fine? **I've run the tests with gdb and determined that net.isIP() passes since *ip ends up as the string "undefined" when uv_inet_pton is called. So the wrapper is more conservative than net.isIP() but in a way we already discussed and throwing separate missing/non string typeerrors independently EINVALs provides better debugging while calling net.isIP() would duplicate code just to rename EINVAL something else. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @lostnet would you please cover the missing parts for |
||||||
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', | ||||||
'interfaceAddress', | ||||||
'string'); | ||||||
} | ||||||
|
||||||
const err = this._handle.setMulticastInterface(interfaceAddress); | ||||||
if (err) { | ||||||
throw errnoException(err, 'setMulticastInterface'); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It should use the new error system |
||||||
} | ||||||
}; | ||||||
|
||||||
Socket.prototype.addMembership = function(multicastAddress, | ||||||
interfaceAddress) { | ||||||
this._healthCheck(); | ||||||
|
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.
I do not think that "should" is the right term. here and in the previous paragraph.
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.
I've no idea in terms of informal English. In RFC terminology, Line 315 has the only actual MUST, leaving some strong recommendations that I would call SHOULD:
In the IPv4 case, I mean SHOULD in the sense that any OS MAY be choosing other behaviors for other IPs that one should not rely on for maximum portability.
In the IPv6 case, I mean SHOULD as in it is technically optional to add the default scope, but I would recommend including it specifically to avoid bugs through miscommunication.