Maxime Villard
2019-01-29 08:32:15 UTC
Hi,
I'm working on jumbo frame support for axen(4) and mue(4). However,
unfortunately, I find that we do not have proper kernel frameworks
(1)
We do not have external mbuf cluster capable for receiving jumbo
frames. We need to use m_devget(9) [ale(4), sk(4), ...], or have
per driver pool [bge(4), dge(4), ...]. The former has a performance
problem, whereas the latter makes drivers complicated.
FreeBSD has 4 cluster sizes, MCLBYTES, pagesize, 9KiB, and 16KiB.
https://www.freebsd.org/cgi/man.cgi?query=m_getjcl
Here, the "size" argument should be chosen from 4 values above
(description in man page is misleading).
OpenBSD has 8 cluster sizes, MCLBYTES, MCLBYTES + 2 (ether aligned
2KiB buffer), 4, 8, 9, 12, 16, and 64KiB. They are usable via
http://man.openbsd.org/MCLGETI
Unlike FreeBSD, the "len" argument can be arbitrary; proper cluster
is chosen automatically. The "ifp" argument is not used.
We have several problems when it comes to clusters.I'm working on jumbo frame support for axen(4) and mue(4). However,
unfortunately, I find that we do not have proper kernel frameworks
(1)
We do not have external mbuf cluster capable for receiving jumbo
frames. We need to use m_devget(9) [ale(4), sk(4), ...], or have
per driver pool [bge(4), dge(4), ...]. The former has a performance
problem, whereas the latter makes drivers complicated.
FreeBSD has 4 cluster sizes, MCLBYTES, pagesize, 9KiB, and 16KiB.
https://www.freebsd.org/cgi/man.cgi?query=m_getjcl
Here, the "size" argument should be chosen from 4 values above
(description in man page is misleading).
OpenBSD has 8 cluster sizes, MCLBYTES, MCLBYTES + 2 (ether aligned
2KiB buffer), 4, 8, 9, 12, 16, and 64KiB. They are usable via
http://man.openbsd.org/MCLGETI
Unlike FreeBSD, the "len" argument can be arbitrary; proper cluster
is chosen automatically. The "ifp" argument is not used.
- As you said, we have only one cluster size to begin with.
- MEXTMALLOC is here to allocate variable-sized clusters, but it is
really ugly and should be removed. Same for MEXTADD.
- The error handling of MCLGET() is error-prone.
- The naming of certain functions is confusing, like m_getcl, which
is one typo away from m_clget.
I propose we do this:
- Find another name for m_add, or just remove it (only two users).
- Rename m_clget to m_addjcl, add a 'size' argument to it, and make
it return -1 on error. MCLGET() will be an alias to it with MCLBYTES
as size. The callers are not forced to read the return value and
can still check for M_EXT. We can then smoothly switch them to
handle the return value instead.
- Add m_getjcl, same as FreeBSD, and make it call m_addjcl. This should
solve your problem.
- Make m_getcl an alias to m_getjcl with MCLBYTES as size.
In short, something like:
int
m_addjcl(struct mbuf *m, int how, int size)
{
// transform 'm' into a cluster of size 'size'
// return -1 on error, 0 otherwise
}
#define MCLGET(m, how) m_addjcl((m), (how), MCLBYTES)
struct mbuf *
m_getjcl(int how, int type, int flags, int size)
{
... allocate an mbuf 'm'...
if (m_addjcl(m, how, size) != 0) {
m_free(m);
return NULL;
}
return m;
}
#define m_getcl(how, type, flags) m_getjcl(how, type, flags, MCLBYTES)
This design should simplify a lot of things: we will be able to remove
MEXTMALLOC, improve the error handling of MCLGET, and have a unified
way to allocate jumbo frames.
Changes I would then like to see (but which don't matter right now):
- Rename MCLGET to m_addcl, and switch each caller to handle errors via
the return value. This will be a great improvement.
- If MEXTADD is still needed, rename it to m_addraw. But normally it
shouldn't be needed, and we should be able to remove it.
What do you think about that?
--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de