Commit c26a2c2d authored by Vladimir Oltean's avatar Vladimir Oltean Committed by David S. Miller

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: f0ee7acf ("gianfar: Add hardware TX timestamping support")
Signed-off-by: default avatarVladimir Oltean <olteanv@gmail.com>
Acked-by: default avatarRichard Cochran <richardcochran@gmail.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 270fe2ce
...@@ -2205,13 +2205,17 @@ static void gfar_clean_tx_ring(struct gfar_priv_tx_q *tx_queue) ...@@ -2205,13 +2205,17 @@ static void gfar_clean_tx_ring(struct gfar_priv_tx_q *tx_queue)
skb_dirtytx = tx_queue->skb_dirtytx; skb_dirtytx = tx_queue->skb_dirtytx;
while ((skb = tx_queue->tx_skbuff[skb_dirtytx])) { while ((skb = tx_queue->tx_skbuff[skb_dirtytx])) {
bool do_tstamp;
do_tstamp = (skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP) &&
priv->hwts_tx_en;
frags = skb_shinfo(skb)->nr_frags; frags = skb_shinfo(skb)->nr_frags;
/* When time stamping, one additional TxBD must be freed. /* When time stamping, one additional TxBD must be freed.
* Also, we need to dma_unmap_single() the TxPAL. * Also, we need to dma_unmap_single() the TxPAL.
*/ */
if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_IN_PROGRESS)) if (unlikely(do_tstamp))
nr_txbds = frags + 2; nr_txbds = frags + 2;
else else
nr_txbds = frags + 1; nr_txbds = frags + 1;
...@@ -2225,7 +2229,7 @@ static void gfar_clean_tx_ring(struct gfar_priv_tx_q *tx_queue) ...@@ -2225,7 +2229,7 @@ static void gfar_clean_tx_ring(struct gfar_priv_tx_q *tx_queue)
(lstatus & BD_LENGTH_MASK)) (lstatus & BD_LENGTH_MASK))
break; break;
if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_IN_PROGRESS)) { if (unlikely(do_tstamp)) {
next = next_txbd(bdp, base, tx_ring_size); next = next_txbd(bdp, base, tx_ring_size);
buflen = be16_to_cpu(next->length) + buflen = be16_to_cpu(next->length) +
GMAC_FCB_LEN + GMAC_TXPAL_LEN; GMAC_FCB_LEN + GMAC_TXPAL_LEN;
...@@ -2235,7 +2239,7 @@ static void gfar_clean_tx_ring(struct gfar_priv_tx_q *tx_queue) ...@@ -2235,7 +2239,7 @@ static void gfar_clean_tx_ring(struct gfar_priv_tx_q *tx_queue)
dma_unmap_single(priv->dev, be32_to_cpu(bdp->bufPtr), dma_unmap_single(priv->dev, be32_to_cpu(bdp->bufPtr),
buflen, DMA_TO_DEVICE); buflen, DMA_TO_DEVICE);
if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_IN_PROGRESS)) { if (unlikely(do_tstamp)) {
struct skb_shared_hwtstamps shhwtstamps; struct skb_shared_hwtstamps shhwtstamps;
u64 *ns = (u64 *)(((uintptr_t)skb->data + 0x10) & u64 *ns = (u64 *)(((uintptr_t)skb->data + 0x10) &
~0x7UL); ~0x7UL);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment