mbuf: Only allow extpg mbufs if the system has a direct map

[markj]

Summary

Some upcoming changes will modify software checksum routines like
in_cksum() to operate using m_apply(), which uses the direct map to
access packet data for unmapped mbufs. This approach of course does not
work on platforms without a direct map, so we have to disallow the use
of unmapped mbufs on such platforms.

I believe this is the right tradeoff: we only configure KTLS on amd64
and arm64 today (and one KTLS consumer, NFS TLS, requires a direct map
already), and the use of unmapped mbufs with plain sendfile is a recent
optimization that is probably not very relevant for 32-bit platforms.
If need be, we could potentially modify m_apply() to use CPU-private
sendfile buffers on platforms without a direct map.

So, change mb_use_ext_pgs to be hard-wired to zero on systems without a
direct map. Note that PMAP_HAS_DMAP is not a compile-time constant on
some systems, so the default value of mb_use_ext_pgs has to be
determined during boot.

[gallatin]

To be honest, when I did early testing of extpgs, I found by far, hands down, the most improvement for sendfile use of extpgs on 32 bit platforms (x86), and the cases we're worried about (sw checksum) are corner cases that most people should never run into. So I'd prefer if it just defaulted to off on platforms without a direct map, but the user was allowed to force it.

[markj]

In D32940#743740, @gallatin wrote:

To be honest, when I did early testing of extpgs, I found by far, hands down, the most improvement for sendfile use of extpgs on 32 bit platforms (x86), and the cases we're worried about (sw checksum) are corner cases that most people should never run into. So I'd prefer if it just defaulted to off on platforms without a direct map, but the user was allowed to force it.

Based on the number of bug reports we've gotten since 13.0, there's a fair number of users who end up having to use sw checksums. Note for example that any interface that's part of a bridge interface has its txcsum capability masked off.

I'd be ok with allowing users to configure use_mb_ext_pgs on 32-bit platforms if they really wanted, but do we expect anyone would do that?

[gallatin]

We already have code in ip_output() to convert to normal mbuf chains in the case that checksums are off on an egress interface. Is there something special about bridges that they fall through the cracks here?

[gallatin]

I honestly don't care about anything other than amd64 and arm64 and wish we could remove all other architectures, so I don't want to stand in the way too much here. If you think this is the best direction to go in, then that's fine.

[markj]

In D32940#743764, @gallatin wrote:

We already have code in ip_output() to convert to normal mbuf chains in the case that checksums are off on an egress interface. Is there something special about bridges that they fall through the cracks here?

Sorry, I should have provided more background. My plan is to remove that code. See revisions D32941 and D32942 where I did this for sctp_delayed_cksum(). My point about if_bridge is just that it's not that unusual to have to fall back to software checksuming.

The problem is that there's still a number of places, e.g., in pf(4), which calculate checksums but are missing the corresponding mb_unmapped_to_ext() calls. Some of them are hard to fix since we have to modify several layers of code which currently don't expect errors (mb_unmapped_to_ext() can fail to allocate memory) and don't expect the mbuf chain to change (as happens if mb_unmapped_to_ext() is required). Rather than doing that, my idea was to instead modify the software checksum code to handle unmapped mbufs, using m_apply(). But this requires the direct map, hence this change.

[jhb]

sys/kern/kern_mbuf.c

200

So this permits an override if you set the tunable to 1/true, yes?

sys/rpc/rpcsec_tls/rpctls_impl.c

714

I think you have to keep this check in the case that a user forces mb_use_ext_pgs on via the tunable on a 32-bit system.

[gallatin]

In D32940#743788, @markj wrote:

In D32940#743764, @gallatin wrote:

We already have code in ip_output() to convert to normal mbuf chains in the case that checksums are off on an egress interface. Is there something special about bridges that they fall through the cracks here?

Sorry, I should have provided more background. My plan is to remove that code. See revisions D32941 and D32942 where I did this for sctp_delayed_cksum(). My point about if_bridge is just that it's not that unusual to have to fall back to software checksuming.

The problem is that there's still a number of places, e.g., in pf(4), which calculate checksums but are missing the corresponding mb_unmapped_to_ext() calls. Some of them are hard to fix since we have to modify several layers of code which currently don't expect errors (mb_unmapped_to_ext() can fail to allocate memory) and don't expect the mbuf chain to change (as happens if mb_unmapped_to_ext() is required). Rather than doing that, my idea was to instead modify the software checksum code to handle unmapped mbufs, using m_apply(). But this requires the direct map, hence this change.

OK, that makes more sense in context.

Thanks!

[markj]

sys/kern/kern_mbuf.c

200

Oops. It was actually my intent to ignore the tunable value if !PMAP_HAS_DMAP. That is, the policy is not meant to be overridable.

[jhb]

While part of me would like a way to force it on still for 32-bit systems, realistically 32-bit systems are in the past and it's not really worth spending time trying to make them work for this.

[markj]

Ignore the tunable unless PMAP_HAS_DMAP is true.