netdev - Re: [net 02/11] net/mlx5e: Fix multicast counter not up-to-date in "ip -s"

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200703105938.2b7b0b48@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Fri, 3 Jul 2020 10:59:38 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Saeed Mahameed <saeedm@...lanox.com>
Cc:     "davem@...emloft.net" <davem@...emloft.net>,
        Tariq Toukan <tariqt@...lanox.com>,
        Ron Diskin <rondi@...lanox.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [net 02/11] net/mlx5e: Fix multicast counter not up-to-date in
 "ip -s"

On Fri, 3 Jul 2020 06:15:09 +0000 Saeed Mahameed wrote:
> > > To read mcast counter we need to execute FW command which is
> > > blocking,
> > > we can't block in atomic context .ndo_get_stats64 :( .. we have to
> > > count in SW. 
> > > 
> > > the previous approach wasn't accurate as we read the mcast counter
> > > in a
> > > background thread triggered by the previous read.. so we were off
> > > by
> > > the interval between two reads.  
> > 
> > And that's bad enough to cause trouble? What's the worst case time
> > delta you're seeing?
> 
> Depends on the user frequency to read stats,
> if you read stats once every 5 minutes then mcast stats are off by 5
> minutes..
> 
> Just thinking out loud, is it ok of we busy loop and wait for FW
> response for mcast stats commands ? 
> 
> In ethtool -S though, they are accurate since we grab them on the spot
> from FW.

I don't really feel too strongly, I'm just trying to get the details
because I feel like the situation is going to be increasingly common.
It'd be quite sad if drivers had to reimplement all stats in sw.

I thought it would be entirely reasonable for the driver to read the
stats from a delayed work every 1/2 HZ and cache that. We do have a
knob in ethtool IRQ coalescing settings for stats writeback frequency.

I'm not sure what locks procfs actually holds, if its something that
could impact reading other files - it'd probably be a bad idea to busy
wait.