Closed Bug 1297418 Opened 6 years ago Closed 5 years ago

Update sctp library from upstream

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox51 --- wontfix
firefox59 --- fixed

People

(Reporter: jesup, Assigned: jesup)

References

Details

(Keywords: stale-bug)

Attachments

(4 files, 4 obsolete files)

Update sctp library from upstream, in prep for supporting sctp channel prioritization and pull callbacks
Rank: 15
Unchanged from last import; all other mods since then are in upstream
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
Attachment #8784000 - Flags: feedback?(tuexen)
Attachment #8784001 - Flags: review+
Attachment #8784000 - Flags: review+
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?
Flags: needinfo?(rjesup)
(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.
Will do.
(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.
Attachment #8783999 - Flags: review?(docfaraday) → review+
Michael - any info on a bug fix for that issue?
Rank: 15 → 17
Flags: needinfo?(tuexen)
Still working on it (I was side tracked by the day job...). Will let you know once it is stable...
Flags: needinfo?(tuexen)
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
You might want to wait until https://github.com/sctplab/usrsctp/issues/137 is resolved.
Remaining rollup patches after update to d5f916d2a42606a7660384e8cbc9a05a933815b4 from upstream repo
(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
Blocks: 1384292
Mass change P1->P2 to align with new Mozilla triage process
Priority: P1 → P2
Still WIP; will need to update to a later upstream rev
all remaining non-upstream changes (very little)
Attachment #8784000 - Attachment is obsolete: true
Attachment #8784000 - Flags: feedback?(tuexen)
Attachment #8784001 - Attachment is obsolete: true
Attachment #8841059 - Attachment is obsolete: true
Attachment #8913869 - Flags: review+
minor tweak to add the android patch to the repo as we do for other imports; patch in a separate queue entry
Attachment #8932691 - Flags: review?(docfaraday)
Attachment #8783999 - Attachment is obsolete: true
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.
Attachment #8932691 - Flags: review?(docfaraday) → review+
Pushed by rjesup@wgate.com:
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
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: mozilla59 → ---
Pushed by rjesup@wgate.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e311eb750f43
reland SCTP library update to rev 0e076261b rs=jesup,drno r=tuexen,drno
https://hg.mozilla.org/mozilla-central/rev/e311eb750f43
Status: REOPENED → RESOLVED
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Depends on: 1445448

Clearing old ni.

Flags: needinfo?(rjesup)
You need to log in before you can comment on or make changes to this bug.