Date: Mon, 23 Oct 2017 12:41:46 -0700 From: Adrian Chadd <adrian.chadd@gmail.com> To: Gleb Smirnoff <glebius@freebsd.org> Cc: Karim Fodil-Lemelin <kfodil-lemelin@xiplink.com>, FreeBSD Net <freebsd-net@freebsd.org>, "Andrey V. Elsukov" <bu7cher@yandex.ru> Subject: Re: m_move_pkthdr leaves m_nextpkt 'dangling' Message-ID: <CAJ-VmonRSSz9hFuALcF1SLni%2B9qTJBgwbmiO1K-1J4EOTLLEGA@mail.gmail.com> In-Reply-To: <20171016175738.GA1100@FreeBSD.org> References: <59567148.1020902@xiplink.com> <CAJ-VmomhJVbZO-G1Ki2sg5Wxrn6xL-zYU1ggoEKS-qPGuocG2g@mail.gmail.com> <31535133-f95a-5db6-a04c-acc0175fa287@yandex.ru> <59DFD3CC.2000401@xiplink.com> <CAJ-Vmo=JhFwo%2B7FgsZUgQMwOSimcoS8zHL%2BAJFONKS-%2Btv7Eww@mail.gmail.com> <20171013211026.GB1055@FreeBSD.org> <59E4C40E.9060103@xiplink.com> <20171016175738.GA1100@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 16 October 2017 at 10:57, Gleb Smirnoff <glebius@freebsd.org> wrote: > Karim, > > On Mon, Oct 16, 2017 at 10:37:02AM -0400, Karim Fodil-Lemelin wrote: > K> > Not only mbufs of M_PKTHDR may have m_nextpkt set. However, I tend to agree > K> > with the patch. But shouldn't we first copy the m_nextpkt to the new mbuf: > K> > > K> > + to->m_nextpkt = from->m_nextpkt; > K> > + from->m_nextpkt = NULL; > K> > > K> > Same way as we deal with tags. > K> > > K> > > K> > K> I think you are correct. If we look at the 'spirit' of m_move_pkthdr(); > K> In my mind, it is to deep copy all fields related to a packet header and > K> since m_nextpkt should only be carried by packet headers, it makes sense > K> to copy it within m_move_pkthdr(). > K> > K> This also raises the question (my apologies in advance from bringing > K> this up...) of weather or not m_nextpkt belongs in struct m_hdr and not > K> in struct pkthdr. > K> > K> In our case we are copying it explicitly outside the function as most of > K> users of m_move_pkthdr() do. > > Moving m_nextpkt from m_hdr to m_pkthdr would be much more intrusive > change, we can't handle that. > > I think an mbuf with m_nextpkt and no M_PKTRHDR is a valid one. In > a datagram socket buffer that could hold a record. (didn't check that, > just guessing). > > So, any objections on commiting this addition to m_move_pkthdr? > > + to->m_nextpkt = from->m_nextpkt; > + from->m_nextpkt = NULL; None from me. (I haven't checked to see if you've done it yet or not.) -adrian > -- > Gleb Smirnoff
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAJ-VmonRSSz9hFuALcF1SLni%2B9qTJBgwbmiO1K-1J4EOTLLEGA>