10 Feb
2018
10 Feb
'18
7:32 a.m.
On 2/9/2018 5:01 PM, Ferenc Wágner wrote:
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.
Agreed here, we are on the same page.
Fabio
"Fabio M. Di Nitto"
<fdinitto(a)redhat.com> writes:
right.
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. 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? 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?