Re: npf: nbuf_ensure_contig and large options

[Maxime Villard <max%m00nbsd.net@localhost>]

Le 08/04/2018 à 15:54, Mindaugas Rasiukevicius a écrit :
> Maxime Villard <max%m00nbsd.net@localhost> wrote:
> > For the record, an issue I spotted some time ago; nbuf_ensure_contig calls
> > m_ensure_contig with the first mbuf of the chain, but m_ensure_contig has
> > a maximum 'len' argument of MHLEN, which is below the allowed option
> > offset for IPv6. So if you send a big IPv6 packet, and if an option
> > happens to be split across two mbufs, NPF fails to parse the packet.
> >
> > There probably aren't many reasons for an IPv6 packet to have an option
> > located this far, but that's theoretically possible, and more importantly,
> > correct specwise.
> >
> > NPF needs to be able to do an ensure_contig on secondary mbufs, without
> > moving everything into the first one. Not sure how to achieve this, I will
> > probably just file a PR.
>
> I do not see that as an NPF problem.  It expects the network stack to
> support certain operations and, in this case, it is a limitation of the
> mbuf interface and m_ensure_contig().

No it's not, the mbuf interface provides a way to read the packet entirely
(via mbuf chains), and npf already handles mbuf chains correctly everywhere
except in nbuf_ensure_contig. Here the limitation comes from NPF only, not
from the kernel.

> The mbuf(9) API and the underlying structuring could be improved (it is
> really dated by now).  While mbufs of an arbitrary size can already be
> allocated, it is very much desirable to have a fixed-size pool cache of
> them (MLEN = 512 is also sensitive to performance).

Fixed-size pools are already 99% of the mbufs allocated by the kernel; either
sizeof(struct mbuf) (normal) or MCLBYTES (cluster). Arbitrary sizes are rare,
only come from some drivers, and they have good reasons for using them
(optimization, avoid reallocating mbufs and copying the driver buffers all
the time).

It doesn't seem to me there are many optimizations to be found here.

> I would argue that it is not worth investing a lot into this problem of
> handling an edge case unless weird IPv6 option offsets become a reality.

Yes, basically it's not worth spending too much time fixing this, because
such edge cases generally don't happen in the wild. My email was just for
the record, I don't really intend to work on that.