netdev - Re: [PATCH net-next v2 0/5] net: sched: move back qlen to per CPU accounting

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: <20190410.122633.1413169098501647229.davem@davemloft.net>
Date:   Wed, 10 Apr 2019 12:26:33 -0700 (PDT)
From:   David Miller <davem@...emloft.net>
To:     pabeni@...hat.com
Cc:     netdev@...r.kernel.org, jhs@...atatu.com, xiyou.wangcong@...il.com,
        jiri@...nulli.us, edumazet@...gle.com, ivecera@...hat.com
Subject: Re: [PATCH net-next v2 0/5] net: sched: move back qlen to per CPU
 accounting

From: Paolo Abeni <pabeni@...hat.com>
Date: Wed, 10 Apr 2019 14:32:36 +0200

> The commit 46b1c18f9deb ("net: sched: put back q.qlen into a single location")
> introduced some measurable regression in the contended scenarios for
> lock qdisc.
> 
> As Eric suggested we could replace q.qlen access with calls to qdisc_is_empty()
> in the datapath and revert the above commit. The TC subsystem updates 
> qdisc->is_empty in a somewhat loose way: notably 'is_empty' is set only when
> the qdisc dequeue() calls return a NULL ptr. That is, the invocation after
> the last packet is dequeued.
> 
> The above is good enough for BYPASS implementation - the only downside is that
> we end up avoiding the optimization for a very small time-frame - but will
> break hard things when internal structures consistency for classful qdisc
> relies on child qdisc_is_empty().
> 
> A more strict 'is_empty' update adds a relevant complexity to its life-cycle, so
> this series takes a different approach: we allow lockless qdisc to switch from
> per CPU accounting to global stats accounting when the NOLOCK bit is cleared.
> Since most pieces of infrastructure are already in place, this requires very
> little changes to the pfifo_fast qdisc, and any later NOLOCK qdisc can hook
> there with little effort - no need to maintain two different implementations.
> 
> The first 2 patches removes direct qlen access from non core TC code, the 3rd
> and 4th patches place and use the infrastructure to allow stats account
> switching and the 5th patch is the actual revert.
> 
>  v1 -> v2:
>   - fixed build issues
>   - more descriptive commit message for patch 5/5

I really like these changes, series applied, thanks Paolo.