netdev - Re: [RFC PATCH net-next 05/13] net: bonding: hold the netdev lists lock when retrieving device statistics

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]

Date:   Wed, 9 Dec 2020 01:28:09 +0000
From:   Vladimir Oltean <vladimir.oltean@....com>
To:     Jakub Kicinski <kuba@...nel.org>
CC:     "David S . Miller" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        Paul Gortmaker <paul.gortmaker@...driver.com>,
        Pablo Neira Ayuso <pablo@...filter.org>,
        Jiri Benc <jbenc@...hat.com>,
        Cong Wang <xiyou.wangcong@...il.com>,
        Jamal Hadi Salim <jhs@...atatu.com>,
        Stephen Hemminger <stephen@...workplumber.org>,
        Eric Dumazet <edumazet@...gle.com>,
        George McCollister <george.mccollister@...il.com>,
        Oleksij Rempel <o.rempel@...gutronix.de>,
        Jay Vosburgh <j.vosburgh@...il.com>,
        Veaceslav Falico <vfalico@...il.com>,
        Andy Gospodarek <andy@...yhouse.net>
Subject: Re: [RFC PATCH net-next 05/13] net: bonding: hold the netdev lists
 lock when retrieving device statistics

On Wed, Dec 09, 2020 at 03:14:04AM +0200, Vladimir Oltean wrote:
> net_failover and bonding are the only drivers that are creating this
> recursivity requirement in dev_get_stats. Other one-over-many stackable
> interfaces, like the bridge, just use dev_get_tstats64. I'm almost
> thinking that it would be cleaner to convert these two to dev_get_tstats64
> too, that would simplify things enormously. Even team uses something
> that is based on software counters, something reminiscent of
> dev_get_tstats64, definitely not counters retrieved from the underlying
> device. Of course, the disadvantage with doing that is going to be that
> virtual interfaces cannot retrieve statistics recursively from their
> lower interface. I'm trying to think how much of a real disadvantage
> that will be. For offloaded interfaces they will be completely off,
> that's for sure. And this is one of the reasons that mandated the DSA
> rework to expose MAC-based counters in dev_get_stats in the first place.
> But thinking in the larger sense. An offloading interface that supports
> IP forwarding, with 50 VLAN uppers. How could the statistics counters of
> those VLAN uppers ever be correct. It's not realistic to expect of the
> underlying hardware to keep separate statistics for each upper, that the
> upper could then go ahead and just query. Sure, in the cases where a
> lower can have only one upper at a time (bridge, bonding) what I said is
> not applicable, but the general goal of having accurate counters for
> offloading interfaces everywhere seems to not be really achievable.

Ok, putting some more thought into it after sending the email, maybe it
isn't unreasonable for devices with IP fwd offload to keep statistics
for each upper. They need to be aware of those uppers for a plethora of
other reasons, after all.

So, here's another idea for eliminating the recursion. Maybe we just add
a new netdevice feature, NETIF_F_FOLDING_STATS, or something like that,
which bonding and failover will use. Then dev_get_stats sees that flag,
and instead of calling ->ndo_get_stats64 for it, it just iterates
through its bottom-most level of lower interfaces and aggregates the
stats by itself.