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