On= Sat, 24 Aug, 2019, 7:30 PM Gao Xiang, <hsiangkao@aol.com&= gt; wrote:

Hi Pratik,

On Sat, Aug 24, 2019 at 06:08:03PM +0530, Pratik Shinde wrote:
> Adding code for superblock checksum calculation.
>
> incorporated the changes suggested in previous patch.
>
> Signed-off-by: Pratik Shinde <pratiksh= inde320@gmail.com>

Thanks for your v2 patch.

Actually, I have some concern about the length of checksum,
sizeof(struct erofs_super_block) could be changed in the
later version, it's bad for EROFS future scalablity.

And I tend not to add another on-disk field to record
the size of erofs_super_block as well, because the old
Linux kernel cannot handle more about the new size,
so it has little use except for checksum calculation.

Few hours ago, I discussed with Chao about this concern,
I think this feature can be changed to do multiple-block
checksum at the mount time, e.g:
=C2=A0- for small images, we can check the whole image once
=C2=A0 =C2=A0at the mount time;
=C2=A0- for the large image, we can check the superblock
=C2=A0 =C2=A0at the mount time, the rest can be handled by
=C2=A0 =C2=A0block-based verification layer.

But we agreed that don't add this for this round
since it's quite a new feature.

All in all, it's a new feature since we are addressing moving
out of staging for this round. I tend to postpone this feature
for now. I understand that you are very interested in EROFS.
Considering EROFS current staging status, it's not such a place
to add new features at all! I have marked your patch down and
I will work with you later. Hope to get your understanding...

Thanks,
Gao Xiang

