Page MenuHomeFreeBSD

sockets: make pr_shutdown fully protocol specific method
ClosedPublic

Authored by glebius on Jan 12 2024, 10:08 AM.
Tags
None
Referenced Files
F102777681: D43413.diff
Sun, Nov 17, 1:22 AM
Unknown Object (File)
Fri, Oct 18, 3:36 PM
Unknown Object (File)
Sep 9 2024, 8:24 AM
Unknown Object (File)
Sep 5 2024, 5:20 AM
Unknown Object (File)
Sep 2 2024, 12:02 AM
Unknown Object (File)
Aug 30 2024, 12:55 PM
Unknown Object (File)
Aug 3 2024, 6:00 PM
Unknown Object (File)
Aug 3 2024, 5:55 PM
Subscribers

Details

Summary

Disassemble a one-for-all soshutdown() into protocol specific methods.
This creates a small amount of copy & paste, but makes code a lot more
self documented, as protocol specific method would execute only the code
that is relevant to that protocol and nothing else. This also fixes a
couple recent regressions and reduces risk of future regressions. The
extended KPI for the new pr_shutdown removes need for the extra pr_flush
which was added for the sake of SCTP which could not perform its shutdown
properly with the old one. Particularly for SCTP this change streamlines
a lot of code.

Some notes on why certain parts of code were copied or were not to certain
protocols:

  • The (SS_ISCONNECTED | SS_ISCONNECTING | SS_ISDISCONNECTING) check is needed only for those protocols that may be connected or disconnected.
  • The above reduces into only SS_ISCONNECTED for those protocols that always connect instantly.
  • The ENOTCONN and continue processing hack is left only for datagram protocols.
  • The SOLISTENING(so) block is copied to those protocols that listen(2).
  • sorflush() on SHUT_RD is copied almost to every protocol, but that will be refactored later.
  • wakeup(&so->so_timeo) is copied to protocols that can make a non-instant connect(2), can SO_LINGER or can accept(2).

There are three protocols (netgraph(4), Bluetooth, SDP) that did not have
pr_shutdown, but old soshutdown() would still perform sorflush() on
SHUT_RD for them and also wakeup(9). Those protocols partially supported
shutdown(2) returning EOPNOTSUP for SHUT_WR/SHUT_RDWR, now they fully lost
shutdown(2) support. I'm pretty sure netgraph(4) and Bluetooth are okay
about that and SDP is almost abandoned anyway.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

I like the changes. Once this hits the tree, I need to upstream it. This might result in undoing some whitespace changes, where you insert line breaks to enforce a 80 character line limit and I might use KASSERT instead of MPASS, since I think I don't use MPASS in the userland stack. But I can deal with this.
I really like making shutdown() protocol specific. This allows discussing TCP or SCTP specific behavior. In some case, I think, a different behavior in some error cases would make sense. But I always did not start discussing it, since this would have touched several protocols.

This revision is now accepted and ready to land.Jan 13 2024, 1:39 PM