Import libsctp from the FreeBSD implementation (already broken out as a user library). Contacts are Michael Tuexen, Randall Stewart (SCTP author) and Salvatore Loreto (of Ericsson); they're willing to make mods if we need them.

cc-ing Jonathan, who worked on bug 383180 and might be interested to know what we're doing here.

First-cut patch to import SCTP. To import sctp (with this patch applied): First, check out SCTP: # sctp usrlib source is available (via cvs) at: # :ext:anoncvs@stewart.chicago.il.us:/usr/sctpCVS # password: sctp The do the import: mkdir network/protocol/sctp/src/netinet mkdir network/protocol/sctp/src/netinet6 netwerk/protocol/sctp/update_sctp.sh dir_where_sctp_KERN_is hg qnew sctp_files There is one fix currently: line 5782 of netinet/sctputil.c needs to filter out linux as well (bug report sent upstream).

Note: there might eventually be a use for SCTP in general web-browsing; see http://www.eecis.udel.edu/~leighton/firefox.html http://www.eecis.udel.edu/~leighton/SPDY.html and http://www.eecis.udel.edu/~amer/PEL/leighton.movies/index.html

Oh, just to note: while they haven't finished packaging it up, the userspace libsctp should be BSD-licensed (per the norm, since it comes from FreeBSD).

updated integration patch - move to netwerk out of protocols

Apply the import patch attachment 608763 [details] [diff] [review] first, then attachment 608762 [details] [diff] [review]

many of these will go away with coming interface changes to userland libsctp

Apply base library, then "fixes and updates", then "build within the Mozilla tree". All of these should be basically identical between alder/default and paris_demo

These are the updated DataChannel drops for the new top-level SCTP API (with proper licensing, etc). This builds on Windows, but doesn't seem to run (there's fancy footwork going on to make it work on WinXP...).

May not actually be changed

Comment on attachment 649080 [details] [diff] [review] Import updated sctp library as of 2012/8/5 3:51am EDT Current tests (xpcshell raw_xpcshell_data.js) work fine with this change. Had to add a #include sctp_constants.h if SCTP_DEBUG is on for SCTP_DEBUG_ALL, etc Checked into alder: http://hg.mozilla.org/projects/alder/rev/f20dfda693fe

Comment on attachment 649213 [details] [diff] [review] Update sctp - adds SO_LINGER to socket options http://hg.mozilla.org/projects/alder/rev/5edc5a402d7f

This is an update of jesup's SCTP patch, these are all the changes to SCTP upstream I needed to get it to compile on Windows.

One additional change to jesup's DataChannel patch to fix the SCTP stdint issue on Windows. With both of these patches applied I can build netwerk/sctp on Windows.

current base patch for netwerk/sctp/src for m-c

current mods to usrsctp library (proposed upstream patch)

removed Makefile.in, which shouldn't have been included

