[PATCH 0/7] net: fixup time stamping batch #1

[PATCH 0/7] net: fixup time stamping batch #1

[Richard Cochran]

As Eric Dumazet helpfully pointed out, the placing the transmit time
stamp hook after giving the skb to hardware races with the completion
ISR, for those drivers which free the buffer in the ISR.

This series fixes up the first batch of time stamping hook patches.

Richard Cochran (7):
  net: correct comment on where to place transmit time stamp hook.
  fec: fix race in transmit time stamping.
  tg3: fix race in transmit time stamping.
  r6040: fix race in transmit time stamping.
  stmmac: fix race in transmit time stamping.
  smsc9420: fix race in transmit time stamping.
  davinci_emac: fix race in transmit time stamping.

 drivers/net/davinci_emac.c       |    3 ++-
 drivers/net/fec.c                |    4 ++--
 drivers/net/r6040.c              |    5 +++--
 drivers/net/smsc9420.c           |    4 ++--
 drivers/net/stmmac/stmmac_main.c |    4 ++--
 drivers/net/tg3.c                |    4 ++--
 include/linux/skbuff.h           |    3 +--
 7 files changed, 14 insertions(+), 13 deletions(-)

[PATCH 1/7] net: correct comment on where to place transmit time stamp hook.

[Richard Cochran]

The comment for the skb_tx_timestamp() function suggests calling it just
after a buffer is released to the hardware for transmission. However,
for drivers that free the buffer in an ISR, this produces a race between
the time stamp code and the ISR. This commit changes the comment to advise
placing the call just before handing the buffer over to the hardware.

Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
---
 include/linux/skbuff.h |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index c0a4f3a..3e54337 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2028,8 +2028,7 @@ static inline void sw_tx_timestamp(struct sk_buff *skb)
  * skb_tx_timestamp() - Driver hook for transmit timestamping
  *
  * Ethernet MAC Drivers should call this function in their hard_xmit()
- * function as soon as possible after giving the sk_buff to the MAC
- * hardware, but before freeing the sk_buff.
+ * function immediately before giving the sk_buff to the MAC hardware.
  *
  * @skb: A socket buffer.
  */
-- 
1.7.0.4

[PATCH 2/7] fec: fix race in transmit time stamping.

[Richard Cochran]

Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
---
 drivers/net/fec.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/fec.c b/drivers/net/fec.c
index 965fe90..7ae3f28 100644
--- a/drivers/net/fec.c
+++ b/drivers/net/fec.c
@@ -324,10 +324,10 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 
 	fep->cur_tx = bdp;
 
-	spin_unlock_irqrestore(&fep->hw_lock, flags);
-
 	skb_tx_timestamp(skb);
 
+	spin_unlock_irqrestore(&fep->hw_lock, flags);
+
 	return NETDEV_TX_OK;
 }
 
-- 
1.7.0.4

[PATCH 3/7] tg3: fix race in transmit time stamping.

[Richard Cochran]

Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
---
 drivers/net/tg3.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index 743d997..34ea37d 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -6088,11 +6088,11 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb, struct net_device *dev)
 		entry = NEXT_TX(tnapi->tx_prod);
 	}
 
+	skb_tx_timestamp(skb);
+
 	/* Packets are ready, update Tx producer idx local and on card. */
 	tw32_tx_mbox(tnapi->prodmbox, entry);
 
