Re: [dm-crypt] [dm-devel] [RFC PATCH 0/2] dm crypt: Allow unaligned buffer lengths for skcipher devices

[Sudhakar Panneerselvam <sudhakar.panneerselvam@oracle.com>]

> 
> On Thu, 24 Sep 2020, Sudhakar Panneerselvam wrote:
> 
> > > By copying it to a temporary aligned buffer and issuing I/O on this
> > > buffer.
> >
> > I don't like this idea. Because, you need to allocate additional pages
> > for the entire I/O size(for the misaligned case, if you think through
> 
> You can break the I/O to smaller pieces. You can use mempool for
> pre-allocation of the pages.

Assuming we do this, how is this code simpler(based on your
comment below) than the fix in dm-crypt? In fact, this approach 
would make the code change look bad in vhost, at the same time
having performance penalty. By doing this, we are just moving the 
responsibility to other unrelated component.

> 
> > carefully, you will know why we have to allocate a temporary buffer that
> > is as big as the original IO) and on top of it, copying the buffer from
> > original to temporary buffer which is all unnecessary while it can
> > simply be fixed in dm-crypt without any of these additional overheads.
> >
> > >
> > > > Only other
> > > > possibility I see is to have windows fix it by always sending 512 byte
> > > > aligned buffer lengths, but going with my earlier point that every other
> > > > component in the Linux IO path handles this case well except for
> > > > dm-crypt, so it make more sense to fix it in dm-crypt.
> > > >
> > > > Thanks
> > > > Sudhakar
> > >
> > > Are you sure that the problem is only with dm-crypt? You haven't tried
> all
> > > the existing block device drivers, have you?
> >
> > My question is, why dm-crypt worries about alignment requirement of
> > other layers? Is there anything that impacts dm-crypt if the segment
> > lengths are not aligned?(I believe this case is just not handled so far
> 
> Because the code is simpler if it assumes aligned buffers.

Did you get a chance to review my changes? If you want more documentation,
improve the code, etc, let me know, I can do that if there is scope for that.

> 
> > in dm-crypt and my patch addresses it). Should dm-crypt not just pass on
> > all those I/O requests to those respective layers to handle it which
> > will be more graceful?
> >
> > -Sudhakar
> 
> So, suppose that we change dm-crypt to handle your workload - what are
> you
> going to do with other block device drivers that assume aligned bio vector
> length? How will you find all the other drivers that need to be changed?
> 
> You are claiming that "every other component in the Linux IO path handles
> this case well except for dm-crypt", but this claim is wrong. There are
> other driver that don't handle misaligned length as well.

I should not have said, "every other component", I take that back, sorry. How
about doing something like this in crypt_convert_block_skcipher():

Add a check that looks at the alignment requirement of the low-level driver
and reject the I/O if it doesn't meet that requirement. This means, we still
need to handle the case in this function where the low lever driver support
unaligned buffer lengths, that means, my other changes in this function
would still be needed.

Is this acceptable to everyone?

-Sudhakar

> 
> Mikulas
> 
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel
>