Fix TFO pending counter leaks

[pkelsey]

Summary

The issue is described in Bug 213424 (https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=213424).

This patch is based on the one originally discussed in D8218.

The fix implemented here handles all of the necessary TFO-failure corrections to the pending counter in a single place in syncache_add().

The tfo_done label has been renamed to tfo_socket_created for clarity.

[jtl]

Despite some nits with the comments/label, this functionally works. But, those are non-functional nits, so I think its OK to go in "as is" if the submitter wants to commit it.

sys/netinet/tcp_syncache.c

1508 ↗

(On Diff #21331)

This is not strictly true. syncache_tfo_expand() may fail to create a socket, and the code path doesn't cross here.

1514 ↗

(On Diff #21331)

This label is slightly misleading, since syncache_tfo_expand() may fail to create a socket, but follows the "goto" path to here.

[pkelsey]

Adjusted label names and comments

I changed the label name in syncache_add() to tfo_expanded, as
tfo_socket_created is misleading in the expand failure case (as jtl
pointed out), and tfo_expanded emphasizes that it is only used if we
make it to syncache_tfo_expand() (as opposed to being a more general
purpose destination during TFO processing).

I also changed the new_tfo_socket label in tcp_input() to
tfo_socket_result as sometimes you have a NULL socket at the point it
is used.

The new comments should now be correct.

I updated the comment at the top of syncache_add() to focus on
describing the TFO ACKed-data exception case (the original intent)
without attempting to describe any of the detailed mechanics of when
that path is followed (a distraction and maintenance hazard).

I did try to find a way to truly collapse all the failure-case pending
counter decrements to one location by moving the TFO socket creation
failure decrement out of syncache_tfo_expand(), but I couldn't come up
with an approach that resulted in an overall simplification.

[jtl]

In D8235#171283, @pkelsey wrote:

Adjusted label names and comments

Thanks!

I did try to find a way to truly collapse all the failure-case pending
counter decrements to one location by moving the TFO socket creation
failure decrement out of syncache_tfo_expand(), but I couldn't come up
with an approach that resulted in an overall simplification.

That's OK. It isn't necessary. This approach is fine.

sys/netinet/tcp_syncache.c

1157 ↗

(On Diff #21369)

This is missing some important information from the previous comment ("a new socket is created and returned via lsop" (most of the time, at least) and "1 is returned to indicate the TFO-socket-creation path was taken").

From my perspective, its not necessary to modify this comment -- certainly it isn't necessary for this change. But, you are, of course, welcome to do so if you feel it would be clearer.

[jtl]

As before, its fine "as is". Feel free to address/ignore my nit as you see fit.