tcp_bbr: fix bbr pacing rate for internal pacing
Subsystems (3)
PATCH 1/1 tcp_bbr: fix bbr pacing rate for internal pacing
From: Eric Dumazet <[email protected]>
This commit makes BBR use only the MSS (without any headers) to calculate pacing rates when internal TCP-layer pacing is used.
This is necessary to achieve the correct pacing behavior in this case, since tcp_internal_pacing() uses only the payload length to calculate pacing delays.
--- include/net/tcp.h | 11 +++++++++++ net/ipv4/tcp_bbr.c | 6 +++++- net/ipv4/tcp_output.c | 14 -------------- 3 files changed, 16 insertions(+), 15 deletions(-)
include/net/tcp.h +11 -0
@@ -1184,6 +1184,17 @@ static inline bool tcp_is_cwnd_limited(const struct sock *sk)return tp->is_cwnd_limited; } +/* BBR congestion control needs pacing. +* Same remark for SO_MAX_PACING_RATE. +* sch_fq packet scheduler is efficiently handling pacing, +* but is not always installed/used. +* Return true if TCP stack should pace packets itself. +*/ +static inline bool tcp_needs_internal_pacing (const struct sock *sk) +{ +return smp_load_acquire (&sk->sk_pacing_status) ==SK_PACING_NEEDED ; +} +/* Something is really bad, we could not queue an additional packet, * because qdisc is full or receiver sent a 0 window. * We do not want to add fuel to the fire, or abort too early,
net/ipv4/tcp_bbr.c +5 -1
@@ -205,7 +205,11 @@ static u32 bbr_bw(const struct sock *sk)*/ static u64 bbr_rate_bytes_per_sec (struct sock *sk,u64 rate,int gain) { - rate *=tcp_mss_to_mtu (sk,tcp_sk (sk)->mss_cache); +unsigned int mss =tcp_sk (sk)->mss_cache; + +if (!tcp_needs_internal_pacing (sk)) + mss =tcp_mss_to_mtu (sk, mss); + rate *= mss; rate *= gain; rate >>=BBR_SCALE ; rate *=USEC_PER_SEC ;
net/ipv4/tcp_output.c +0 -14
@@ -973,17 +973,6 @@ enum hrtimer_restart tcp_pace_kick(struct hrtimer *timer)return HRTIMER_NORESTART ; } -/* BBR congestion control needs pacing. -* Same remark for SO_MAX_PACING_RATE. -* sch_fq packet scheduler is efficiently handling pacing, -* but is not always installed/used. -* Return true if TCP stack should pace packets itself. -*/ -static bool tcp_needs_internal_pacing (const struct sock *sk) -{ -return smp_load_acquire (&sk->sk_pacing_status) ==SK_PACING_NEEDED ; -} -static void tcp_internal_pacing (struct sock *sk,const struct sk_buff *skb) {u64 len_ns;
@@ -995,9 +984,6 @@ static void tcp_internal_pacing(struct sock *sk, const struct sk_buff *skb)if (!rate || rate == ~0U )return ; -/* Should account for header sizes as sch_fq does, -* but lets make things simple. -*/ len_ns = (u64 )skb->len *NSEC_PER_SEC ;do_div (len_ns, rate); hrtimer_start(&tcp_sk(sk)->pacing_timer,
Signature · 1 line
2.18.0.rc1.244.gcf134e6275-goog
From: Kevin Yang <[email protected]>
Date: Wed, 20 Jun 2018 16:07:35 -0400
Quoted 5 lines
From: Eric Dumazet <[email protected]>
This commit makes BBR use only the MSS (without any headers) to calculate pacing rates when internal TCP-layer pacing is used.
Quoted 7 lines
This is necessary to achieve the correct pacing behavior in this case, since
tcp_internal_pacing()uses only the payload length to calculate pacing delays.Signed-off-by Kevin Yang <[email protected]>Signed-off-by Eric Dumazet <[email protected]>Reviewed-by Neal Cardwell <[email protected]>
Applied, thank you.