otrd., 2021. g. 2. marts, plkst. 16:18 — lietotājs David Sterba
(<dste...@suse.cz>) rakstīja:
>
> On Sat, Feb 27, 2021 at 10:07:02PM +0200, Dāvis Mosāns wrote:
> > Currently only single checksum byte is outputted.
> > This fixes it so that full checksum is outputted.
>
> Thanks, that really needs fixing.
>
> > Signed-off-by: Dāvis Mosāns <davis...@gmail.com>
> > ---
> > kernel-shared/disk-io.c | 47 ++++++++++++++++++++++++++++++++++++-----
> > 1 file changed, 42 insertions(+), 5 deletions(-)
> >
> > diff --git a/kernel-shared/disk-io.c b/kernel-shared/disk-io.c
> > index 6f584986..8773eed7 100644
> > --- a/kernel-shared/disk-io.c
> > +++ b/kernel-shared/disk-io.c
> > @@ -160,10 +160,45 @@ int btrfs_csum_data(u16 csum_type, const u8 *data, u8
> > *out, size_t len)
> > return -1;
> > }
> >
> > +int btrfs_format_csum(u16 csum_type, const char *data, char *output)
> > +{
> > + int i;
> > + int csum_len = 0;
> > + int position = 0;
> > + int direction = 1;
> > + switch (csum_type) {
> > + case BTRFS_CSUM_TYPE_CRC32:
> > + csum_len = 4;
>
> This duplicates the per-csum size, you could use btrfs_csum_type_size
>
Didn't notice this, fixed in v2.
> > + position = csum_len - 1;
> > + direction = -1;
> > + break;
> > + case BTRFS_CSUM_TYPE_XXHASH:
> > + csum_len = 8;
> > + position = csum_len - 1;
> > + direction = -1;
>
> So the direction says if it's big endian or little endian, right, so for
> xxhash it's bigendian but the crc32c above it's little.
>
The problem is that both crc and xxhash uses native CPU endianess -
they're 32-bit and 64-bit numbers. But sha256 and blake2 always uses
big endian when displayed.
So here I implemented this difference.
> In kernel the format is 0x%*phN, which translates to 'hexadecimal with
> variable length', so all the work is hidden inside printk. But still
> there are no changes in the 'direction'.
I wasn't aware of such format specifier, but unfortunately here in
btrfs-progs printk is just alias to fprintf which doesn't support this
format. So not sure if there's any better way to implement this.
> I haven't actually checked if the printed format matches what kernel
> does but would think that there should be no direction adjustment
> needed.
I looked at kernel's implementation and it always outputs in big
endian and that's why there's no such direction.
kernel output:
> checksum verify failed on 21057101103104 wanted 0x753cdd5f found 0x9c0ba035
> level 0
this patch's output:
> checksum verify failed on 21057101103104 wanted 5FDD3C75 found 35A00B9C
As you can see for crc32c they're reversed. This isn't really big
problem but it can be confusing as most tools output crc as number
which converted to hex won't match kernel's output.
Not sure if we should stick to how kernel is outputting for sake of
consistency or if this would be better...
