netdev - Re: bnxt_en: Incorrect tx timestamp report

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: <CAGtf3iaO+Q=He7xyCCfzfPQDH_dHYYG1rHbpaUe-oBo90JBtjA@mail.gmail.com>
Date: Fri, 21 Mar 2025 17:17:33 +0200
From: Kamil Zaripov <zaripov-kamil@...ide.ai>
To: Jacob Keller <jacob.e.keller@...el.com>
Cc: netdev@...r.kernel.org
Subject: Re: bnxt_en: Incorrect tx timestamp report

> That depends. If it has only one underlying clock, but each PF has its
> own register space, it may functionally be independent clocks in
> practice. I don't know the bnxt_en driver or hardware well enough to
> know if that is the case.

> If it really is one clock with one set of registers to control it, then
> it should only expose one PHC. This may be tricky depending on the
> driver design. (See ice as an example where we've had a lot of
> challenges in this space because of the multiple PFs)

I can only guess, from looking at the __bnxt_hwrm_ptp_qcfg function,
that it depends on hardware and/or firmware (see
https://elixir.bootlin.com/linux/v6.13.7/source/drivers/net/ethernet/broadcom/bnxt/bnxt.c#L9427-L9431).
I hope that broadcom folks can clarify this.

> This part of the driver is tricky. ASYNC_EVENT_CMPL_EVENT_ID_PHC_UPDATE
> reports only 16 bits of 64 bits timestamp, 48-63 range, which doesn't
> overlap with anything else. The assumption is that when the driver
> processes this event, the register which reports bits of range 0-47 has
> already overflowed and holds new value. Unfortunately, there is a time
> gap between register overflow and update of MSB of the cached timestamp.

Indeed, PHC counter reading is pretty complex in the case of the
bnxt_en driver. Final timestamp that is sent to the userspace is
combined from 3 parts which are stored in different places and updated
using different mechanics. Apparently in some corner cases the driver
fails to produce the correct result.

> There is no easy way to solve this problem, but we may add additional
> check on every read, probably... Not sure, though

Right now I've just added an extra check into
bnxt_ptp_rtc_timecounter_init function, it should work in some cases
but I do not believe that it is the right way to fix the original
issue.