Hi, On Wed, 2018-02-21 at 06:43 -0800, Eric Dumazet wrote: > From: Eric Dumazet <eduma...@google.com> > > BBR uses tcp_tso_autosize() in an attempt to probe what would be the > burst sizes and to adjust cwnd in bbr_target_cwnd() with following > gold formula : > > /* Allow enough full-sized skbs in flight to utilize end systems. */ > cwnd += 3 * bbr->tso_segs_goal; > > But GSO can be lacking or be constrained to very small > units (ip link set dev ... gso_max_segs 2) > > What we really want is to have enough packets in flight so that both > GSO and GRO are efficient. > > So in the case GSO is off or downgraded, we still want to have the same > number of packets in flight as if GSO/TSO was fully operational, so > that GRO can hopefully be working efficiently. > > To fix this issue, we make tcp_tso_autosize() unaware of > sk->sk_gso_max_segs > > Only tcp_tso_segs() has to enforce the gso_max_segs limit. > > Tested: > > ethtool -K eth0 tso off gso off > tc qd replace dev eth0 root pfifo_fast > > Before patch: > for f in {1..5}; do ./super_netperf 1 -H lpaa24 -- -K bbr; done > 691 (ss -temoi shows cwnd is stuck around 6 ) > 667 > 651 > 631 > 517 > > After patch : > # for f in {1..5}; do ./super_netperf 1 -H lpaa24 -- -K bbr; done > 1733 (ss -temoi shows cwnd is around 386 ) > 1778 > 1746 > 1781 > 1718 > > Fixes: 0f8782ea1497 ("tcp_bbr: add BBR congestion control") > Signed-off-by: Eric Dumazet <eduma...@google.com> > Reported-by: Oleksandr Natalenko <oleksa...@natalenko.name> > --- > net/ipv4/tcp_output.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c > index > b2bca373f8bee35267df49b5947a6793fed71a12..6818042cd8a9a1778f54637861647091afd9a769 > 100644 > --- a/net/ipv4/tcp_output.c > +++ b/net/ipv4/tcp_output.c > @@ -1730,7 +1730,7 @@ u32 tcp_tso_autosize(const struct sock *sk, unsigned > int mss_now, > */ > segs = max_t(u32, bytes / mss_now, min_tso_segs); > > - return min_t(u32, segs, sk->sk_gso_max_segs); > + return segs; > } > EXPORT_SYMBOL(tcp_tso_autosize); >
Very minor nit, why don't: return max_t(u32, bytes / mss_now, min_tso_segs); and drop the 'segs' local variable? Thanks, Paolo