Comment on attachment 656168 [details] [diff] [review] rebuilt import of usrsctp library Looking for reviews on the SCTP library portion of datachannels. There are 3 bugs here: the import itself, a patch intended for upstreaming (which they're already looking at), and a patch to make it build in our tree (Makefile.in's, configure.in, etc. The will come bug 729512 (DataChannel protocol), and bug 733002 (DataChannel DOM). Once they update the library to resolve the issues in the patch to their lib, we expect to continue with no mods to the imported files.

Comment on attachment 655519 [details] [diff] [review] build libsctp within Mozilla tree Patch applies to m-c with a few line offsets - build SCTP within the mozilla system. patch 3 of 3 to get the SCTP protocol building.

From Randall Stewart (SCTP author): The project is accessed via: svn co --username your.user.name https://sctp-refimpl.googlecode.com/svn/trunk sctpSVN If you are just wanting a read-only copy use svn co --username your.user.name http://sctp-refimpl.googlecode.com/svn/trunk sctpSVN

Comment on attachment 655519 [details] [diff] [review] build libsctp within Mozilla tree Adding ted to review the build system stuff for SCTP (which is most of this patch; patrick and biesi may want to defer on it to him)

Comment on attachment 655519 [details] [diff] [review] build libsctp within Mozilla tree +++ b/netwerk/sctp/sctp_update.log is it useful to have the references to your local directory in this file? +++ b/netwerk/sctp/src/Makefile.in +#EXPORTS_mozilla/net/netinet = \ Why's this commented out? Can you either add a comment stating why, or remove it? +DEFINES_Debug += \ + $(NULL) just remove this. (I'll note that DEFINES_debug below has lowercase debug...) +++ b/netwerk/sctp/update_sctp.sh +# sctp usrlib source is available (via cvs) at: +# :ext:anoncvs@stewart.chicago.il.us:/usr/sctpCVS +# password: sctp should be updated with the new URL you mentioned in the comment +# hand after this process, or use a more complex on. on -> one Can you add a README in netwerk/sctp that this is upstream library and mention its URL? I wish we had a better place for third-party libraries other than dumping them in random locations in our source tree, indistinguishable from our own code...

Comment on attachment 656168 [details] [diff] [review] rebuilt import of usrsctp library this is just the output of the update_sctp.sh script, unmodified from upstream, right? r=biesi if so

(In reply to Christian :Biesinger (don't email me, ping me on IRC) from comment #58) > Comment on attachment 655519 [details] [diff] [review] > build libsctp within Mozilla tree > > +++ b/netwerk/sctp/sctp_update.log > > is it useful to have the references to your local directory in this file? Nope. Revised script to say "Updated from SVN version XXXX on YYYYY"; edited import file > > +++ b/netwerk/sctp/src/Makefile.in > +#EXPORTS_mozilla/net/netinet = \ > > Why's this commented out? Can you either add a comment stating why, or > remove it? That was needed until they did a revision to clean up the "external" include (usrsctp.h). Removed. > > +DEFINES_Debug += \ > + $(NULL) > > just remove this. (I'll note that DEFINES_debug below has lowercase debug...) Cruft, removed. > +++ b/netwerk/sctp/update_sctp.sh > +# sctp usrlib source is available (via cvs) at: > +# :ext:anoncvs@stewart.chicago.il.us:/usr/sctpCVS > +# password: sctp > > should be updated with the new URL you mentioned in the comment Done. > +# hand after this process, or use a more complex on. > > on -> one Done, and added comments about how one would do that if needed. > Can you add a README in netwerk/sctp that this is upstream library and > mention its URL? > > I wish we had a better place for third-party libraries other than dumping > them in random locations in our source tree, indistinguishable from our own > code... Added. And I agree... or at least a standard nomenclature like "external/" or "imported/" (netwerk/sctp/src/imported or netwerk/sctp/imported/src, or imported/netwerk/sctp). But it makes little sense to do it for one module and not the entire tree. Another mass-rename/re-org, though I doubt this one will ever happen. Even a top-level file listing what we've imported, what the license is, what the update policy and procedure is, etc would be a HUGE improvement.... I may suggest that to Gerv.

Comment on attachment 656712 [details] [diff] [review] import of usrsctp library svn rev 8119 r=biesi Review of attachment 656712 [details] [diff] [review]: ----------------------------------------------------------------- Will carry forward r=biesi; pinging Gerv for license approval

Comment on attachment 656759 [details] [diff] [review] build libsctp within Mozilla tree +++ b/netwerk/sctp/sctp_update.log @@ -0,0 +1,8 @@ +sctp updated from CVS on Tue Mar 20 01:04:08 EDT 2012 +sctp updated from CVS on Fri Jun 1 01:31:43 EDT 2012 +sctp updated from CVS on Tue Jul 10 12:30:59 EDT 2012 +sctp updated from CVS on Thu Jul 12 00:31:32 EDT 2012 +sctp updated from CVS on Tue Jul 24 15:38:37 EDT 2012 +sctp updated from CVS on Sun Aug 5 03:51:50 EDT 2012 +sctp updated from CVS on Mon Aug 6 04:17:12 EDT 2012 +sctp updated to version 8119 from SVN on Wed Aug 29 22:06:23 EDT 2012 I'd just remove all the CVS lines; don't think the pre-hg import updates are relevant +svn co --username your.username http://sctp-refimpl.googlecode.com/svn/trunk sctpSVN fairly sure this one doesn't need a username

Comment on attachment 656759 [details] [diff] [review] build libsctp within Mozilla tree Review of attachment 656759 [details] [diff] [review]: ----------------------------------------------------------------- Is this patch against mozilla-central? If so you'll need some bits from my patch on bug 782936. r- for that and a few other things. ::: configure.in @@ +2934,5 @@ > MOZ_CHECK_HEADERS(sys/quota.h sys/sysmacros.h) > +MOZ_CHECK_HEADERS([linux/quota.h],,,[#include <sys/socket.h>]) > + > +dnl SCTP support - needs various network include headers > +MOZ_CHECK_HEADERS([linux/if_addr.h linux/rtnetlink.h],,,[#include <sys/socket.h>]) I'd just mash these two MOZ_CHECK_HEADERS lines together if they're both going to require sys/socket.h. @@ +2941,5 @@ > dnl NB - later gcc versions require -mmmx for this header to be successfully > dnl included (or another option which implies it, such as -march=pentium-mmx) > MOZ_CHECK_HEADERS(mmintrin.h) > > +dnl Check for sin_len and sin6_len - used by SCTP; only appears in Mac/*BSD generally This is basically the same diff hunk from bug 783843, but with sin6_len as well? @@ +2944,5 @@ > > +dnl Check for sin_len and sin6_len - used by SCTP; only appears in Mac/*BSD generally > +MOZ_CHECK_HEADERS(sys/types.h) > +AC_MSG_CHECKING(for sockaddr_in.sin_len) > +AC_CACHE_VAL(ac_cv_sockaddr_in_sin_len, AC_CACHE_CHECK, as per the other patch. ::: netwerk/Makefile.in @@ +25,5 @@ > ipc \ > $(NULL) > > +ifdef MOZ_SCTP > +PARALLEL_DIRS += sctp Just make this sctp/src and get rid of sctp/Makefile.in. ::: netwerk/sctp/Makefile.in @@ +15,5 @@ > + $(NULL) > + > +include $(topsrcdir)/config/rules.mk > + > +DEFINES += -DIMPL_NS_NET This Makefile doesn't do anything useful. Get rid of it. ::: netwerk/sctp/src/Makefile.in @@ +79,5 @@ > +ifeq ($(OS_TARGET),WINNT) > +DEFINES += \ > + -D__Userspace_os_Windows=1 \ > + -D_LIB=1 \ > + $(NULL) Two-space indent. @@ +80,5 @@ > +DEFINES += \ > + -D__Userspace_os_Windows=1 \ > + -D_LIB=1 \ > + $(NULL) > +CFLAGS += /I. /W3 I think you probably want to use -I. and -W3, but I'm not really sure why these are here anyway. @@ +84,5 @@ > +CFLAGS += /I. /W3 > +else > +ifeq ($(OS_TARGET),Darwin) > +DEFINES += \ > + -D__Userspace_os_Darwin=1 \ Here as well. @@ +94,5 @@ > +DEFINES += -D__Userspace_os_Linux=1 > +else > +ifeq ($(OS_TARGET),FreeBSD) > +DEFINES += \ > + -D__Userspace_os_FreeBSD=1 \ Two-space indent. @@ +98,5 @@ > + -D__Userspace_os_FreeBSD=1 \ > + -U__FreeBSD__ \ > + $(NULL) > +else > +#default_fallback; probably doesn't work I'd prefer that this error out in configure on unsupported platforms then instead of people hitting weird build errors in the SCTP code. ::: netwerk/sctp/src/README.sctp @@ +3,5 @@ > +The project is accessed via: > + > +svn co --username your.username https://sctp-refimpl.googlecode.com/svn/trunk sctpSVN > + > +If you are just wanting a read-only copy use "If you just want a read-only copy use" ::: netwerk/sctp/update_sctp.sh @@ +11,5 @@ > +# > +# also assumes we've made *NO* changes to the SCTP sources! If we do, we have to merge by > +# hand after this process, or use a more complex one. > +# > +# For example, one could update an sctp library import head, and merge back to default. Or keep a Extraneous tailing space here.

merged in bug 783843 (which is r=ted)

bug 787204 is the licensing (r+'d)

Comment on attachment 656179 [details] [diff] [review] Proposed upstream fixes to usrsctp library Rev 8176 has merged these all in

Should deal with all comments from Ted's review

Comment on attachment 659507 [details] [diff] [review] (and Bug 783843) build libsctp within Mozilla tree Review of attachment 659507 [details] [diff] [review]: ----------------------------------------------------------------- r=me with comments addressed. ::: configure.in @@ +2971,5 @@ > + [], > + [ac_cv_sockaddr_in6_sin6_len=true], > + [ac_cv_sockaddr_in6_sin6_len=false])]) > +if test "$ac_cv_sockaddr_in6_sin6_len" = true ; then > + AC_DEFINE(HAVE_SIN6_LEN) Is it an error if only one of HAVE_SIN[6]_LEN is set? I'd assume only a seriously broken toolchain would hit that. ::: netwerk/sctp/src/Makefile.in @@ +2,5 @@ > +# This Source Code Form is subject to the terms of the Mozilla Public > +# License, v. 2.0. If a copy of the MPL was not distributed with this > +# file, You can obtain one at http://mozilla.org/MPL/2.0/. > + > +DEPTH = ../../.. DEPTH = @DEPTH@ @@ +23,5 @@ > + > +EXPORTS_NAMESPACES = mozilla/net > + > +XPIDLSRCS = \ > + $(NULL) Why is this here? Just remove it. @@ +25,5 @@ > + > +XPIDLSRCS = \ > + $(NULL) > + > +CSRCS = \ nit: here and elsewhere you should use := if you're assigning something that can be immediately evaluated. @@ +58,5 @@ > + usrsctp.h \ > + $(NULL) > + > +LOCAL_INCLUDES = \ > + -I$(srcdir)/../../base/src \ This would read better starting from $(topsrcdir), so it's clear where it's pointing. @@ +97,5 @@ > + -D__Userspace_os_FreeBSD=1 \ > + -U__FreeBSD__ \ > + $(NULL) > +else > +#error Unsupported platform! #error is not a make thing. You want $(error). This is silly though, we should be testing platforms in configure, not here. I'd get rid of the nested ifeq/else chain entirely. ::: netwerk/sctp/src/README.sctp @@ +8,5 @@ > + > +svn co http://sctp-refimpl.googlecode.com/svn/trunk sctpSVN > + > +The usrsctp library is based on the FreeBSD kernel implementation, and the > +userspace implementation was largely done my Michael Tuexen and Irene typo: done by

https://hg.mozilla.org/integration/mozilla-inbound/rev/9fb243800b26 (usrsctp rev 8279) https://hg.mozilla.org/integration/mozilla-inbound/rev/d85136cf566a https://hg.mozilla.org/integration/mozilla-inbound/rev/059041691d7f

https://hg.mozilla.org/mozilla-central/rev/9fb243800b26 https://hg.mozilla.org/mozilla-central/rev/d85136cf566a https://hg.mozilla.org/mozilla-central/rev/059041691d7f