STUN logging is reporting wrong transport

RESOLVED FIXED in mozilla30

Status

()

Core
WebRTC: Networking
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: drno, Assigned: bwc)

Tracking

Trunk
mozilla30
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

5 years ago
In case of a network which blocks all UDP and the call goes over TURN TCP the STUN logs still reports reading from UDP.

It seems the string representation of the transport is outdated for some reason, as the attached patch which updates the string every time after reading from it fixes the issue (with the patch UDP no longer shows up in the log).

We should probably find a better way of updating the string less often, but still have been reported correctly.
(Reporter)

Comment 1

5 years ago
Created attachment 8368986 [details] [diff] [review]
fix_incorect_stun_udp_logging.patch
(Reporter)

Updated

5 years ago
Assignee: nobody → docfaraday
(Assignee)

Comment 2

5 years ago
So, I'm seeing some places where we are hard-coding UDP nr_transport_addr, but what I'm not seeing is where we could be switching that addr over to TCP, allowing a subsequent call to nr_transport_addr_fmt_addr_string to fix things up. Also, using ice_unittest, I can see that TURN TCP traffic is logged with the right string, whereas after it is unwrapped it is always logged as UDP (this is consistent with my reading of the code), both before and after the patch is applied.

Can you send r_log at DEBUG so I can try to figure out whether we're seeing the same thing?
Flags: needinfo?(drno)
(Assignee)

Updated

4 years ago
Status: NEW → ASSIGNED
(Reporter)

Comment 3

4 years ago
Created attachment 8389636 [details]
stun.log.bz2

This contains lines like this one:
(ice/DEBUG) ICE(PC:1394599591666615 (id=19 url=https://apprtc.appspot.com/?r=56411989)): Read 999 bytes from IP4:23.236.53.1:51895/UDP
although TURN TCP was used.
Flags: needinfo?(drno)
(Reporter)

Comment 4

4 years ago
Are you saying that after unwrapping a STUN packet from a TCP connection would always log UDP?
I understand that we will get the full STUN packet from the far end as it got wrapped into the send indication by the far end. But this original packet does not contain a transport indication, or? Because the log file above was taken on the same machine with two browsers and the machines firewall blocked UDP. So there really should be no UDP on the media path.
(Assignee)

Comment 5

4 years ago
(In reply to Nils Ohlmeier [:drno] from comment #4)
> Are you saying that after unwrapping a STUN packet from a TCP connection
> would always log UDP?

   Yes. This is because we try to log using the tuple that the sender is using to talk to the relay, as indicated in the XOR_PEER_ADDR attribute, but this attribute does not indicate transport protocol. So we just assume UDP (since there is no reason to think otherwise).
(Assignee)

Comment 6

4 years ago
Created attachment 8389926 [details] [diff] [review]
Make logging more clear when processing an unwrapped indication.

This should make the logging more clear.
(Assignee)

Updated

4 years ago
Attachment #8368986 - Attachment is obsolete: true
(Assignee)

Comment 7

4 years ago
Comment on attachment 8389926 [details] [diff] [review]
Make logging more clear when processing an unwrapped indication.

Would this clarify things sufficiently?
Attachment #8389926 - Flags: review?(drno)
(Reporter)

Comment 8

4 years ago
Comment on attachment 8389926 [details] [diff] [review]
Make logging more clear when processing an unwrapped indication.

lgtm
Attachment #8389926 - Flags: review?(drno) → review+
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/7884913fe074
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in before you can comment on or make changes to this bug.