Date: Wed, 5 Jul 2017 09:23:09 -0700 From: Adrian Chadd <adrian.chadd@gmail.com> To: Karim Fodil-Lemelin <kfodil-lemelin@xiplink.com> Cc: FreeBSD Net <freebsd-net@freebsd.org> Subject: Re: m_move_pkthdr leaves m_nextpkt 'dangling' Message-ID: <CAJ-VmomhJVbZO-G1Ki2sg5Wxrn6xL-zYU1ggoEKS-qPGuocG2g@mail.gmail.com> In-Reply-To: <59567148.1020902@xiplink.com> References: <59567148.1020902@xiplink.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On 30 June 2017 at 08:42, Karim Fodil-Lemelin <kfodil-lemelin@xiplink.com> wrote: > Hi, > > As many of you know, when dealing with IP fragments the kernel will build a > list of packets (fragments) chained together through the m_nextpkt pointer. > This is all good until someone tries to do a M_PREPEND on one of the packet > in the chain and the M_PREPEND has to create an extra mbuf to prepend at the > beginning of the chain. > > When doing so m_move_pkthdr is called to copy the current PKTHDR fields > (tags and flags) to the mbuf that was prepended. The function also does: > > to->m_pkthdr = from->m_pkthdr; > > This, for the case I am interested in, essentially leaves the 'from' mbuf > with a dangling pointer m_nextpkt pointing to the next fragment. While this > is mostly harmless because only mbufs of pkthdr types are supposed to have > m_nextpkt it triggers some panics when running with INVARIANTS in NetGraph > (see ng_base.c :: CHECK_DATA_MBUF(m)): > > ... > if (n->m_nextpkt != NULL) \ > panic("%s: m_nextpkt", __func__); \ > } > ... > > So I would like to propose the following patch: > > @@ -442,10 +442,11 @@ m_move_pkthdr(struct mbuf *to, struct mbuf *from) > if ((to->m_flags & M_EXT) == 0) > to->m_data = to->m_pktdat; > to->m_pkthdr = from->m_pkthdr; /* especially tags */ > SLIST_INIT(&from->m_pkthdr.tags); /* purge tags from src */ > from->m_flags &= ~M_PKTHDR; > + from->m_nextpkt = NULL; > } > > It will reset the m_nextpkt so we don't have two mbufs pointing to the same > next packet. This is fairly harmless and solves a problem for us here at > XipLink. This seems like a no-brainer. :-) Any objections? -adrian
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAJ-VmomhJVbZO-G1Ki2sg5Wxrn6xL-zYU1ggoEKS-qPGuocG2g>