Page MenuHomeFreeBSD

cxgbe(4): Save proper zone index on low memory in refill_fl().
ClosedPublic

Authored by mav on Feb 16 2021, 10:11 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Apr 14, 2:11 AM
Unknown Object (File)
Thu, Apr 11, 12:21 PM
Unknown Object (File)
Sun, Mar 31, 6:51 AM
Unknown Object (File)
Fri, Mar 29, 3:29 PM
Unknown Object (File)
Mar 3 2024, 11:09 PM
Unknown Object (File)
Feb 4 2024, 10:46 PM
Unknown Object (File)
Jan 31 2024, 12:20 AM
Unknown Object (File)
Jan 29 2024, 11:08 AM
Subscribers

Details

Summary

When refill_fl() fails to allocate large (9/16KB) mbuf cluster, it falls back to safe (4KB) ones. But it still saved into sd->zidx the original fl->zidx instead of fl->safe_zidx. It caused number of problems with the later use of that cluster, including possibility of memory and/or data corruption.

While there, make refill_fl() to fallback to the safe zone for all the following clusters of the call.

Test Plan

Fragment memory into 12KB chunks and try to write to iSCSI target from another host. Without the patch the iSCSI connection fails on timeout and/or incorrect values in received iSCSI headers. With the patch the write is slow, but it is working.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

mav requested review of this revision.Feb 16 2021, 10:11 PM
This revision is now accepted and ready to land.Feb 16 2021, 10:25 PM

I've slightly modified the patch to remove lowmem variable.

This revision now requires review to proceed.Feb 16 2021, 10:27 PM
This revision is now accepted and ready to land.Feb 16 2021, 10:28 PM

So lowmem is just an optimization to keep using safe_zidx for the rest of the loop once a zidx has failed once? I could see perhaps doing this as two small commits if so, one to fix the bug (sd->zidx being wrong), and the second to add the lowmem optimization.

In D28716#642898, @jhb wrote:

So lowmem is just an optimization to keep using safe_zidx for the rest of the loop once a zidx has failed once?

Right.

I could see perhaps doing this as two small commits if so, one to fix the bug (sd->zidx being wrong), and the second to add the lowmem optimization.

In the new version I've just uploaded I've done that without extra variable, so now it is even smaller change to split between two commits.