iscsi: Support unmapped I/O requests in the default initiator.

[jhb]

Summary

• Add icl_pdu_append_bio and icl_pdu_get_bio methods.

• When ICL_NOCOPY is used to append data from an unmapped I/O request to a PDU, construct unmapped mbufs from the relevant pages backing the struct bio.

• Use m_apply with a helper to compute crc32 digests on mbuf chains to handle unmapped mbufs. Since m_apply requires PMAP_HAS_DMAP for unmapped mbufs, only support unmapped requests when PMAP_HAS_DMAP is true.

[mav]

This code looks good to me, but you should also either disable ICL_NOCOPY part if CRC32 is enabled, or teach icl_mbuf_to_crc32c() to work on unmapped mbufs. Also I wonder what happen if you try to connect to local iSCSI target, which also expects mapped mbufs on receive. Will network stack map the buffers automagically?

[jhb]

In D34406#779713, @mav wrote:

This code looks good to me, but you should also either disable ICL_NOCOPY part if CRC32 is enabled, or teach icl_mbuf_to_crc32c() to work on unmapped mbufs. Also I wonder what happen if you try to connect to local iSCSI target, which also expects mapped mbufs on receive. Will network stack map the buffers automagically?

Hmmm, I hadn't tested with digests, yes. @markj id work recently in TCP to efficiently handle checksums on unmapped buffers, so fixing icl_mbuf_to_crc32c() is probably the way to go there.

For loopback, I believe that since the lo(4) driver doesn't advertise support for unmapped mbufs, the mbufs get converted back to mapped mbufs during ip_output. At least, KTLS over loopback works and it also uses unmapped mbufs.

[markj]

In D34406#781016, @jhb wrote:

In D34406#779713, @mav wrote:

This code looks good to me, but you should also either disable ICL_NOCOPY part if CRC32 is enabled, or teach icl_mbuf_to_crc32c() to work on unmapped mbufs. Also I wonder what happen if you try to connect to local iSCSI target, which also expects mapped mbufs on receive. Will network stack map the buffers automagically?

Hmmm, I hadn't tested with digests, yes. @markj id work recently in TCP to efficiently handle checksums on unmapped buffers, so fixing icl_mbuf_to_crc32c() is probably the way to go there.

One caveat, though, is that the m_apply() solution only works when a direct map is available. I'm not sure how much of a concern that is here.

For loopback, I believe that since the lo(4) driver doesn't advertise support for unmapped mbufs, the mbufs get converted back to mapped mbufs during ip_output. At least, KTLS over loopback works and it also uses unmapped mbufs.

[jhb]

In D34406#781032, @markj wrote:

In D34406#781016, @jhb wrote:

In D34406#779713, @mav wrote:

This code looks good to me, but you should also either disable ICL_NOCOPY part if CRC32 is enabled, or teach icl_mbuf_to_crc32c() to work on unmapped mbufs. Also I wonder what happen if you try to connect to local iSCSI target, which also expects mapped mbufs on receive. Will network stack map the buffers automagically?

Hmmm, I hadn't tested with digests, yes. @markj id work recently in TCP to efficiently handle checksums on unmapped buffers, so fixing icl_mbuf_to_crc32c() is probably the way to go there.

One caveat, though, is that the m_apply() solution only works when a direct map is available. I'm not sure how much of a concern that is here.

Yes, I have made it so that icl_soft only declares support for unmapped I/O for PMAP_HAS_DMAP now.

[jhb]

• Handle checksums for unmapped mbufs.

[jhb]

sys/dev/iscsi/icl_soft.c

1193

This could perhaps now just use PHYS_TO_DMAP instead. OTOH, we could perhaps still enable unmapped even for the non-PMAP_HAS_DMAP case as long as checksums are disabled for the data payload. That would mean setting value of 'unmapped' in the handoff hook, but I'm not sure that would work. It's also probably not worth trying to optimize for 32-bit platforms at this point. That might argue for just using PHYS_TO_DMAP here.