[Intel-wired-lan] [PATCH 4/4] igc: Fix race condition in PTP tx code
Brown, Aaron F
aaron.f.brown at intel.com
Fri Aug 14 03:09:42 UTC 2020
> From: Intel-wired-lan <intel-wired-lan-bounces at osuosl.org> On Behalf Of
> Andre Guedes
> Sent: Tuesday, July 28, 2020 4:38 PM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [PATCH 4/4] igc: Fix race condition in PTP tx code
>
> Currently, the igc driver supports timestamping only one tx packet at a
> time. During the transmission flow, the skb that requires hardware
> timestamping is saved in adapter->ptp_tx_skb. Once hardware has the
> timestamp, an interrupt is delivered, and adapter->ptp_tx_work is
> scheduled. In igc_ptp_tx_work(), we read the timestamp register, update
> adapter->ptp_tx_skb, and notify the network stack.
>
> While the thread executing the transmission flow (the user process
> running in kernel mode) and the thread executing ptp_tx_work don't
> access adapter->ptp_tx_skb concurrently, there are two other places
> where adapter->ptp_tx_skb is accessed: igc_ptp_tx_hang() and
> igc_ptp_suspend().
>
> igc_ptp_tx_hang() is executed by the adapter->watchdog_task worker
> thread which runs periodically so it is possible we have two threads
> accessing ptp_tx_skb at the same time. Consider the following scenario:
> right after __IGC_PTP_TX_IN_PROGRESS is set in igc_xmit_frame_ring(),
> igc_ptp_tx_hang() is executed. Since adapter->ptp_tx_start hasn't been
> written yet, this is considered a timeout and adapter->ptp_tx_skb is
> cleaned up.
>
> This patch fixes the issue described above by adding the ptp_tx_lock to
> protect access to ptp_tx_skb and ptp_tx_start fields from igc_adapter.
> Since igc_xmit_frame_ring() called in atomic context by the networking
> stack, ptp_tx_lock is defined as a spinlock.
>
> With the introduction of the ptp_tx_lock, the __IGC_PTP_TX_IN_PROGRESS
> flag doesn't provide much of a use anymore so this patch gets rid of it.
>
> Signed-off-by: Andre Guedes <andre.guedes at intel.com>
> ---
> drivers/net/ethernet/intel/igc/igc.h | 5 ++-
> drivers/net/ethernet/intel/igc/igc_main.c | 7 +++-
> drivers/net/ethernet/intel/igc/igc_ptp.c | 49 ++++++++++++++---------
> 3 files changed, 40 insertions(+), 21 deletions(-)
>
Tested-by: Aaron Brown <aaron.f.brown at intel.com>
More information about the Intel-wired-lan
mailing list