[PATCH v5 6/6] ceph: replace local base64 helpers with lib/base64

Guan-Chun Wu posted 6 patches 1 month ago
[PATCH v5 6/6] ceph: replace local base64 helpers with lib/base64
Posted by Guan-Chun Wu 1 month ago
Remove the ceph_base64_encode() and ceph_base64_decode() functions and
replace their usage with the generic base64_encode() and base64_decode()
helpers from lib/base64.

This eliminates the custom implementation in Ceph, reduces code
duplication, and relies on the shared Base64 code in lib.
The helpers preserve RFC 3501-compliant Base64 encoding without padding,
so there are no functional changes.

This change also improves performance: encoding is about 2.7x faster and
decoding achieves 43-52x speedups compared to the previous local
implementation.

Reviewed-by: Kuan-Wei Chiu <visitorckw@gmail.com>
Signed-off-by: Guan-Chun Wu <409411716@gms.tku.edu.tw>
---
 fs/ceph/crypto.c | 60 ++++--------------------------------------------
 fs/ceph/crypto.h |  6 +----
 fs/ceph/dir.c    |  5 ++--
 fs/ceph/inode.c  |  2 +-
 4 files changed, 9 insertions(+), 64 deletions(-)

diff --git a/fs/ceph/crypto.c b/fs/ceph/crypto.c
index 7026e794813c..b6016dcffbb6 100644
--- a/fs/ceph/crypto.c
+++ b/fs/ceph/crypto.c
@@ -15,59 +15,6 @@
 #include "mds_client.h"
 #include "crypto.h"
 
-/*
- * The base64url encoding used by fscrypt includes the '_' character, which may
- * cause problems in snapshot names (which can not start with '_').  Thus, we
- * used the base64 encoding defined for IMAP mailbox names (RFC 3501) instead,
- * which replaces '-' and '_' by '+' and ','.
- */
-static const char base64_table[65] =
-	"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+,";
-
-int ceph_base64_encode(const u8 *src, int srclen, char *dst)
-{
-	u32 ac = 0;
-	int bits = 0;
-	int i;
-	char *cp = dst;
-
-	for (i = 0; i < srclen; i++) {
-		ac = (ac << 8) | src[i];
-		bits += 8;
-		do {
-			bits -= 6;
-			*cp++ = base64_table[(ac >> bits) & 0x3f];
-		} while (bits >= 6);
-	}
-	if (bits)
-		*cp++ = base64_table[(ac << (6 - bits)) & 0x3f];
-	return cp - dst;
-}
-
-int ceph_base64_decode(const char *src, int srclen, u8 *dst)
-{
-	u32 ac = 0;
-	int bits = 0;
-	int i;
-	u8 *bp = dst;
-
-	for (i = 0; i < srclen; i++) {
-		const char *p = strchr(base64_table, src[i]);
-
-		if (p == NULL || src[i] == 0)
-			return -1;
-		ac = (ac << 6) | (p - base64_table);
-		bits += 6;
-		if (bits >= 8) {
-			bits -= 8;
-			*bp++ = (u8)(ac >> bits);
-		}
-	}
-	if (ac & ((1 << bits) - 1))
-		return -1;
-	return bp - dst;
-}
-
 static int ceph_crypt_get_context(struct inode *inode, void *ctx, size_t len)
 {
 	struct ceph_inode_info *ci = ceph_inode(inode);
@@ -318,7 +265,7 @@ int ceph_encode_encrypted_dname(struct inode *parent, char *buf, int elen)
 	}
 
 	/* base64 encode the encrypted name */
-	elen = ceph_base64_encode(cryptbuf, len, p);
+	elen = base64_encode(cryptbuf, len, p, false, BASE64_IMAP);
 	doutc(cl, "base64-encoded ciphertext name = %.*s\n", elen, p);
 
 	/* To understand the 240 limit, see CEPH_NOHASH_NAME_MAX comments */
@@ -412,7 +359,8 @@ int ceph_fname_to_usr(const struct ceph_fname *fname, struct fscrypt_str *tname,
 			tname = &_tname;
 		}
 
