1230965 - WebRTC datachannel ignores SCTP packet with multiple data chunks

Status: RESOLVED FIXED

(Core :: WebRTC: Networking, defect, P3)

Description

[xmo]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/46.0.2490.86 Safari/537.36

Steps to reproduce:

1. My app opens a WebRTC data channel (SCTP/DTLS/UDP) towards Firefox
2. My app performs file transfer over this data channel.  File size is a few 10MBs.



Actual results:

Whenever an SCTP packet contains multiple data chunks, it's like Firefox ignores it:
 - No SACK is sent back (related to this packet)
 - When subsequent packets are sent, the SACK's TSN is the one of the previous packet.

I can provide SCTP traces offline.


Expected results:

Transfer should complete.

Comment 1

[Maire Reavy [:mreavy]]

Is this a dup?

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

Is this a dup of bug 1214269?

Comment 3

[xmo]

(In reply to Randell Jesup [:jesup] from comment #2)
> Is this a dup of bug 1214269?

No, this one was related to DTLS.
The new one is kind of similar, but at SCTP layer.

I will send SCTP traces to you and Niels.

BR
Xavier

Comment 4

[Randell Jesup [:jesup] (needinfo me)]

Please send the traces to myself and tuexen (cc'd here)

Comment 5

[xmo]

done
(sorry for Nils, cc'd too ;)

Comment 6

[Michael Tüxen]

OK, I looked at the tracefile. Everything seems more or less fine until frame 2272. This contains the TSNs 74 and 75 (I'm only using the last two digits of the TSN). The DATA chunks with TSNs 76, ..., 79 are also missing at the receiver. The receiver SACKs up to 73 and from 80 onwards. The sender correctly does fast retransmits. The problem is that the bundled frames in frame 2272 don't get accepted.
The reason could be that the SCTP doesn't handle bundled DATA chunks correctly. But we tested it and it works at least in general. But what if the SCTP stack doesn't get the packet. The receiver is missing 5 packets. Could it be that the packet in frame 2272 is too long and screws up the receiver for a short while. That would explain, why frame 2272 and its retransmissions are dropped.

Is it possible to provide a tracefile from Firefox as described in
https://github.com/nplab/WebRTC-Data-Channel-Playground/wiki/Analyze-Data-Channel-traffic-with-Wireshark

Comment 7

[Nils Ohlmeier [:drno]]

I think Michael meant frame 2277, not 2272. But yeah that is the first frame which is bigger then 1294 bytes, and it appears to be the one where the trouble starts. I'll try to have quick look if I can find a similar buffer problem as with the DTLS layer.

Comment 8

[Nils Ohlmeier [:drno]]

This looks a little bit suspicious to me:
  https://dxr.mozilla.org/mozilla-central/source/netwerk/sctp/src/user_socket.c#3223
There we properly parse the length of the first SCTP header, but then:
  https://dxr.mozilla.org/mozilla-central/source/netwerk/sctp/src/user_socket.c#3226
and some following calls keep using the length information from the wire. My guess is we would need to put a loop around that code to be able to handle multiple SCTP packets coming in at once.

Should be fairly easy to test once bug 1214269 lands as the sctp_unittest uses the same transportlayerloopback code as the DTLS tests. So the extensions from bug 1214269 should allow us to add tests to the sctp_unittest for multiple SCTP packets as well.

Comment 9

[Michael Tüxen]

usrsctp_conninput() only expects a single SCTP packet containing of a common header and a number of chunks.
That looks good to me.

Comment 10

[Randell Jesup [:jesup] (needinfo me)]

I'm on PTO, but IIRC we disabled PMTU because of problems getting errors back when we tried sending too-large packets through ICE/DTLS;check back on bugs (open and closed) that Michael is cc'd on or look at hg logs from DataChannel.cpp

Comment 11

[Randell Jesup [:jesup] (needinfo me)]

See bug 1194817 where we disabled PMTU

I suspect a similar issue, where the packets aren't passing through the ICE stack due to limits in the ICE implementation.

Comment 12

[Randell Jesup [:jesup] (needinfo me)]

Confirming, -> Networking
We may well end up an mtransport/ICE bug for large-packet support

Comment 13

[Bulk Bug Changes for mreavy's org]

Mass change P2->P3 to align with new Mozilla triage process.

Comment 14

[Randell Jesup [:jesup] (needinfo me)]

Nils, is this fixed?

Comment 15

[Nils Ohlmeier [:drno]]

Declaring NI bankruptcy.