compression plugin compress level validation

Fabio M. Di Nitto fdinitto at redhat.com
Sat Feb 10 08:32:05 CET 2018


On 2/9/2018 5:01 PM, Ferenc Wágner wrote:
> "Fabio M. Di Nitto" <fdinitto at redhat.com> writes:
>
>> AFAIR all plugins accept an int value for compress_level so from that
>> perspective it shouldn´t be too much of a problem (aka no API/ABI changes).
> The plugins do accept an integer compress level because that's what they
> get from libknet.  And this is a good fit for most, but requires an
> arbitrary mapping for lzo2, which has no concept of compress level, only
> different compression algorithms, directly selectable by the user.

right.

>
>> I think Feri´s suggestion to drop the filter completely might be too
>> much, instead, after thinking a bit, it might be perfectly reason to
>> change the internals of val_level to try and compress a small buffer to
>> test that the value is good.
>> The val_level is used only at configuration time, so it´s not too
>> expensive to compress one small data chunk for validation and it would
>> remove the whole hardcoded filter of different compress_levels.
> While I can imagine it work, it still somehow feels conceptually
> backwards to me.  What if we let a "bad" value through?  The first send
> will fail, I guess.  Is that really worse than filtering during
> configuration?

That is exactly why we should still try to compress a random 0 byte
buffer with the value during knet_compress_config and verify if the
value is accepted by the underneath library. The config change happen
with a write lock and there is no traffic passing by at all.

so basically:

- normal operations, traffic is going
- user calls knet_compress_config
  - internally:
    - traffic stops (this happens now as well, no changes here)
    - compress level validation API will try to compress a random static
buffer to see if the value is
      accepted by the compression library
   - if not, return error and etc..
   - if yes, move on and release the hell hounds :-)

In short I am suggesting to delegate the val_level to the external
library. I don´t see how that´s worse than filtering during
configuration tho and doesn´t affect any TX traffic.

>
>> For special plugins like lzo2, I think it´s important we improve the
>> logging at configuration time to make sure users at least warned of
>> supported values (as you can see each values map to a specific lzo2 API
>> call to compress).
> It's easy to do and maintain and saves a round trip to the fine
> documentation, so why not?

Agreed here, we are on the same page.

Fabio



More information about the Devel mailing list