On Thu, Jan 19, 2017 at 11:57:01PM -0800, Simon Ratner wrote:
> Thanks Chris,
> 
> It appears to me that there is questionable benefit to having mbufs sized
> larger than the largest L2CAP fragment size (plus overhead), i.e. the 80
> bytes that Will mentioned. Is that a reasonable statement, or am I missing
> something?
> 
> For incoming data, you always waste memory with larger mbufs, and for
> outgoing data host will take longer to free the memory (since you can't
> free the payload mbuf until the last fragment, as opposed to freeing
> smaller mbufs as you go), and you don't save on the number of copies in the
> host. You will save something on mbuf allocations and mbuf header overhead
> in the app as you are generating the payload, though.

I agree with your analysis.  This is just for BLE data, of course.  If
you use msys for other things (e.g., an alternative to malloc for
internal use), that obviously has an affect on the optimum msys buffer
size.

> When allocating mbufs for the payload, is there something I should do to
> reserve enough leading space for the ACL header to make sure host doesn't
> need to re-allocate it?

Yes - you should use ble_hs_mbuf_att_pkt() to allocate all your mbufs.
This function ensures the resulting mbuf has sufficient leading space
for:
    * ACL data header
    * Basic L2CAP header
    * Four bytes for an ATT command

The last item (four bytes) is imprecise because different ATT commands
have different payload sizes, and this function doesn't know which
command you'll be sending.  Four bytes is the most the host would never
need to prepend to application attribute data.

We should expose some more functions from
net/nimble/host/src/ble_hs_mbuf.c that give the user more control over
the amount of leading space.  This would be useful for when the
application wants to send an ATT command that doesn't need the full four
bytes.

Just to be clear - if you allocate an mbuf that lacks sufficient leading
space (e.g., via os_msys_get_pkthdr()) and pass it to the host, the host
won't reallocate and copy the entire chain; it will just allocate a
single buffer and prepend it to the chain.  This is still wasteful, but
it can be completely avoided by using the ble_hs_mbuf_att_pkt()
function.

> Also, at least in theory, it sounds like you could size mbufs to match the
> fragment exactly -- or pre-fragment the mbuf chain as you are generating
> the payload -- and have zero copies in the host. Could be useful in a
> low-memory situation, if the host was smart enough to take advantage of
> that?

The host isn't smart enough :).  The complication arises from the
"os_mbuf_pkthdr" that the leading buffer contains.  The presence of this
header causes the first buffer to have less capacity than subsequent
buffers.  The fragmentation procedure never frees the chain's leading
buffer.  The assumption is that the data is packed in the mbuf chain,
and that the second buffer doesn't have sufficient leading space to
contain the pkthdr.

A smarter procedure might check how much unused space the second buffer
contains.  If there is sufficient room for the pkthdr, the procedure
would move the data to make room for the pkthdr at the front of the
buffer, then copy the pkthdr into the buffer.  After this, the leading
buffer could be freed.  This might be worth looking into.

Thanks,
Chris

Reply via email to