As discussed, this change is not necessary.

The latest FreeBSD MAN page for TCP(4) shows the TCP_MAXSEG allows to determine the result of negotiation between sender and receiver.

Does not seem to be needed as the abandon reason is in https://reviews.freebsd.org/D47218#1253529.

In D47218#1253522, @rrs wrote:

What other CC modules are you contemplating using this for?

My one comment on the change is that now you will always be setting something into the cwnd.. the old code only
did it when there was a change. Have not looked at the cache line layout of the data, I suspect it makes no difference
since you probably have the cwnd in cache anyway :)

Does not seem to be needed.

In D53104#1217628, @adrian wrote:

ok, so this looks fine, but I'm trying to remember where I used to assign the flowid/flowtype for a connection back in the day. On the mbuf side it used to be done in the driver side and then overriden by the network stack input path if it didn't match what was required (eg the magic required in ip reassembly, which is still there, but also to support completely software flow hashing.)

(remember way WAY back in the day the flowid/flowtype was also used by kip's flowdirector stuff for doing more explicit control over where flows went, and RSS just 'also' used it.)

For reference, the Linux kernel recently changed it from 6MB to 32MB.

Looks good to me. I only have two minor suggestions.

In D7986#1184580, @kbowling wrote:

@imp if I recall this was something we were looking at on my team at LLNW, if it were in Bugzilla I'd say "Close - OBE" is appropriate.

@cc since you've been working on CUBIC lately, the only thing possibly still relevant here is turning the first level if into the early returns.. it removes one level of indention which might help justify a couple other nearby style(9) fixes but it's not worth holding this review open.

Thanks for the following improvements:

I am ok with this patch. But I want to be sure how snd_recover shrinks. Is it because tp->snd_recover = tp->snd_recover_prev in CC_RTO_ERR? But that shall not be in SACK recovery.

Other than the fd is not used in the test code, I am good with this change.

Thanks for the elaboration. Looks good to me now.

Looks I am missing the context. Given the fact that the socket is in TCPS_TIME_WAIT while switching to the default stack, was this assert a day one issue that was not test covered before? Or was this panic caused by recent changes?

This is for the fast path fix. Just want to confirm with your that a second patch may be around to improve both of the fast and slow path?

Didn't see any surprising regression from my test result for this patch:
testD49047

code update based on Richard's comments

re-base, add a missing part and update based on Richard's comment in the meeting

In D49047#1118719, @glebius wrote:

Question: why does this function returns signed value? And why some of the sackhint counters are signed?

There are several calls to tcp_compute_pipe() that did not check the V_tcp_do_newsack: 2 in tcp_output and 1 in tcp_input. Their behavior will change with revised SACK turned off. Commit message should explain why this is okay.

Why not move the old_method: label above the stack variables' declaration? I think it may be cleaner to read.
Like this:

Additional comment:

update:
Looks this patch has some significant reduction on fragment (data_size % MSS) > 0 out of TSO data chunks: testD47474
TSO not enabled:

In D47474#1084985, @rscheff wrote:

Well, I had the same thought - the full MSS (including options) is less frequently used, that the mss without options...

So, it would certainly be more efficient to store the MSS exkluding options in tcpcb, and calculating the MSS inkl. option space only where that is needed...

But that should a different Diff IMHO, as this one is more in the break/fix category.

I think you meant the title be:
tcp: consistently set CWND to MSS => tcp: consistently set CWND to 1
in case of SYN/SYN ACK retransmissions => in case of SYN retransmissions

OK. I am approving it now as my test in https://wiki.freebsd.org/chengcui/testD43470 shows some improvement. Any bug related observations can be fixed later.

Also, please correct the SUMMARY section:

In D30155#1076291, @kbowling wrote:

In D30155#1075233, @cc wrote:

From my test result in testD30155, I didn't find any significant improvement under my eyes:

• no significant difference in ping latency
• no significant iperf3 performance improvement due to bad performance (3.x Gbps) in FreeBSD 15-current vs. (9.x Gbps) in stock Linux kernel 5.15.

Thanks for the results @cc. Something seems very strange with the throughput there, the main system I am testing is a xeon-d that is much less than 1/4th as powerful and can line rate both directions no issues and I also have an older 2x Xeon E5-2695 v2 (two NUMA domains) without throughput limitations. I will see if I can find my emulab credentials and take a look there, it seems like these might be 4-way NUMA machines but it is not expected to me that that would cause this magnitude of throughput issues, especially at the 10gbit data rate.

update code based on discussion

From my test result in testD30155, I didn't find any significant improvement under my eyes:

Better now. But it can be cleaner.

Add the __inline keyword to avoid overhead when possible.