Skip to content

Commit c26a2c2

Browse files
vladimirolteandavem330
authored andcommitted
gianfar: Fix TX timestamping with a stacked DSA driver
The driver wrongly assumes that it is the only entity that can set the SKBTX_IN_PROGRESS bit of the current skb. Therefore, in the gfar_clean_tx_ring function, where the TX timestamp is collected if necessary, the aforementioned bit is used to discriminate whether or not the TX timestamp should be delivered to the socket's error queue. But a stacked driver such as a DSA switch can also set the SKBTX_IN_PROGRESS bit, which is actually exactly what it should do in order to denote that the hardware timestamping process is undergoing. Therefore, gianfar would misinterpret the "in progress" bit as being its own, and deliver a second skb clone in the socket's error queue, completely throwing off a PTP process which is not expecting to receive it, _even though_ TX timestamping is not enabled for gianfar. There have been discussions [0] as to whether non-MAC drivers need or not to set SKBTX_IN_PROGRESS at all (whose purpose is to avoid sending 2 timestamps, a sw and a hw one, to applications which only expect one). But as of this patch, there are at least 2 PTP drivers that would break in conjunction with gianfar: the sja1105 DSA switch and the felix switch, by way of its ocelot core driver. So regardless of that conclusion, fix the gianfar driver to not do stuff based on flags set by others and not intended for it. [0]: https://www.spinics.net/lists/netdev/msg619699.html Fixes: f0ee7ac ("gianfar: Add hardware TX timestamping support") Signed-off-by: Vladimir Oltean <[email protected]> Acked-by: Richard Cochran <[email protected]> Signed-off-by: David S. Miller <[email protected]>
1 parent 270fe2c commit c26a2c2

File tree

1 file changed

+7
-3
lines changed

1 file changed

+7
-3
lines changed

drivers/net/ethernet/freescale/gianfar.c

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2205,13 +2205,17 @@ static void gfar_clean_tx_ring(struct gfar_priv_tx_q *tx_queue)
22052205
skb_dirtytx = tx_queue->skb_dirtytx;
22062206

22072207
while ((skb = tx_queue->tx_skbuff[skb_dirtytx])) {
2208+
bool do_tstamp;
2209+
2210+
do_tstamp = (skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP) &&
2211+
priv->hwts_tx_en;
22082212

22092213
frags = skb_shinfo(skb)->nr_frags;
22102214

22112215
/* When time stamping, one additional TxBD must be freed.
22122216
* Also, we need to dma_unmap_single() the TxPAL.
22132217
*/
2214-
if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_IN_PROGRESS))
2218+
if (unlikely(do_tstamp))
22152219
nr_txbds = frags + 2;
22162220
else
22172221
nr_txbds = frags + 1;
@@ -2225,7 +2229,7 @@ static void gfar_clean_tx_ring(struct gfar_priv_tx_q *tx_queue)
22252229
(lstatus & BD_LENGTH_MASK))
22262230
break;
22272231

2228-
if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_IN_PROGRESS)) {
2232+
if (unlikely(do_tstamp)) {
22292233
next = next_txbd(bdp, base, tx_ring_size);
22302234
buflen = be16_to_cpu(next->length) +
22312235
GMAC_FCB_LEN + GMAC_TXPAL_LEN;
@@ -2235,7 +2239,7 @@ static void gfar_clean_tx_ring(struct gfar_priv_tx_q *tx_queue)
22352239
dma_unmap_single(priv->dev, be32_to_cpu(bdp->bufPtr),
22362240
buflen, DMA_TO_DEVICE);
22372241

2238-
if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_IN_PROGRESS)) {
2242+
if (unlikely(do_tstamp)) {
22392243
struct skb_shared_hwtstamps shhwtstamps;
22402244
u64 *ns = (u64 *)(((uintptr_t)skb->data + 0x10) &
22412245
~0x7UL);

0 commit comments

Comments
 (0)