Page MenuHomeFreeBSD

Extend m_apply() to support unmapped mbufs.
ClosedPublic

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

Details

Summary

m_apply() invokes the callback function separately on each segment of
an unmapped mbuf: the TLS header, individual pages, and the TLS
trailer.

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

sys/kern/uipc_mbuf.c
1250

Just a suggestion, but I'd find it nicer to handle the M_EXTPG case in a separate function.

1253

I thought m_data == 0 for EXTPG mbufs. When is that not true?

1263

Doesn't pgoff need to be reset to 0 after the first iteration of the loop?

1276

It would be nice to call m_epg_pagelen() and save the result in a local var, there are three calls in this loop.

sys/kern/uipc_mbuf.c
1250

Hmm, most of this is a single function? Would you rather inline the M_EXTPG check below and rename this to 'm_apply_extpg()' or would you want m_apply_one() to still do the check and have a separate 'm_apply_extpg_one'?

1253

m_data is the offset into the mbuf, so when transmitting a portion of a TLS record via TCP m_data can be non-zero. For the purposes of KTLS SW crypto, m_data should always be zero, but all the other places that iterate over an unmapped mbuf honor the m_data and m_len subset range of the backing store so it is consistent (and safest for any potential future uses) to honor it here.

1263

Oof, so it does.

sys/kern/uipc_mbuf.c
1250

Either suggestion seems fine, I think the latter is a bit better. I'm ok keeping it as-is. I just don't like the fact that the extpg code has extra indentation even though almost all the function is extpg-specific. I'm ok keeping it as-is too though.

jhb marked 4 inline comments as done.May 13 2021, 11:17 PM
jhb added inline comments.
sys/kern/uipc_mbuf.c
1278

I've also simplified the logic a bit here (and added some assertions). Since off and len are constrained to the bounds of the mbuf (m_len), the offset and length should never be above the bounds of the mbuf, so this first test here should always be true, and similarly, the 'min' below should always return 'len'.

This revision is now accepted and ready to land.May 25 2021, 1:35 PM
This revision was automatically updated to reflect the committed changes.