On Sun, Aug 30, 2020 at 10:14 PM Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote: > > On Sun, Aug 30, 2020 at 03:32:09AM -0700, Sagi Grimberg wrote: > > Currently we allocate rx buffers in a single contiguous buffers > > for headers (iser and iscsi) and data trailer. This means > > that most likely the data starting offset is aligned to 76 > > bytes (size of both headers). > > > > This worked fine for years, but at some point this broke. > > To fix this, we should avoid passing unaligned buffers for > > I/O. > > > > Instead, we allocate our recv buffers with some extra space > > such that we can have the data portion align to 512 byte boundary. > > This also means that we cannot reference headers or data using > > structure but rather accessors (as they may move based on alignment). > > Also, get rid of the wrong __packed annotation from iser_rx_desc > > as this has only harmful effects (not aligned to anything). > > > > This affects the rx descriptors for iscsi login and data plane. > > What are the symptoms of the breakage? You don't get the same blocks back that you just wrote. A simple program that tags sectors with sector numbers gets blocks that are shifted by 72 bytes. Shows up basically instantly. Completely non-functional for pretty much any application. I tested this on 5.4.61 with both Mellanox Connect X3 (ROCE) and Chelsio T580 cards (iWarp). Both required that ImmediateData be turned off on the target side before the patch. After the patch, it works both ways. This is from a thread that started late last year and went thru about March. Sagi then seems to drop it, until I pinged him on it a few days ago. He asked me to test the patch and then I did. My tests were only two systems and I did not dig into the patch. I don't think Sagi has the hardware to test this end to end himself. I am not sure how much the ImmediateData hurts performance. I get wire speed either way on my single socket E5-1650 v3 server. Doug Dumitru [posting twice, forgot to turn plain text on, sorry] -- Doug Dumitru EasyCo LLC
