linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ceph: optimize ceph_base64_encode() with block processing
@ 2025-08-30 13:28 Guan-Chun Wu
  2025-09-02 19:37 ` Viacheslav Dubeyko
  0 siblings, 1 reply; 7+ messages in thread
From: Guan-Chun Wu @ 2025-08-30 13:28 UTC (permalink / raw)
  To: xiubli, idryomov; +Cc: ceph-devel, linux-kernel, Guan-Chun Wu

Previously, ceph_base64_encode() used a bitstream approach, handling one
input byte at a time and performing extra bit operations. While correct,
this method was suboptimal.

This patch processes input in 3-byte blocks, mapping directly to 4 output
characters. Remaining 1 or 2 bytes are handled according to standard Base64
rules. This reduces computation and improves performance.

Performance test (5 runs) for ceph_base64_encode():

64B input:
-------------------------------------------------------
| Old method | 123 | 115 | 137 | 119 | 109 | avg ~121 ns |
-------------------------------------------------------
| New method |  84 |  83 |  86 |  85 |  84 | avg ~84 ns  |
-------------------------------------------------------

1KB input:
--------------------------------------------------------
| Old method | 1217 | 1150 | 1146 | 1149 | 1149 | avg ~1162 ns |
--------------------------------------------------------
| New method |  776 |  772 |  772 |  774 |  770 | avg ~773 ns  |
--------------------------------------------------------

Signed-off-by: Guan-Chun Wu <409411716@gms.tku.edu.tw>
---
Tested on Linux 6.8.0-64-generic x86_64
with Intel Core i7-10700 @ 2.90GHz

Test is executed in the form of kernel module.

Test script:

static int encode_v1(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;
}

static int encode_v2(const u8 *src, int srclen, char *dst)
{
	u32 ac = 0;
	int i = 0;
	char *cp = dst;

	while (i + 2 < srclen) {
		ac = ((u32)src[i] << 16) | ((u32)src[i + 1] << 8) | (u32)src[i + 2];
		*cp++ = base64_table[(ac >> 18) & 0x3f];
		*cp++ = base64_table[(ac >> 12) & 0x3f];
		*cp++ = base64_table[(ac >> 6) & 0x3f];
		*cp++ = base64_table[ac & 0x3f];
		i += 3;
	}

	switch (srclen - i) {
	case 2:
		ac = ((u32)src[i] << 16) | ((u32)src[i + 1] << 8);
		*cp++ = base64_table[(ac >> 18) & 0x3f];
		*cp++ = base64_table[(ac >> 12) & 0x3f];
		*cp++ = base64_table[(ac >> 6) & 0x3f];
		break;
	case 1:
		ac = ((u32)src[i] << 16);
		*cp++ = base64_table[(ac >> 18) & 0x3f];
		*cp++ = base64_table[(ac >> 12) & 0x3f];
		break;
	}
	return cp - dst;
}

static void run_test(const char *label, const u8 *data, int len)
{
    char *dst1, *dst2;
    int n1, n2;
    u64 start, end;

    dst1 = kmalloc(len * 2, GFP_KERNEL);
    dst2 = kmalloc(len * 2, GFP_KERNEL);

    if (!dst1 || !dst2) {
        pr_err("%s: Failed to allocate dst buffers\n", label);
        goto out;
    }

    pr_info("[%s] input size = %d bytes\n", label, len);

    start = ktime_get_ns();
    n1 = encode_v1(data, len, dst1);
    end = ktime_get_ns();
    pr_info("[%s] encode_v1 time: %lld ns\n", label, end - start);

    start = ktime_get_ns();
    n2 = encode_v2(data, len, dst2);
    end = ktime_get_ns();
    pr_info("[%s] encode_v2 time: %lld ns\n", label, end - start);

    if (n1 != n2 || memcmp(dst1, dst2, n1) != 0)
        pr_err("[%s] Mismatch detected between encode_v1 and encode_v2!\n", label);
    else
        pr_info("[%s] Outputs are identical.\n", label);

out:
    kfree(dst1);
    kfree(dst2);
}
---
 fs/ceph/crypto.c | 33 ++++++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/fs/ceph/crypto.c b/fs/ceph/crypto.c
index 3b3c4d8d401e..a35570fd8ff5 100644
--- a/fs/ceph/crypto.c
+++ b/fs/ceph/crypto.c
@@ -27,20 +27,31 @@ static const char base64_table[65] =
 int ceph_base64_encode(const u8 *src, int srclen, char *dst)
 {
 	u32 ac = 0;
-	int bits = 0;
-	int i;
+	int i = 0;
 	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);
+	while (i + 2 < srclen) {
+		ac = ((u32)src[i] << 16) | ((u32)src[i + 1] << 8) | (u32)src[i + 2];
+		*cp++ = base64_table[(ac >> 18) & 0x3f];
+		*cp++ = base64_table[(ac >> 12) & 0x3f];
+		*cp++ = base64_table[(ac >> 6) & 0x3f];
+		*cp++ = base64_table[ac & 0x3f];
+		i += 3;
+	}
+
+	switch (srclen - i) {
+	case 2:
+		ac = ((u32)src[i] << 16) | ((u32)src[i + 1] << 8);
+		*cp++ = base64_table[(ac >> 18) & 0x3f];
+		*cp++ = base64_table[(ac >> 12) & 0x3f];
+		*cp++ = base64_table[(ac >> 6) & 0x3f];
+		break;
+	case 1:
+		ac = ((u32)src[i] << 16);
+		*cp++ = base64_table[(ac >> 18) & 0x3f];
+		*cp++ = base64_table[(ac >> 12) & 0x3f];
+		break;
 	}
-	if (bits)
-		*cp++ = base64_table[(ac << (6 - bits)) & 0x3f];
 	return cp - dst;
 }
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re:  [PATCH] ceph: optimize ceph_base64_encode() with block processing
  2025-08-30 13:28 [PATCH] ceph: optimize ceph_base64_encode() with block processing Guan-Chun Wu
@ 2025-09-02 19:37 ` Viacheslav Dubeyko
  2025-09-02 21:05   ` Kuan-Wei Chiu
  0 siblings, 1 reply; 7+ messages in thread
From: Viacheslav Dubeyko @ 2025-09-02 19:37 UTC (permalink / raw)
  To: 409411716@gms.tku.edu.tw, Xiubo Li, idryomov@gmail.com
  Cc: ceph-devel@vger.kernel.org, linux-kernel@vger.kernel.org

On Sat, 2025-08-30 at 21:28 +0800, Guan-Chun Wu wrote:
> Previously, ceph_base64_encode() used a bitstream approach, handling one
> input byte at a time and performing extra bit operations. While correct,
> this method was suboptimal.
> 

Sounds interesting!

Is ceph_base64_decode() efficient then?
Do we have something in crypto library of Linux kernel? Maybe we can use
something efficient enough from there?

> This patch processes input in 3-byte blocks, mapping directly to 4 output
> characters. Remaining 1 or 2 bytes are handled according to standard Base64
> rules. This reduces computation and improves performance.
> 

So, why namely 3-byte blocks? Could you please explain in more details your
motivation and improved technique in commit message? How exactly your technique
reduces computation and improves performance?

> Performance test (5 runs) for ceph_base64_encode():
> 
> 64B input:
> -------------------------------------------------------
> > Old method | 123 | 115 | 137 | 119 | 109 | avg ~121 ns |
> -------------------------------------------------------
> > New method |  84 |  83 |  86 |  85 |  84 | avg ~84 ns  |
> -------------------------------------------------------
> 
> 1KB input:
> --------------------------------------------------------
> > Old method | 1217 | 1150 | 1146 | 1149 | 1149 | avg ~1162 ns |
> --------------------------------------------------------
> > New method |  776 |  772 |  772 |  774 |  770 | avg ~773 ns  |
> --------------------------------------------------------
> 
> Signed-off-by: Guan-Chun Wu <409411716@gms.tku.edu.tw>
> ---
> Tested on Linux 6.8.0-64-generic x86_64
> with Intel Core i7-10700 @ 2.90GHz
> 

I assume that it is still the commit message. So, I think this portion should be
before Signed-off-by.

> Test is executed in the form of kernel module.

> Test script:
> 

Is it finally script or kernel module? As far as I can see, it is not complete
source code. So, I am not sure that everybody will be capable to build and test
this module.

What's about to introduce this as Kunit test or self-test that can be used by
everybody in CephFS kernel client for testing and checking performance? I am
working on initial set of Kunit tests for CephFS kernel client right now.

> static int encode_v1(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;
> }
> 
> static int encode_v2(const u8 *src, int srclen, char *dst)
> {
> 	u32 ac = 0;
> 	int i = 0;
> 	char *cp = dst;
> 
> 	while (i + 2 < srclen) {
> 		ac = ((u32)src[i] << 16) | ((u32)src[i + 1] << 8) | (u32)src[i + 2];
> 		*cp++ = base64_table[(ac >> 18) & 0x3f];
> 		*cp++ = base64_table[(ac >> 12) & 0x3f];
> 		*cp++ = base64_table[(ac >> 6) & 0x3f];
> 		*cp++ = base64_table[ac & 0x3f];
> 		i += 3;
> 	}
> 
> 	switch (srclen - i) {
> 	case 2:
> 		ac = ((u32)src[i] << 16) | ((u32)src[i + 1] << 8);
> 		*cp++ = base64_table[(ac >> 18) & 0x3f];
> 		*cp++ = base64_table[(ac >> 12) & 0x3f];
> 		*cp++ = base64_table[(ac >> 6) & 0x3f];
> 		break;
> 	case 1:
> 		ac = ((u32)src[i] << 16);
> 		*cp++ = base64_table[(ac >> 18) & 0x3f];
> 		*cp++ = base64_table[(ac >> 12) & 0x3f];
> 		break;
> 	}
> 	return cp - dst;
> }
> 
> static void run_test(const char *label, const u8 *data, int len)
> {
>     char *dst1, *dst2;
>     int n1, n2;
>     u64 start, end;
> 
>     dst1 = kmalloc(len * 2, GFP_KERNEL);
>     dst2 = kmalloc(len * 2, GFP_KERNEL);
> 
>     if (!dst1 || !dst2) {
>         pr_err("%s: Failed to allocate dst buffers\n", label);
>         goto out;
>     }
> 
>     pr_info("[%s] input size = %d bytes\n", label, len);
> 
>     start = ktime_get_ns();
>     n1 = encode_v1(data, len, dst1);
>     end = ktime_get_ns();
>     pr_info("[%s] encode_v1 time: %lld ns\n", label, end - start);
> 
>     start = ktime_get_ns();
>     n2 = encode_v2(data, len, dst2);
>     end = ktime_get_ns();
>     pr_info("[%s] encode_v2 time: %lld ns\n", label, end - start);
> 
>     if (n1 != n2 || memcmp(dst1, dst2, n1) != 0)
>         pr_err("[%s] Mismatch detected between encode_v1 and encode_v2!\n", label);
>     else
>         pr_info("[%s] Outputs are identical.\n", label);
> 
> out:
>     kfree(dst1);
>     kfree(dst2);
> }
> ---
>  fs/ceph/crypto.c | 33 ++++++++++++++++++++++-----------
>  1 file changed, 22 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/ceph/crypto.c b/fs/ceph/crypto.c
> index 3b3c4d8d401e..a35570fd8ff5 100644
> --- a/fs/ceph/crypto.c
> +++ b/fs/ceph/crypto.c
> @@ -27,20 +27,31 @@ static const char base64_table[65] =
>  int ceph_base64_encode(const u8 *src, int srclen, char *dst)
>  {
>  	u32 ac = 0;
> -	int bits = 0;
> -	int i;
> +	int i = 0;
>  	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);
> +	while (i + 2 < srclen) {

Frankly speaking, I am not completely happy about hardcoded constants. As a
result, it makes code hard to understand, modify and support. Could you please
introduce named constants instead of hardcoded numbers?


> +		ac = ((u32)src[i] << 16) | ((u32)src[i + 1] << 8) | (u32)src[i + 2];
> +		*cp++ = base64_table[(ac >> 18) & 0x3f];
> +		*cp++ = base64_table[(ac >> 12) & 0x3f];
> +		*cp++ = base64_table[(ac >> 6) & 0x3f];
> +		*cp++ = base64_table[ac & 0x3f];
> +		i += 3;
> +	}
> +
> +	switch (srclen - i) {
> +	case 2:
> +		ac = ((u32)src[i] << 16) | ((u32)src[i + 1] << 8);
> +		*cp++ = base64_table[(ac >> 18) & 0x3f];
> +		*cp++ = base64_table[(ac >> 12) & 0x3f];
> +		*cp++ = base64_table[(ac >> 6) & 0x3f];
> +		break;
> +	case 1:
> +		ac = ((u32)src[i] << 16);
> +		*cp++ = base64_table[(ac >> 18) & 0x3f];
> +		*cp++ = base64_table[(ac >> 12) & 0x3f];
> +		break;
>  	}
> -	if (bits)
> -		*cp++ = base64_table[(ac << (6 - bits)) & 0x3f];
>  	return cp - dst;
>  }
>  

Let me test your patch and check that it doesn't introduce regression(s).

Thanks,
Slava.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] ceph: optimize ceph_base64_encode() with block processing
  2025-09-02 19:37 ` Viacheslav Dubeyko
