Skip to content

Commit

Permalink
igc: Check if hardware TX timestamping is enabled earlier
Browse files Browse the repository at this point in the history
Before requesting a packet transmission to be hardware timestamped,
check if the user has TX timestamping enabled. Fixes an issue that if
a packet was internally forwarded to the NIC, and it had the
SKBTX_HW_TSTAMP flag set, the driver would mark that timestamp as
skipped.

In reality, that timestamp was "not for us", as TX timestamp could
never be enabled in the NIC.

Checking if the TX timestamping is enabled earlier has a secondary
effect that when TX timestamping is disabled, there's no need to check
for timestamp timeouts.

We should only take care to free any pending timestamp when TX
timestamping is disabled, as that skb would never be released
otherwise.

Fixes: 2c344ae ("igc: Add support for TX timestamping")
Suggested-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
Tested-by: Naama Meir <naamax.meir@linux.intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
  • Loading branch information
Vinicius Costa Gomes authored and Tony Nguyen committed Jun 22, 2023
1 parent 9c50e2b commit ce58c7c
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 5 deletions.
1 change: 1 addition & 0 deletions drivers/net/ethernet/intel/igc/igc.h
Original file line number Diff line number Diff line change
Expand Up @@ -581,6 +581,7 @@ enum igc_ring_flags_t {
IGC_RING_FLAG_TX_CTX_IDX,
IGC_RING_FLAG_TX_DETECT_HANG,
IGC_RING_FLAG_AF_XDP_ZC,
IGC_RING_FLAG_TX_HWTSTAMP,
};

#define ring_uses_large_buffer(ring) \
Expand Down
5 changes: 3 additions & 2 deletions drivers/net/ethernet/intel/igc/igc_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -1585,15 +1585,16 @@ static netdev_tx_t igc_xmit_frame_ring(struct sk_buff *skb,
}
}

if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP)) {
if (unlikely(test_bit(IGC_RING_FLAG_TX_HWTSTAMP, &tx_ring->flags) &&
skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP)) {
/* FIXME: add support for retrieving timestamps from
* the other timer registers before skipping the
* timestamping request.
*/
unsigned long flags;

spin_lock_irqsave(&adapter->ptp_tx_lock, flags);
if (adapter->tstamp_config.tx_type == HWTSTAMP_TX_ON && !adapter->ptp_tx_skb) {
if (!adapter->ptp_tx_skb) {
skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
tx_flags |= IGC_TX_FLAGS_TSTAMP;

Expand Down
42 changes: 39 additions & 3 deletions drivers/net/ethernet/intel/igc/igc_ptp.c
Original file line number Diff line number Diff line change
Expand Up @@ -536,22 +536,60 @@ static void igc_ptp_enable_rx_timestamp(struct igc_adapter *adapter)
wr32(IGC_TSYNCRXCTL, val);
}

static void igc_ptp_clear_tx_tstamp(struct igc_adapter *adapter)
{
unsigned long flags;

cancel_work_sync(&adapter->ptp_tx_work);

spin_lock_irqsave(&adapter->ptp_tx_lock, flags);

dev_kfree_skb_any(adapter->ptp_tx_skb);
adapter->ptp_tx_skb = NULL;

spin_unlock_irqrestore(&adapter->ptp_tx_lock, flags);
}

static void igc_ptp_disable_tx_timestamp(struct igc_adapter *adapter)
{
struct igc_hw *hw = &adapter->hw;
int i;

/* Clear the flags first to avoid new packets to be enqueued
* for TX timestamping.
*/
for (i = 0; i < adapter->num_tx_queues; i++) {
struct igc_ring *tx_ring = adapter->tx_ring[i];

clear_bit(IGC_RING_FLAG_TX_HWTSTAMP, &tx_ring->flags);
}

/* Now we can clean the pending TX timestamp requests. */
igc_ptp_clear_tx_tstamp(adapter);

wr32(IGC_TSYNCTXCTL, 0);
}

static void igc_ptp_enable_tx_timestamp(struct igc_adapter *adapter)
{
struct igc_hw *hw = &adapter->hw;
int i;

wr32(IGC_TSYNCTXCTL, IGC_TSYNCTXCTL_ENABLED | IGC_TSYNCTXCTL_TXSYNSIG);

/* Read TXSTMP registers to discard any timestamp previously stored. */
rd32(IGC_TXSTMPL);
rd32(IGC_TXSTMPH);

/* The hardware is ready to accept TX timestamp requests,
* notify the transmit path.
*/
for (i = 0; i < adapter->num_tx_queues; i++) {
struct igc_ring *tx_ring = adapter->tx_ring[i];

set_bit(IGC_RING_FLAG_TX_HWTSTAMP, &tx_ring->flags);
}

}

/**
Expand Down Expand Up @@ -1026,9 +1064,7 @@ void igc_ptp_suspend(struct igc_adapter *adapter)
if (!(adapter->ptp_flags & IGC_PTP_ENABLED))
return;

cancel_work_sync(&adapter->ptp_tx_work);
dev_kfree_skb_any(adapter->ptp_tx_skb);
adapter->ptp_tx_skb = NULL;
igc_ptp_clear_tx_tstamp(adapter);

if (pci_device_is_present(adapter->pdev)) {
igc_ptp_time_save(adapter);
Expand Down

0 comments on commit ce58c7c

Please sign in to comment.