> ---
>=C2=A0 include/erofs/config.h |=C2=A0 1 +
>=C2=A0 include/erofs_fs.h=C2=A0 =C2=A0 =C2=A0| 10 ++++++++
>=C2=A0 mkfs/main.c=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 | 64 ++++++= +++++++++++++++++++++++++++++++++++++++++++-
>=C2=A0 3 files changed, 74 insertions(+), 1 deletion(-)
>
> diff --git a/include/erofs/config.h b/include/erofs/config.h
> index 05fe6b2..40cd466 100644
> --- a/include/erofs/config.h
> +++ b/include/erofs/config.h
> @@ -22,6 +22,7 @@ struct erofs_configure {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0char *c_src_path;
>=C2=A0 =C2=A0 =C2=A0 =C2=A0char *c_compr_alg_master;
>=C2=A0 =C2=A0 =C2=A0 =C2=A0int c_compr_level_master;
> +=C2=A0 =C2=A0 =C2=A0int c_feature_flags;
>=C2=A0 };
>=C2=A0
>=C2=A0 extern struct erofs_configure cfg;
> diff --git a/include/erofs_fs.h b/include/erofs_fs.h
> index 601b477..9ac2635 100644
> --- a/include/erofs_fs.h
> +++ b/include/erofs_fs.h
> @@ -20,6 +20,16 @@
>=C2=A0 #define EROFS_REQUIREMENT_LZ4_0PADDING=C2=A0 =C2=A0 =C2=A0 =C2= =A00x00000001
>=C2=A0 #define EROFS_ALL_REQUIREMENTS=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0EROFS_REQUIREMENT_LZ4_0PADDING
>=C2=A0
> +/*
> + * feature definations.
> + */
> +#define EROFS_DEFAULT_FEATURES=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0EROFS_FEATURE_SB_CHKSUM
> +#define EROFS_FEATURE_SB_CHKSUM=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 0x0001
> +
> +
> +#define EROFS_HAS_COMPAT_FEATURE(super,mask) \
> +=C2=A0 =C2=A0 =C2=A0( le32_to_cpu((super)->features) & (mask) = )
> +
>=C2=A0 struct erofs_super_block {
>=C2=A0 /*=C2=A0 0 */__le32 magic;=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0/* in the little endian */
>=C2=A0 /*=C2=A0 4 */__le32 checksum;=C2=A0 =C2=A0 =C2=A0 =C2=A0 /* crc3= 2c(super_block) */
> diff --git a/mkfs/main.c b/mkfs/main.c
> index f127fe1..355fd2c 100644
> --- a/mkfs/main.c
> +++ b/mkfs/main.c
> @@ -31,6 +31,45 @@ static void usage(void)
>=C2=A0 =C2=A0 =C2=A0 =C2=A0fprintf(stderr, " -EX[,...] X=3Dextende= d options\n");
>=C2=A0 }
>=C2=A0
> +#define CRCPOLY=C2=A0 =C2=A0 =C2=A0 0x82F63B78
> +static inline u32 crc32c(u32 seed, unsigned char const *in, size_t le= n)
> +{
> +=C2=A0 =C2=A0 =C2=A0int i;
> +=C2=A0 =C2=A0 =C2=A0u32 crc =3D seed;
> +
> +=C2=A0 =C2=A0 =C2=A0while (len--) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0crc ^=3D *in++;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0for (i =3D 0; i < = 8; i++) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0crc =3D (crc >> 1) ^ ((crc & 1) ? CRCPOLY : 0);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}
> +=C2=A0 =C2=A0 =C2=A0}
> +=C2=A0 =C2=A0 =C2=A0erofs_dump("calculated crc: 0x%x\n", cr= c);
> +=C2=A0 =C2=A0 =C2=A0return crc;
> +}
> +
> +char *feature_opts[] =3D {
> +=C2=A0 =C2=A0 =C2=A0"nosbcrc", NULL
> +};
> +#define O_SB_CKSUM=C2=A0 =C2=A00
> +
> +static int parse_feature_subopts(char *opts)
> +{
> +=C2=A0 =C2=A0 =C2=A0char *arg;
> +
> +=C2=A0 =C2=A0 =C2=A0cfg.c_feature_flags =3D EROFS_DEFAULT_FEATURES; > +=C2=A0 =C2=A0 =C2=A0while (*opts !=3D '\0') {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0switch(getsubopt(&= ;opts, feature_opts, &arg)) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0case O_SB_CKSUM:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0cfg.c_feature_flags |=3D (~EROFS_FEATURE_SB_CHKSUM);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0break;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0default:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0erofs_err("incorrect suboption");
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0return -EINVAL;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}
> +=C2=A0 =C2=A0 =C2=A0}
> +=C2=A0 =C2=A0 =C2=A0return 0;
> +}
> +
>=C2=A0 static int parse_extended_opts(const char *opts)
>=C2=A0 {
>=C2=A0 #define MATCH_EXTENTED_OPT(opt, token, keylen) \
> @@ -79,7 +118,8 @@ static int mkfs_parse_options_cfg(int argc, char *a= rgv[])
>=C2=A0 {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0int opt, i;
>=C2=A0
> -=C2=A0 =C2=A0 =C2=A0while ((opt =3D getopt(argc, argv, "d:z:E:&q= uot;)) !=3D -1) {
> +=C2=A0 =C2=A0 =C2=A0cfg.c_feature_flags =3D EROFS_DEFAULT_FEATURES; > +=C2=A0 =C2=A0 =C2=A0while ((opt =3D getopt(argc, argv, "d:z:E:o:= ")) !=3D -1) {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0switch (opt) { >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0case 'z'= :
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0if (!optarg) {
> @@ -113,6 +153,12 @@ static int mkfs_parse_options_cfg(int argc, char = *argv[])
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return opt;
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0break;
>=C2=A0
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0case 'O':
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0opt =3D parse_feature_subopts(optarg);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0if (opt)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return opt;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0break;
> +
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0default: /* '= ;?' */
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0return -EINVAL;
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}
> @@ -144,6 +190,15 @@ static int mkfs_parse_options_cfg(int argc, char = *argv[])
>=C2=A0 =C2=A0 =C2=A0 =C2=A0return 0;
>=C2=A0 }
>=C2=A0
> +u32 erofs_superblock_checksum(struct erofs_super_block *sb)
> +{
> +=C2=A0 =C2=A0 =C2=A0u32 crc;
> +=C2=A0 =C2=A0 =C2=A0crc =3D crc32c(~0, (const unsigned char *)sb,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0sizeof(= struct erofs_super_block));
> +=C2=A0 =C2=A0 =C2=A0erofs_dump("superblock checksum: 0x%x\n"= ;, crc);
> +=C2=A0 =C2=A0 =C2=A0return crc;
> +}
> +
>=C2=A0 int erofs_mkfs_update_super_block(struct erofs_buffer_head *bh,<= br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0erofs_nid_t root_nid)
>=C2=A0 {
> @@ -155,6 +210,7 @@ int erofs_mkfs_update_super_block(struct erofs_buf= fer_head *bh,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0.meta_blkaddr=C2= =A0 =3D sbi.meta_blkaddr,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0.xattr_blkaddr = =3D 0,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0.requirements = =3D cpu_to_le32(sbi.requirements),
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0.features =3D cpu_to_= le32(cfg.c_feature_flags),
>=C2=A0 =C2=A0 =C2=A0 =C2=A0};
>=C2=A0 =C2=A0 =C2=A0 =C2=A0const unsigned int sb_blksize =3D
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0round_up(EROFS_S= UPER_END, EROFS_BLKSIZ);
> @@ -169,6 +225,12 @@ int erofs_mkfs_update_super_block(struct erofs_bu= ffer_head *bh,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0sb.blocks=C2=A0 =C2=A0 =C2=A0 =C2=A0=3D cpu_= to_le32(erofs_mapbh(NULL, true));
>=C2=A0 =C2=A0 =C2=A0 =C2=A0sb.root_nid=C2=A0 =C2=A0 =C2=A0=3D cpu_to_le= 16(root_nid);
>=C2=A0
> +=C2=A0 =C2=A0 =C2=A0if (EROFS_HAS_COMPAT_FEATURE(&sb, EROFS_FEATU= RE_SB_CHKSUM)) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0sb.checksum =3D 0; > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0u32 crc =3D erofs_sup= erblock_checksum(&sb);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0sb.checksum =3D cpu_t= o_le32(crc);
> +=C2=A0 =C2=A0 =C2=A0}
> +
>=C2=A0 =C2=A0 =C2=A0 =C2=A0buf =3D calloc(sb_blksize, 1);
>=C2=A0 =C2=A0 =C2=A0 =C2=A0if (!buf) {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0erofs_err("= Failed to allocate memory for sb: %s",
> --
> 2.9.3
>