@ 2025-09-02 21:05   ` Kuan-Wei Chiu
  2025-09-02 21:21     ` Viacheslav Dubeyko
  0 siblings, 1 reply; 7+ messages in thread
From: Kuan-Wei Chiu @ 2025-09-02 21:05 UTC (permalink / raw)
  To: Viacheslav Dubeyko, Guan-Chun Wu
  Cc: 409411716@gms.tku.edu.tw, Xiubo Li, idryomov@gmail.com,
	ceph-devel@vger.kernel.org, linux-kernel@vger.kernel.org

On Tue, Sep 02, 2025 at 07:37:22PM +0000, Viacheslav Dubeyko wrote:
> On Sat, 2025-08-30 at 21:28 +0800, Guan-Chun Wu wrote:
> > Previously, ceph_base64_encode() used a bitstream approach, handling one
> > input byte at a time and performing extra bit operations. While correct,
> > this method was suboptimal.
> > 
> 
> Sounds interesting!
> 
> Is ceph_base64_decode() efficient then?
> Do we have something in crypto library of Linux kernel? Maybe we can use
> something efficient enough from there?
> 
Hi Viacheslav,

FYI, we already have base64 encode/decode implementations in
lib/base64.c. As discussed in another thread [1], I think we can put
the optimized version there and have users switch to call the library
functions.

[1]: https://lore.kernel.org/lkml/38753d95-8503-4b72-9590-cb129aa49a41@t-8ch.de/

Hi Guan-Chun,

I was also trying optimizing base64 performance, but I saw your patch
first. Happy to help if you need any assistance!

Regards,
Kuan-Wei

> > This patch processes input in 3-byte blocks, mapping directly to 4 output
> > characters. Remaining 1 or 2 bytes are handled according to standard Base64
> > rules. This reduces computation and improves performance.
> > 
> 
> So, why namely 3-byte blocks? Could you please explain in more details your
> motivation and improved technique in commit message? How exactly your technique
> reduces computation and improves performance?
> 
> > Performance test (5 runs) for ceph_base64_encode():
> > 
> > 64B input:
> > -------------------------------------------------------
> > > Old method | 123 | 115 | 137 | 119 | 109 | avg ~121 ns |
> > -------------------------------------------------------
> > > New method |  84 |  83 |  86 |  85 |  84 | avg ~84 ns  |
> > -------------------------------------------------------
> > 
> > 1KB input:
> > --------------------------------------------------------
> > > Old method | 1217 | 1150 | 1146 | 1149 | 1149 | avg ~1162 ns |
> > --------------------------------------------------------
> > > New method |  776 |  772 |  772 |  774 |  770 | avg ~773 ns  |
> > --------------------------------------------------------
> > 
> > Signed-off-by: Guan-Chun Wu <409411716@gms.tku.edu.tw>
> > ---
> > Tested on Linux 6.8.0-64-generic x86_64
> > with Intel Core i7-10700 @ 2.90GHz
> > 
> 
> I assume that it is still the commit message. So, I think this portion should be
> before Signed-off-by.
> 
> > Test is executed in the form of kernel module.
> 
> > Test script:
> > 
> 
> Is it finally script or kernel module? As far as I can see, it is not complete
> source code. So, I am not sure that everybody will be capable to build and test
> this module.
> 
> What's about to introduce this as Kunit test or self-test that can be used by
> everybody in CephFS kernel client for testing and checking performance? I am
> working on initial set of Kunit tests for CephFS kernel client right now.
> 
> > static int encode_v1(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;
> > }
> > 
> > static int encode_v2(const u8 *src, int srclen, char *dst)
> > {
> > 	u32 ac = 0;
> > 	int i = 0;
> > 	char *cp = dst;
> > 
> > 	while (i + 2 < srclen) {
> > 		ac = ((u32)src[i] << 16) | ((u32)src[i + 1] << 8) | (u32)src[i + 2];
> > 		*cp++ = base64_table[(ac >> 18) & 0x3f];
> > 		*cp++ = base64_table[(ac >> 12) & 0x3f];
> > 		*cp++ = base64_table[(ac >> 6) & 0x3f];
> > 		*cp++ = base64_table[ac & 0x3f];
> > 		i += 3;
> > 	}
> > 
> > 	switch (srclen - i) {
> > 	case 2:
> > 		ac = ((u32)src[i] << 16) | ((u32)src[i + 1] << 8);
> > 		*cp++ = base64_table[(ac >> 18) & 0x3f];
> > 		*cp++ = base64_table[(ac >> 12) & 0x3f];
> > 		*cp++ = base64_table[(ac >> 6) & 0x3f];
> > 		break;
> > 	case 1:
> > 		ac = ((u32)src[i] << 16);
> > 		*cp++ = base64_table[(ac >> 18) & 0x3f];
> > 		*cp++ = base64_table[(ac >> 12) & 0x3f];
> > 		break;
> > 	}
> > 	return cp - dst;
> > }
> > 
> > static void run_test(const char *label, const u8 *data, int len)
> > {
> >     char *dst1, *dst2;
> >     int n1, n2;
> >     u64 start, end;
> > 
> >     dst1 = kmalloc(len * 2, GFP_KERNEL);
> >     dst2 = kmalloc(len * 2, GFP_KERNEL);
> > 
> >     if (!dst1 || !dst2) {
> >         pr_err("%s: Failed to allocate dst buffers\n", label);
> >         goto out;
> >     }
> > 
> >     pr_info("[%s] input size = %d bytes\n", label, len);
> > 
> >     start = ktime_get_ns();
> >     n1 = encode_v1(data, len, dst1);
> >     end = ktime_get_ns();
> >     pr_info("[%s] encode_v1 time: %lld ns\n", label, end - start);
> > 
> >     start = ktime_get_ns();
> >     n2 = encode_v2(data, len, dst2);
> >     end = ktime_get_ns();
> >     pr_info("[%s] encode_v2 time: %lld ns\n", label, end - start);
> > 
> >     if (n1 != n2 || memcmp(dst1, dst2, n1) != 0)
> >         pr_err("[%s] Mismatch detected between encode_v1 and encode_v2!\n", label);
> >     else
> >         pr_info("[%s] Outputs are identical.\n", label);
> > 
> > out:
> >     kfree(dst1);
> >     kfree(dst2);
> > }
> > ---
> >  fs/ceph/crypto.c | 33 ++++++++++++++++++++++-----------
> >  1 file changed, 22 insertions(+), 11 deletions(-)
> > 
> > diff --git a/fs/ceph/crypto.c b/fs/ceph/crypto.c
> > index 3b3c4d8d401e..a35570fd8ff5 100644
> > --- a/fs/ceph/crypto.c
> > +++ b/fs/ceph/crypto.c
> > @@ -27,20 +27,31 @@ static const char base64_table[65] =
> >  int ceph_base64_encode(const u8 *src, int srclen, char *dst)
> >  {
> >  	u32 ac = 0;
> > -	int bits = 0;
> > -	int i;
> > +	int i = 0;
> >  	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);
> > +	while (i + 2 < srclen) {
> 
> Frankly speaking, I am not completely happy about hardcoded constants. As a
> result, it makes code hard to understand, modify and support. Could you please
> introduce named constants instead of hardcoded numbers?
> 
> 
> > +		ac = ((u32)src[i] << 16) | ((u32)src[i + 1] << 8) | (u32)src[i + 2];
> > +		*cp++ = base64_table[(ac >> 18) & 0x3f];
> > +		*cp++ = base64_table[(ac >> 12) & 0x3f];
> > +		*cp++ = base64_table[(ac >> 6) & 0x3f];
> > +		*cp++ = base64_table[ac & 0x3f];
> > +		i += 3;
> > +	}
> > +
> > +	switch (srclen - i) {
> > +	case 2:
> > +		ac = ((u32)src[i] << 16) | ((u32)src[i + 1] << 8);
> > +		*cp++ = base64_table[(ac >> 18) & 0x3f];
> > +		*cp++ = base64_table[(ac >> 12) & 0x3f];
> > +		*cp++ = base64_table[(ac >> 6) & 0x3f];
> > +		break;
> > +	case 1:
> > +		ac = ((u32)src[i] << 16);
> > +		*cp++ = base64_table[(ac >> 18) & 0x3f];
> > +		*cp++ = base64_table[(ac >> 12) & 0x3f];
> > +		break;
> >  	}
> > -	if (bits)
> > -		*cp++ = base64_table[(ac << (6 - bits)) & 0x3f];
> >  	return cp - dst;
> >  }
> >  
> 
> Let me test your patch and check that it doesn't introduce regression(s).
> 
> Thanks,
> Slava.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH] ceph: optimize ceph_base64_encode() with block processing
  2025-09-02 21:05   ` Kuan-Wei Chiu
@ 2025-09-02 21:21     ` Viacheslav Dubeyko
  2025-09-03  7:55       ` Luis Henriques
  0 siblings, 1 reply; 7+ messages in thread
From: Viacheslav Dubeyko @ 2025-09-02 21:21 UTC (permalink / raw)
  To: 409411716@gms.tku.edu.tw, visitorckw@gmail.com
  Cc: Xiubo Li, idryomov@gmail.com, linux-kernel@vger.kernel.org,
	ceph-devel@vger.kernel.org

On Wed, 2025-09-03 at 05:05 +0800, Kuan-Wei Chiu wrote:
> On Tue, Sep 02, 2025 at 07:37:22PM +0000, Viacheslav Dubeyko wrote:
> > On Sat, 2025-08-30 at 21:28 +0800, Guan-Chun Wu wrote:
> > > Previously, ceph_base64_encode() used a bitstream approach, handling one
> > > input byte at a time and performing extra bit operations. While correct,
> > > this method was suboptimal.
> > > 
> > 
> > Sounds interesting!
> > 
> > Is ceph_base64_decode() efficient then?
> > Do we have something in crypto library of Linux kernel? Maybe we can use
> > something efficient enough from there?
> > 
> Hi Viacheslav,
> 
> FYI, we already have base64 encode/decode implementations in
> lib/base64.c. As discussed in another thread [1], I think we can put
> the optimized version there and have users switch to call the library
> functions.
> 
> [1]: https://lore.kernel.org/lkml/38753d95-8503-4b72-9590-cb129aa49a41@t-8ch.de/  
> 
> 

Sounds great! Generalized version of this algorithm is much better than
supporting some implementation in Ceph code.

Thanks,
Slava.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] ceph: optimize ceph_base64_encode() with block processing
  2025-09-02 21:21     ` Viacheslav Dubeyko
@ 2025-09-03  7:55       ` Luis Henriques
  2025-09-03  8:10         ` Kuan-Wei Chiu
  0 siblings, 1 reply; 7+ messages in thread
From: Luis Henriques @ 2025-09-03  7:55 UTC (permalink / raw)
  To: Viacheslav Dubeyko
  Cc: 409411716@gms.tku.edu.tw, visitorckw@gmail.com, Xiubo Li,
	idryomov@gmail.com, linux-kernel@vger.kernel.org,
	ceph-devel@vger.kernel.org

On Tue, Sep 02, 2025 at 09:21:14PM +0000, Viacheslav Dubeyko wrote:
> On Wed, 2025-09-03 at 05:05 +0800, Kuan-Wei Chiu wrote:
> > On Tue, Sep 02, 2025 at 07:37:22PM +0000, Viacheslav Dubeyko wrote:
> > > On Sat, 2025-08-30 at 21:28 +0800, Guan-Chun Wu wrote:
> > > > Previously, ceph_base64_encode() used a bitstream approach, handling one
> > > > input byte at a time and performing extra bit operations. While correct,
> > > > this method was suboptimal.
> > > > 
> > > 
> > > Sounds interesting!
> > > 
> > > Is ceph_base64_decode() efficient then?
> > > Do we have something in crypto library of Linux kernel? Maybe we can use
> > > something efficient enough from there?
> > > 
> > Hi Viacheslav,
> > 
> > FYI, we already have base64 encode/decode implementations in
> > lib/base64.c. As discussed in another thread [1], I think we can put
> > the optimized version there and have users switch to call the library
> > functions.
> > 
> > [1]: https://lore.kernel.org/lkml/38753d95-8503-4b72-9590-cb129aa49a41@t-8ch.de/  
> > 
> > 
> 
> Sounds great! Generalized version of this algorithm is much better than
> supporting some implementation in Ceph code.

