Re: [sctp fix] Re: CVS commit: src/sys/kern
Le 28/04/2020 à 09:16, Luke Mewburn a écrit : On 20-04-26 18:15, Maxime Villard wrote: | - There was no demonstrated use-case justifying importing it. In addition, |major OSes like Windows and macOS do not implement SCTP. There just is no |demand for SCTP on the market; and on NetBSD, proportionally even less. SCTP is used in mobile telco environments; the control plane for 3G and 4G networks uses SCTP (or TCP as an option, but mostly SCTP). That may be true. I am mostly aware of the multimedia use cases. Le 01/05/2020 à 18:46, m...@netbsd.org a écrit : We can setup an equivalence: put as much effort into the SCTP removal proposal as there was for the SCTP introduction proposal. Since SCTP was just dropped in src without any prior discussion, I don't think we need any discussion for removing it. That would be fair, yes. Le 02/05/2020 à 13:55, m...@netbsd.org a écrit : I'm sorry for picking on SCTP in particular. Apparently it was added because it was listed in src/doc/roadmaps.networking (and it's still listed there). But this doesn't address your own point, does it. The one-liner in src/doc/roadmaps/networking gives no explanation on why we would want SCTP in the first place. It doesn't even say where the code was imported from. This brings the question of who, exactly, made this list. Several of the items on this wanted list are actively *not* wanted, as Christos noted in five of his "Comment[christos]". At this point there is no doubt that SCTP in the NetBSD kernel has no future. The only viable option I see is usrsctp: https://github.com/sctplab/usrsctp A userland version of the code, but portable, and actively maintained. While here, notice the crazy buffer overflows that were fixed in it and are still present in the NetBSD SCTP kernel code... Adds to my point, that the code is of extremely poor quality. Maxime
Re: [sctp fix] Re: CVS commit: src/sys/kern
On Fri, May 01, 2020 at 04:46:36PM +, m...@netbsd.org wrote: > We can setup an equivalence: put as much effort into the SCTP removal > proposal as there was for the SCTP introduction proposal. > > Since SCTP was just dropped in src without any prior discussion, I don't > think we need any discussion for removing it. I'm sorry for picking on SCTP in particular. Apparently it was added because it was listed in src/doc/roadmaps.networking (and it's still listed there).
Re: [sctp fix] Re: CVS commit: src/sys/kern
We can setup an equivalence: put as much effort into the SCTP removal proposal as there was for the SCTP introduction proposal. Since SCTP was just dropped in src without any prior discussion, I don't think we need any discussion for removing it.
Re: [sctp fix] Re: CVS commit: src/sys/kern
On 20-04-26 18:15, Maxime Villard wrote: | - There was no demonstrated use-case justifying importing it. In addition, |major OSes like Windows and macOS do not implement SCTP. There just is no |demand for SCTP on the market; and on NetBSD, proportionally even less. SCTP is used in mobile telco environments; the control plane for 3G and 4G networks uses SCTP (or TCP as an option, but mostly SCTP). NetBSD is a server OS; and could be viable in that market, which currently is mostly Linux (AFAICT), which does have a viable SCTP stack. It's understandable why macOS (a client OS) doesn't support SCTP. Besides the rest of your arguments (which may be valid), this particular one is not. regards, Luke.
[sctp fix] Re: CVS commit: src/sys/kern
Le 26/04/2020 à 16:21, Jonathan A. Kollasch a écrit : > Module Name: src > Committed By: jakllsch > Date: Sun Apr 26 14:21:14 UTC 2020 > > Modified Files: > src/sys/kern: uipc_socket.c > > Log Message: > Implement SCTP bug fixes found by maxv@. > > Adding these seems to improve the SCTP situation. Yeah, thanks... I remember I had sent an email about these bugs, no one cared, so I just put big XXXs and left the broken thing as-is. That was more than a year ago. I remember also pointing out at some point the severe deficiencies of the SCTP code we have, with no one caring once again (not even me actually). In its current state (that is, the state it has been in ever since it was imported five years ago), our SCTP code is a near-perfect example of gigantic, buggy and useless bloat. Specifically, as far as I remember: - Last I checked, SCTP occupies, in number of lines of code, half of our IPv4 network stack. In other words, when it was imported, our kernel netinet stack suddenly doubled in size. I don't see how we will ever MP-ify all of that. - There was no demonstrated use-case justifying importing it. In addition, major OSes like Windows and macOS do not implement SCTP. There just is no demand for SCTP on the market; and on NetBSD, proportionally even less. - The code is of remarkably poor quality, with bugs in all directions, complex pieces of logic that seem rarely justified, and implementation errors like the pointer bugs you just fixed. The bloat and bugs are already evident as early as in the first twenty lines of code of the sctp_input() entry point. - IIRC the mbuf API usage is very poor (though I don't remember the specifics here; I must have kept paper notes somewhere). - It seems that no one is maintaining it? Nothing has improved over the last five years, and there are no apparent signs that this situation will ever change. There are piecemeal changes like yours and mine, to accommodate API changes and fix obvious bugs, and that's about it, as far as I can tell. At one point I hesitated about doing a big cleanup of SCTP. But I concluded that it is too structurally broken, and that fixing it is a waste of time; rewriting it would be faster and would result in a better, more functional, and easier to maintain code. Overall I think this is an example of bad policy. It's good to have support for new protocols, but at some point, we need to question whether landing 40K lines of buggy yet critical kernel code out of nowhere with no one maintaining it and little justification for the feature is a good thing to do. CC'ing core@ in case they are interested in making a useful statement for once. In all cases, this code is disabled by default, which is a good thing given its state, but results in us not giving a damn about it. Maxime
