Page MenuHomeFreeBSD

ktls: Propagate EPG_FLAG_ANON to mapped mbufs
Needs ReviewPublic

Authored by markj on Sat, Jun 13, 12:30 AM.
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 73847
Build 70730: arc lint + arc unit

Event Timeline

This makes sense. You just need an ifnet without NOMAP support. Are you seeing this over the loopback interface? I guess we don't support unmapped mbufs for receive, so the loopback needs to convert any unmapped mbufs to mapped when looping them through. Sort of sucks to have to burn another flag just for this. Or rather, this makes me think that we should just define an EXT_FLAG_ANON and eventually use it to replace EPG_FLAG_ANON.

Hmm, or maybe we really should just be using M_RDONLY for the non-ANON case? It almost makes better sense to invert the flag to be more like EXT_FLAG_NONANON as most external buffers are anon-backed, but then the flag probably just mirrors M_RDONLY.

I think adding EXT_FLAG_ANON is fine for now, especially for purposes of an EN. I do wonder if we could retire the ANON flags entirely in main though and instead move to setting M_RDONLY for the non-ANON case. That would also make the code of the original SA patch much simpler (just fail if any of the mbufs are read-only).

sys/kern/kern_mbuf.c
940

So this is used for the path that converts M_EXTPG to "plain" M_EXT via sf_bufs, e.g. when you don't have a NIC that supports NOMAP mbufs?

In D57557#1319182, @jhb wrote:

This makes sense. You just need an ifnet without NOMAP support. Are you seeing this over the loopback interface?

Yes, this is over loopback, I'm experimenting with writing some simple tests for NFS+TLS.

I did some more testing today and verified that NFS-over-TLS works ok with a NIC that supports MEXTPG, so it seems the regression only appears if you're using a non-EXTPG-capable ifnet.

I guess we don't support unmapped mbufs for receive, so the loopback needs to convert any unmapped mbufs to mapped when looping them through. Sort of sucks to have to burn another flag just for this. Or rather, this makes me think that we should just define an EXT_FLAG_ANON and eventually use it to replace EPG_FLAG_ANON.

I don't think there's much pressure on the EXT_FLAG_* space? Most of the bits are unused. Certainly it's better to avoid adding new flags if possible though.

Hmm, or maybe we really should just be using M_RDONLY for the non-ANON case? It almost makes better sense to invert the flag to be more like EXT_FLAG_NONANON as most external buffers are anon-backed, but then the flag probably just mirrors M_RDONLY.

I think adding EXT_FLAG_ANON is fine for now, especially for purposes of an EN. I do wonder if we could retire the ANON flags entirely in main though and instead move to setting M_RDONLY for the non-ANON case. That would also make the code of the original SA patch much simpler (just fail if any of the mbufs are read-only).

I will try it.

I do wonder at the motivation behind D46784 though. Is it just a seatbelt?