2020-12-17 12:13:17

by Toke Høiland-Jørgensen

[permalink] [raw]
Subject: Re: [PATCH 1/7] net/fq_impl: bulk-free packets from a flow on overmemory

Felix Fietkau <[email protected]> writes:

> On 2020-12-16 21:54, Johannes Berg wrote:
>> On Wed, 2020-12-16 at 21:43 +0100, Felix Fietkau wrote:
>>>
>>> +static int fq_flow_drop(struct fq *fq, struct fq_flow *flow,
>>> + fq_skb_free_t free_func)
>>> +{
>>> + unsigned int packets = 0, bytes = 0, truesize = 0;
>>> + struct fq_tin *tin = flow->tin;
>>> + struct sk_buff *skb;
>>> + int pending;
>>> +
>>> + lockdep_assert_held(&fq->lock);
>>> +
>>> + pending = min_t(int, 32, skb_queue_len(&flow->queue) / 2);
>>>
>>
>> Why 32?
> I guess I forgot got make it configurable. sch_fq_codel uses 64, but
> that seemed a bit excessive to me.

I'm not sure it's worth a configuration knob. It's basically an
arbitrary choice anyway, and only kicks in when an unresponsive flows
keeps flooding a queue to the point of overflow (if it's many smaller
flows the "never drop more than half a flow's backlog" should keep it
from being excessive).

This (hopefully) only happens relatively rarely and hitting it with a
really big hammer is the right thing to do in such a case to keep the
box from falling over. Not sure if 32 or 64 makes much difference; guess
it depends on the CPU-to-bandwidth ratio of the particular machine.

-Toke