1297418 - Update sctp library from upstream

Status: RESOLVED FIXED

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

Description

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

Update sctp library from upstream, in prep for supporting sctp channel prioritization and pull callbacks

Comment 1

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

Attachment: Rework sctp update script to fetch from new repo location (git)

Comment 2

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

Attachment: update usrsctp to rev d1208ae from upstream rs=jesup

Comment 3

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

Attachment: rollup of sctp modifications for Android rs=jesup

Unchanged from last import; all other mods since then are in upstream

Comment 4

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

Comment on attachment 8784000 [details] [diff] [review]
update usrsctp to rev d1208ae from upstream rs=jesup

Just verifying this is a good rev to import

Comment 5

[Michael Tüxen]

Hi Randell,

I really appreciate that you want to update the userland stack.
The current status is:
* Support for the I-DATA chunk at the receiver side has been implemented.
* Support for the I-DATA chunk at the sender side has been implemented.
* Available schedulers do support user message interleaving.
* Support for the WFQ scheduler required for WebRTC is still missing.
* Support for explicit EOR on multiple streams on the sender side is still missing.

For supporting I-DATA, the data receive path has been rewritten and is used for DATA and
I-DATA chunks.
We did some stress testing and I'm aware of one bug: When sending a lot of large
un-ordered unreliable user messages using DATA chunks and have substantial packet loss (10 %),
the receiver aborts the data transfer due to a bug.
I personally would like to prefer to fix the issue before it hits released versions of FF.
The still missing features are not an issue, since they are not used by FF right now.
What do you think?

Comment 6

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=565724dfe80b

Comment 7

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

(In reply to Michael Tüxen from comment #5)

> We did some stress testing and I'm aware of one bug: When sending a lot of
> large
> un-ordered unreliable user messages using DATA chunks and have substantial
> packet loss (10 %),
> the receiver aborts the data transfer due to a bug.
> I personally would like to prefer to fix the issue before it hits released
> versions of FF.
> The still missing features are not an issue, since they are not used by FF
> right now.
> What do you think?

Right.  Once the bug is fixed let me know here (needinfo me and/or email).  It's easy for me to update the import.

Comment 8

[Michael Tüxen]

Will do.

Comment 9

[Michael Tüxen]

(In reply to Randell Jesup [:jesup] from comment #4)
> Comment on attachment 8784000 [details] [diff] [review]
> update usrsctp to rev d1208ae from upstream rs=jesup
> 
> Just verifying this is a good rev to import

Just clearing the "need info" flag" as I answered this request.

Comment 10

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

Michael - any info on a bug fix for that issue?

Comment 11

[Michael Tüxen]

Still working on it (I was side tracked by the day job...). Will let you know once it is stable...

Comment 12

[Michael Tüxen]

Hi Randall,

I think you can now integrate the master branch of usrsctp. The regression issues I'm aware of are resolved.

Best regards
Michael

Comment 14

[Michael Tüxen]

You might want to wait until https://github.com/sctplab/usrsctp/issues/137 is resolved.

Comment 15

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

Attachment: rollup of sctp modifications rs=jesup

Remaining rollup patches after update to d5f916d2a42606a7660384e8cbc9a05a933815b4 from upstream repo

Comment 16

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

(In reply to Michael Tüxen from comment #14)
> You might want to wait until https://github.com/sctplab/usrsctp/issues/137
> is resolved.

I've updated my import, but can do so again once that's available.  

Good timing for the report, though :-)

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4b751c28750a90bb97f0372cdfb99d60b69e98ae

Comment 17

[Emma Humphries ☕️🎸🧞‍♀️✨ (she/they) [:emceeaich] (Pacific Time) use needinfo]

This is an assigned P1 bug without activity in two weeks. 

If you intend to continue working on this bug for the current release/iteration/sprint, remove the 'stale-bug' keyword.

Otherwise we'll reset the priority of the bug back to '--' on Monday, August 28th.

Comment 18

[Nils Ohlmeier [:drno]]

Looks like we should at least take https://github.com/sctplab/usrsctp/commit/1313bd57676c1f22db222000059fc2e914ad94fa to fix bug 1384292 as well.

Comment 19

[Bulk Bug Changes for mreavy's org]

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

Comment 20

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

Attachment: update usrsctp to rev 7737b4a5 from upstream rs=jesup

Still WIP; will need to update to a later upstream rev

Comment 21

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

Attachment: rollup of sctp modifications for Android and a capitalization change rs=jesup

all remaining non-upstream changes (very little)

Comment 22

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

Attachment: Rework sctp update script to fetch from new repo location (git)

minor tweak to add the android patch to the repo as we do for other imports; patch in a separate queue entry

Comment 23

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

Attachment: small android patch to upstream sctp library rs=jesup

Comment 24

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

Green Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=db76da4f027af15351f575159765242c6c9cbf8d

Comment 25

[Byron Campen [:bwc]]

Comment on attachment 8932691 [details] [diff] [review]
Rework sctp update script to fetch from new repo location (git)

Review of attachment 8932691 [details] [diff] [review]:
-----------------------------------------------------------------

One simplification I would like to see, otherwise fine.

::: netwerk/sctp/update_sctp.sh
@@ +17,5 @@
>  # For example, one could update an sctp library import head, and merge back to default.  Or keep a
>  # separate repo with this in it, and pull from there to m-c and merge.
>  if [ "$1" ] ; then
>    export date=`date`
> +  export revision=`(cd $1; git log --pretty=oneline | head -1 | cut -c 1-40)`

"git rev-parse HEAD" seems to do the same thing.

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/148e39639161
Rework sctp update script to fetch from new repo location (git) r=bwc
https://hg.mozilla.org/integration/mozilla-inbound/rev/f955466975c1
small android patch to upstream sctp library rs=jesup
https://hg.mozilla.org/integration/mozilla-inbound/rev/8f1108e2c944
update usrsctp to rev 7737b4a5 from upstream rs=jesup

Comment 27

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/148e39639161
https://hg.mozilla.org/mozilla-central/rev/f955466975c1
https://hg.mozilla.org/mozilla-central/rev/8f1108e2c944

Comment 28

[Cristian Brindusan [:cbrindusan]]

Backed out for causing bug 1421963:
https://hg.mozilla.org/mozilla-central/rev/fed6b20cf2beeb05c4a8f0e49895ab45ce1d1392

Comment 30

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e311eb750f43
reland SCTP library update to rev 0e076261b rs=jesup,drno r=tuexen,drno

Comment 31

[Cosmin Sabou [:CosminS]]

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

Comment 32

[Dan Minor [:dminor]]

Clearing old ni.