Page MenuHomeFreeBSD

rack, bbr: cleanup ack throttling
ClosedPublic

Authored by tuexen on Jul 22 2024, 9:55 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Nov 21, 5:11 PM
Unknown Object (File)
Wed, Nov 20, 2:15 AM
Unknown Object (File)
Sat, Nov 9, 11:07 PM
Unknown Object (File)
Tue, Nov 5, 12:35 PM
Unknown Object (File)
Nov 3 2024, 4:23 AM
Unknown Object (File)
Oct 27 2024, 4:08 PM
Unknown Object (File)
Oct 19 2024, 9:31 PM
Unknown Object (File)
Oct 18 2024, 3:00 AM

Details

Summary

Use the variable in the TCPCB, not the one in the stack specific data structure. This simplifies the code and brings the functionality to BBR without any change.

This changes builds upon D46066.

Diff Detail

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

Event Timeline

tuexen retitled this revision from rack, bbr to rack, bbr: cleanup ack throttling.
tuexen edited the summary of this revision. (Show Details)
cc requested changes to this revision.Jul 24 2024, 6:43 PM
cc added a subscriber: cc.
cc added inline comments.
sys/netinet/tcp_stacks/rack_bbr_common.c
535

I think the logic here can be split into two functions, such that the refined logic 1 on checking if we should send ACK can be re-used across multiple places.

logic 1:

bool
is_ack_unlimited(struct tcpcb *tp)
{
    /* focus on checking the epoch and
     * if should send ACK, return true; else return false
     */ 
}

logic 2:

void
set_tp_acknow(struct tcpcb *tp)
{
    if (is_ack_unlimited(tp))
        tp->t_flags |= TF_ACKNOW;
    else
        tp->t_flags &= ~TF_ACKNOW;
}
761–762

From my above comment, we can re-use the logic 1. Like this:

if (is_ack_unlimited(tp))
    /* Send challenge ACK. */
This revision now requires changes to proceed.Jul 24 2024, 6:44 PM
sys/netinet/tcp_stacks/rack_bbr_common.c
761–762

I think this code block could be replaced by tcp_send_challenge_ack() (and leaving the counter increment)?

  • Rebase after vnetifying sysctl-able variables.
  • Address Peter's comment.
tuexen added inline comments.
sys/netinet/tcp_stacks/rack_bbr_common.c
535

I would like to reuse tcp_send_challenge_ack() instead of the alternate used here. But that is a followup commit.

761–762

Not needed, we can just use tcp_send_challenge_ack().

cc added inline comments.
sys/netinet/tcp_stacks/rack_bbr_common.c
535

I would like to reuse tcp_send_challenge_ack() instead of the alternate used here. But that is a followup commit.

I am OK if you have a plan to replace ctf_ack_war_checks() with tcp_send_challenge_ack() for the re-use.

761–762

Not needed, we can just use tcp_send_challenge_ack().

Thanks for the update.

This revision is now accepted and ready to land.Aug 7 2024, 3:41 PM
This revision was automatically updated to reflect the committed changes.
tuexen marked 2 inline comments as done.