1464914 - WebRTC DataChannel unordered is not working if channel is negotiated

Status: RESOLVED FIXED

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

Description

[Mario Carbajal]

Attachment: negotiated.zip

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:60.0) Gecko/20100101 Firefox/60.0
Build ID: 20180516032328

Steps to reproduce:

I create a set of 3 negotiated datachannels of which some are created with {ordered: false}

I then transmit data through the unordered channels with packet loss between the two peers.


Actual results:

The datachannel behaves as if it was an ordered channel.


Expected results:

The datachannel should allow unordered messages to arrive.

I'm attaching relevant firefox logs obtained using these instructions: https://github.com/nplab/WebRTC-Data-Channel-Playground/wiki/Analyze-Data-Channel-traffic-with-Wireshark

In those logs the channel with label "uu" is supposed to be unreliable unordered yet it behaves as if it were reliable and ordered.

Comment 1

[Lennart Grahl [:lgrahl]]

It seems to me as if the `DATA_CHANNEL_FLAGS_WAITING_ACK` flag is incorrectly set for a negotiated channel: https://dxr.mozilla.org/comm-central/source/mozilla/netwerk/sctp/datachannel/DataChannel.cpp#2724. Randell, any thoughts?

Comment 2

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

Yes, probably the mFlags |= WAITING_ACK if statment should be inside the "if (!(mFlags & EXTERNAL_NEGOTIATED)) {" since we'll never get an ACK for an external negotiated channel

Comment 3

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Marking confirmed based on code inspection in comment 2.

Comment 4

[Victor Boivie]

Attachment: Bug 1464914: Relax sending ordered until OPEN_ACK

RFC8832: "However, before the DATA_CHANNEL_ACK message or any other message
has been received on a data channel, all other messages ... MUST be sent
ordered, ... After the DATA_CHANNEL_ACK or any other message has been received
on the data channel, messages containing user data MUST be sent ordered on
ordered data channels and MUST be sent unordered on unordered data channels."

Before this commit, it always expected DATA_CHANNEL_ACK, but as some clients
don't send it (spec violation, but that's how it is), it would then always send
ordered even on unordered channels.

What is mentioned above from the RFC shouldn't apply to pre-negotiated data
channels, as those don't explicitly send DATA_CHANNEL_OPEN/ACK. This commit
also ensures that those send unordered on unordered channels from the start.

Depends on D191299

Comment 5

[Byron Campen [:bwc]]

Try pushes:
https://treeherder.mozilla.org/jobs?repo=try&revision=853a0b8bc0377647ab1aebedd643652bdd3dbb83
https://treeherder.mozilla.org/jobs?repo=try&revision=0e1776995cb3f540ceada3a126307ed642f667aa

Comment 6

[Byron Campen [:bwc]]

Try looks fine.

Comment 7

[Victor Boivie]

I have manually verified that pre-negotiated channels can now send unordered SCTP data chunks;

Frame 43: 174 bytes on wire (1392 bits), 174 bytes captured (1392 bits) on interface Fake IF, text2pcap, id 0 (outbound)
Ethernet II, Src: Receive_00 (20:52:45:43:56:00), Dst: Send_00 (20:53:45:4e:44:00)
Internet Protocol Version 4, Src: 10.2.2.2, Dst: 10.1.1.1
Stream Control Transmission Protocol, Src Port: 5000 (5000), Dst Port: 5000 (5000)
    Source port: 5000
    Destination port: 5000
    Verification tag: 0x471dff3c
    [Association index: disabled (enable in preferences)]
    Checksum: 0x68a35404 [unverified]
    [Checksum Status: Unverified]
    DATA chunk (unordered, complete segment, TSN: 3124953040, SID: 113, SSN: 0, PPID: 53, payload length: 109 bytes)
Data (109 bytes)
    Data: <redacted>…
    [Length: 109]

Reproduced using http://meet.new - check e.g. data sent on SCTP stream ID 113 or 115.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ff8bbef7ae74
Relax sending ordered until OPEN_ACK r=bwc,jesup

Comment 9

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/ff8bbef7ae74