btrfs development - question about crypto api integration

btrfs development - question about crypto api integration

[Jean Fobe]

Hi all,
    I've been studying LZ4 and other compression algorithms on the
kernel, and seen other projects such as zram and ubifs using the
crypto api. Is there a technical reason for not using the crypto api
for compression (and possibly for encryption) in btrfs?
    I did not find any design/technical implementation choices in
btrfs development in the developer's FAQ on the wiki. If I completely
missed it, could someone point me in the right direction?
    Lastly, if there is no technical reason for this, would it be
something interesting to have implemented?

Best regards

Re: btrfs development - question about crypto api integration

[Nikolay Borisov]

On 29.11.18 г. 18:43 ч., Jean Fobe wrote:
> Hi all,
>     I've been studying LZ4 and other compression algorithms on the
> kernel, and seen other projects such as zram and ubifs using the
> crypto api. Is there a technical reason for not using the crypto api
> for compression (and possibly for encryption) in btrfs?
>     I did not find any design/technical implementation choices in
> btrfs development in the developer's FAQ on the wiki. If I completely
> missed it, could someone point me in the right direction?
>     Lastly, if there is no technical reason for this, would it be
> something interesting to have implemented?

I personally think it would be better if btrfs' exploited the generic
framework. And in fact when you look at zstd, btrfs does use the
generic, low-level ZSTD routines but not the crypto library wrappers. If
I were I'd try and convert zstd (since it's the most recently added
algorithm) to using the crypto layer to see if there are any lurking
problems.

> 
> Best regards
> 

Re: btrfs development - question about crypto api integration

[Chris Mason]

On 29 Nov 2018, at 12:37, Nikolay Borisov wrote:

> On 29.11.18 г. 18:43 ч., Jean Fobe wrote:
>> Hi all,
>>     I've been studying LZ4 and other compression algorithms on the
>> kernel, and seen other projects such as zram and ubifs using the
>> crypto api. Is there a technical reason for not using the crypto api
>> for compression (and possibly for encryption) in btrfs?
>>     I did not find any design/technical implementation choices in
>> btrfs development in the developer's FAQ on the wiki. If I completely
>> missed it, could someone point me in the right direction?
>>     Lastly, if there is no technical reason for this, would it be
>> something interesting to have implemented?
>
> I personally think it would be better if btrfs' exploited the generic
> framework. And in fact when you look at zstd, btrfs does use the
> generic, low-level ZSTD routines but not the crypto library wrappers. 
> If
> I were I'd try and convert zstd (since it's the most recently added
> algorithm) to using the crypto layer to see if there are any lurking
> problems.

Back when I first added the zlib support, the zlib API was both easier 
to use and a better fit for our async worker threads.  That doesn't mean 
we shouldn't switch, it's just how we got to step one ;)

-chris

Re: btrfs development - question about crypto api integration

[Nikolay Borisov]

On 30.11.18 г. 17:22 ч., Chris Mason wrote:
> On 29 Nov 2018, at 12:37, Nikolay Borisov wrote:
> 
>> On 29.11.18 г. 18:43 ч., Jean Fobe wrote:
>>> Hi all,
>>>     I've been studying LZ4 and other compression algorithms on the
>>> kernel, and seen other projects such as zram and ubifs using the
>>> crypto api. Is there a technical reason for not using the crypto api
>>> for compression (and possibly for encryption) in btrfs?
>>>     I did not find any design/technical implementation choices in
>>> btrfs development in the developer's FAQ on the wiki. If I completely
>>> missed it, could someone point me in the right direction?
>>>     Lastly, if there is no technical reason for this, would it be
>>> something interesting to have implemented?
>>
>> I personally think it would be better if btrfs' exploited the generic
>> framework. And in fact when you look at zstd, btrfs does use the
>> generic, low-level ZSTD routines but not the crypto library wrappers. 
>> If
>> I were I'd try and convert zstd (since it's the most recently added
>> algorithm) to using the crypto layer to see if there are any lurking
>> problems.
> 
> Back when I first added the zlib support, the zlib API was both easier 
> to use and a better fit for our async worker threads.  That doesn't mean 
> we shouldn't switch, it's just how we got to step one ;)

And what about zstd? WHy is it also using the low level api and not the
crypto wrappers?

> 
> -chris
> 

Re: btrfs development - question about crypto api integration

[David Sterba]

On Fri, Nov 30, 2018 at 06:27:58PM +0200, Nikolay Borisov wrote:
> 
> 
> On 30.11.18 г. 17:22 ч., Chris Mason wrote:
> > On 29 Nov 2018, at 12:37, Nikolay Borisov wrote:
> > 
> >> On 29.11.18 г. 18:43 ч., Jean Fobe wrote:
> >>> Hi all,
> >>>     I've been studying LZ4 and other compression algorithms on the
> >>> kernel, and seen other projects such as zram and ubifs using the
> >>> crypto api. Is there a technical reason for not using the crypto api
> >>> for compression (and possibly for encryption) in btrfs?
> >>>     I did not find any design/technical implementation choices in
> >>> btrfs development in the developer's FAQ on the wiki. If I completely
> >>> missed it, could someone point me in the right direction?
> >>>     Lastly, if there is no technical reason for this, would it be
> >>> something interesting to have implemented?
> >>
> >> I personally think it would be better if btrfs' exploited the generic
> >> framework. And in fact when you look at zstd, btrfs does use the
> >> generic, low-level ZSTD routines but not the crypto library wrappers. 
> >> If
> >> I were I'd try and convert zstd (since it's the most recently added
> >> algorithm) to using the crypto layer to see if there are any lurking
> >> problems.
> > 
> > Back when I first added the zlib support, the zlib API was both easier 
> > to use and a better fit for our async worker threads.  That doesn't mean 
> > we shouldn't switch, it's just how we got to step one ;)
> 
> And what about zstd? WHy is it also using the low level api and not the
> crypto wrappers?

I think beacuse it just copied the way things already were.

Here's fs/ubifs/compress.c as an example use of the API in a filesystem:

ubifs_compress
  crypto_comp_compress
    crypto_comp_crt -- address of, dereference
    ->cot_commpress -- takes another address of something, indirect
		       function call
      with value 'crypto_compress'
	-- this does 2 pointer dereferences and indirect call to
	   coa_compress
	   coa_compress = lzo_compress
	     lzo_compress -- pointer dereferences for crypto context
	       lzo1x_1_compress -- through static __lzo_compress (no overhead)

while in btrfs:

btrfs_compress_pages
  ->compress_pages
    lzo1x_1_compress

The crypto API adds several pointer and function call indirectinos, I'm
not sure I want to get rid of the direct calls just yet.