netdev - Re: [PATCH] net/mlx5e: Report rx_discards_phy via rx_missed

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: <20241106171717.1bf7331f@kernel.org>
Date: Wed, 6 Nov 2024 17:17:17 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: Gal Pressman <gal@...dia.com>
Cc: Yafang Shao <laoar.shao@...il.com>, Tariq Toukan
 <ttoukan.linux@...il.com>, saeedm@...dia.com, tariqt@...dia.com,
 leon@...nel.org, netdev@...r.kernel.org, linux-rdma@...r.kernel.org
Subject: Re: [PATCH] net/mlx5e: Report rx_discards_phy via rx_missed_errors

On Wed, 6 Nov 2024 21:23:47 +0200 Gal Pressman wrote:
> > It appears that rx_fifo_errors is a more appropriate counter for this purpose.
> > I will submit a v2. Thanks for your suggestion.  
> 
> Probably not a good idea:
>  *   This statistics was used interchangeably with @rx_over_errors.
>  *   Not recommended for use in drivers for high speed interfaces.

FWIW we can change the definition. Let me copy paste below the commit
which added the docs because it has the background.

tl;dr is that I was trying to push drivers towards a single stat to
keep things simple. If we have a clear definition of how rx_fifo_errors
would differ - we can reuse it and update the doc. For example if
rx_discards_phy usually means that the adapter itself is overwhelmed
(too many rules etc) that would be a pretty clear, since rx_missed is
supposed to primarily indicate that the host rings are full or perhaps
the PCIe interface of the NIC is struggling. But not the packet
processing.



commit 0db0c34cfbc9838c1a14cb04dd880602abd699a7
Author: Jakub Kicinski <kuba@...nel.org>
Date:   Thu Sep 3 16:14:31 2020 -0700

    net: tighten the definition of interface statistics
    
    This patch is born out of an investigation into which IEEE statistics
    correspond to which struct rtnl_link_stats64 members. Turns out that
    there seems to be reasonable consensus on the matter, among many drivers.
    To save others the time (and it took more time than I'm comfortable
    admitting) I'm adding comments referring to IEEE attributes to
    struct rtnl_link_stats64.
    
    Up until now we had two forms of documentation for stats - in
    Documentation/ABI/testing/sysfs-class-net-statistics and the comments
    on struct rtnl_link_stats64 itself. While the former is very cautious
    in defining the expected behavior, the latter feel quite dated and
    may not be easy to understand for modern day driver author
    (e.g. rx_over_errors). At the same time modern systems are far more
    complex and once obvious definitions lost their clarity. For example
    - does rx_packet count at the MAC layer (aFramesReceivedOK)?
    packets processed correctly by hardware? received by the driver?
    or maybe received by the stack?
    
    I tried to clarify the expectations, further clarifications from
    others are very welcome.
    
    The part hardest to untangle is rx_over_errors vs rx_fifo_errors
    vs rx_missed_errors. After much deliberation I concluded that for
    modern HW only two of the counters will make sense. The distinction
    between internal FIFO overflow and packets dropped due to back-pressure
    from the host is likely too implementation (driver and device) specific
    to expose in the standard stats.
    
    Now - which two of those counters we select to use is anyone's pick:
    
    sysfs documentation suggests rx_over_errors counts packets which
    did not fit into buffers due to MTU being too small, which I reused.
    There don't seem to be many modern drivers using it (well, CAN drivers
    seem to love this statistic).
    
    Of the remaining two I picked rx_missed_errors to report device drops.
    bnxt reports it and it's folded into "drop"s in procfs (while
    rx_fifo_errors is an error, and modern devices usually receive the frame
    OK, they just can't admit it into the pipeline).
    
    Of the drivers I looked at only AMD Lance-like and NS8390-like use all
    three of these counters. rx_missed_errors counts missed frames,
    rx_over_errors counts overflow events, and rx_fifo_errors counts frames
    which were truncated because they didn't fit into buffers. This suggests
    that rx_fifo_errors may be the correct stat for truncated packets, but
    I'd think a FIFO stat counting truncated packets would be very confusing
    to a modern reader.