hey, thanks for all the fixes!
I'm trying to import a driver (while being very new to
networking/driver stuff), and updating to -current fixed an issue I've
had, most likely from this commit.
Now my ported driver is close to working :-)
On Sat, Apr 28, 2018 at 08:16:15AM +0000, Maxime Villard wrote:
> Module Name: src
> Committed By: maxv
> Date: Sat Apr 28 08:16:15 UTC 2018
>
> Modified Files:
> src/sys/kern: uipc_mbuf.c
>
> Log Message:
> Modify m_defrag, so that it never frees the first mbuf of the chain. While
> here use the given 'flags' argument, and not M_DONTWAIT.
>
> We have a problem with several drivers: they poll an mbuf chain from their
> queues and call m_defrag on them, but m_defrag could update the mbuf
> pointer, so the mbuf in the queue is no longer valid. It is not easy to
> fix each driver, because doing pop+push will reorder the queue, and we
> don't really want that to happen.
>
> This problem was independently spotted by me, Kengo, Masanobu, and other
> people too it seems (perhaps PR/53218).
>
> Now m_defrag leaves the first mbuf in place, and compresses the chain
> only starting from the second mbuf in the chain.
>
> It is important not to compress the first mbuf with hacks, because the
> storage of this first mbuf may be shared with other mbufs.
>
>
> To generate a diff of this commit:
> cvs rdiff -u -r1.210 -r1.211 src/sys/kern/uipc_mbuf.c
>
> Please note that diffs are not public domain; they are subject to the
> copyright notices on the relevant files.
>
> Modified files:
>
> Index: src/sys/kern/uipc_mbuf.c
> diff -u src/sys/kern/uipc_mbuf.c:1.210 src/sys/kern/uipc_mbuf.c:1.211
> --- src/sys/kern/uipc_mbuf.c:1.210 Fri Apr 27 19:06:48 2018
> +++ src/sys/kern/uipc_mbuf.c Sat Apr 28 08:16:15 2018
> @@ -1,4 +1,4 @@
> -/* $NetBSD: uipc_mbuf.c,v 1.210 2018/04/27 19:06:48 maxv Exp $ */
> +/* $NetBSD: uipc_mbuf.c,v 1.211 2018/04/28 08:16:15 maxv Exp $ */
>
> /*
> * Copyright (c) 1999, 2001 The NetBSD Foundation, Inc.
> @@ -62,7 +62,7 @@
> */
>
> #include <sys/cdefs.h>
> -__KERNEL_RCSID(0, "$NetBSD: uipc_mbuf.c,v 1.210 2018/04/27 19:06:48 maxv Exp
> $");
> +__KERNEL_RCSID(0, "$NetBSD: uipc_mbuf.c,v 1.211 2018/04/28 08:16:15 maxv Exp
> $");
>
> #ifdef _KERNEL_OPT
> #include "opt_mbuftrace.h"
> @@ -1475,27 +1475,32 @@ enobufs:
> }
>
> /*
> - * Copy the mbuf chain to a new mbuf chain that is as short as possible.
> - * Return the new mbuf chain on success, NULL on failure. On success,
> - * free the old mbuf chain.
> + * Compress the mbuf chain. Return the new mbuf chain on success, NULL on
> + * failure. The first mbuf is preserved, and on success the pointer returned
> + * is the same as the one passed.
> */
> struct mbuf *
> m_defrag(struct mbuf *mold, int flags)
> {
> struct mbuf *m0, *mn, *n;
> - size_t sz = mold->m_pkthdr.len;
> + int sz;
>
> KASSERT((mold->m_flags & M_PKTHDR) != 0);
>
> - m0 = m_gethdr(flags, MT_DATA);
> + if (mold->m_next == NULL)
> + return mold;
> +
> + m0 = m_get(flags, MT_DATA);
> if (m0 == NULL)
> return NULL;
> - M_COPY_PKTHDR(m0, mold);
> mn = m0;
>
> + sz = mold->m_pkthdr.len - mold->m_len;
> + KASSERT(sz >= 0);
> +
> do {
> - if (sz > MHLEN) {
> - MCLGET(mn, M_DONTWAIT);
> + if (sz > MLEN) {
> + MCLGET(mn, flags);
> if ((mn->m_flags & M_EXT) == 0) {
> m_freem(m0);
> return NULL;
> @@ -1511,7 +1516,7 @@ m_defrag(struct mbuf *mold, int flags)
>
> if (sz > 0) {
> /* need more mbufs */
> - n = m_get(M_NOWAIT, MT_DATA);
> + n = m_get(flags, MT_DATA);
> if (n == NULL) {
> m_freem(m0);
> return NULL;
> @@ -1522,9 +1527,10 @@ m_defrag(struct mbuf *mold, int flags)
> }
> } while (sz > 0);
>
> - m_freem(mold);
> + m_freem(mold->m_next);
> + mold->m_next = m0;
>
> - return m0;
> + return mold;
> }
>
> void
>
