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: <875xp1azla.fsf@nvidia.com>
Date: Tue, 05 Nov 2024 18:02:57 -0800
From: Rahul Rameshbabu <rrameshbabu@...dia.com>
To: "Arinzon, David" <darinzon@...zon.com>
Cc: Gal Pressman <gal@...dia.com>,  Jakub Kicinski <kuba@...nel.org>,  David
 Miller <davem@...emloft.net>,  "netdev@...r.kernel.org"
 <netdev@...r.kernel.org>,  Eric Dumazet <edumazet@...gle.com>,  Paolo
 Abeni <pabeni@...hat.com>,  Richard Cochran <richardcochran@...il.com>,
  "Woodhouse, David" <dwmw@...zon.co.uk>,  "Machulsky, Zorik"
 <zorik@...zon.com>,  "Matushevsky, Alexander" <matua@...zon.com>,
  "Bshara, Saeed" <saeedb@...zon.com>,  "Wilson, Matt" <msw@...zon.com>,
  "Liguori, Anthony" <aliguori@...zon.com>,  "Bshara, Nafea"
 <nafea@...zon.com>,  "Schmeilin, Evgeny" <evgenys@...zon.com>,  "Belgazal,
 Netanel" <netanel@...zon.com>,  "Saidi, Ali" <alisaidi@...zon.com>,
  "Herrenschmidt, Benjamin" <benh@...zon.com>,  "Kiyanovski, Arthur"
 <akiyano@...zon.com>,  "Dagan, Noam" <ndagan@...zon.com>,  "Bernstein,
 Amit" <amitbern@...zon.com>,  "Agroskin, Shay" <shayagr@...zon.com>,
  "Abboud, Osama" <osamaabb@...zon.com>,  "Ostrovsky, Evgeny"
 <evostrov@...zon.com>,  "Tabachnik, Ofir" <ofirt@...zon.com>,
  "Machnikowski, Maciek" <maciek@...hnikowski.net>
Subject: Re: [PATCH v3 net-next 3/3] net: ena: Add PHC documentation

On Tue, 05 Nov, 2024 16:52:11 +0000 "Arinzon, David" <darinzon@...zon.com> wrote:
>> >>> +=================
>> >> ======================================================
>> >>> +**phc_cnt**         Number of successful retrieved timestamps (below
>> >> expire timeout).
>> >>> +**phc_exp**         Number of expired retrieved timestamps (above
>> >> expire timeout).
>> >>> +**phc_skp**         Number of skipped get time attempts (during block
>> >> period).
>> >>> +**phc_err**         Number of failed get time attempts (entering into
>> block
>> >> state).
>> >>> +=================
>> >> ======================================================
>> >>
>> >> I seem to recall we had an unpleasant conversation about using
>> >> standard stats recently. Please tell me where you looked to check if
>> >> Linux has standard stats for packet timestamping. We need to add the
>> right info there.
>> >> --
>> >> pw-bot: cr
>> >
>> > Hi Jakub,
>> >
>> > Just wanted to clarify that this feature and the associated
>> > documentation are specifically intended for reading a HW timestamp, not
>> for TX/RX packet timestamping.
>> > We reviewed similar drivers that support HW timestamping via
>> > `gettime64` and `gettimex64` APIs, and we couldn't identify any that
>> capture or report statistics related to reading a HW timestamp.
>> > Let us know if further details would be helpful.
>> 
>> David, did you consider Rahul's recent timestamping stats API?
>> 0e9c127729be ("ethtool: add interface to read Tx hardware timestamping
>> statistics")
>
> Hi Gal,
>
> We've looked into the `get_ts_stats` ethtool hook, and it refers to TX HW packet timestamping
> and not HW timestamp which is retrieved through `gettime64` and `gettimex64`.

Hi folks,

I think everyone might be on the same page now, but I wanted to provide
some clarifications just in case.

The use case that the TX HW timestamping statistics covers


    ┌─────────────────────────────────────┐
    │                                     │
    │                                     │
    │                              ┌──────┤
    │           NIC hw             │      │             Packets out
    │                              │  PF  ├────────────────────────────────────────────────►
    │                              │      │
    │                              └───┬──┤
    │                                  │  │
    │                                  │  │
    │                                  │  │
    │                             ┌────┘  │
    │                             │       │
    └─────────────────────────────┼───────┘
                                  │Hw timestamp information per packet
    ┌─────────────────────────────┼───────┐
    │                             │       │
    │                       ┌─────▼────┐  │
    │                       │  cmsg    │  │
    │                       │          │  │
    │                       │  queue   │  │
    │                       │          │  │
    │                       └──────────┘  │
    │                                     │
    │                                     │
    │         Linux Kernel Stack          │
    │                                     │
    │                                     │
    │                                     │
    └─────────────────────────────────────┘

We are collecting statistics on every packet being sent out the wire.
The use case being described here


    ┌──────────────────────────────────┐
    │                                  │
    │                                  │
    │                          ┌───────┤
    │            NIC hw        │       │
    │                          │   PF  │
    │    ┌───────────────────┐ │       │
    │    │                   │ └───────┤
    │    │        PHC        │         │
    │    │                   │         │
    │    │                   │         │
    │    │ (just a clock dev)│         │
    │    └─────────┬─────────┘         │
    │              │                   │
    └──────────────┼───────────────────┘
                   │ Query device's clock for current time
    ┌──────────────┼───────────────────┐      ┌─────────────────────────────────┐
    │              │                   │      │                                 │
    │              │                   │      │                                 │
    │   ┌──────────┼─────────────┐     │      │                                 │
    │   │          │             │     │      │    ┌───────────────────────┐    │
    │   │          │             │     │      │    │                       │    │
    │   │          ▼          ───┼─────┼──────┼────┼──────────►            │    │
    │   │                        │     │      │    │                       │    │
    │   │ .gettimex64 callback   │     │      │    │  clock_gettime syscall│    │
    │   └────────────────────────┘     │      │    └───────────────────────┘    │
    │                                  │      │                                 │
    │        Linux Kernel Stack        │      │            Userspace            │
    │                                  │      │                                 │
    └──────────────────────────────────┘      └─────────────────────────────────┘

The model above is about getting the time from the NIC hardware in the
userspace application (which has not involvement with TX/RX traffic).

I do think the phc-to-host related statistics are on the niche side of
things. The following drivers are error free in their gettimex64 paths.

* AMD pesando/ionic
* Broadcom Tigon3
* Intel ixgbe
* Intel igc
* NVIDIA mlxsw
* NVIDIA mlx5_core

The above drivers would definitely not benefit from having "phc
(nic)"-to-host related statistics being presented here. I am more in
favor of making these statistics specific to amazon's ENA driver since I
think most drivers do not have a complex . Also, what value is there in
the count of phc-to-host successful/failed operations versus just
keeping track of the errors in userspace for whoever is calling
clock_gettime. I am somewhat ok with these counters, but I honestly
cannot imagine any practical use to this especially since they are not
related to anything over-the-wire. So the errors in userspace would be
enough of an indicator of whether there is excessive utilization of the
requests and the counters seem redundant to that (at least to me). Feel
free to share how you feel these counters would be helpful beyond
handling the return codes through clock_gettime. I might just be missing
something.

Hope this helps.

-- 
Thanks,

Rahul Rameshbabu

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