On 16 October 2017 at 10:57, Gleb Smirnoff <gleb...@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 _______________________________________________ freebsd-net@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
