[PATCH net v3 0/3] sfc: fix PTP synchronization races

[Alex Austin]

This series fixes PTP synchronization races found by code inspection, and
a related frequency-adjustment bug found while reviewing the locking.

efx_ptp_start() resets the driver's cached current_adjfreq as though
MC_CMD_PTP_OP_ENABLE also resets the NIC's frequency adjustment on the
MC. It does not, so the next time-only adjustment reprograms the clock
from a stale cache. Patch 1 removes this reset.

efx_ptp_pps_worker() and the SIOCSHWTSTAMP path through
efx_ptp_change_mode() both call efx_ptp_synchronize(). Concurrent calls
can interfere with the shared DMA handshake and the shared timeset
results, because the MCDI interface is released inside
efx_mcdi_rpc_finish() and acquired after the handshake flag is cleared.
Patch 2 serializes a function's synchronization transactions and clock
operations under a new mutex. Patch 3 returns the PPS timestamp in
caller-owned storage to prevent concurrent synchronization from
overwriting it while the PPS worker reads it.

Tested on two hosts with X2522-25G NICs cabled back to back at 25G.
Both PCI functions share one clock.

Clock operations on the function that registers the clock were run
against repeated efx_ptp_change_mode() -> efx_ptp_synchronize() on the
other function:

  hwstamp_ctl -i ens41f0np0 -r 1 -t 1
  hwstamp_ctl -i ens41f1np1 -r 1 -t 1

  for i in $(seq 150); do
          phc_ctl /dev/ptp1 freq 1000
          phc_ctl /dev/ptp1 adj 0.0001
          phc_ctl /dev/ptp1 get
  done &
  for i in $(seq 40); do
          hwstamp_ctl -i ens41f1np1 -r 0 -t 0
          hwstamp_ctl -i ens41f1np1 -r 1 -t 1
  done &
  wait

  ethtool -S ens41f1np1 | grep ptp_

                                without    with
  ptp_good_syncs                     400     400
  ptp_bad_syncs                        0       0
  ptp_sync_timeouts                    0       0
  ptp_no_time_syncs                    0       0
  ptp_invalid_sync_windows             0       0
  ptp_undersize_sync_windows           0       8
  ptp_oversize_sync_windows            0       0

The undersize counter rejects readings as implausibly fast rather than
inaccurate. FW reports a 40ns minimum sync window; without the series
the fastest readings sit exactly on that limit, and with it they sit a
few nanoseconds below. The shift tracks the barrier in the mutex
acquisition rather than the serialization it provides: an smp_mb() alone
where the mutex is taken reproduces the counter on an otherwise
unmodified driver, while a plain memory access there does not. No
synchronization failed: each takes eight readings and needs one to
succeed.

End-to-end PTP sync quality is unchanged (tested with ptp4l and
phc2sys).

Changes since v2:
 - add patch 1, dropping the stale current_adjfreq reset in
   efx_ptp_start() found while re-reviewing the locking this series
   adds.
Changes since v1:
 - document in efx_ptp_synchronize() why a secondary function needs no
   cross-function locking.
 - take phc_lock in efx_ptp_start() around the current_adjfreq reset
 - do not take phc_lock in efx_phc_gettime(): it reads no shared state
   and MCDI already serializes the command, so _efx_phc_gettime() is
   gone and efx_phc_settime() calls efx_phc_gettime() under the lock
 - keep the comment above efx_phc_settime() (Edward Cree)
 - add the test results above
 - return the PPS timestamp through the caller's own pps_event_time
   rather than adding a struct.

Alex Austin (3):
  sfc: ptp: drop the stale frequency reset in efx_ptp_start()
  sfc: ptp: serialize host and MC synchronization transactions
  sfc: ptp: avoid racing PPS timestamp updates

 drivers/net/ethernet/sfc/ptp.c | 108 ++++++++++++++++++++++-----------
 1 file changed, 71 insertions(+), 37 deletions(-)

-- 
2.34.1