From 106ae4078642dd30624e9f6bfccf6747c9a88969 Mon Sep 17 00:00:00 2001
From: sephe <sephe@ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f>
Date: Wed, 15 Jun 2016 05:16:37 +0000
Subject: [PATCH] MFC 296178

    buf_ring/drbr: Add buf_ring_peek_clear_sc and use it in drbr_peek

    Unlike buf_ring_peek, it only supports single consumer mode, and it
    clears the cons_head if DEBUG_BUFRING/INVARIANTS is defined.

    The normal use case of drbr_peek for network drivers is:

    m = drbr_peek(br);
    err = hw_spec_encap(&m); /* could m_defrag/m_collapse */
    (*)
    if (err) {
        if (m == NULL)
            drbr_advance(br);
        else
            drbr_putback(br, m);
        /* break the loop */
    }
    drbr_advance(br);

    The race is:
    If hw_spec_encap() m_defrag or m_collapse the mbuf, i.e. the old mbuf
    was freed, or like the Hyper-V's network driver, that transmission-
    done does not even require the TX lock; then on the other CPU at the
    (*) time, the freed mbuf could be recycled and being drbr_enqueue even
    before the current CPU had the chance to call drbr_{advance,putback}.
    This triggers a panic in drbr_enqueue duplicated element check, if
    DEBUG_BUFRING/INVARIANTS is defined.

    Use buf_ring_peek_clear_sc() in drbr_peek() to fix the above race.

    This change is a NO-OP, if neither DEBUG_BUFRING nor INVARIANTS are
    defined.

    MFC after:  1 week
    Sponsored by:       Microsoft OSTC
    Differential Revision:      https://reviews.freebsd.org/D5416


git-svn-id: svn://svn.freebsd.org/base/stable/10@301913 ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f
---
 sys/net/if_var.h   |  2 +-
 sys/sys/buf_ring.h | 31 +++++++++++++++++++++++++++++++
 2 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/sys/net/if_var.h b/sys/net/if_var.h
index 1ff7a4beb..01ca26c1c 100644
--- a/sys/net/if_var.h
+++ b/sys/net/if_var.h
@@ -705,7 +705,7 @@ drbr_peek(struct ifnet *ifp, struct buf_ring *br)
 		return (m);
 	}
 #endif
-	return(buf_ring_peek(br));
+	return(buf_ring_peek_clear_sc(br));
 }
 
 static __inline void
diff --git a/sys/sys/buf_ring.h b/sys/sys/buf_ring.h
index ef930569f..b752bfa95 100644
--- a/sys/sys/buf_ring.h
+++ b/sys/sys/buf_ring.h
@@ -263,6 +263,37 @@ buf_ring_peek(struct buf_ring *br)
 	return (br->br_ring[br->br_cons_head]);
 }
 
+static __inline void *
+buf_ring_peek_clear_sc(struct buf_ring *br)
+{
+#ifdef DEBUG_BUFRING
+	void *ret;
+
+	if (!mtx_owned(br->br_lock))
+		panic("lock not held on single consumer dequeue");
+#endif	
+	/*
+	 * I believe it is safe to not have a memory barrier
+	 * here because we control cons and tail is worst case
+	 * a lagging indicator so we worst case we might
+	 * return NULL immediately after a buffer has been enqueued
+	 */
+	if (br->br_cons_head == br->br_prod_tail)
+		return (NULL);
+
+#ifdef DEBUG_BUFRING
+	/*
+	 * Single consumer, i.e. cons_head will not move while we are
+	 * running, so atomic_swap_ptr() is not necessary here.
+	 */
+	ret = br->br_ring[br->br_cons_head];
+	br->br_ring[br->br_cons_head] = NULL;
+	return (ret);
+#else
+	return (br->br_ring[br->br_cons_head]);
+#endif
+}
+
 static __inline int
 buf_ring_full(struct buf_ring *br)
 {
-- 
2.45.0