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]

Date:   Fri, 3 Jul 2020 06:15:09 +0000
From:   Saeed Mahameed <saeedm@...lanox.com>
To:     "kuba@...nel.org" <kuba@...nel.org>
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 Thu, 2020-07-02 at 21:25 -0700, Jakub Kicinski wrote:
> On Fri, 3 Jul 2020 03:45:45 +0000 Saeed Mahameed wrote:
> > On Thu, 2020-07-02 at 18:47 -0700, Jakub Kicinski wrote:
> > > On Thu,  2 Jul 2020 15:19:14 -0700 Saeed Mahameed wrote:  
> > > > From: Ron Diskin <rondi@...lanox.com>
> > > > 
> > > > Currently the FW does not generate events for counters other
> > > > than
> > > > error
> > > > counters. Unlike ".get_ethtool_stats", ".ndo_get_stats64"
> > > > (which ip
> > > > -s
> > > > uses) might run in atomic context, while the FW interface is
> > > > non
> > > > atomic.
> > > > Thus, 'ip' is not allowed to issue fw commands, so it will only
> > > > display
> > > > cached counters in the driver.
> > > > 
> > > > Add a SW counter (mcast_packets) in the driver to count rx
> > > > multicast
> > > > packets. The counter also counts broadcast packets, as we
> > > > consider
> > > > it a
> > > > special case of multicast.
> > > > Use the counter value when calling "ip -s"/"ifconfig".  Display
> > > > the
> > > > new
> > > > counter when calling "ethtool -S", and add a matching counter
> > > > (mcast_bytes) for completeness.  
> > > 
> > > What is the problem that is being solved here exactly?
> > > 
> > > Device counts mcast wrong / unsuitably?
> > >   
> > 
> > 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.

> > > > Fixes: f62b8bb8f2d3 ("net/mlx5: Extend mlx5_core to support
> > > > ConnectX-4 Ethernet functionality")
> > > > Signed-off-by: Ron Diskin <rondi@...lanox.com>
> > > > Reviewed-by: Tariq Toukan <tariqt@...lanox.com>
> > > > Signed-off-by: Saeed Mahameed <saeedm@...lanox.com>