On Wed, Nov 21, 2018 at 3:47 PM Yuchung Cheng <ych...@google.com> wrote: > > On Wed, Nov 21, 2018 at 2:47 PM, Eric Dumazet <eric.duma...@gmail.com> wrote: > > > > > > On 11/21/2018 02:40 PM, Yuchung Cheng wrote: > >> On Wed, Nov 21, 2018 at 9:52 AM, Eric Dumazet <eduma...@google.com> wrote: > >>> Under high stress, and if GRO or coalescing does not help, > >>> we better make room in backlog queue to be able to keep latest > >>> packet coming. > >>> > >>> This generally helps fast recovery, given that we often receive > >>> packets in order. > >> > >> I like the benefit of fast recovery but I am a bit leery about head > >> drop causing HoLB on large read, while tail drops can be repaired by > >> RACK and TLP already. Hmm - > > > > This is very different pattern here. > > > > We have a train of packets coming, the last packet is not a TLP probe... > > > > Consider this train coming from an old stack without burst control nor > > pacing. > > > > This patch guarantees last packet will be processed, and either : > > > > 1) We are a receiver, we will send a SACK. Sender will typically start > > recovery > > > > 2) We are a sender, we will process the most recent ACK sent by the > > receiver. > > > > Sure on the sender it's universally good. > > On the receiver side my scenario was not the last packet being TLP. > AFAIU the new design will first try coalesce the incoming skb to the > tail one then exit. Otherwise it's queued to the back with an > additional 64KB space credit. This patch checks the space w/o the > extra credit and drops the head skb. If the head skb has been > coalesced, we might end dropping the first big chunk that may need a > few rounds of fast recovery to repair. But I am likely to > misunderstand the patch :-)
This situation happens only when we are largely over committing the memory, due to bad skb->len/skb->truesize ratio. Think about MTU=9000 and some drivers allocating 16 KB of memory per frame, even if the packet has 1500 bytes in it. > > Would it make sense to check the space first before the coalesce > operation, and drop just enough bytes of the head to make room? This is basically what the patch does, the while loop breaks when we have freed just enough skbs.
