User-Agent: Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1.4) Gecko/20070515 Firefox/2.0.0.4 Build Identifier: NSPR 4.6.5 NSPR does not currently support the SCTP transport protocol. While hacked versions of NSPR have been created to support SCTP, the goal of this enhancement request is to get SCTP accepted into NSPR. Reproducible: Always Steps to Reproduce: 1.(Not applicable for enhancement request) 2. 3. Actual Results: (Not applicable for enhancement request) Expected Results: (Not applicable for enhancement request)

I view bug 235681 as describing all the work that must be done to higher layers of mozilla to use the SCTP API, once it is implemented in NSPR. I look forward to your contribution.

Preethi Natarajan hacked Firefox 1.5 to run on SCTP, some time ago. I've gone through the exercise of duplicating that hack in Firefox 2.0.0.4, mostly to confirm that there have been no significant changes to how Firefox uses NSPR's transport layer services. The hack appears to work fine with no meaningful changes.

Preethi met with Wan-Teh a couple of weeks ago to talk about adding SCTP to NSPR, and some of her related work with SCTP. Wan-Teh agreed that we can use 2 of NSPR's reserved PRIOmethods funcitons to implement sctp_sendmsg and sctp_recvmsg. This is good news (as I don't see any good approach), and I'm proceeding down that path.

