Date: Fri, 7 Jul 2017 17:46:39 +0300 From: "Andrey V. Elsukov" <bu7cher@yandex.ru> To: Adrian Chadd <adrian.chadd@gmail.com>, Karim Fodil-Lemelin <kfodil-lemelin@xiplink.com> Cc: FreeBSD Net <freebsd-net@freebsd.org>, Gleb Smirnoff <glebius@freebsd.org> Subject: Re: m_move_pkthdr leaves m_nextpkt 'dangling' Message-ID: <31535133-f95a-5db6-a04c-acc0175fa287@yandex.ru> In-Reply-To: <CAJ-VmomhJVbZO-G1Ki2sg5Wxrn6xL-zYU1ggoEKS-qPGuocG2g@mail.gmail.com> References: <59567148.1020902@xiplink.com> <CAJ-VmomhJVbZO-G1Ki2sg5Wxrn6xL-zYU1ggoEKS-qPGuocG2g@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
This is an OpenPGP/MIME signed message (RFC 4880 and 3156)
--Kq9FClqC02i4JQKr81nFaqfiAEbJgprfb
Content-Type: multipart/mixed; boundary="UCeCAcPu8bPqcLXC4DbbO7b8n2MgrWxVw";
protected-headers="v1"
From: "Andrey V. Elsukov" <bu7cher@yandex.ru>
To: Adrian Chadd <adrian.chadd@gmail.com>,
Karim Fodil-Lemelin <kfodil-lemelin@xiplink.com>
Cc: FreeBSD Net <freebsd-net@freebsd.org>, Gleb Smirnoff <glebius@freebsd.org>
Message-ID: <31535133-f95a-5db6-a04c-acc0175fa287@yandex.ru>
Subject: Re: m_move_pkthdr leaves m_nextpkt 'dangling'
References: <59567148.1020902@xiplink.com>
<CAJ-VmomhJVbZO-G1Ki2sg5Wxrn6xL-zYU1ggoEKS-qPGuocG2g@mail.gmail.com>
In-Reply-To: <CAJ-VmomhJVbZO-G1Ki2sg5Wxrn6xL-zYU1ggoEKS-qPGuocG2g@mail.gmail.com>
--UCeCAcPu8bPqcLXC4DbbO7b8n2MgrWxVw
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: quoted-printable
On 05.07.2017 19:23, Adrian Chadd wrote:
>> As many of you know, when dealing with IP fragments the kernel will bu=
ild a
>> list of packets (fragments) chained together through the m_nextpkt poi=
nter.
>> This is all good until someone tries to do a M_PREPEND on one of the p=
acket
>> in the chain and the M_PREPEND has to create an extra mbuf to prepend =
at the
>> beginning of the chain.
>>
>> When doing so m_move_pkthdr is called to copy the current PKTHDR field=
s
>> (tags and flags) to the mbuf that was prepended. The function also doe=
s:
>>
>> to->m_pkthdr =3D from->m_pkthdr;
>>
>> This, for the case I am interested in, essentially leaves the 'from' m=
buf
>> with a dangling pointer m_nextpkt pointing to the next fragment. While=
this
>> is mostly harmless because only mbufs of pkthdr types are supposed to =
have
>> m_nextpkt it triggers some panics when running with INVARIANTS in NetG=
raph
>> (see ng_base.c :: CHECK_DATA_MBUF(m)):
>>
>> ...
>> if (n->m_nextpkt !=3D NULL) =
\
>> panic("%s: m_nextpkt", __func__); =
\
>> }
>> ...
>>
>> So I would like to propose the following patch:
>>
>> @@ -442,10 +442,11 @@ m_move_pkthdr(struct mbuf *to, struct mbuf *from=
)
>> if ((to->m_flags & M_EXT) =3D=3D 0)
>> to->m_data =3D to->m_pktdat;
>> to->m_pkthdr =3D from->m_pkthdr; /* especially tags *=
/
>> SLIST_INIT(&from->m_pkthdr.tags); /* purge tags from src=
*/
>> from->m_flags &=3D ~M_PKTHDR;
>> + from->m_nextpkt =3D NULL;
>> }
>>
>> It will reset the m_nextpkt so we don't have two mbufs pointing to the=
same
>> next packet. This is fairly harmless and solves a problem for us here =
at
>> XipLink.
>=20
> This seems like a no-brainer. :-) Any objections?
I think the change is reasonable.
But from other side m_demote_pkthdr() may also need this change.
Maybe we can wait when Gleb will be back and review this? Also he is the
author of the mentioned assertion in netgraph code.
--=20
WBR, Andrey V. Elsukov
--UCeCAcPu8bPqcLXC4DbbO7b8n2MgrWxVw--
--Kq9FClqC02i4JQKr81nFaqfiAEbJgprfb
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: OpenPGP digital signature
Content-Disposition: attachment; filename="signature.asc"
-----BEGIN PGP SIGNATURE-----
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iQEzBAEBCAAdFiEE5lkeG0HaFRbwybwAAcXqBBDIoXoFAllfns8ACgkQAcXqBBDI
oXq01Af/UPqo0Bxyqb4m+GN1rlS9ML7e2J9n4eQGiLnwdAz8lBn1MK/6Chrr4eJg
lHAEu4+dzzCl1a+rWnkaHtG5ONfpHL1ZDijYo0y4DcsHZTUMQSOCV89V1ZJwjs40
Ik/Kp+eoUp5INyr4b5t8NAaL62jqSvYc2HZI2Y8h3s/3toQ6wi865CnrL8Lu02y1
NtX7/ig9uQN2nr4SZwKT8eOr3dBbdqiu6Nuca9MJJsKToFEbgeiobxMraaXjF1RS
wxa5YO80T0sGnWdT30s/jsMfQ2c2H/lYxCNxSKzmu+E1S0PWG1R7zia8b/BNY/XX
NIDa3VA9tRiraI2CerROnl7cwMhhwA==
=Cc77
-----END PGP SIGNATURE-----
--Kq9FClqC02i4JQKr81nFaqfiAEbJgprfb--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?31535133-f95a-5db6-a04c-acc0175fa287>