New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 768916 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Fuchsia
Pri: 3
Type: Bug



Sign in to add a comment

Implement multicast support in net::UDPSocket on Fuchsia

Project Member Reported by sergeyu@chromium.org, Sep 26 2017

Issue description

Currently setsockopt() doesn't behave properly on Fuchsia. Particularly for multicast operations (IP_ADD_MEMBERSHIP and IP_DROP_MEMBERSHIP) it's always successful, even when it's expected to fails, see https://fuchsia.atlassian.net/browse/NET-193 .
This issue breaks UDPSocketTest.JoinMulticastGroup. The test will need to be updated once the problem is fixed in Fuchsia.
 
Cc: mpcomplete@chromium.org
Summary: Implement multicast support in net::UDPSocket on Fuchsia (was: Update UDPSocketTest.JoinMulticastGroup once setsockopt() is fixed to report errors.)
There are some other issues with Multicast support in net::UDPSocket:
 - UDPSocketPosix::JoinGroup() uses ip_mreqn instead of ip_mreq, which is a non-standard extension on Linux, not supported on Fuchsia.

 - For MacOS JoinGroup() can use ip_mreq, but then it depends on if_indextoname() and ioctl(SIOCGIFADDR) which are not implemented on Fuchsia, so MacOS version of JoinGroup() also won't work.

 - JoinMulticastGroup test doesn't call SetMulticastInterface(), which implies that the system is expected to select the default interface (JoinGroup() sets ip_mreq.imr_interface to INADDR_ANY). This feature is not supported on Fuchsia. It's questionable if it's a good idea to support it, may be better to update the test to select interface. +mpcomplete, WDYT?
Fuchsia should support the standard interface. Setting ip_mreq.imr_interface to INADDR_ANY is standard, AFAICT, so we should support it.

I'm ambivalent about the ip_mreqn issue. It might be better the add a FIDL interface for managing multicast groups. Would that work for you?
Filed NET-194 to support interface selection by index. Don't feel strongly about it, feel free to close it as WontFix.
Also filed NET-195 for INADDR_ANY.
Owner: sergeyu@chromium.org
Status: Started (was: Untriaged)
Project Member

Comment 5 by bugdroid1@chromium.org, Oct 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2beef3dfec082beea9ad2c0f2031ac80bacbcf87

commit 2beef3dfec082beea9ad2c0f2031ac80bacbcf87
Author: Sergey Ulanov <sergeyu@chromium.org>
Date: Thu Oct 19 21:53:53 2017

Implement net::GetNetworkList() for Fuchsia.

Previously GetNetworkList was calling getifaddrs(), but it isn't
implemented on Fuchsia yet. Replaced it with a GetNetworkList()
implementation that calls Fuchsia-specific ioctl.

Bug:  768916 
Change-Id: I5e6e5c2f06867c3d4a3ec08cf2526bb24f4b7e60
Reviewed-on: https://chromium-review.googlesource.com/728919
Reviewed-by: Eric Roman <eroman@chromium.org>
Commit-Queue: Sergey Ulanov <sergeyu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510222}
[modify] https://crrev.com/2beef3dfec082beea9ad2c0f2031ac80bacbcf87/net/BUILD.gn
[add] https://crrev.com/2beef3dfec082beea9ad2c0f2031ac80bacbcf87/net/base/network_interfaces_fuchsia.cc
[modify] https://crrev.com/2beef3dfec082beea9ad2c0f2031ac80bacbcf87/net/base/network_interfaces_unittest.cc
[modify] https://crrev.com/2beef3dfec082beea9ad2c0f2031ac80bacbcf87/testing/buildbot/filters/fuchsia.net_unittests.filter

Project Member

Comment 6 by bugdroid1@chromium.org, Oct 21 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fea2f07f106ebcb718a736770be8d35dbf71a07a

commit fea2f07f106ebcb718a736770be8d35dbf71a07a
Author: Sergey Ulanov <sergeyu@chromium.org>
Date: Sat Oct 21 04:34:26 2017

Roll Fuchsia SDK to 492eec9e40c9832dd8772f4718b7db6aac4d7921

In the new SDK:
 - NIC indices are 1-based now, which is useful for multicast UDP
   socket (for details see
   https://chromium-review.googlesource.com/c/chromium/src/+/729538)
 - LP_CLONE_FDIO_CWD was removed.

TBR=thakis@chromium.org

Bug: 707030,  768916 
Change-Id: If282c06ea01e7ec5e8f77d312529340e35ecc868
Reviewed-on: https://chromium-review.googlesource.com/731671
Commit-Queue: Sergey Ulanov <sergeyu@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Reviewed-by: Wez <wez@chromium.org>
Reviewed-by: Sergey Ulanov <sergeyu@chromium.org>
Reviewed-by: Kevin Marshall <kmarshall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510660}
[modify] https://crrev.com/fea2f07f106ebcb718a736770be8d35dbf71a07a/DEPS
[modify] https://crrev.com/fea2f07f106ebcb718a736770be8d35dbf71a07a/base/process/launch.h
[modify] https://crrev.com/fea2f07f106ebcb718a736770be8d35dbf71a07a/base/process/launch_fuchsia.cc
[modify] https://crrev.com/fea2f07f106ebcb718a736770be8d35dbf71a07a/content/common/sandbox_policy_fuchsia.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Oct 23 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/20007f6c4846bd8b37cc54001fbcbba0e4556d22

commit 20007f6c4846bd8b37cc54001fbcbba0e4556d22
Author: Sergey Ulanov <sergeyu@chromium.org>
Date: Mon Oct 23 20:33:54 2017

Bugfixes for multicast support on Fuchsia.

- Updated UDPSocketPosix to pass correct arguments when joining/leaving
  multicast groups.
- Enabled unittests.

Bug:  768916 
Change-Id: I96a8170cad3c52b8cf922cac86e13c2bc4f97021
Reviewed-on: https://chromium-review.googlesource.com/729538
Commit-Queue: Sergey Ulanov <sergeyu@chromium.org>
Reviewed-by: Eric Roman <eroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510905}
[modify] https://crrev.com/20007f6c4846bd8b37cc54001fbcbba0e4556d22/net/base/network_interfaces_fuchsia.cc
[modify] https://crrev.com/20007f6c4846bd8b37cc54001fbcbba0e4556d22/net/socket/udp_socket_posix.cc
[modify] https://crrev.com/20007f6c4846bd8b37cc54001fbcbba0e4556d22/net/socket/udp_socket_unittest.cc

Status: Fixed (was: Started)

Sign in to add a comment