Please note that ceph can not use the default base64 implementation because
it uses the '_' character in the encoding, as explained in commit

  64e86f632bf1 ("ceph: add base64 endcoding routines for encrypted names")

That's why it implements it's own version according to an IMAP RFC, which
uses '+' and ',' instead of '-' and '_'.

Cheers,
--
Luis

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] ceph: optimize ceph_base64_encode() with block processing
  2025-09-03  7:55       ` Luis Henriques
@ 2025-09-03  8:10         ` Kuan-Wei Chiu
  2025-09-03  8:42           ` Luis Henriques
  0 siblings, 1 reply; 7+ messages in thread
From: Kuan-Wei Chiu @ 2025-09-03  8:10 UTC (permalink / raw)
  To: Luis Henriques
  Cc: Viacheslav Dubeyko, 409411716@gms.tku.edu.tw, Xiubo Li,
	idryomov@gmail.com, linux-kernel@vger.kernel.org,
	ceph-devel@vger.kernel.org

On Wed, Sep 03, 2025 at 08:55:36AM +0100, Luis Henriques wrote:
> On Tue, Sep 02, 2025 at 09:21:14PM +0000, Viacheslav Dubeyko wrote:
> > On Wed, 2025-09-03 at 05:05 +0800, Kuan-Wei Chiu wrote:
> > > On Tue, Sep 02, 2025 at 07:37:22PM +0000, Viacheslav Dubeyko wrote:
> > > > On Sat, 2025-08-30 at 21:28 +0800, Guan-Chun Wu wrote:
> > > > > Previously, ceph_base64_encode() used a bitstream approach, handling one
> > > > > input byte at a time and performing extra bit operations. While correct,
> > > > > this method was suboptimal.
> > > > > 
> > > > 
> > > > Sounds interesting!
> > > > 
> > > > Is ceph_base64_decode() efficient then?
> > > > Do we have something in crypto library of Linux kernel? Maybe we can use
> > > > something efficient enough from there?
> > > > 
> > > Hi Viacheslav,
> > > 
> > > FYI, we already have base64 encode/decode implementations in
> > > lib/base64.c. As discussed in another thread [1], I think we can put
> > > the optimized version there and have users switch to call the library
> > > functions.
> > > 
> > > [1]: https://lore.kernel.org/lkml/38753d95-8503-4b72-9590-cb129aa49a41@t-8ch.de/  
> > > 
> > > 
> > 
> > Sounds great! Generalized version of this algorithm is much better than
> > supporting some implementation in Ceph code.
> 
> Please note that ceph can not use the default base64 implementation because
> it uses the '_' character in the encoding, as explained in commit
> 
>   64e86f632bf1 ("ceph: add base64 endcoding routines for encrypted names")
> 
> That's why it implements it's own version according to an IMAP RFC, which
> uses '+' and ',' instead of '-' and '_'.
> 
Perhaps we could modify the API to allow users to provide a custom
base64 table or an extra parameter to specify which RFC standard to use
for encoding/decoding?

Regards,
Kuan-Wei

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] ceph: optimize ceph_base64_encode() with block processing
  2025-09-03  8:10         ` Kuan-Wei Chiu
