Allow SCTP to be build as a module

[markj]

Summary

This is not intended to be committed as-is and should be split into
several pieces. For instance:

• dtrace probes can be moved to sctp_kdtrace.c separately
• rt_addrmsg eventhandler can be added separately
• addition of the SCTP_SUPPORT option can be done separately

There are some XXX comments that I plan to address.

My goal is to enable removal of SCTP from our GENERIC kernels: I believe
there are relatively few users, and SCTP presents a relatively large
attack surface. Consider that IPSEC has more users and is similarly not
in GENERIC; one can set ipsec_enable="YES" in /etc/rc.conf and have it
automatically be loaded.

Test Plan

I tested tsctp and was able to pass traffic.

[markj]

sys/netinet/sctp_usrreq.c

95 ↗

(On Diff #63186)

I forgot to add the code to register SCTP for IPv6, will fix.

[tuexen]

I would suggest to get

• the generic fixes from above in head first
• then get the eventhandler stuff or the dtrace changes in as separate commits.

That reduces the patch set and really focusses on the changes related to allowing SCTP to be a module.

If you agree, you can choose if we should do the eventhandler or dtrace stuff first. I would like to test things for the kernel stack be getting it in.

I would prefer to add a file sctp_module.c or so which contains the module loading and unloading stuff instead of adding it to sctp_usrreq.c.
I guess the unloading of the module needs some changes. You can only unload the stack if there are no endpoints. Then you need to perform the teardown which also gets rid of zones as some timers.

sys/dev/xen/netback/netback.c

49 ↗

(On Diff #63186)

Can you get this committed, since it doesn't seem to be related to this change, but it more a general fix.

sys/modules/pf/Makefile

9 ↗

(On Diff #63186)

Can you get this committed, since it doesn't seem to be related to this change, but it more a general fix.

sys/netinet/sctp_crc32.h

36 ↗

(On Diff #63186)

Not sure why this is needed. We use this pattern in all SCTP related files. So why change it only here. If this is wrong, we need to change it separately in all files.

sys/netinet6/ip6_forward.c

41 ↗

(On Diff #63186)

Can you get this committed, since it doesn't seem to be related to this change, but it more a general fix.

sys/netinet6/sctp6_usrreq.c

60 ↗

(On Diff #63186)

I guess removing extern struct protosw inetsw[]; is a general fix. Should I run this upstream and commit it to head or do you prefer to commit it to head first and I will upstream it then.

sys/netpfil/pf/pf.c

45 ↗

(On Diff #63186)

The changes to this this files except the || defined(SCTP_SUPPORT) are a general fix. Can you commit them?

[tuexen]

sys/netinet/sctp_usrreq.c

176 ↗

(On Diff #63186)

I would prefer to have almost all this stuff in a separate file, sctp_module.c, for example.

[markj]

sys/netinet/sctp_crc32.h

36 ↗

(On Diff #63186)

I will just revert this hunk. __FBSDID is typically only used in .c files, but it doesn't really matter.

sys/netinet/sctp_usrreq.c

176 ↗

(On Diff #63186)

Ok

sys/netinet6/sctp6_usrreq.c

60 ↗

(On Diff #63186)

I'm not sure why I deleted it. It's not necessary for the rest of the changes, so please go ahead and commit it however you prefer.

[tuexen]

sys/netinet6/sctp6_usrreq.c

60 ↗

(On Diff #63186)

Committed in r353466

[tuexen]

I'll focus on the eventhandler changes...

[markj]

• Rebase
• Fix some XXX comments
• Fix handling of COMPAT32 syscalls
• Call ipproto_register() as well as pf_proto_register()

[tuexen]

I think the changes to in_kdtrace.c and sctp_kdtrace.c are not needed anymore, since they are in head.

[markj]

• Rebase on a more recent head revision
• Update kernel configurations

[markj]

Fix compilation guard for sopeeloff().

[tuexen]

Create a file sctp_module.c which handles all modules specific things which are FreeBSD specific. Also add sctp to the global list of modules.

[markj]

A next incremental step might be to add the SCTP_SUPPORT option to sys/conf, and commit the changes to convert ifdef SCTP to if defined(SCTP) || defined(SCTP_SUPPORT).

sys/netinet/sctp_module.c

1 ↗

(On Diff #63629)

The file is missing a license header.

75 ↗

(On Diff #63629)

Extra newline.

93 ↗

(On Diff #63629)

I think we could address this by keeping a set of 4 global flags which indicate whether the protocol tuple <socket type, domain> has been registered. Then we conditionalize deregistration on those flags.

[markj]

In D21997#483847, @markj wrote:

A next incremental step might be to add the SCTP_SUPPORT option to sys/conf, and commit the changes to convert ifdef SCTP to if defined(SCTP) || defined(SCTP_SUPPORT).

Michael, do you have any objection to my committing some pieces of this? I would really like to move this work along.

[tuexen]

In D21997#558809, @markj wrote:

In D21997#483847, @markj wrote:

A next incremental step might be to add the SCTP_SUPPORT option to sys/conf, and commit the changes to convert ifdef SCTP to if defined(SCTP) || defined(SCTP_SUPPORT).

Michael, do you have any objection to my committing some pieces of this? I would really like to move this work along.

Right now I'm busy bug fixing, not adding features... Get it in as long as it works...

[markj]

• Updates after testing with VIMAGE jails.
• Don't modify kernel configs in this diff.

[markj]

I'll commit this in the next day or two if there are no objections, and start a mailing list thread before changing GENERIC.

[emaste]

sys/netinet/sctp_module.c

4 ↗

(On Diff #74220)

now 2020

[markj]

Update copyright year.