| Message ID | 20180409070314.12409-1-richardcochran@gmail.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net] net: dsa: mv88e6xxx: Fix receive time stamp race condition. | expand |
On Mon, Apr 09, 2018 at 12:03:14AM -0700, Richard Cochran wrote: > This patch fixes the race by moving the queue onto a list on the stack > before reading out the latched time stamp value. > > Fixes: c6fe0ad2c3499 ("net: dsa: mv88e6xxx: add rx/tx timestamping support") Dave, please hold off on this patch. I am seeing new problems in my testing with this applied. I still need to get to the bottom of this. Thanks, Richard
On Mon, Apr 09, 2018 at 07:19:31AM -0700, Richard Cochran wrote: > Dave, please hold off on this patch. I am seeing new problems in my > testing with this applied. I still need to get to the bottom of > this. Looks like the new problems are a HW/board glitch. The patch is good to go. Thanks, Richard
From: Richard Cochran <richardcochran@gmail.com> Date: Thu, 12 Apr 2018 10:35:40 -0700 > On Mon, Apr 09, 2018 at 07:19:31AM -0700, Richard Cochran wrote: >> Dave, please hold off on this patch. I am seeing new problems in my >> testing with this applied. I still need to get to the bottom of >> this. > > Looks like the new problems are a HW/board glitch. > > The patch is good to go. Ok, applied and queued up for -stable. Thanks.
diff --git a/drivers/net/dsa/mv88e6xxx/hwtstamp.c b/drivers/net/dsa/mv88e6xxx/hwtstamp.c index ac7694c71266..a036c490b7ce 100644 --- a/drivers/net/dsa/mv88e6xxx/hwtstamp.c +++ b/drivers/net/dsa/mv88e6xxx/hwtstamp.c @@ -285,10 +285,18 @@ static void mv88e6xxx_get_rxts(struct mv88e6xxx_chip *chip, struct sk_buff_head *rxq) { u16 buf[4] = { 0 }, status, seq_id; - u64 ns, timelo, timehi; struct skb_shared_hwtstamps *shwt; + struct sk_buff_head received; + u64 ns, timelo, timehi; + unsigned long flags; int err; + /* The latched timestamp belongs to one of the received frames. */ + __skb_queue_head_init(&received); + spin_lock_irqsave(&rxq->lock, flags); + skb_queue_splice_tail_init(rxq, &received); + spin_unlock_irqrestore(&rxq->lock, flags); + mutex_lock(&chip->reg_lock); err = mv88e6xxx_port_ptp_read(chip, ps->port_id, reg, buf, ARRAY_SIZE(buf)); @@ -311,7 +319,7 @@ static void mv88e6xxx_get_rxts(struct mv88e6xxx_chip *chip, /* Since the device can only handle one time stamp at a time, * we purge any extra frames from the queue. */ - for ( ; skb; skb = skb_dequeue(rxq)) { + for ( ; skb; skb = __skb_dequeue(&received)) { if (mv88e6xxx_ts_valid(status) && seq_match(skb, seq_id)) { ns = timehi << 16 | timelo;
The DSA stack passes received PTP frames to this driver via mv88e6xxx_port_rxtstamp() for deferred delivery. The driver then queues the frame and kicks the worker thread. The work callback reads out the latched receive time stamp and then works through the queue, delivering any non-matching frames without a time stamp. If a new frame arrives after the worker thread has read out the time stamp register but enters the queue before the worker finishes processing the queue, that frame will be delivered without a time stamp. This patch fixes the race by moving the queue onto a list on the stack before reading out the latched time stamp value. Fixes: c6fe0ad2c3499 ("net: dsa: mv88e6xxx: add rx/tx timestamping support") Signed-off-by: Richard Cochran <richardcochran@gmail.com> --- drivers/net/dsa/mv88e6xxx/hwtstamp.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)