[FlyWeb] Use unbound UDP socket for mDNS queries

RESOLVED FIXED in Firefox 50

Status

()

Core
Networking
RESOLVED FIXED
10 months ago
9 months ago

People

(Reporter: djvj, Assigned: djvj)

Tracking

(Blocks: 1 bug)

unspecified
mozilla50
Points:
---

Firefox Tracking Flags

(firefox50 fixed)

Details

(Whiteboard: [necko-active])

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

10 months ago
One of the implementation issues we ran into on Windows 10 is that win10 system processes open and hold UDP port 5353 on the mDNS multicast address.

Our current fallback implementation also uses a SO_REUSE UDP socket on port 5353, but is hampered by conflicts.  The main issue is that only broadcast packets sent to the multicast group are delivered to the gecko UDP socket.  Directed UDP messages to the machine on that port are not delivered, and presumably go to the system socket instead.

The workaround suggested is to use a single query socket opened on an arbitrary port.  The query socket does NOT join the mDNS multicast IP group, but DOES send packets to that group.  Since this socket is assigned its own unique port, query responses addressed to it will resolve properly.
(Assignee)

Comment 1

10 months ago
Created attachment 8770685 [details] [diff] [review]
use-unbound-query-socket.patch

Tested this on Linux, Android, and my Win10 box.  Seems to work quite well.
Attachment #8770685 - Flags: review?(jdarcangelo)

Updated

10 months ago
Assignee: nobody → kvijayan
Whiteboard: [necko-active]
Comment on attachment 8770685 [details] [diff] [review]
use-unbound-query-socket.patch

Review of attachment 8770685 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM. See my comment about removing the old block of commented-out code.

::: netwerk/dns/mdns/libmdns/fallback/MulticastDNS.jsm
@@ +343,5 @@
>        sockets.forEach((socket) => {
>          socket.send(MDNS_MULTICAST_GROUP, MDNS_PORT, data, data.length);
>        });
>      });
> +    */

We should probably just remove this block instead of commenting it out.
Attachment #8770685 - Flags: review?(jdarcangelo) → review+
(Assignee)

Comment 3

10 months ago
Created attachment 8771453 [details] [diff] [review]
discovery-fixes.patch

Updated patch.
Attachment #8770685 - Attachment is obsolete: true
Attachment #8771453 - Flags: review?(jdarcangelo)
Attachment #8771453 - Flags: review?(jdarcangelo) → review+

Comment 4

10 months ago
Pushed by kvijayan@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/048a09249037
Use dedicated custom-port query socket to do mDNS queries.  Respond to queries using per-interface sockets. r=justindarc
(Assignee)

Updated

10 months ago
Blocks: 1228662

Comment 5

9 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/048a09249037
Status: NEW → RESOLVED
Last Resolved: 9 months ago
status-firefox50: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50

Comment 6

9 months ago
this caused a small perf improvement on android (nexus 4.1: https://treeherder.mozilla.org/perf.html#/alerts?id=2167 ):
Summary of tests that improved:

  remote-blank summary android-4-2-armv7-api15 opt - 2.08% better


thanks for the win!
You need to log in before you can comment on or make changes to this bug.