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.
Details
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)| 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. | |
| sys/netinet/tcp_input.c | ||
|---|---|---|
| 1532 | Thanks. This got somehow dropped between revisions. | |