netdev - Re: [RFC net-next 2/2] net: sched: fq

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <87tvf8zxki.fsf@toke.dk>
Date:   Mon, 08 Apr 2019 10:54:37 +0200
From:   Toke Høiland-Jørgensen <toke@...hat.com>
To:     Gautam Ramakrishnan <gautamramk@...il.com>
Cc:     Jamal Hadi Salim <jhs@...atatu.com>, davem@...emloft.net,
        netdev@...r.kernel.org,
        "Mohit P . Tahiliani" <tahiliani@...k.edu.in>,
        "Sachin D . Patil" <sdp.sachin@...il.com>,
        Mohit Bhasi <mohitbhasi1998@...il.com>,
        "V . Saicharan" <vsaicharan1998@...il.com>,
        Leslie Monis <lesliemonis@...il.com>,
        Dave Taht <dave.taht@...il.com>
Subject: Re: [RFC net-next 2/2] net: sched: fq_pie: Flow Queue PIE AQM

Gautam Ramakrishnan <gautamramk@...il.com> writes:

> I was trying to refactor the code and I ran into some issues.
> 1. I moved some of the parameters such as flows_cnt into a new struct
> called fq_pie_params, instead of keeping them in fq_pie_sched_data.
> Should I move those parameters back into fq_pie_sched_data?

Hmm, this is getting into taste here, but I think I would just put them
directly into fq_pie_sched_data; after all, you are not reusing them in
multiple places, and things like 'q->params.p_params.target' is a bit
much for my taste :)

> 2. fq_codel maintains the backlog variable as a list in the
> fq_codel_sched_data, whereas I maintain a backlog in the struct
> fq_pie_flow. What approach should I follow?

The reason fq_codel does this is to keep that memory contiguous, so that
when it loops through it all (in fq_codel_drop()), it is less memory
that has to be loaded into cache. Since you're not doing that in fq_pie,
it's not strictly necessary to mirror this (admittedly, somewhat
confusing) aspect of fq_codel :)

> 3. Would maintaining a per flow pie_stats be useful in the future? I
> do not use the per flow stats anywhere in the code.

Hard to know what is useful in the future. Is it useful now? It's easier
to add new features later on than to remove them once they are there...
So if you don't have an immediate use case for it, my suggestion would
be to remove it, and add it back if someone really ends up needing it :)

-Toke