Page MenuHomeFreeBSD

Extend m_apply() to support unmapped mbufs.
ClosedPublic

Authored by jhb on May 5 2021, 9:29 PM.
Tags
None
Referenced Files
F81696064: D30132.diff
Sat, Apr 20, 2:19 AM
F81622471: D30132.diff
Fri, Apr 19, 3:21 AM
Unknown Object (File)
Thu, Apr 11, 12:28 PM
Unknown Object (File)
Sun, Mar 31, 6:28 AM
Unknown Object (File)
Mar 3 2024, 11:57 AM
Unknown Object (File)
Mar 1 2024, 11:24 PM
Unknown Object (File)
Mar 1 2024, 11:24 PM
Unknown Object (File)
Mar 1 2024, 11:24 PM
Subscribers

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
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 39421
Build 36310: arc lint + arc unit

Event Timeline

sys/kern/uipc_mbuf.c
1251

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

1254

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

1264

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

1277

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
1251

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'?

1254

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.

1264

Oof, so it does.

sys/kern/uipc_mbuf.c
1251

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
1279

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.