done

Out goes CLOSE WAIT :)

commited

added close wait.. and fixed all my earlier errors :)

include close-wait

fix my earlier bobble :)

fix comment

Fix the compile bug and get the tested code in.

Updates per latest comments.

Sending ack's in general (ESTABLISHED) will of course not cause issues. But what concerns me
is that we *do* use acks to drive the various state-machines when we are in the front or back-states.
Can ack's driven by CC/ECN interfere .. I don't know.. is it protocol or implementation probably both.
I just think there needs to be careful thought on when the CC/ECN overrides and says "ACK" if
we are not in established.

In D55460#1269209, @tuexen wrote:

In D55460#1269031, @rrs wrote:

The reason I am suggesting to include FIN WAIT 1/2 is that we stopped sending, but the
peer can send us data for an unlimited amount of time.. Shouldn't this data transfer towards
us be able to use ECN? That would mean we should send ACKs based on the received ECN markings.
Am I wrong with this? Why shouldn't half closed TCP connections not use ECN for the
direction data is still sent?

Hmm FIN WAIT 2, yes I can see that since at that point you have transitioned to fully closed. And yes
the peer could keep in established on its side a long time.. But are you not in the middle of your close
in FIN WAIT 1?

Have a look at state diagram.
When you close your writing end, you send a FIN and enter FIN WAIT 1. When you receive the ACK for this FIN, you enter FIN WAIT 2. From the perspective of your peer, there is no difference if you are in FIN WAIT 1 or FIN WAIT 2 (or ESTABLISHED). The peer can send data. As long as it can send data, ECN should be usable, I think.

The reason I am suggesting to include FIN WAIT 1/2 is that we stopped sending, but the
peer can send us data for an unlimited amount of time.. Shouldn't this data transfer towards
us be able to use ECN? That would mean we should send ACKs based on the received ECN markings.
Am I wrong with this? Why shouldn't half closed TCP connections not use ECN for the
direction data is still sent?

In D55460#1268967, @tuexen wrote:

In D55460#1268958, @rscheff wrote:

LGTM

I think CLOSE WAIT should be replaced by FIN WAIT 1 and FIN WAIT 2. My mistake.

And I have found the case where we are getting the initial ack that causes the ack-war beginning.. not the
src of the continued ack-war (thats in tcp_timewait) but the initial extra ack :)

Ok I have added the CLOSE_WAIT stage Michael suggested. However I am not
sure that is a wise thing. It may mean an extra ack in the closing state i.e. while you
are waiting for your user to close.. but on the other hand that may take a *very* long
time.. so I am ambivalent about it

In D55460#1268502, @tuexen wrote:

What is with BBR?

In D55454#1268359, @bms wrote:

Any plans to add gencnt to SCTP sockets?

Olivier Van Acker did a GenAI lemming run with Cursor last week to auto-generate sctpsso(8) from tcpsso(8) as a template.

Whilst it generated an (at first) viable looking code artefact that compiled without error against FreeBSD 15.0-RELEASE, the AI model concerned had hallucinated the existence of two major things: gencnt existing for SCTP sockets, and a *.sockopt sysctl for SCTP to actually set the options.

In D55338#1265942, @tuexen wrote:

This is work in progress. There will be some more changes.

What other CC modules are you contemplating using this for?

In D53832#1237840, @tuexen wrote:

How does this algorithm relate to the reordering tolerance of RACK defined by step 4 of RFC 8985?

I think you forget that I am the author of read_bbrlog and it will slowly change over time as I work on the next features I am planning in rack. I admit
I am a bit slow, since retirement keeps getting in the way :-D, but I anticipate getting a lot more done on measurements and improvements using
the measurements to DGP. As that all occurs read_bbrlog will change. As will other base things in FreeBSD... Gleb had asked me to move the socket
read/write code of of the generic for TCP.. I plan on doing that first and getting rid of the socket buffer locks. From there it will be onward to get the
measurements in.. hmm come to think about it I need to complete my improvements for reordering and get rid of the sysctl stuff that can make
rack use the old reorder methods from draft 1 of Rack... something that the recent fun with reordering proves is not a good idea :)

committed

committed

Adopt Gleb's suggestion on __inline -> inline (other places need that change too) :)

Testing is a good thing so I think this is well worth having. I will say on the rumor of name changes inside tcp_hpts due to the idea of possibly supporting
more than just TCP. I am unsupportive of this UNTIL we have tcp_hpts being able to handle multiple protocols. When I originally wrote hpts (called pacing not hpts.. Robert
renamed it hpts) I did envision expanding it to include not just TCP but SCTP and maybe even UDP. But the fundamental problem I hit was the fact that choosing where
to dice the thing such that you would have a common call point into the protocol was not a cut and dried thing. For TCP it was easy since there was a clear demark between
the initial input of the packet and the actual segment processing, which I used when I broke out the multiple stacks idea. However for SCTP and UDP no such clear
basis exists that is common with TCP..... or even between SCTP and UDP... so I back down and decided to implement it has a TCP thing. Now if someone comes up
with a clean and elegant way to call into to TCP/SCTP and UDP then name changes would be appropriate within the code.. without that elegant set of proposed
changes name changes I am STRONGLY against and will object to since they are just irrelevant until HTPS can support multiple protocols... so in summary

In D50019#1141170, @ivy wrote:

here's the thing though, you can already accept TCP connections to an anycast address by simply not marking the address as IFF_ANYCAST, and this is what everyone does today. if you force users to set a sysctl to do this, they will just not bother marking addresses as IFF_ANYCAST and will be more likely to run into problems from e.g. accidentally originating outgoing connections from an anycast address.

or in other words, the choice here isn't "should we let users accept TCP connections to anycast addresses?" because it's impossible to prevent that, it's "should we allow users to correctly mark their existing anycast addresses as anycast addresses?". i don't see any advantage to forcing users to set a sysctl in order to configure networking more correctly.

more sysctls is more confusing and creates more pain for users, removing special cases and unnecessary code is more beautiful. :-)

however, i would be willing to add some text to ifconfig.8 to mention the potential issues with TCP anycast and refer them to some appropriate documentation.

How about a compromise here... I do think Michael has a valid point...

Only question I have is why the change from hlen -> sizeof(struct ip6_hdr)

To me this is marginally less readable in the changed form. Both are much better than the original but considering your comments about what the compiler actually does I suggest we leave it the way it is!

Note I do agree with Gleb here, these macros are just nasty and not very clear. However I think that making such a change should be a separate commit from this one!

I agree with Peter :)