hpts: remove call into TCP HPTS from userret()

[glebius]

Summary

This hack introduced in d7955cc0ffdf and e3cbc572f154 proved to have more
ill side effects than benefits. Sorry for that.

Now the HPTS soft clock is called only after the LRO completion. Refactor
HPTS module linkage to address that and share the pointer only between
HPTS and LRO.

[kib]

sys/netinet/tcp_hpts.h

96

This 'extern' is not needed.

sys/netinet/tcp_lro.c

1261–1262

I suggest to read the pointer into a local var with atomic_load(), then test the local and call through it. Otherwise, compiler is allowed to compile this into two reads, first might see non-NULL pointer, and second could read NULL.

[glebius]

sys/netinet/tcp_lro.c

1261–1262

My code is a typical code pattern for loadable (but not unloadable) modules. The HPTS is just not safe to unload. It will fail to unload unless called as kldunload -f. However, Randall and others liked to load/unload it for development purposes. The atomic_store_ptr() at unload was added by them. If I add atomic here as you suggest, we will pretend even more that the module support unload, while it doesn't. If I add atomic(9) here, I will not make it safe. The pointer can be cleared and HPTS structs be freed right after we atomically checked the pointer and entered _tcp_hpts_softlock().

I would rather remove the atomic from tcp_lro_hpts_uninit() than pretend that we are carefully covering unload.

[glebius]

• Remove unneeded extern.
• Remove all comments that talk about userret().
• Remove atomics from module unload, they don't make it safe, just pretend.

[rrs]

sys/netinet/tcp_lro.c

1261–1262

Gleb:

I never unloaded (or loaded) HPTS as a module. I never liked the idea of making it a module. It IMO is a core
function in FreeBSD. So no I don't see it as something that should be ever unloaded and as far as I am concerned
it should just be compiled in :)

R

[glebius]

sys/netinet/tcp_lro.c

1261–1262

Thanks, Randall! Then my memory skewed the events. I don't remember for whom I made it unloadable (with -f). Anyway, I'd prefer to keep it this way and don't pretend it is unloadable in a safe manner neither make it forcibly static.

[kib]

I am fine with sys/sys and sys/kern changes, the network bits are up to you.

sys/netinet/tcp_lro.c

1261–1262

Having something available as module has nothing to do with basic or non-basic functionality. It is about making the user able to use some features in modular way, without enforcing the hassle of recompiling if a feature appeared to be needed, or in reverse, not needed.