m_move_pkthdr leaves m_nextpkt 'dangling'
Gleb Smirnoff
glebius at FreeBSD.org
Mon Oct 16 17:57:46 UTC 2017
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
More information about the freebsd-net
mailing list