d/freebsd-dev
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

buf_ring/drbr: Add buf_ring_peek_clear_sc and use it in drbr_peek

Browse Source 
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
This commit is contained in:
Sepherosa Ziehau 2016-02-29 03:54:51 +00:00
parent 0aeed3e993
commit 1321c5029e
Notes: svn2git 2020-12-20 02:59:44 +00:00 
svn path=/head/; revision=296178
2 changed files with 32 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
sys/net/ifq.h
View File

@ -369,7 +369,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

31
sys/sys/buf_ring.h
View File

@ -268,6 +268,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)
{