@ 2025-09-03  8:42           ` Luis Henriques
  0 siblings, 0 replies; 7+ messages in thread
From: Luis Henriques @ 2025-09-03  8:42 UTC (permalink / raw)
  To: Kuan-Wei Chiu
  Cc: Viacheslav Dubeyko, 409411716@gms.tku.edu.tw, Xiubo Li,
	idryomov@gmail.com, linux-kernel@vger.kernel.org,
	ceph-devel@vger.kernel.org

On Wed, Sep 03 2025, Kuan-Wei Chiu wrote:

> On Wed, Sep 03, 2025 at 08:55:36AM +0100, Luis Henriques wrote:
>> On Tue, Sep 02, 2025 at 09:21:14PM +0000, Viacheslav Dubeyko wrote:
>> > On Wed, 2025-09-03 at 05:05 +0800, Kuan-Wei Chiu wrote:
>> > > On Tue, Sep 02, 2025 at 07:37:22PM +0000, Viacheslav Dubeyko wrote:
>> > > > On Sat, 2025-08-30 at 21:28 +0800, Guan-Chun Wu wrote:
>> > > > > Previously, ceph_base64_encode() used a bitstream approach, handling one
>> > > > > input byte at a time and performing extra bit operations. While correct,
>> > > > > this method was suboptimal.
>> > > > > 
>> > > > 
>> > > > Sounds interesting!
>> > > > 
>> > > > Is ceph_base64_decode() efficient then?
>> > > > Do we have something in crypto library of Linux kernel? Maybe we can use
>> > > > something efficient enough from there?
>> > > > 
>> > > Hi Viacheslav,
>> > > 
>> > > FYI, we already have base64 encode/decode implementations in
>> > > lib/base64.c. As discussed in another thread [1], I think we can put
>> > > the optimized version there and have users switch to call the library
>> > > functions.
>> > > 
>> > > [1]: https://lore.kernel.org/lkml/38753d95-8503-4b72-9590-cb129aa49a41@t-8ch.de/  
>> > > 
>> > > 
>> > 
>> > Sounds great! Generalized version of this algorithm is much better than
>> > supporting some implementation in Ceph code.
>> 
>> Please note that ceph can not use the default base64 implementation because
>> it uses the '_' character in the encoding, as explained in commit
>> 
>>   64e86f632bf1 ("ceph: add base64 endcoding routines for encrypted names")
>> 
>> That's why it implements it's own version according to an IMAP RFC, which
>> uses '+' and ',' instead of '-' and '_'.
>> 
> Perhaps we could modify the API to allow users to provide a custom
> base64 table or an extra parameter to specify which RFC standard to use
> for encoding/decoding?

Yes, sure.  That should work as well.  If I remember correctly, I didn't
bother doing that back then because ceph was the only place that needed a
custom base64.  But I not really sure, that was long ago.

Cheers,
-- 
Luís

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-09-03  8:42 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-30 13:28 [PATCH] ceph: optimize ceph_base64_encode() with block processing Guan-Chun Wu
2025-09-02 19:37 ` Viacheslav Dubeyko
2025-09-02 21:05   ` Kuan-Wei Chiu
2025-09-02 21:21     ` Viacheslav Dubeyko
2025-09-03  7:55       ` Luis Henriques
2025-09-03  8:10         ` Kuan-Wei Chiu
2025-09-03  8:42           ` Luis Henriques

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).