Correction to last post - This is good news (as I don't see any _other_ good approach)...

(In reply to comment #3) > Preethi met with Wan-Teh a couple of weeks ago to talk about adding SCTP to > NSPR, and some of her related work with SCTP. Wan-Teh agreed that we can use 2 > of NSPR's reserved PRIOmethods funcitons to implement sctp_sendmsg and > sctp_recvmsg. This is good news (as I don't see any good approach), and I'm > proceeding down that path. > Would this work on Linux only (since it's the only mainstream OS that has builtin support for it), or would it be cross-platform ?

(In reply to comment #5) > Would this work on Linux only (since it's the only mainstream OS that has > builtin support for it), or would it be cross-platform ? > The idea is to have cross-platform support. I doubt it will be accepted into NSPR without it. The initial work will be done on FreeBSD since that is what we have the most experience with, and because (as far as I'm aware), the Kame SCTP stack for FreeBSD is the most complete kernel space implementation available. The Kame SCTP stack is included in the distribution as of FreeBSD 7.0. I have an unofficial list of systems that NSPR supports, but I haven't yet made any effort to understand which of those have support for SCTP. As far as mainstream OS's go, Linux and Mac OS X (Kame stack works here), will certainly have to be on the list.

This is an update on the effort to add SCTP to NSPR. It is also a request for some guidance from the owners of NSPR and a request for feedback/input from any other interested parties. At this point I have duplicated much of the TCP functionality with similar SCTP functionality, and the changes are described below. Some of this functionality has been tested with a modified version of the socket.c test code, which sends and receives SCTP messages over different streams. I have added two new file descriptor types: PR_DESC_SOCKET_SCTP_1TO1 PR_DESC_SOCKET_SCTP_1TOMANY This was necessary because file descriptor types for sockets are specific to both the socket type and protocol, which precludes using a single file descriptor type for both 1-to-1 and 1-to-many style SCTP sockets. Functions added to NSPR include: PR_NewSCTPSocket() PR_NewSCTP1To1Socket() PR_NewSCTP1ToManySocket() PR_OpenSCTPSocket() PR_OpenSCTP1To1Socket() PR_OpenSCTP1ToManySocket() PR_Sctp_Sendmsg() PR_Sctp_Recvmsg() The last two functions include the same timeout argument that is used in functions such as PR_Send() and PR_Recv(), so that polling is handled in the same way, however the byte counting used in PR_Send() is not included in PR_Sctp_Sendmsg() because SCTP preserves message boundaries and only delivers complete messages. I have added the SCTP options: PR_SockOpt_Sctp_Default_Send_Param PR_SockOpt_Sctp_Use_Ext_RcvInfo PR_SockOpt_Sctp_Events PR_SockOpt_Sctp_InitMsg I believe that this is most of the functionality needed to support adding HTTP over SCTP capability to Firefox, but I would welcome any input on other functions/options that might be needed or desirable. I'd like to get some feedback from the owners of NSPR on one issue in particular. When I added sctp_sendmsg() and sctp_recvmsg() to the PRIOMethods structure I thought that there were only 4 reserved functions slots available (reserved_fn_0-3), however I later noticed that reserved_fn_5&6 are also available. Although I used reserved_fn_2&3 for sctp_sendmsg() and sctp_recvmsg(), it would be a simple task to change over to using reserved_fn_5&6 instead. This would allow the last four entries in the PRIOMethods structure to remain reserved (reserved_fn_0-3). Please let me know if you have a preference.

Jonathan: Thanks for the status update. It is fine to add two new file descriptor types for SCTP if you need to use two PRIOMethods to implement the SCTP I/O functions. I just checked our source code and it seems that the file descriptor type is only used by NSPR to select an appropriate PRIOMethods. You don't need to add new PR_NewXXXSocket() functions. The existing PR_NewXXXSocket() functions are hardcoded to use IPv4. We prefer the PR_OpenXXXSocket() functions, which can use either IPv4 or IPv6. PR_Sctp_Sendmsg() and PR_Sctp_Recvmsg() don't follow NSPR's naming convention for functions. Ideally they should be named PR_SendMsg() and PR_RecvMsg(). Can the prototypes of these two functions be defined to be protocol-neutral? I can't comment on what other functions or options might be needed because I am not familiar with SCTP. You can use reserved_fn_5&6. They used to be PR_GetSockOpt and PR_SetSockOpt, which were removed in NSPR 4.0. Since we're already at NSPR 4.6.x, it is safe to use these two reserved fields.

Wan-Teh: Thanks for the helpful comments. I think I still need your input regarding the PR_Sctp_XxxxMsg() function names (explained below). > It is fine to add two new file descriptor types for SCTP if > you need to use two PRIOMethods to implement the SCTP I/O > functions. I just checked our source code and it seems > that the file descriptor type is only used by NSPR to select > an appropriate PRIOMethods. SCTP supports both a 1-to-1 style connection and 1-to-many style connection. They both use the IPPROTO_SCTP protocol, however the 1-to-1 style uses the SOCK_STREAM type, while the 1-to-many style uses the SOCK_SEQPACKET type. Since both the type and protocol are fixed for a given file descriptor, two new file descriptors were needed. We actually have no plans to use the 1-to-many style connection, so I'll remove that file descriptor and its PRIOMethod. > You don't need to add new PR_NewXXXSocket() functions. The > existing PR_NewXXXSocket() functions are hardcoded to use IPv4. > We prefer the PR_OpenXXXSocket() functions, which can use either > IPv4 or IPv6. I'll remove the PR_NewXXXSocket() functions. > PR_Sctp_Sendmsg() and PR_Sctp_Recvmsg() don't follow NSPR's > naming convention for functions. Ideally they should be named > PR_SendMsg() and PR_RecvMsg(). Can the prototypes of these > two functions be defined to be protocol-neutral? The function names were based on the SCTP API system calls, which are sctp_sendmsg() and sctp_recvmsg(). I thought that the PR_Sctp_SendMsg() and PR_Sctp_RecvMsg() function names might be a problem, but I also thought they might follow the naming convention. I avoided calling them PR_SendMsg() and PR_RecvMsg() because I didn't want anyone thinking that they were an implementation of the sendmsg() and recvmsg() system calls. The sendmsg() and recvmsg() functions exist independently of SCTP and are protocol-neutral, but they are typically not used because they are cumbersome (for SCTP). I chose the sctp_xxxxmsg() functions in order to provide the same ease-of-use for SCTP that send() and recv() provide for TCP. The send() and recv() functions are also protocol-independent, but they do not allow access the stream id number (or most other features of SCTP), and having that additional information is a necessity for effectively implementing HTTP over SCTP. I feel I need to implement the sctp_sendmsg() and sctp_recvmsg() systems calls even though they are not protocol-independent, but I can name them whatever is appropriate. Please let me know if you still prefer PR_SendMsg() and PR_RecvMsg(), or something else. > You can use reserved_fn_5&6. They used to be PR_GetSockOpt > and PR_SetSockOpt, which were removed in NSPR 4.0. Since > we're already at NSPR 4.6.x, it is safe to use these two > reserved fields. I'll change the code to use reserved_fn_5&6.

The file descriptor type for 1-to-many style SCTP sockets has been removed. All PR_NewXXXSocket() functions for SCTP sockets have been removed. I have replaced PR_Sctp_SendMsg() and PR_Sctp_RecvMsg() with PR_SctpSendMSg() and PR_SctpRecvMsg(), respectively. In the PRIOMethods structure, PR_SctpSendMsg() and PR_SctpRecvMsg() have replaced reserved_fn_5&6, and reserved_fn_0-3 are now the remaining four reserved functions.

This is an update to let interested parties know that I currently have working versions of NSPR with the SCTP API, for FreeBSD, Mac OS X/Intel, and Linux. I'm working on a Mac OS X/PPC version, as well as using pre-processor directives to combine all versions into a common source. I recently learned of an effort to bring the KAME stack to 32 bit Windows XP/Vista, and I'll be looking into that as well. I would like to add a build option to enable/disable the SCTP API, as well as configure checks for SCTP support. I've been reading up on the use of autoconf, etc., but the GNU build system is considerably more complex than anything I've used before. If anyone can provide some information on where and how NSPR defines such options and checks it would be very helpful.

I currently have a version of NSPR that supports SCTP on FreeBSD, Mac OS X Intel/PPC, and Linux. It includes a configure option to enable sctp support with checks for the presence and functionality of SCTP, as well as some test code. I'm working on extending support to Solaris, but the SCTP API on Solaris is far behind the current API spec. There may be a possibility of getting an update to OpenSolaris in the near term with better support for SCTP. Can someone tell me if NSPR "oficially" supports both Solaris and OpenSolaris?

Yes, NSPR officially supports both Solaris and OpenSolaris. For Solaris, we support Solaris 8 - Solaris 10 right now.

I now have a version of NSPR that supports SCTP on FreeBSD, Linux, Mac OS X (ppc/intel), Solaris, and OpenSolaris. I have also nearly completed work on a version that includes support for XP and Vista, though that work is currently on hold while the developer investigates a possible bug in the Windows SCTP kernel driver. I have not yet been able to get access to either an AIX or an HP-UX machine to work on those versions. As far as I am aware, other platforms supported by NSPR do not support SCTP. One dangling issue is that I'm having some trouble implementing machine dependent functions in a manner consistent with NSPR's approach. I've posted a question in the dev.tech.nspr newsgroup asking for help on how to do that properly. If anyone reading this can provide some help with that issue I'd be very grateful. I'd like to submit a patch for review/evaluation once I have the machine dependent function issue resolved, and I'm hoping to get some input from the NSPR owners on how that process works. My code is currently based on NSPR 4.7.3 and I'm wondering if I should update it to 4.8 beta2 or wait for 4.8 official before preparing a patch. - Jon

The code is now updated to NSPR 4.8 release, and I'm posting this patch for review. Run configure with --enable-sctp to enable SCTP. Note that I have not implemented NSPR versions of bindx() or connectx() as this would consume two of the four remaining unused pointers in the PRIOMethods table. These can be added in the future if there's sufficient interest.

I've update the code and am now working off the CVS trunk. This patch should be more in line with standard practice as it's based on the trunk (cvs diff -u8pN nsprpub/ > nsprpub_trunk_sctp.patch).

Jonathan, Your latest patch is almost 50% bigger than the previous one. Does it patch more files and more functions? Or is there some other reason why it grew so much? (Most I'm just curious)

Sorry for the late response Nelson. The patch is bigger because I inadvertently forgot to delete nsprpub/configure from my source code before creating the patch. That's the only difference, but it accounts for several thousand more lines in the patch. - Jon

I've made a few changes to the code based on some comments I received. The changes include fixing comments, improving consistency of style, and fixing a couple of errors. I'd like some guidance on how to handle the license block for a new file. I'm wondering what to put for "Original Code" and "Initial Developer". Does "Original Code" refer to just that file, or to NSPR as a whole? The "Initial Developer" depends on how I interpret that. If anyone has any comments please pass them along. I'll plan to post a new patch in a couple of days. - Jon

The most recent update to OpenSolaris (2009.06) contains a bug that causes getsocktop(SO_ERROR) to return "Option not supported by protocol" if the protocol is SCTP. This means that connections to non-blocking SCTP sockets that return EINPROGRESS ultimately fail because NSPR's polling loop for connect() calls getsockopt(SO_ERROR). I'm going to hold off posting another patch until I get some more feedback from the OpenSolaris folks.

I have started to review this patch (attachment 383528 [details] [diff] [review]). I'm not going to review it all at once, but rather in pieces. I won't give it an r+ or r- review flag until I'm done, but I can see already that it will be an r- due to a number of issues. Don't rush to create a new patch until I've gotten farther. When you do finally produce the next version of your patch, please remove all the "JTL - <date>" comments from it. Initially my comments will be about the overall approach, the way the code is injected into NSPR's existing structures. I won't be discussing details of code in individual functions as much as discussing the appropriateness of the API. Also, in some cases, I'll be asking questions whose answers might be apparent to me after reviewing the entire patch. I'll also be displaying my ignorance of SCTP, so bear with me. File nspr.def: NSPR uses a "versioned API". It is only changed by addition. When new public functions are added to the API, they are added as part of a new version. Each version incorporates all the previous versions by reference. Once a version is released, it is not changed thereafter. Only new versions are added. If you look over the entire file, you will see its structure, and will intuit how new versions are added. This patch adds new functions into the declaration of an old version of the API, instead of adding a new version of the API. You'll have to redo your patch to this file to add a new version. file prio.h: Struct PRFileDesc is intended to be absolutely free of any members that are only meaningful for some types of files. No file-type-dependent members here. All File-type-dependent members belong in the PRFilePrivate struct, whose definition is file type dependent, and whose address is found in the PRFileDesc. If you want/need to define a new public struct as part of the API for this file type, It should still not be in the PRFileDesc. The comments above are pretty certain, not likely negotiable. The comments below are more exploratory, questioning why things are. If I question something that you believe is important, please defend it. The patch adds numerous manifest constants to the PRSockOpt enumeration and numerous members to the struct PRSocketOptionData. Some of these appear at first glance to be duplicative of existing constants/members. For example: PR_SockOpt_NoDelay PR_SockOpt_Sctp_NoDelay PR_SockOpt_MaxSegment PR_SockOpt_Sctp_MaxSegment Does SCTP really have its own definitions of these things that conflict with the existing definitions? I'd rather that we reuse existing definitions where there is no conflict. I really don't want two ways to set NoDelay if it's really the same thing in both cases. If the values are fundamentally the same, but the underlying OS nonetheless requires that they be handled differently (e.g. different sockopts), then that difference should be handled in NSPR's IOMethods code, and hidden from the application where possible. Some of the new PRSockOpt constants seem very similar in name to other functions that accomplish similarly named purposes for other types of sockets. For example, the names PR_SockOpt_Sctp_Set_Peer_Primary_Addr and PR_SockOpt_Sctp_Set_Primary_Addr suggest functionality that seems similar to that provided by the functions connect and bind for UDP (datagram) sockets. Could those functions be used instead of new socket options? The structs named PRSctp_SetPrim and PRSctp_SetPeerPrim appear to be identical to each other. Is there any reason that a single struct cannot be used for both purposes? Isn't this struct effectively the equivalent of a PRNetAddr for SCTP? How confident are you that the underlying platforms' declarations of structs sctp_assoc_t and sockaddr_storage (and any other structs declared in <netinet/sctp.h> ) will be the same on all platforms? Unless the answer is "VERY VERY SURE", these potentially platform dependent structs should be replaced by structs declared in NSPR headers. Why does PR_SctpRecvmsg need a fromlen argument to go with a PRNetAddr, and why does PR_SctpSendmsg need a tolen argument? PR_SendTo and PR_RecvFrom also use PRNetAddrs, and they manage to avoid using an explicit length argument, even on BSD-ish systems that have lengths in their underlying sockaddr structures. This protects applications from having to behave differently on platforms where sockaddrs have lengths than on platforms where they don't. Can something similar be done for SCTP? Or is the result too awful to imagine? The patch adds 3 function declarations to 4 "md" headers. The new declarations appear to be identical in all 4 headers. They don't seem to be machine-dependent declarations. Do they really belong in md headers? That's enough for now. More to follow, hopefully before too long.

Thanks very much for starting to look at this Nelson. JTL comments will now be removed before a patch is created. File nspr.def: mods moved to correct location. File prio.h: filetype dependent mods moved to PRFilePrivate in primpl.h SCTP_NODELAY SCTP does in fact define the socket options SCTP_NODELAY and SCTP_MAXSEG, but I was unsure about whether to implement these in a way that programmers familiar with SCTP would expect, or to merge them under the existing TCP implementations. Using the existing PR_SockOpt_NoDelay would be straightforward since TCP_NODELAY and SCTP_NODELAY both take a boolean argument (though they are different socket options). When you say "NSPR's IOMethods" code, do you mean the implementations in priometh.c? The implementations in priometh.c aren't used for pthreads, so they aren't used for SCTP (unless the Windows SCTP kernel extension ever gets fixed), so I would implement this in pt_GetSocketOption and pt_SetSocketOption (both in ptio.c). Is there somewhere else I should do this? SCTP_MAXSEG Unlike TCP_MAXSEG which takes an int, SCTP_MAXSEG takes an sctp_assoc_value structure. The sctp_assoc_value is needed for 1-to-many associations, but I have only implemented 1-to-1 associations. This means that we could implement SCTP_MAXSEG under the existing implementation of TCP_MAXSEG, but if we ever add support for 1-to-many associations, we'll have to change the SCTP_MAXSEG implementation in a way that would break code based on the previous implementation. I'm pretty sure we don't want to do that, so giving the SCTP_MAXSEG its own implementation makes sense to me. I'll get rid of PR_SockOpt_SCTP_NoDelay, but leave PR_SockOpt_Sctp_MaxSegment in, if I don't hear back from you on this issue. PR_SockOpt_Sctp_Set_Peer_Primary_Addr PR_SockOpt_Sctp_Set_Primary_Addr An SCTP association binds to a set of IP addresses on each end, one of which is considered the primary address. In normal SCTP (i.e., without any experimental extensions), the additional addresses are only used for failover in the event that the primary address fails. Without a failure, transfer is done between the two primary addresses. PR_SockOpt_Sctp_Set_Peer_Primary_Addr requests that the peer use a particular local address as the primary address of the local host. PR_SockOpt_Sctp_Set_Primary_Addr requests that the local SCTP stack use a particular peer address as the peer's primary address. These operations can't be done with connect and bind. PRSctp_SetPrim PRSctp_SetPeerPrim These structures are identical, as you pointed out. I've changed prsctp.h to use a single struct for them. sctp_assoc_t I'm not at all confident that this structure is the same on all platforms, but like the pthread_t structure, it is opaque, so like the pthreads implementations in NSPR, I think we have to use it. sockaddr_storage I don't know how likely it is that this structure will be the same on all platforms, so I need to look into this more, and probably define an NSPR structure that's isomorphic. PR_SctpSendmsg PR_SctpRecvmsg The SCTP API for sctp_sendmsg() and sctp_recvmsg() specifies the tolen and fromlen parameters, with fromlen being an in/out parameter. I'm not sure why, given that the to and from parameters are always struct sockaddr*. I'll look into that. "md" headers The three functions all have slightly different implementations based on slight differences in how closely each platform adheres to the SCTP sockets API ID. Should I move the headers somewhere else?

Oops - PRSctp_SetPrim and PRSctp_SetPeerPrim are slightly different. The members of PRSctp_SetPrim have a prefix of ssp_, while the members of PRSctp_SetPeerPrim have a prefix of sspp_. This follows the specification in the SCTP sockets API ID.

Comment on attachment 383528 [details] [diff] [review] Patch to add SCTP support to NSPR 4.8, based on CVS trunk Due to the personal circumstances in which the members of Sun's NSS/NSPR team presently find ourselves, we/I will be unable to complete this review in a timely fashion. Perhaps one of the other NSPR peers/owners can help. Sorry. :(

If anyone has any information on the status of this patch review (such as who might review it and when), I'd like very much to know. I understand that the situation at Sun has made things more difficult, but the last input from a reviewer was nearly four months ago, and I'm anxious for the patch to get some attention. Thanks.

Jonathan: I will try to review your patch. Sorry about the delay.

Thanks very much Wan-Teh. The patch is based on the trunk as of 2009-06-16. Let me know if you'd like me to update it to the current trunk. The only change I've made to the code since then is to convert PR_MSG_PEEK to MSG_PEEK in pt_Sctp_Recvmsg(), in nsprpub/pr/src/pthreads/ptio.c

Comment on attachment 383528 [details] [diff] [review] Patch to add SCTP support to NSPR 4.8, based on CVS trunk Hi Jonathon, I took a quick look at this patch last Wednesday night. I only looked at the changes to the public headers. 1. nsprpub/configure In the future please omit the changes to nsprpub/configure in your patches for code review because they are generated from changes to nsprpub/configure.in and are usually very long because of the line number changes. 2. nsprpub/configure.in Please remove the "JTL - mm/dd/yy" comments throughout your patch. 3. nsprpub/pr/include/prio.h > struct PRFileDesc { > const PRIOMethods *methods; /* the I/O methods table */ > PRFilePrivate *secret; /* layer dependent data */ > PRFileDesc *lower, *higher; /* pointers to adjacent layers */ > void (PR_CALLBACK *dtor)(PRFileDesc *fd); > /* A destructor function for layer */ > PRDescIdentity identity; /* Identity of this particular layer */ >+ >+/* >+ * JTL - 11/28/07, per PN, adding variables for stream id bookeeping. This is >+ * done specifically to support an existing implementation (hack) of HTTP over >+ * SCTP in firefox, however the whole approach to supporting HTTP over SCTP in >+ * firefox needs to be re-thought, and it is expected that this will not be the >+ * way that stream bookkeeping is handled. >+ */ >+#if defined(ENABLE_SCTP) >+ PRUint16 NumberOfSctpStreams; /* Number of SCTP streams available for >+ * sending GETs. MIN(op, ip) >+ * since a response to a GET comes back >+ * on the same stream ... >+#endif > }; We can't add new fields to the PRFileDesc structure. These fields need to be added to the PRFilePrivate structure for the NSPR layer (pointed to by the 'secret' field of PRFileDesc). The PRFilePrivate structure for the NSPR layer is defined in primpl.h: http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/nsprpub/pr/include/private/primpl.h&rev=3.91&mark=1717#1717 >+ PR_SockOpt_Sctp_Default_Send_Param, /* set default outgoing paramaeter values */ >+ PR_SockOpt_Sctp_Use_Ext_RcvInfo, /* enable/disable extrcvinfo notifications */ >+ PR_SockOpt_Sctp_Events, /* enable/disable sndrcvinfo notifications */ >+ PR_SockOpt_Sctp_InitMsg, /* get/set INIT chunk parameters */ >+ PR_SockOpt_Sctp_AssocInfo, /* get/set/change association info */ >+ PR_SockOpt_Sctp_NoDelay, /* enable/disable Nagle algorithm */ >+ PR_SockOpt_Sctp_MaxSegment, /* get/set maximum fragment size */ >+ PR_SockOpt_Sctp_Status, /* get current state of association */ >+ PR_SockOpt_Sctp_Set_Peer_Primary_Addr, /* set primary address of peer */ >+ PR_SockOpt_Sctp_Set_Primary_Addr, /* set primary address */ Nit: the current style uses no underscores after PR_SockOpt_. > typedef enum PRDescType > { > PR_DESC_FILE = 1, > PR_DESC_SOCKET_TCP = 2, > PR_DESC_SOCKET_UDP = 3, > PR_DESC_LAYERED = 4, >- PR_DESC_PIPE = 5 >+ PR_DESC_PIPE = 5, >+ PR_DESC_SOCKET_SCTP_1TO1 = 6, /* JTL - 9/4/07 */ > } PRDescType; Nit: I find "1TO1" a little hard to read -- "1" looks like a capital I or a lowercase l with some fonts. I wonder if we should be verbose and use ONE_TO_ONE instead. 4. nsprpub/pr/include/prsctp.h >+/* Need to this to define the platform dependent opaque sctp_assoc_t structure. */ >+#include <netinet/sctp.h> Is it possible to avoid using the platform-specific sctp_assoc_t structure? Can we use PRNetAddr instead of struct sockaddr_storage?

(In reply to comment #28) Hi Wan-Teh - I very much appreciate you finding some time to look at this. My responses are below, and I'll follow up with a revised patch. > 1. nsprpub/configure > > In the future please omit the changes to nsprpub/configure Yes - this was done by mistake and shouldn't happen again. > 2. nsprpub/configure.in > > Please remove the "JTL - mm/dd/yy" comments throughout your Yes - I knew this was a problem and since last July I've been removing them before creating any patches. You should not see them again. > 3. nsprpub/pr/include/prio.h > > We can't add new fields to the PRFileDesc structure. All SCTP related fields added to PRFileDesc and PRFilePrivate are no longer needed and have been removed. > > >+ PR_SockOpt_Sctp_Default_Send_Param, /* set default outgoing paramaeter values */ > >+ PR_SockOpt_Sctp_Use_Ext_RcvInfo, /* enable/disable extrcvinfo notifications */ > >+ PR_SockOpt_Sctp_Events, /* enable/disable sndrcvinfo notifications */ > >+ PR_SockOpt_Sctp_InitMsg, /* get/set INIT chunk parameters */ > >+ PR_SockOpt_Sctp_AssocInfo, /* get/set/change association info */ > >+ PR_SockOpt_Sctp_NoDelay, /* enable/disable Nagle algorithm */ > >+ PR_SockOpt_Sctp_MaxSegment, /* get/set maximum fragment size */ > >+ PR_SockOpt_Sctp_Status, /* get current state of association */ > >+ PR_SockOpt_Sctp_Set_Peer_Primary_Addr, /* set primary address of peer */ > >+ PR_SockOpt_Sctp_Set_Primary_Addr, /* set primary address */ > > Nit: the current style uses no underscores after PR_SockOpt_. I've renamed all SCTP related socket options to conform to the current style. I do have an issue with SCTP_NODELAY and SCTP_MAXSEGMENT. PR_SockOpt_NoDelay and PR_SockOpt_MaxSegment already exist in NSPR, but they operate at the IPPROTO_TCP level rather than IPPROTO_SCTP. I have therefore retained the SCTP versions as PR_SockOpt_SctpNoDelay and PR_SockOpt_SctpMaxSegment. I could remove them and use the existing PR_SockOpt_NoDelay and PR_SockOpt_MaxSegment, but that would require checking PRDescType before calling get/setsockopt and changing the level from IPPROTO_TCP to IPROTO_SCTP on the fly. That seems pretty ugly. Do you have a suggestion for a better way to do this? > >+ PR_DESC_SOCKET_SCTP_1TO1 = 6, /* JTL - 9/4/07 */ > > Nit: I find "1TO1" a little hard to read -- "1" looks like a > capital I or a lowercase l with some fonts. I wonder if we > should be verbose and use ONE_TO_ONE instead. I think that's a good idea. I've changed 1TO1 to ONE_TO_ONE and 1To1 to OneToOne. > 4. nsprpub/pr/include/prsctp.h > > >+/* Need to this to define the platform dependent opaque sctp_assoc_t structure. */ > >+#include <netinet/sctp.h> > > Is it possible to avoid using the platform-specific sctp_assoc_t structure? I don't know how we can avoid it since sctp_assoc_t is a platform dependent opaque structure. It's like using the opaque structure pthread_t in the definition of NSPR's PThread structure (primpl.h). Do you have a suggestion for how I might work around it? > Can we use PRNetAddr instead of struct sockaddr_storage? I think this should work. I'll go ahead and make the change.

The patch has been updated per comment 29. In addition, all occurrences of 1to1 have been changed to one_to_one (only occurred in ptio.c). This patch includes the fix for the OpenSolaris bug mentioned in comment 20. The patch should support recent versions of FreeBSD, Linux, Mac OS X, Solaris, and OpenSolaris. Run configure with --enable-sctp option.