netdev - Re: [PATCH net-next 0/3] ice: restore timestamp config after reset

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

Message-ID: <b0bc87bc-cc65-4a45-9110-0dbaf6d39211@intel.com>
Date: Mon, 30 Oct 2023 14:38:11 -0700
From: Jacob Keller <jacob.e.keller@...el.com>
To: <netdev@...r.kernel.org>, David Miller <davem@...emloft.net>, "Jakub
 Kicinski" <kuba@...nel.org>, Jesse Brandeburg <jesse.brandeburg@...el.com>
CC: Karol Kolacinski <karol.kolacinski@...el.com>
Subject: Re: [PATCH net-next 0/3] ice: restore timestamp config after reset



On 10/27/2023 5:23 PM, Jacob Keller wrote:
> We recently discovered during internal validation that the ice driver has
> not been properly restoring Tx timestamp configuration after a device reset,
> which resulted in application failures after a device reset.
> 
> It turned out this problem is two-fold. Since the introduction of the PTP
> support, the driver has been clobbering the user space configuration
> settings during reset. Thus after a reset, the driver will not restore
> timestamps, and will report timestamps as disabled if SIOCGHWTSTAMP ioctl is
> issued.
> 
> In addition, the recently merged auxiliary bus support code missed that
> PFINT_TSYN_MSK must be reprogrammed on the clock owner for E822 devices.
> Failure to restore this register configuration results in the driver no
> longer responding to interrupts from other ports. Depending on the traffic
> pattern, this can either result in increased latency responding to
> timestamps on the non-owner ports, or it can result in the driver never
> reporting any timestamps.
> 
> This series fixes both issues, as well as removes a redundant Tx ring field
> since we can rely on the skb flag as the primary detector for a Tx timestamp
> request.
> 
> I opted to send this to net-next, because my primary focus was on fixing the
> E822 issue which was not introduced until the auxiliary bus which isn't in
> the net tree. I do not know if the fix for the overall timestamp
> configuration issue is applicable to net, since we have had a lot of
> refactor going into the driver to support auxiliary bus as well as in
> preparation for new hardware support.
> 
> I'd like to see this merged so that the timestamping issues are fixed in
> 6.6.
> 

Heh, re-reading this now, I meant 6.7 here. The commit with the PTP
auxiliary busy support is in the code headed for 6.7, not in 6.6

Thanks,
Jake

> Jacob Keller (3):
>   ice: remove ptp_tx ring parameter flag
>   ice: unify logic for programming PFINT_TSYN_MSK
>   ice: restore timestamp configuration after device reset
> 
>  drivers/net/ethernet/intel/ice/ice_main.c |  12 +-
>  drivers/net/ethernet/intel/ice/ice_ptp.c  | 146 ++++++++++++----------
>  drivers/net/ethernet/intel/ice/ice_ptp.h  |   5 +-
>  drivers/net/ethernet/intel/ice/ice_txrx.c |   3 -
>  drivers/net/ethernet/intel/ice/ice_txrx.h |   1 -
>  5 files changed, 84 insertions(+), 83 deletions(-)
> 
> 
> base-commit: 3a04927f8d4b7a4f008f04af41e31173002eb1ea