-	skb_tx_timestamp(skb);
-
 	tnapi->tx_prod = entry;
 	if (unlikely(tg3_tx_avail(tnapi) <= (MAX_SKB_FRAGS + 1))) {
 		netif_tx_stop_queue(txq);
-- 
1.7.0.4

[PATCH 4/7] r6040: fix race in transmit time stamping.

[Richard Cochran]

Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
---
 drivers/net/r6040.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/r6040.c b/drivers/net/r6040.c
index 5ee5f8f..00f06e9 100644
--- a/drivers/net/r6040.c
+++ b/drivers/net/r6040.c
@@ -836,6 +836,9 @@ static netdev_tx_t r6040_start_xmit(struct sk_buff *skb,
 	descptr->buf = cpu_to_le32(pci_map_single(lp->pdev,
 		skb->data, skb->len, PCI_DMA_TODEVICE));
 	descptr->status = DSC_OWNER_MAC;
+
+	skb_tx_timestamp(skb);
+
 	/* Trigger the MAC to check the TX descriptor */
 	iowrite16(0x01, ioaddr + MTPR);
 	lp->tx_insert_ptr = descptr->vndescp;
@@ -846,8 +849,6 @@ static netdev_tx_t r6040_start_xmit(struct sk_buff *skb,
 
 	spin_unlock_irqrestore(&lp->lock, flags);
 
-	skb_tx_timestamp(skb);
-
 	return NETDEV_TX_OK;
 }
 
-- 
1.7.0.4

[PATCH 5/7] stmmac: fix race in transmit time stamping.

[Richard Cochran]

Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
---
 drivers/net/stmmac/stmmac_main.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/stmmac/stmmac_main.c b/drivers/net/stmmac/stmmac_main.c
index 5b85f4c..d4adc80 100644
--- a/drivers/net/stmmac/stmmac_main.c
+++ b/drivers/net/stmmac/stmmac_main.c
@@ -1079,10 +1079,10 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
 
 	dev->stats.tx_bytes += skb->len;
 
-	priv->hw->dma->enable_dma_transmission(priv->ioaddr);
-
 	skb_tx_timestamp(skb);
 
+	priv->hw->dma->enable_dma_transmission(priv->ioaddr);
+
 	return NETDEV_TX_OK;
 }
 
-- 
1.7.0.4

[PATCH 6/7] smsc9420: fix race in transmit time stamping.

[Richard Cochran]

Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
---
 drivers/net/smsc9420.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/smsc9420.c b/drivers/net/smsc9420.c
index 2d84f92..ae4d60a 100644
--- a/drivers/net/smsc9420.c
+++ b/drivers/net/smsc9420.c
@@ -1030,12 +1030,12 @@ static netdev_tx_t smsc9420_hard_start_xmit(struct sk_buff *skb,
 	pd->tx_ring[index].status = TDES0_OWN_;
 	wmb();
 
+	skb_tx_timestamp(skb);
+
 	/* kick the DMA */
 	smsc9420_reg_write(pd, TX_POLL_DEMAND, 1);
 	smsc9420_pci_flush_write(pd);
 
-	skb_tx_timestamp(skb);
-
 	return NETDEV_TX_OK;
 }
 
-- 
1.7.0.4

[PATCH 7/7] davinci_emac: fix race in transmit time stamping.

[Richard Cochran]

Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
---
 drivers/net/davinci_emac.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/net/davinci_emac.c b/drivers/net/davinci_emac.c
index f5688ff..55c8245 100644
--- a/drivers/net/davinci_emac.c
+++ b/drivers/net/davinci_emac.c
@@ -1083,6 +1083,8 @@ static int emac_dev_xmit(struct sk_buff *skb, struct net_device *ndev)
 		goto fail_tx;
 	}
 
+	skb_tx_timestamp(skb);
+
 	ret_code = cpdma_chan_submit(priv->txchan, skb, skb->data, skb->len,
 				     GFP_KERNEL);
 	if (unlikely(ret_code != 0)) {
@@ -1090,7 +1092,6 @@ static int emac_dev_xmit(struct sk_buff *skb, struct net_device *ndev)
 			dev_err(emac_dev, "DaVinci EMAC: desc submit failed");
 		goto fail_tx;
 	}
-	skb_tx_timestamp(skb);
 
 	return NETDEV_TX_OK;
 
-- 
1.7.0.4

Re: [PATCH 0/7] net: fixup time stamping batch #1

[David Miller]

From: Richard Cochran <richardcochran@gmail.com>
Date: Sun, 19 Jun 2011 15:31:38 +0200

> As Eric Dumazet helpfully pointed out, the placing the transmit time
> stamp hook after giving the skb to hardware races with the completion
> ISR, for those drivers which free the buffer in the ISR.
> 
> This series fixes up the first batch of time stamping hook patches.

Series applied, thanks.