This patch is submitted in a3aa6f652904

type converting fix is in https://reviews.freebsd.org/D40323

Thank you Michael for this review!

update based on comments

In D40185#917355, @tuexen wrote:

You might want to wait also for Richards' approval, since he is much more familiar with the code than I am.

correct the lower bound for min_rtt_usecs and correct some space and comments

Add comment above this fix to explain why it can improve precision.

If keep the flowlist of the footnote summary, I have to rework the code to keep ip/port tuples when use flowid as the key.

further cleanup

clean up function siftr_get_flowid()

cleanup change and fix a bug in this change

Looks good to me. Thanks for taking care of this. :)

update inp->inp_flowid usage in compare

In D39848#910632, @tuexen wrote:

Changing the algorithm is one thing I agree with. But why change the output in the summary from IP addresses / port numbers to flow IDs? For me the IP addresses and port numbers are easier to read and more helpful.

correct siftr_get_flowid() in SIFTR_IPV6 compile

update man page

update man page too

In D39835#906901, @cc wrote:

In D39835#906803, @tuexen wrote:

In D39835#906795, @cc wrote:

In D39835#906794, @tuexen wrote:

What do we gain from removing this feature? My understanding is that if it is not enabled, it does not have a substantial CPU overhead. I have no idea who uses siftr and if they use this feature...

This feature is not used AFAIK. We have less carbon print (at least 10 chars of "0x00000000") per record after removing this feature.

OK, so I understand that it makes sense not to print it, if the feature is not used. But I have (in general) a hard time to tell, which features are used and which are not... So how do you know that you are not removing a feature considered important by some people?
An argument I see in favor of this change is that I'm not aware of any tool reading a .pcap file and computing the hashes for the packets.

Let's assume this is a dead feature, or the original authors do not need it any longer. Also, a hash value is hard to use for a packet identification. I will consider tcp.seq, tcp.ack, and tcp.len in the tcphdr for such identification. Besides, the inconsistency between IPv4 and IPv6 for this feature does not make sense.

In D39835#906803, @tuexen wrote:

In D39835#906795, @cc wrote:

In D39835#906794, @tuexen wrote:

What do we gain from removing this feature? My understanding is that if it is not enabled, it does not have a substantial CPU overhead. I have no idea who uses siftr and if they use this feature...

This feature is not used AFAIK. We have less carbon print (at least 10 chars of "0x00000000") per record after removing this feature.

OK, so I understand that it makes sense not to print it, if the feature is not used. But I have (in general) a hard time to tell, which features are used and which are not... So how do you know that you are not removing a feature considered important by some people?
An argument I see in favor of this change is that I'm not aware of any tool reading a .pcap file and computing the hashes for the packets.

In D39835#906794, @tuexen wrote:

What do we gain from removing this feature? My understanding is that if it is not enabled, it does not have a substantial CPU overhead. I have no idea who uses siftr and if they use this feature...

Add date update in man page.

In D39698#906322, @glebius wrote:

c3c20de3b2d1357d081ce6c41f15f23cd1020e35

In D39803#906274, @tuexen wrote:

Isn't that another backwards incompatible change?

Update date in man page, and inflate siftr version.

Offline discussion includes packetdrill tests from Richard and my Emulab dumbbell testbed tests. I found it hard to attach my Emulab test result here. But this patch looks good to me.

In D39716#904013, @glebius wrote:

I'm fine with this change. If you commit it quickly, I will respin values in D39698. Alternatively the value can remain as is, just add explicit comments that bits 0x1000, 0x2000, 0x4000, 0x8000 are unused and available.

In D39698#903847, @rrs wrote:

Why if 0x10000 is not used would it matter?

In D39299#902591, @rscheff wrote:

tcp_compute_pipe is always controlled by mostly V_tcp_do_newsack in the output path and CC modules, and V_tcp_do_newcwv in the input path for deciding if the session is cwnd limited or not. New cwnd validation is disabled by default, newsack (formerly rfc6675_pipe) is enabled by default.

Without the improved Pipe calculation, PRR can not deliver the expected results (in high loss-fraction fast recovery scenarios).

Looks good to me. If the ')' char can be removed as pointed out in my inline comment, that will be better!