Page MenuHomeFreeBSD

tcp: Correctly compute the TCP goodput in bits per second by using SEQ_SUB().
ClosedPublic

Authored by hselasky on Jun 16 2022, 3:10 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, May 8, 1:54 PM
Unknown Object (File)
Wed, May 8, 1:53 PM
Unknown Object (File)
Wed, May 8, 12:16 PM
Unknown Object (File)
Sun, Apr 28, 9:28 AM
Unknown Object (File)
Apr 1 2024, 1:15 PM
Unknown Object (File)
Feb 15 2024, 1:50 AM
Unknown Object (File)
Jan 14 2024, 9:59 AM
Unknown Object (File)
Dec 20 2023, 4:22 AM

Diff Detail

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

Event Timeline

Should we generally use the SEQ_SUB macro throughout, to prevent casting issues? (mechanically replace all instances, where a type tcpseq is subtracted from another type tcpseq?)

This revision is now accepted and ready to land.Jun 16 2022, 6:15 AM

Maybe. Most places where TCP sequence numbers are subtracted, they end up in integers, so then there is no problem.

I need to look at closer at this.

sys/netinet/tcp_input.c
324

I think we need to add SEQ_GET(th->th_ack, tp->gput_seq) here too, just to ensure that the SEQ_SUB() doesn't return a signed negative value.

324

SEQ_GET -> SEQ_GEQ

BTW: The current code seems OK, but may be confusing to review.

BTW: The current code seems OK, but may be confusing to review.

You shouldn't end up doing this calculation on ACKs from below the left edge of the window (th_ack < snd_una)

Right, so then the SEQ_SUB() shouldn't become negative?

Right, so then the SEQ_SUB() shouldn't become negative?

During normal processing, th_ack should always be in the range snd_una ... snd_max. All cases outside of this window should be addressed before normal congestion-control processing. The cases with explicit unsigned int typcasting should be adressed for formal reasons, but generally, those calculations should return positive values.