966564 - STUN logging is reporting wrong transport

Status: RESOLVED FIXED

(Core :: WebRTC: Networking, defect)

Description

[Nils Ohlmeier [:drno]]

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.

Comment 1

[Nils Ohlmeier [:drno]]

Attachment: fix_incorect_stun_udp_logging.patch

Comment 2

[Byron Campen [:bwc]]

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?

Comment 3

[Nils Ohlmeier [:drno]]

Attachment: 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.

Comment 4

[Nils Ohlmeier [:drno]]

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.

Comment 5

[Byron Campen [:bwc]]

(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).

Comment 6

[Byron Campen [:bwc]]

Attachment: Make logging more clear when processing an unwrapped indication.

This should make the logging more clear.

Comment 7

[Byron Campen [:bwc]]

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

Would this clarify things sufficiently?

Comment 8

[Nils Ohlmeier [:drno]]

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

lgtm

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7884913fe074

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7884913fe074