Date: Mon, 16 Oct 2017 10:57:38 -0700 From: Gleb Smirnoff <glebius@FreeBSD.org> To: Karim Fodil-Lemelin <kfodil-lemelin@xiplink.com> Cc: Adrian Chadd <adrian.chadd@gmail.com>, FreeBSD Net <freebsd-net@freebsd.org>, "Andrey V. Elsukov" <bu7cher@yandex.ru> Subject: Re: m_move_pkthdr leaves m_nextpkt 'dangling' Message-ID: <20171016175738.GA1100@FreeBSD.org> In-Reply-To: <59E4C40E.9060103@xiplink.com> 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>
next in thread | previous in thread | raw e-mail | index | archive | help
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; -- Gleb Smirnoff
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20171016175738.GA1100>