Hi, The mbuf_queue API allows read access to integer variables which another CPU may change simultaneously. To prevent miss-optimisations by the compiler, they need the READ_ONCE() macro. Otherwise there could be two read operations with inconsistent values.
Writing to integer in mq_set_maxlen() needs mutex protection. Otherwise the value could change during critical sections. Again the compiler could optimize to multiple read operations within the critical section. With inconsistent values, the behavior is undefined. The header file diff survived a make release. ok? bluhm Index: kern/uipc_mbuf.c =================================================================== RCS file: /data/mirror/openbsd/cvs/src/sys/kern/uipc_mbuf.c,v retrieving revision 1.284 diff -u -p -r1.284 uipc_mbuf.c --- kern/uipc_mbuf.c 14 Aug 2022 01:58:28 -0000 1.284 +++ kern/uipc_mbuf.c 4 May 2023 07:24:13 -0000 @@ -1776,6 +1776,14 @@ mq_hdatalen(struct mbuf_queue *mq) return (hdatalen); } +void +mq_set_maxlen(struct mbuf_queue *mq, u_int maxlen) +{ + mtx_enter(&mq->mq_mtx); + mq->mq_maxlen = maxlen; + mtx_leave(&mq->mq_mtx); +} + int sysctl_mq(int *name, u_int namelen, void *oldp, size_t *oldlenp, void *newp, size_t newlen, struct mbuf_queue *mq) @@ -1793,11 +1801,8 @@ sysctl_mq(int *name, u_int namelen, void case IFQCTL_MAXLEN: maxlen = mq->mq_maxlen; error = sysctl_int(oldp, oldlenp, newp, newlen, &maxlen); - if (!error && maxlen != mq->mq_maxlen) { - mtx_enter(&mq->mq_mtx); - mq->mq_maxlen = maxlen; - mtx_leave(&mq->mq_mtx); - } + if (!error && maxlen != mq->mq_maxlen) + mq_set_maxlen(mq, maxlen); return (error); case IFQCTL_DROPS: return (sysctl_rdint(oldp, oldlenp, newp, mq_drops(mq))); Index: sys/mbuf.h =================================================================== RCS file: /data/mirror/openbsd/cvs/src/sys/sys/mbuf.h,v retrieving revision 1.255 diff -u -p -r1.255 mbuf.h --- sys/mbuf.h 15 Aug 2022 16:15:37 -0000 1.255 +++ sys/mbuf.h 4 May 2023 07:26:00 -0000 @@ -526,6 +526,8 @@ unsigned int ml_hdatalen(struct mbuf_li * mbuf queues */ +#include <sys/atomic.h> + #define MBUF_QUEUE_INITIALIZER(_maxlen, _ipl) \ { MUTEX_INITIALIZER(_ipl), MBUF_LIST_INITIALIZER(), (_maxlen), 0 } @@ -538,12 +540,12 @@ void mq_delist(struct mbuf_queue *, st struct mbuf * mq_dechain(struct mbuf_queue *); unsigned int mq_purge(struct mbuf_queue *); unsigned int mq_hdatalen(struct mbuf_queue *); +void mq_set_maxlen(struct mbuf_queue *, u_int); -#define mq_len(_mq) ml_len(&(_mq)->mq_list) -#define mq_empty(_mq) ml_empty(&(_mq)->mq_list) -#define mq_full(_mq) (mq_len((_mq)) >= (_mq)->mq_maxlen) -#define mq_drops(_mq) ((_mq)->mq_drops) -#define mq_set_maxlen(_mq, _l) ((_mq)->mq_maxlen = (_l)) +#define mq_len(_mq) READ_ONCE((_mq)->mq_list.ml_len) +#define mq_empty(_mq) (mq_len(_mq) == 0) +#define mq_full(_mq) (mq_len((_mq)) >= READ_ONCE((_mq)->mq_maxlen)) +#define mq_drops(_mq) READ_ONCE((_mq)->mq_drops) #endif /* _KERNEL */ #endif /* _SYS_MBUF_H_ */