[sctp fix] Re: CVS commit: src/sys/kern

[Maxime Villard <max%m00nbsd.net@localhost>]

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