Page MenuHomeFreeBSD

HTPS has actually three states not two so the macro needs to account for that.
ClosedPublic

Authored by rrs on Feb 29 2024, 4:07 PM.
Tags
None
Referenced Files
F103376106: D44157.diff
Sun, Nov 24, 5:32 AM
F103349230: D44157.id135224.diff
Sat, Nov 23, 9:39 PM
F103307845: D44157.diff
Sat, Nov 23, 8:54 AM
Unknown Object (File)
Fri, Nov 22, 10:26 PM
Unknown Object (File)
Fri, Nov 22, 12:19 PM
Unknown Object (File)
Thu, Nov 21, 6:20 PM
Unknown Object (File)
Thu, Nov 21, 1:49 PM
Unknown Object (File)
Wed, Nov 20, 1:59 AM
Subscribers

Details

Summary

Ok lets fix up the tcp_in_hpts() so that it also says yes if you
are in the race state moving and you are scheduled to be put in.
This also requires changing the MPASS to be the old version non
inline function of tcp_in_hpts().

This change also adds a new inline macro so that a uint64_t timestamp can be
obtained by a transport (aka Rack will use this).

Diff Detail

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

Event Timeline

sys/netinet/tcp_hpts.h
214

Is adding this function intended? At least I would not expect it based on the description of the review...

sys/netinet/tcp_hpts.h
214

Well it is definitely used in the updated rack. When I split out the
diff it came with. I can move it back if you think it worth it....

sys/netinet/tcp_hpts.h
214

Either move it back or leave it, but cover it in the commit message.

rrs added inline comments.
sys/netinet/tcp_hpts.h
214

I have expanded the commit message to mention the new uit64_t function :)

This revision is now accepted and ready to land.Feb 29 2024, 8:29 PM

The change is missing a change to tcp_hpts_insert_diag():

--- a/FreeBSD/sys/netinet/tcp_hpts.c
+++ b/FreeBSD/sys/netinet/tcp_hpts.c
@@ -816,7 +816,7 @@ tcp_hpts_insert_diag(struct tcpcb *tp, uint32_t slot, int32_t line, struct hpts_
 
        INP_WLOCK_ASSERT(tptoinpcb(tp));
        MPASS(!(tptoinpcb(tp)->inp_flags & INP_DROPPED));
-       MPASS(!tcp_in_hpts(tp));
+       MPASS(!(tp->t_in_hpts == IHPTS_ONQUEUE));
 
        /*
         * We now return the next-slot the hpts will be on, beyond its
This revision now requires changes to proceed.Feb 29 2024, 10:14 PM
sys/netinet/tcp_hpts.h
214

Shouldn't this chunk just go to the big rack update patch?

Good catch Gleb, I missed the hpts.c changes.. I will update it :=)

Add in the additional MPASS as Gleb suggested.

Thanks! That matches what you did internally.

I still don't understand what is the point of glueing this changeset together with tcp_get_u64_usecs(). If it is possible, IMHO, better make that two separate commits (doesn't need yet another phab review of course!).

This revision is now accepted and ready to land.Mar 1 2024, 4:11 PM

If possible, commit the addition of tcp_get_u64_usecs() separately.