udp: add source-specific multicast support #2202
Conversation
src/unix/udp.c
Outdated
| const char* source_addr, | ||
| uv_membership membership) { | ||
| int err; | ||
| union sockaddr { |
There was a problem hiding this comment.
why not use struct sockadr_storage here?
There was a problem hiding this comment.
Yes, I thought about it when going through the changes but went for this approach for a couple of reasons:
- I didn't want to change the original PR much.
- The current multicast method already uses strings instead of
sockaddr_structs (which I guess that's the reason why this interface was chosen).
That said, I think it makes more sense using sockaddr_ structs in both methods, but I guess that'll have to go into master?
There was a problem hiding this comment.
@saghul disregard my last comment. I though you were talking about the function arguments 🤦♂️
About using struct sockaddr_storage I already commented about it in the original PR, but thought this option looked cleaner.
There was a problem hiding this comment.
I see. Not my cup of tea, but I won't block this based on that.
|
Would someone be so kind and kick off the CI and land this if everything is fine? @santigimeno @saghul @cjihrig @bnoordhuis It would be great to get this in libuv so we can progress on a long outstanding Node.js PR. |
|
CI: https://ci.nodejs.org/view/libuv/job/libuv-test-commit/1253/ As expected, we're getting the same failures as in #2185. Let's see if we can resolve those before merging this. |
|
@saghul I've updated the PR to use CI: https://ci.nodejs.org/view/libuv/job/libuv-test-commit/1257/ |
|
Polite bump/ping. nodejs/node#15735 is currently blocked on this. |
|
I would love landing this, but it's blocked by #2185 which, unfortunately, I don't think we're close to. |
|
Rebased to current |
PR-URL: libuv#2202 Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com> Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
PR-URL: libuv#2202 Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
All the supported platforms support specific source multicast. PR-URL: libuv#2202 Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
Use the correct socket options: `MCAST_JOIN_SOURCE_GROUP` and `MCAST_LEAVE_SOURCE_GROUP`. Set mreq.gsr_interface = 0 if iface_addr = NULL. PR-URL: libuv#2202 Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
PR-URL: libuv#2202 Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
To support source specific multicast operations. PR-URL: libuv#2202 Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
PR-URL: libuv#2202 Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
|
CI is green except for the known flaky tests. Landing. |
This is a continuation of #964 with some fixes and tests.
For the tests, I've been working on top of #2185, so you can skip that commit for the review.
CI is fine except for the failures already reported in #2185 (comment).
CI: https://ci.nodejs.org/view/libuv/job/libuv-test-commit/1245/