Page MenuHomeFreeBSD

tcp: pass maxseg around instead of locally calculating anew
ClosedPublic

Authored by rscheff on Jan 21 2024, 7:06 PM.
Tags
None
Referenced Files
F84019463: D43536.id133188.diff
Sat, May 18, 4:56 AM
F84019446: D43536.id133229.diff
Sat, May 18, 4:55 AM
F84019434: D43536.id133187.diff
Sat, May 18, 4:55 AM
F84019305: D43536.id133271.diff
Sat, May 18, 4:52 AM
Unknown Object (File)
Thu, May 9, 11:55 PM
Unknown Object (File)
Apr 16 2024, 2:55 PM
Unknown Object (File)
Apr 12 2024, 10:20 AM
Unknown Object (File)
Feb 3 2024, 5:52 PM
Subscribers

Details

Summary

Improve slowpath processing (reordering, retransmissions)
slightly by passing maxseg, once calculated, around. This typically
saves one of two calls to tcp_maxseg() which is relatively complex.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 55562
Build 52451: arc lint + arc unit

Event Timeline

I'm fine with the patch as is, but here is some idea you may consider.

Current code will call either tcp_sack_partialack() or tcp_do_prr_ack() and only once. It is:

if (foo) {
                if (bar)
                                tcp_do_prr_ack(tp, th, &to, sack_changed,
                                    (maxseg ? maxseg : tcp_maxseg(tp)));
                else
                                tcp_sack_partialack(tp, th,
                                    (maxseg ? maxseg : tcp_maxseg(tp)));
} else if (baz)
                        tcp_do_prr_ack(tp, th, &to, SACK_CHANGE,
                            (maxseg ? maxseg : tcp_maxseg(tp)));

However, we might have another function interested in maxseg or either of the existing functions called twice. If that happens we are doing two calls to tcp_maxseg() again. So alternative code would be a prototype that passes pointer to maxseg on the stack of caller and allows callee to update it if it is 0.

void
tcp_do_prr_ack(struct tcpcb *tp, struct tcphdr *th, struct tcpopt *to, sackstatus_t sack_changed, u_int *maxseg)
{
         if (*maxseg == 0)
                *maxseg = tcp_maxseg(tp)
  • update uninitialized maxseg for posterity
  • update uninitialized maxseg for posterity (no ptrs)
  • go back to passing a pointer but don't touch body of functions
This revision is now accepted and ready to land.Jan 24 2024, 7:15 AM
sys/netinet/tcp_input.c
1532

The "maxseg" won't be assigned a value after line#2793. So I am wondering why not initialize/zero it in the beginning. There may be techniques that make sure it is always zero, but at least zero it here removes uncertainty.

2792

The "maxseg", in my understanding, won't be assigned a value after line#2793.

  • properly initialize maxseg again
This revision now requires review to proceed.Jan 24 2024, 2:36 PM
rscheff added inline comments.
sys/netinet/tcp_input.c
1532

Thanks. This got somehow dropped between revisions.

This revision is now accepted and ready to land.Jan 24 2024, 2:51 PM
This revision was automatically updated to reflect the committed changes.
rscheff marked an inline comment as done.