Page MenuHomeFreeBSD

crypto: Add a new type of crypto buffer for a single mbuf.
ClosedPublic

Authored by jhb on May 5 2021, 9:30 PM.

Details

Summary

This is intended for use in KTLS transmit where each TLS record is
described by a single mbuf that is itself queued in the socket buffer.
Using the existing CRYPTO_BUF_MBUF would result in
bus_dmamap_load_crp() walking additional mbufs in the socket buffer
that are not relevant, but generating a S/G list that potentially
exceeds the limit of the tag (while also wasting CPU cycles).

Sponsored by: Netflix

Diff Detail

Repository
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

jhb requested review of this revision.May 5 2021, 9:30 PM
bcr added a subscriber: bcr.

OK for the man page update.

share/man/man9/crypto_buffer.9
199
sys/kern/subr_bus_dma.c
189

I'm having trouble figuring out if BUS_DMA_LOAD_MBUF needs to be specified in both cases.

sys/mips/nlm/dev/sec/nlmseclib.c
138

IMHO it could be better to combine this with the CRYPTO_BUF_MBUF case, e.g., by breaking out of the loop after the first iteration in the CRYPTO_BUF_SINGLE_MBUF case.

sys/kern/subr_bus_dma.c
189

Hmm, I was going off of bus_dmamap_load_mbuf_sg() below which does not. The flag is not documented (no comment in sys/sys/bus_dma.h)

Humm, this is some weird hack for bus_dma on arm and mips. We apparently decide that DMA'ing to misaligned buffers that are mbufs don't require bounce buffers (but DMA'ing to other unaligned buffers is not ok and has to bounce?) I suspect this is probably _not_ safe for the m_epg regions as there is other valid data in the mbuf around the header and trailer that we wouldn't want to be overwritten by DMA if the CPU had data cached. The ARM bits don't really have a good comment on why mbufs are special, but MIPS has a bit of a comment, and it explicitly talks about assuming there is no valid data in the "other" part of the cache line, so it's ok to not flush the dcache before doing DMA. I feel like that means it is not safe for epg and that the existing code here is correct. Im still not entirely sure why this is fully safe for plain mbufs though (e.g. header mbufs where the CPU has scribbled data into the mbuf that might still be in the cache)

markj added inline comments.
sys/kern/subr_bus_dma.c
189

Indeed, it's not clear to me why it's safe to specify BUS_DMA_LOAD_MBUF is safe to use when data and header fields occupy the same cache line. I agree that we should specify that flag for extpg mbufs.

This revision is now accepted and ready to land.May 7 2021, 2:31 PM
jhb marked 2 inline comments as done.
  • Various review feedback.
This revision now requires review to proceed.May 24 2021, 11:57 PM
This revision is now accepted and ready to land.May 25 2021, 1:50 PM