Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SCTP SSN handling error #111

Closed
ohcool9898 opened this issue Nov 28, 2016 · 10 comments
Closed

SCTP SSN handling error #111

ohcool9898 opened this issue Nov 28, 2016 · 10 comments

Comments

@ohcool9898
Copy link

I try to send a live streaming through webrtc data channel to browser, but the browser will fail to receive the streaming data 20 minutes later. In the sctp layer, the browser keeps sending ack indicating that it has received no more data. After close investigation, I find out that the problem results from the false variable declaration in function "sctp_queue_data_to_stream"(in usrsctplib/netinet/sctp_indata.c). Defined in rfc 4960, the field for Stream Sequence Number(SSN) is a 16-bit unsigned digit, but 32-bits here.

@tuexen
Copy link
Member

tuexen commented Nov 28, 2016

That is intentional, since we need a 32-bit value when using the I-DATA extension. So the plan is that we are passing around 32-bit values, but do 16-bit arithmetic in case we are not using this extensions.
There might be bugs there... Can you tell me how to reproduce the problem? Just transferring 100000 messages doesn't produce the problem...

@ohcool9898
Copy link
Author

ohcool9898 commented Nov 28, 2016

I was trying to send a sequence of sctp messages, and each sctp messge is about 1000 bytes with itself a complete message(the b bit and e bit are set, ref: https://tools.ietf.org/html/rfc4960#section-3.3.1). After we send more than 65535 messages, the problem happens. (The statement
nxt_todel = strm->last_sequence_delivered + 1;`) will cause the problem.
(Ex: strm->last_sequence_delivered = 65535, then the value of nxt_todel will be 65536, but the value range of control->sinfo_ssn(SSN) is 0 ~ 65535. Therefore, the incoming messages afterwards will be queued in the buffer(by the function call "sctp_place_control_in_stream") till the buffer is over the preset limit.).
What I encounter is that as the buffer size is over "sctp_max_chunks_on_queue" then the incoming messages will all be dropped.

@tuexen
Copy link
Member

tuexen commented Nov 28, 2016

I saw that and I think you are right. We need to detect if the comparison is needed to be based on uint16_t or uint32_t. I just want to reproduce the issue to be able to test the fix.
I can't reproduce this with the kernel stack. Since you are using the userland stack, are you using the callback API?

@tuexen
Copy link
Member

tuexen commented Nov 28, 2016

OK. I can confirm that there is a bug, I was able to reproduce the problem. Will fix it...

@ohcool9898
Copy link
Author

Thanks for your quick response.

@shiretu
Copy link
Contributor

shiretu commented Nov 29, 2016

Was about to report the same issue.

@shiretu
Copy link
Contributor

shiretu commented Nov 29, 2016

I have a full pcap file with the issue if that helps, but is ~320MB in size.

@tuexen
Copy link
Member

tuexen commented Nov 29, 2016

@shiretu I can reproduce the issue locally, not tracefile needed. Thanks for offering!

uqs pushed a commit to freebsd/freebsd-src that referenced this issue Dec 7, 2016
This made a couple of bugs visible in handling SSN wrap-arounds
when using DATA chunks. Now bulk transfer seems to work fine...
This fixes the issue reported in
sctplab/usrsctp#111

MFC after:	1 week


git-svn-id: svn+ssh://svn.freebsd.org/base/head@309682 ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f
uqs pushed a commit to freebsd/freebsd-src that referenced this issue Dec 7, 2016
This made a couple of bugs visible in handling SSN wrap-arounds
when using DATA chunks. Now bulk transfer seems to work fine...
This fixes the issue reported in
sctplab/usrsctp#111

MFC after:	1 week
tuexen added a commit that referenced this issue Dec 7, 2016
@tuexen
Copy link
Member

tuexen commented Dec 7, 2016

Please test this fix. I'm able to transfer several hundred GBs without any problem. This was failing before...

@shiretu
Copy link
Contributor

shiretu commented Dec 7, 2016

@tuexen: thank you sir! Will do so!

uqs pushed a commit to freebsd/freebsd-src that referenced this issue Dec 18, 2016
Cleanup the names of SSN, SID, TSN, FSN, PPID and MID.

This made a couple of bugs visible in handling SSN wrap-arounds
when using DATA chunks. Now bulk transfer seems to work fine...
This fixes the issue reported in
sctplab/usrsctp#111
bdrewery pushed a commit to bdrewery/freebsd that referenced this issue Jan 3, 2017
This made a couple of bugs visible in handling SSN wrap-arounds
when using DATA chunks. Now bulk transfer seems to work fine...
This fixes the issue reported in
sctplab/usrsctp#111

MFC after:	1 week


git-svn-id: svn+ssh://svn.freebsd.org/base/head@309682 ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f
dspinellis pushed a commit to dspinellis/unix-history-repo that referenced this issue Jan 9, 2017
Cleanup the names of SSN, SID, TSN, FSN, PPID and MID.

This made a couple of bugs visible in handling SSN wrap-arounds
when using DATA chunks. Now bulk transfer seems to work fine...
This fixes the issue reported in
sctplab/usrsctp#111
dspinellis pushed a commit to dspinellis/unix-history-repo that referenced this issue Jan 12, 2017
Cleanup the names of SSN, SID, TSN, FSN, PPID and MID.

This made a couple of bugs visible in handling SSN wrap-arounds
when using DATA chunks. Now bulk transfer seems to work fine...
This fixes the issue reported in
sctplab/usrsctp#111
dspinellis pushed a commit to dspinellis/unix-history-repo that referenced this issue Mar 25, 2017
Cleanup the names of SSN, SID, TSN, FSN, PPID and MID.

This made a couple of bugs visible in handling SSN wrap-arounds
when using DATA chunks. Now bulk transfer seems to work fine...
This fixes the issue reported in
sctplab/usrsctp#111
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants