Discussion:
m_defrag
(too old to reply)
c***@sdf.org
2018-09-01 18:48:19 UTC
Permalink
Does netbsd have a version of m_defrag that does something useful, like
what the openbsd version does?
I want to have one mbuf. netbsd gives me two.

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Maxime Villard
2018-09-01 19:01:37 UTC
Permalink
Post by c***@sdf.org
Does netbsd have a version of m_defrag that does something useful, like
what the openbsd version does?
I want to have one mbuf. netbsd gives me two.
My commit message: "It is important not to compress the first mbuf with hacks,
because the storage of this first mbuf may be shared with other mbufs". OpenBSD
does not support shared mbufs.

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
m***@netbsd.org
2018-09-01 20:15:38 UTC
Permalink
Post by Maxime Villard
Post by c***@sdf.org
Does netbsd have a version of m_defrag that does something useful, like
what the openbsd version does?
I want to have one mbuf. netbsd gives me two.
My commit message: "It is important not to compress the first mbuf with hacks,
because the storage of this first mbuf may be shared with other mbufs". OpenBSD
does not support shared mbufs.
OK. sorry. Thanks for explaining why I can't use OpenBSD's code.

So, to elaborate on my situation: I want m_defrag because I want the
mbuf chain to be of a length that I can bus_dmamap_load_mbuf.

I believe m_defrag is a bad API:

- Looks like OpenBSD m_defrag, which returns error, not mbuf.
(This isn't noticed in code ports, and you had to fix one driver who got
this wrong. bwfm at pci, which I ported, also gets this wrong, and this
is why it only looks like it works).

- Doesn't give me any guarantee it is a length I can use, just "tries to
defrag it".

- Looks like other drivers open code this now because they do need a
guarantee (like dev/ic/bwi.c:9182-9216)

FreeBSD opted to write m_collapse, which also seems appealing.
I'm a newbie to networking so it will take a while to come with a diff,
so bear with me.

Here is another case that is wrong found by taylor:
https://nxr.netbsd.org/xref/src/sys/dev/ic/rt2860.c#1784
This won't work, because rt2860 also can only handle length 1:
https://nxr.netbsd.org/xref/src/sys/dev/ic/rt2860.c#529

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
m***@netbsd.org
2018-09-01 20:23:30 UTC
Permalink
Post by m***@netbsd.org
https://nxr.netbsd.org/xref/src/sys/dev/ic/rt2860.c#1784
https://nxr.netbsd.org/xref/src/sys/dev/ic/rt2860.c#529
Oops, this is the wrong case, I got it wrong.

taylor says it is
https://nxr.netbsd.org/xref/src/sys/dev/pci/if_rtwn.c#1980

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Taylor R Campbell
2018-09-01 20:41:56 UTC
Permalink
Context:

Network drivers have a limited number of tx descriptors to program
with DMA addresses when transmitting a packet. When we have an mbuf
ready to transmit, we prepare the IOMMU or whatever and get DMA
addresses to point the tx descriptors at with bus_dmamap_load_mbuf,
which fails if the mbuf has more noncontiguous segments than there are
DMA segments available in the DMA map.

Most drivers have more than one tx descriptor, so most of the time,
even if an mbuf is split into multiple segments, bus_dmamap_load_mbuf
will succeed.

For those cases when there are too many mbuf segments, most of our
drivers have copypasta that will allocate a new single-segment mbuf,
and copy the data into it to transmit.

Some drivers use m_defrag, which does this for _everything after the
first segment_ of the mbuf but leaves the identity of the mbuf intact,
in case there are pointers to it elsewhere.

The trouble with bwfm is that there is only one tx descriptor, so
m_defrag always fails to do what it needs. OpenBSD does some grody
hacks to compress it into a single mbuf without changing the identity
of the mbuf but apparently we can't do that.

Conceivably, there might be drivers with jumbo frames that can't
service either because two segments is not _enough_ segments. FreeBSD
also has a function m_collapse that takes the maximum number of
fragments (but does not preserve the identity of the mbuf).

What should we do? m_collapse seems like a good idea to me, provided
we take care when adapting drivers to use it that it is OK to change
the identity of the mbuf -- which most of the copypasta does anyway.

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Martin Husemann
2018-09-02 10:05:15 UTC
Permalink
Post by Taylor R Campbell
What should we do? m_collapse seems like a good idea to me, provided
we take care when adapting drivers to use it that it is OK to change
the identity of the mbuf -- which most of the copypasta does anyway.
Pre allocate a special buffer for this in the driver and just do m_copydata()
when the chain does not fit?

Martin

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Loading...