-		declen = ceph_base64_decode(name, name_len, tname->name);
+		declen = base64_decode(name, name_len,
+				       tname->name, false, BASE64_IMAP);
 		if (declen <= 0) {
 			ret = -EIO;
 			goto out;
@@ -426,7 +374,7 @@ int ceph_fname_to_usr(const struct ceph_fname *fname, struct fscrypt_str *tname,
 
 	ret = fscrypt_fname_disk_to_usr(dir, 0, 0, &iname, oname);
 	if (!ret && (dir != fname->dir)) {
-		char tmp_buf[CEPH_BASE64_CHARS(NAME_MAX)];
+		char tmp_buf[BASE64_CHARS(NAME_MAX)];
 
 		name_len = snprintf(tmp_buf, sizeof(tmp_buf), "_%.*s_%ld",
 				    oname->len, oname->name, dir->i_ino);
diff --git a/fs/ceph/crypto.h b/fs/ceph/crypto.h
index 23612b2e9837..b748e2060bc9 100644
--- a/fs/ceph/crypto.h
+++ b/fs/ceph/crypto.h
@@ -8,6 +8,7 @@
 
 #include <crypto/sha2.h>
 #include <linux/fscrypt.h>
+#include <linux/base64.h>
 
 #define CEPH_FSCRYPT_BLOCK_SHIFT   12
 #define CEPH_FSCRYPT_BLOCK_SIZE    (_AC(1, UL) << CEPH_FSCRYPT_BLOCK_SHIFT)
@@ -89,11 +90,6 @@ static inline u32 ceph_fscrypt_auth_len(struct ceph_fscrypt_auth *fa)
  */
 #define CEPH_NOHASH_NAME_MAX (180 - SHA256_DIGEST_SIZE)
 
-#define CEPH_BASE64_CHARS(nbytes) DIV_ROUND_UP((nbytes) * 4, 3)
-
-int ceph_base64_encode(const u8 *src, int srclen, char *dst);
-int ceph_base64_decode(const char *src, int srclen, u8 *dst);
-
 void ceph_fscrypt_set_ops(struct super_block *sb);
 
 void ceph_fscrypt_free_dummy_policy(struct ceph_fs_client *fsc);
diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index d18c0eaef9b7..0fa7c7777242 100644
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -998,13 +998,14 @@ static int prep_encrypted_symlink_target(struct ceph_mds_request *req,
 	if (err)
 		goto out;
 
-	req->r_path2 = kmalloc(CEPH_BASE64_CHARS(osd_link.len) + 1, GFP_KERNEL);
+	req->r_path2 = kmalloc(BASE64_CHARS(osd_link.len) + 1, GFP_KERNEL);
 	if (!req->r_path2) {
 		err = -ENOMEM;
 		goto out;
 	}
 
-	len = ceph_base64_encode(osd_link.name, osd_link.len, req->r_path2);
+	len = base64_encode(osd_link.name, osd_link.len,
+			    req->r_path2, false, BASE64_IMAP);
 	req->r_path2[len] = '\0';
 out:
 	fscrypt_fname_free_buffer(&osd_link);
diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
index a6e260d9e420..b691343cb7f1 100644
--- a/fs/ceph/inode.c
+++ b/fs/ceph/inode.c
@@ -958,7 +958,7 @@ static int decode_encrypted_symlink(struct ceph_mds_client *mdsc,
 	if (!sym)
 		return -ENOMEM;
 
-	declen = ceph_base64_decode(encsym, enclen, sym);
+	declen = base64_decode(encsym, enclen, sym, false, BASE64_IMAP);
 	if (declen < 0) {
 		pr_err_client(cl,
 			"can't decode symlink (%d). Content: %.*s\n",
-- 
2.34.1
Re: [PATCH v5 6/6] ceph: replace local base64 helpers with lib/base64
Posted by Viacheslav Dubeyko 1 month ago
On Fri, 2025-11-14 at 14:02 +0800, Guan-Chun Wu wrote:
> Remove the ceph_base64_encode() and ceph_base64_decode() functions and
> replace their usage with the generic base64_encode() and base64_decode()
> helpers from lib/base64.
> 
> This eliminates the custom implementation in Ceph, reduces code
> duplication, and relies on the shared Base64 code in lib.
> The helpers preserve RFC 3501-compliant Base64 encoding without padding,
> so there are no functional changes.
> 
> This change also improves performance: encoding is about 2.7x faster and
> decoding achieves 43-52x speedups compared to the previous local
> implementation.
> 
> Reviewed-by: Kuan-Wei Chiu <visitorckw@gmail.com>
> Signed-off-by: Guan-Chun Wu <409411716@gms.tku.edu.tw>
> ---
>  fs/ceph/crypto.c | 60 ++++--------------------------------------------
>  fs/ceph/crypto.h |  6 +----
>  fs/ceph/dir.c    |  5 ++--
>  fs/ceph/inode.c  |  2 +-
>  4 files changed, 9 insertions(+), 64 deletions(-)
> 
> diff --git a/fs/ceph/crypto.c b/fs/ceph/crypto.c
> index 7026e794813c..b6016dcffbb6 100644
> --- a/fs/ceph/crypto.c
> +++ b/fs/ceph/crypto.c
> @@ -15,59 +15,6 @@
>  #include "mds_client.h"
>  #include "crypto.h"
>  
> -/*
> - * The base64url encoding used by fscrypt includes the '_' character, which may
> - * cause problems in snapshot names (which can not start with '_').  Thus, we
> - * used the base64 encoding defined for IMAP mailbox names (RFC 3501) instead,
> - * which replaces '-' and '_' by '+' and ','.
> - */
> -static const char base64_table[65] =
> -	"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+,";
> -
> -int ceph_base64_encode(const u8 *src, int srclen, char *dst)
> -{
> -	u32 ac = 0;
> -	int bits = 0;
> -	int i;
> -	char *cp = dst;
> -
> -	for (i = 0; i < srclen; i++) {
> -		ac = (ac << 8) | src[i];
> -		bits += 8;
> -		do {
> -			bits -= 6;
> -			*cp++ = base64_table[(ac >> bits) & 0x3f];
> -		} while (bits >= 6);
> -	}
> -	if (bits)
> -		*cp++ = base64_table[(ac << (6 - bits)) & 0x3f];
> -	return cp - dst;
> -}
> -
> -int ceph_base64_decode(const char *src, int srclen, u8 *dst)
> -{
> -	u32 ac = 0;
> -	int bits = 0;
> -	int i;
> -	u8 *bp = dst;
> -
> -	for (i = 0; i < srclen; i++) {
> -		const char *p = strchr(base64_table, src[i]);
> -
> -		if (p == NULL || src[i] == 0)
> -			return -1;
> -		ac = (ac << 6) | (p - base64_table);
> -		bits += 6;
> -		if (bits >= 8) {
> -			bits -= 8;
> -			*bp++ = (u8)(ac >> bits);
> -		}
> -	}
> -	if (ac & ((1 << bits) - 1))
> -		return -1;
> -	return bp - dst;
> -}
> -
>  static int ceph_crypt_get_context(struct inode *inode, void *ctx, size_t len)
>  {
>  	struct ceph_inode_info *ci = ceph_inode(inode);
> @@ -318,7 +265,7 @@ int ceph_encode_encrypted_dname(struct inode *parent, char *buf, int elen)
>  	}
>  
>  	/* base64 encode the encrypted name */
> -	elen = ceph_base64_encode(cryptbuf, len, p);
> +	elen = base64_encode(cryptbuf, len, p, false, BASE64_IMAP);
>  	doutc(cl, "base64-encoded ciphertext name = %.*s\n", elen, p);
>  
>  	/* To understand the 240 limit, see CEPH_NOHASH_NAME_MAX comments */
> @@ -412,7 +359,8 @@ int ceph_fname_to_usr(const struct ceph_fname *fname, struct fscrypt_str *tname,
>  			tname = &_tname;
>  		}
>  
> -		declen = ceph_base64_decode(name, name_len, tname->name);
> +		declen = base64_decode(name, name_len,
> +				       tname->name, false, BASE64_IMAP);
>  		if (declen <= 0) {
>  			ret = -EIO;
>  			goto out;
> @@ -426,7 +374,7 @@ int ceph_fname_to_usr(const struct ceph_fname *fname, struct fscrypt_str *tname,
>  
>  	ret = fscrypt_fname_disk_to_usr(dir, 0, 0, &iname, oname);
>  	if (!ret && (dir != fname->dir)) {
> -		char tmp_buf[CEPH_BASE64_CHARS(NAME_MAX)];
> +		char tmp_buf[BASE64_CHARS(NAME_MAX)];
>  
>  		name_len = snprintf(tmp_buf, sizeof(tmp_buf), "_%.*s_%ld",
>  				    oname->len, oname->name, dir->i_ino);
> diff --git a/fs/ceph/crypto.h b/fs/ceph/crypto.h
> index 23612b2e9837..b748e2060bc9 100644
> --- a/fs/ceph/crypto.h
> +++ b/fs/ceph/crypto.h
> @@ -8,6 +8,7 @@
>  
>  #include <crypto/sha2.h>
>  #include <linux/fscrypt.h>
> +#include <linux/base64.h>
>  
>  #define CEPH_FSCRYPT_BLOCK_SHIFT   12
>  #define CEPH_FSCRYPT_BLOCK_SIZE    (_AC(1, UL) << CEPH_FSCRYPT_BLOCK_SHIFT)
> @@ -89,11 +90,6 @@ static inline u32 ceph_fscrypt_auth_len(struct ceph_fscrypt_auth *fa)
>   */
>  #define CEPH_NOHASH_NAME_MAX (180 - SHA256_DIGEST_SIZE)
>  
> -#define CEPH_BASE64_CHARS(nbytes) DIV_ROUND_UP((nbytes) * 4, 3)
> -
> -int ceph_base64_encode(const u8 *src, int srclen, char *dst);
> -int ceph_base64_decode(const char *src, int srclen, u8 *dst);
> -
>  void ceph_fscrypt_set_ops(struct super_block *sb);
>  
>  void ceph_fscrypt_free_dummy_policy(struct ceph_fs_client *fsc);
> diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
> index d18c0eaef9b7..0fa7c7777242 100644
> --- a/fs/ceph/dir.c
> +++ b/fs/ceph/dir.c
> @@ -998,13 +998,14 @@ static int prep_encrypted_symlink_target(struct ceph_mds_request *req,
>  	if (err)
>  		goto out;
>  
> -	req->r_path2 = kmalloc(CEPH_BASE64_CHARS(osd_link.len) + 1, GFP_KERNEL);
> +	req->r_path2 = kmalloc(BASE64_CHARS(osd_link.len) + 1, GFP_KERNEL);
>  	if (!req->r_path2) {
>  		err = -ENOMEM;
>  		goto out;
>  	}
>  
> -	len = ceph_base64_encode(osd_link.name, osd_link.len, req->r_path2);
> +	len = base64_encode(osd_link.name, osd_link.len,
> +			    req->r_path2, false, BASE64_IMAP);
>  	req->r_path2[len] = '\0';
>  out:
>  	fscrypt_fname_free_buffer(&osd_link);
> diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
> index a6e260d9e420..b691343cb7f1 100644
> --- a/fs/ceph/inode.c
> +++ b/fs/ceph/inode.c
> @@ -958,7 +958,7 @@ static int decode_encrypted_symlink(struct ceph_mds_client *mdsc,
>  	if (!sym)
>  		return -ENOMEM;
>  
> -	declen = ceph_base64_decode(encsym, enclen, sym);
> +	declen = base64_decode(encsym, enclen, sym, false, BASE64_IMAP);
>  	if (declen < 0) {
>  		pr_err_client(cl,
>  			"can't decode symlink (%d). Content: %.*s\n",

Looks good!

Reviewed-by: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>

Have you run xfstests for this patchset?

Thanks,
Slava.
Re: [PATCH v5 6/6] ceph: replace local base64 helpers with lib/base64
Posted by Guan-Chun Wu 4 weeks ago
On Fri, Nov 14, 2025 at 06:07:26PM +0000, Viacheslav Dubeyko wrote:
> On Fri, 2025-11-14 at 14:02 +0800, Guan-Chun Wu wrote:
> > Remove the ceph_base64_encode() and ceph_base64_decode() functions and
> > replace their usage with the generic base64_encode() and base64_decode()
> > helpers from lib/base64.
> > 
> > This eliminates the custom implementation in Ceph, reduces code
> > duplication, and relies on the shared Base64 code in lib.
> > The helpers preserve RFC 3501-compliant Base64 encoding without padding,
> > so there are no functional changes.
> > 
> > This change also improves performance: encoding is about 2.7x faster and
> > decoding achieves 43-52x speedups compared to the previous local
> > implementation.
> > 
> > Reviewed-by: Kuan-Wei Chiu <visitorckw@gmail.com>
> > Signed-off-by: Guan-Chun Wu <409411716@gms.tku.edu.tw>
> > ---
> >  fs/ceph/crypto.c | 60 ++++--------------------------------------------
> >  fs/ceph/crypto.h |  6 +----
> >  fs/ceph/dir.c    |  5 ++--
> >  fs/ceph/inode.c  |  2 +-
> >  4 files changed, 9 insertions(+), 64 deletions(-)
> > 
> > diff --git a/fs/ceph/crypto.c b/fs/ceph/crypto.c
> > index 7026e794813c..b6016dcffbb6 100644
> > --- a/fs/ceph/crypto.c
> > +++ b/fs/ceph/crypto.c
> > @@ -15,59 +15,6 @@
> >  #include "mds_client.h"
> >  #include "crypto.h"
> >  
> > -/*
> > - * The base64url encoding used by fscrypt includes the '_' character, which may
> > - * cause problems in snapshot names (which can not start with '_').  Thus, we
> > - * used the base64 encoding defined for IMAP mailbox names (RFC 3501) instead,
> > - * which replaces '-' and '_' by '+' and ','.
> > - */
> > -static const char base64_table[65] =
> > -	"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+,";
> > -
> > -int ceph_base64_encode(const u8 *src, int srclen, char *dst)
> > -{
> > -	u32 ac = 0;
> > -	int bits = 0;
> > -	int i;
> > -	char *cp = dst;
> > -
> > -	for (i = 0; i < srclen; i++) {
> > -		ac = (ac << 8) | src[i];
> > -		bits += 8;
> > -		do {
> > -			bits -= 6;
> > -			*cp++ = base64_table[(ac >> bits) & 0x3f];
> > -		} while (bits >= 6);
> > -	}
> > -	if (bits)
> > -		*cp++ = base64_table[(ac << (6 - bits)) & 0x3f];
> > -	return cp - dst;
> > -}
> > -
> > -int ceph_base64_decode(const char *src, int srclen, u8 *dst)
> > -{
> > -	u32 ac = 0;
> > -	int bits = 0;
> > -	int i;
> > -	u8 *bp = dst;
> > -
> > -	for (i = 0; i < srclen; i++) {
> > -		const char *p = strchr(base64_table, src[i]);
> > -
> > -		if (p == NULL || src[i] == 0)
> > -			return -1;
> > -		ac = (ac << 6) | (p - base64_table);
> > -		bits += 6;
> > -		if (bits >= 8) {
> > -			bits -= 8;
> > -			*bp++ = (u8)(ac >> bits);
> > -		}
> > -	}
> > -	if (ac & ((1 << bits) - 1))
> > -		return -1;
> > -	return bp - dst;
> > -}
> > -
> >  static int ceph_crypt_get_context(struct inode *inode, void *ctx, size_t len)
> >  {
> >  	struct ceph_inode_info *ci = ceph_inode(inode);
> > @@ -318,7 +265,7 @@ int ceph_encode_encrypted_dname(struct inode *parent, char *buf, int elen)
> >  	}
> >  
> >  	/* base64 encode the encrypted name */
> > -	elen = ceph_base64_encode(cryptbuf, len, p);
> > +	elen = base64_encode(cryptbuf, len, p, false, BASE64_IMAP);
> >  	doutc(cl, "base64-encoded ciphertext name = %.*s\n", elen, p);
> >  
> >  	/* To understand the 240 limit, see CEPH_NOHASH_NAME_MAX comments */
> > @@ -412,7 +359,8 @@ int ceph_fname_to_usr(const struct ceph_fname *fname, struct fscrypt_str *tname,
> >  			tname = &_tname;
> >  		}
> >  
> > -		declen = ceph_base64_decode(name, name_len, tname->name);
> > +		declen = base64_decode(name, name_len,
> > +				       tname->name, false, BASE64_IMAP);
> >  		if (declen <= 0) {
> >  			ret = -EIO;
> >  			goto out;
> > @@ -426,7 +374,7 @@ int ceph_fname_to_usr(const struct ceph_fname *fname, struct fscrypt_str *tname,
> >  
> >  	ret = fscrypt_fname_disk_to_usr(dir, 0, 0, &iname, oname);
> >  	if (!ret && (dir != fname->dir)) {
> > -		char tmp_buf[CEPH_BASE64_CHARS(NAME_MAX)];
> > +		char tmp_buf[BASE64_CHARS(NAME_MAX)];
> >  
> >  		name_len = snprintf(tmp_buf, sizeof(tmp_buf), "_%.*s_%ld",
> >  				    oname->len, oname->name, dir->i_ino);
> > diff --git a/fs/ceph/crypto.h b/fs/ceph/crypto.h
> > index 23612b2e9837..b748e2060bc9 100644
> > --- a/fs/ceph/crypto.h
> > +++ b/fs/ceph/crypto.h
> > @@ -8,6 +8,7 @@
> >  
> >  #include <crypto/sha2.h>
> >  #include <linux/fscrypt.h>
> > +#include <linux/base64.h>
> >  
> >  #define CEPH_FSCRYPT_BLOCK_SHIFT   12
> >  #define CEPH_FSCRYPT_BLOCK_SIZE    (_AC(1, UL) << CEPH_FSCRYPT_BLOCK_SHIFT)
> > @@ -89,11 +90,6 @@ static inline u32 ceph_fscrypt_auth_len(struct ceph_fscrypt_auth *fa)
> >   */
> >  #define CEPH_NOHASH_NAME_MAX (180 - SHA256_DIGEST_SIZE)
> >  
> > -#define CEPH_BASE64_CHARS(nbytes) DIV_ROUND_UP((nbytes) * 4, 3)
> > -
> > -int ceph_base64_encode(const u8 *src, int srclen, char *dst);
> > -int ceph_base64_decode(const char *src, int srclen, u8 *dst);
> > -
> >  void ceph_fscrypt_set_ops(struct super_block *sb);
> >  
> >  void ceph_fscrypt_free_dummy_policy(struct ceph_fs_client *fsc);
> > diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
> > index d18c0eaef9b7..0fa7c7777242 100644
> > --- a/fs/ceph/dir.c
> > +++ b/fs/ceph/dir.c
> > @@ -998,13 +998,14 @@ static int prep_encrypted_symlink_target(struct ceph_mds_request *req,
> >  	if (err)
> >  		goto out;
> >  
> > -	req->r_path2 = kmalloc(CEPH_BASE64_CHARS(osd_link.len) + 1, GFP_KERNEL);
> > +	req->r_path2 = kmalloc(BASE64_CHARS(osd_link.len) + 1, GFP_KERNEL);
> >  	if (!req->r_path2) {
> >  		err = -ENOMEM;
> >  		goto out;
> >  	}
> >  
> > -	len = ceph_base64_encode(osd_link.name, osd_link.len, req->r_path2);
> > +	len = base64_encode(osd_link.name, osd_link.len,
> > +			    req->r_path2, false, BASE64_IMAP);
> >  	req->r_path2[len] = '\0';
> >  out:
> >  	fscrypt_fname_free_buffer(&osd_link);
> > diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
> > index a6e260d9e420..b691343cb7f1 100644
> > --- a/fs/ceph/inode.c
> > +++ b/fs/ceph/inode.c
> > @@ -958,7 +958,7 @@ static int decode_encrypted_symlink(struct ceph_mds_client *mdsc,
> >  	if (!sym)
> >  		return -ENOMEM;
> >  
> > -	declen = ceph_base64_decode(encsym, enclen, sym);
> > +	declen = base64_decode(encsym, enclen, sym, false, BASE64_IMAP);
> >  	if (declen < 0) {
> >  		pr_err_client(cl,
> >  			"can't decode symlink (%d). Content: %.*s\n",
> 
> Looks good!
> 
> Reviewed-by: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>
> 
> Have you run xfstests for this patchset?

Hi Slava,

Thanks for the review.

I haven't run xfstests on this patchset yet.

Best regards,
Guan-Chun

> 
> Thanks,
> Slava.
RE: [PATCH v5 6/6] ceph: replace local base64 helpers with lib/base64
Posted by Viacheslav Dubeyko 3 weeks, 6 days ago
On Sun, 2025-11-16 at 18:36 +0800, Guan-Chun Wu wrote:
> On Fri, Nov 14, 2025 at 06:07:26PM +0000, Viacheslav Dubeyko wrote:
> > On Fri, 2025-11-14 at 14:02 +0800, Guan-Chun Wu wrote:
> > > Remove the ceph_base64_encode() and ceph_base64_decode() functions and
> > > replace their usage with the generic base64_encode() and base64_decode()
> > > helpers from lib/base64.
> > > 
> > > This eliminates the custom implementation in Ceph, reduces code
> > > duplication, and relies on the shared Base64 code in lib.
> > > The helpers preserve RFC 3501-compliant Base64 encoding without padding,
> > > so there are no functional changes.
> > > 
> > > This change also improves performance: encoding is about 2.7x faster and
> > > decoding achieves 43-52x speedups compared to the previous local
> > > implementation.
> > > 
> > > Reviewed-by: Kuan-Wei Chiu <visitorckw@gmail.com>
> > > Signed-off-by: Guan-Chun Wu <409411716@gms.tku.edu.tw>
> > > ---
> > >  fs/ceph/crypto.c | 60 ++++--------------------------------------------
> > >  fs/ceph/crypto.h |  6 +----
> > >  fs/ceph/dir.c    |  5 ++--
> > >  fs/ceph/inode.c  |  2 +-
> > >  4 files changed, 9 insertions(+), 64 deletions(-)
> > > 
> > > diff --git a/fs/ceph/crypto.c b/fs/ceph/crypto.c
> > > index 7026e794813c..b6016dcffbb6 100644
> > > --- a/fs/ceph/crypto.c
> > > +++ b/fs/ceph/crypto.c
> > > @@ -15,59 +15,6 @@
> > >  #include "mds_client.h"
> > >  #include "crypto.h"
> > >  
> > > -/*
> > > - * The base64url encoding used by fscrypt includes the '_' character, which may
> > > - * cause problems in snapshot names (which can not start with '_').  Thus, we
> > > - * used the base64 encoding defined for IMAP mailbox names (RFC 3501) instead,
> > > - * which replaces '-' and '_' by '+' and ','.
> > > - */
> > > -static const char base64_table[65] =
> > > -	"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+,";
> > > -
> > > -int ceph_base64_encode(const u8 *src, int srclen, char *dst)
> > > -{
> > > -	u32 ac = 0;
> > > -	int bits = 0;
> > > -	int i;
> > > -	char *cp = dst;
> > > -
> > > -	for (i = 0; i < srclen; i++) {
> > > -		ac = (ac << 8) | src[i];
> > > -		bits += 8;
> > > -		do {
> > > -			bits -= 6;
> > > -			*cp++ = base64_table[(ac >> bits) & 0x3f];
> > > -		} while (bits >= 6);
> > > -	}
> > > -	if (bits)
> > > -		*cp++ = base64_table[(ac << (6 - bits)) & 0x3f];
> > > -	return cp - dst;
> > > -}
> > > -
> > > -int ceph_base64_decode(const char *src, int srclen, u8 *dst)
> > > -{
> > > -	u32 ac = 0;
> > > -	int bits = 0;
> > > -	int i;
> > > -	u8 *bp = dst;
> > > -
> > > -	for (i = 0; i < srclen; i++) {
> > > -		const char *p = strchr(base64_table, src[i]);
> > > -
> > > -		if (p == NULL || src[i] == 0)
> > > -			return -1;
> > > -		ac = (ac << 6) | (p - base64_table);
> > > -		bits += 6;
> > > -		if (bits >= 8) {
> > > -			bits -= 8;
> > > -			*bp++ = (u8)(ac >> bits);
> > > -		}
> > > -	}
> > > -	if (ac & ((1 << bits) - 1))
> > > -		return -1;
> > > -	return bp - dst;
> > > -}
> > > -
> > >  static int ceph_crypt_get_context(struct inode *inode, void *ctx, size_t len)
> > >  {
> > >  	struct ceph_inode_info *ci = ceph_inode(inode);
> > > @@ -318,7 +265,7 @@ int ceph_encode_encrypted_dname(struct inode *parent, char *buf, int elen)
> > >  	}
> > >  
> > >  	/* base64 encode the encrypted name */
> > > -	elen = ceph_base64_encode(cryptbuf, len, p);
> > > +	elen = base64_encode(cryptbuf, len, p, false, BASE64_IMAP);
> > >  	doutc(cl, "base64-encoded ciphertext name = %.*s\n", elen, p);
> > >  
> > >  	/* To understand the 240 limit, see CEPH_NOHASH_NAME_MAX comments */
> > > @@ -412,7 +359,8 @@ int ceph_fname_to_usr(const struct ceph_fname *fname, struct fscrypt_str *tname,
> > >  			tname = &_tname;
> > >  		}
> > >  
> > > -		declen = ceph_base64_decode(name, name_len, tname->name);
> > > +		declen = base64_decode(name, name_len,
> > > +				       tname->name, false, BASE64_IMAP);
> > >  		if (declen <= 0) {
> > >  			ret = -EIO;
> > >  			goto out;
> > > @@ -426,7 +374,7 @@ int ceph_fname_to_usr(const struct ceph_fname *fname, struct fscrypt_str *tname,
> > >  
> > >  	ret = fscrypt_fname_disk_to_usr(dir, 0, 0, &iname, oname);
> > >  	if (!ret && (dir != fname->dir)) {
> > > -		char tmp_buf[CEPH_BASE64_CHARS(NAME_MAX)];
> > > +		char tmp_buf[BASE64_CHARS(NAME_MAX)];
> > >  
> > >  		name_len = snprintf(tmp_buf, sizeof(tmp_buf), "_%.*s_%ld",
> > >  				    oname->len, oname->name, dir->i_ino);
> > > diff --git a/fs/ceph/crypto.h b/fs/ceph/crypto.h
> > > index 23612b2e9837..b748e2060bc9 100644
> > > --- a/fs/ceph/crypto.h
> > > +++ b/fs/ceph/crypto.h
> > > @@ -8,6 +8,7 @@
> > >  
> > >  #include <crypto/sha2.h>
> > >  #include <linux/fscrypt.h>
> > > +#include <linux/base64.h>
> > >  
> > >  #define CEPH_FSCRYPT_BLOCK_SHIFT   12
> > >  #define CEPH_FSCRYPT_BLOCK_SIZE    (_AC(1, UL) << CEPH_FSCRYPT_BLOCK_SHIFT)
> > > @@ -89,11 +90,6 @@ static inline u32 ceph_fscrypt_auth_len(struct ceph_fscrypt_auth *fa)
> > >   */
> > >  #define CEPH_NOHASH_NAME_MAX (180 - SHA256_DIGEST_SIZE)
> > >  
> > > -#define CEPH_BASE64_CHARS(nbytes) DIV_ROUND_UP((nbytes) * 4, 3)
> > > -
> > > -int ceph_base64_encode(const u8 *src, int srclen, char *dst);
> > > -int ceph_base64_decode(const char *src, int srclen, u8 *dst);
> > > -
> > >  void ceph_fscrypt_set_ops(struct super_block *sb);
> > >  
> > >  void ceph_fscrypt_free_dummy_policy(struct ceph_fs_client *fsc);
> > > diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
> > > index d18c0eaef9b7..0fa7c7777242 100644
> > > --- a/fs/ceph/dir.c
> > > +++ b/fs/ceph/dir.c
> > > @@ -998,13 +998,14 @@ static int prep_encrypted_symlink_target(struct ceph_mds_request *req,
> > >  	if (err)
> > >  		goto out;
> > >  
> > > -	req->r_path2 = kmalloc(CEPH_BASE64_CHARS(osd_link.len) + 1, GFP_KERNEL);
> > > +	req->r_path2 = kmalloc(BASE64_CHARS(osd_link.len) + 1, GFP_KERNEL);
> > >  	if (!req->r_path2) {
> > >  		err = -ENOMEM;
> > >  		goto out;
> > >  	}
> > >  
> > > -	len = ceph_base64_encode(osd_link.name, osd_link.len, req->r_path2);
> > > +	len = base64_encode(osd_link.name, osd_link.len,
> > > +			    req->r_path2, false, BASE64_IMAP);
> > >  	req->r_path2[len] = '\0';
> > >  out:
> > >  	fscrypt_fname_free_buffer(&osd_link);
> > > diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
> > > index a6e260d9e420..b691343cb7f1 100644
> > > --- a/fs/ceph/inode.c
> > > +++ b/fs/ceph/inode.c
> > > @@ -958,7 +958,7 @@ static int decode_encrypted_symlink(struct ceph_mds_client *mdsc,
> > >  	if (!sym)
> > >  		return -ENOMEM;
> > >  
> > > -	declen = ceph_base64_decode(encsym, enclen, sym);
> > > +	declen = base64_decode(encsym, enclen, sym, false, BASE64_IMAP);
> > >  	if (declen < 0) {
> > >  		pr_err_client(cl,
> > >  			"can't decode symlink (%d). Content: %.*s\n",
> > 
> > Looks good!
> > 
> > Reviewed-by: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>
> > 
> > Have you run xfstests for this patchset?
> 
> Hi Slava,
> 
> Thanks for the review.
> 
> I haven't run xfstests on this patchset yet.
> 
> 

I have run the xfstests for CephFS with applied patchset. I don't see any new
issues. We had failures with generic/452 generic/639 before applying the
patchset. So, as far as I can see, patchset works well.

Tested-by: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>

Thanks,
Slava.