Skip to contentSkip to conversation

hash_readwrite_autotest: fix printf parameters

PATCH In review Andre Muezerie 6 participants 34 messages Mar 6, 2025 – Apr 10, 2025

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 hash_readwrite_autotest: fix printf parameters

Compiling with MSVC logs the warnings below, which result in build error:

../app/test/test_hash_readwrite.c(73): warning C4476: 'printf' :
unknown type field character ''' in format specifier
../app/test/test_hash_readwrite.c(75): warning C4474: 'printf' :
too many arguments passed for format string
../app/test/test_hash_readwrite.c(75): note: placeholders and
their parameters expect 2 variadic arguments, but 4 were provided

---
 app/test/test_hash_readwrite.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
app/test/test_hash_readwrite.c +1 -1
index 1867376ade..dc07df709a 100644
@@ -70,7 +70,7 @@ test_hash_readwrite_worker(__rte_unused void *arg)
 	}
 	offset = tbl_rw_test_param.num_insert * i;
 
-	printf("Core #%d inserting and reading %d: %'"PRId64" - %'"PRId64"\n",
+	printf("Core #%d inserting and reading %d: %" PRId64 " - %" PRId64 "\n",
 	       lcore_id, tbl_rw_test_param.num_insert,
 	       offset, offset + tbl_rw_test_param.num_insert - 1);
 
Signature · 1 line
2.48.1.vfs.0.0
#1 lore raw
Quoted 10 lines · Andre Muezerie

Compiling with MSVC logs the warnings below, which result in build error:

../app/test/test_hash_readwrite.c(73): warning C4476: 'printf' :
unknown type field character ''' in format specifier
../app/test/test_hash_readwrite.c(75): warning C4474: 'printf' :
too many arguments passed for format string
../app/test/test_hash_readwrite.c(75): note: placeholders and
their parameters expect 2 variadic arguments, but 4 were provided

Quoted 5 lines
---
 app/test/test_hash_readwrite.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

IF the "'" character is not supported, is there some other method to do thousands grouping in MSVC?

/Bruce

#2 lore raw
Quoted 16 lines · Bruce Richardson
Quoted 10 lines · Andre Muezerie

Compiling with MSVC logs the warnings below, which result in build error:

../app/test/test_hash_readwrite.c(73): warning C4476: 'printf' :
unknown type field character ''' in format specifier
../app/test/test_hash_readwrite.c(75): warning C4474: 'printf' :
too many arguments passed for format string
../app/test/test_hash_readwrite.c(75): note: placeholders and
their parameters expect 2 variadic arguments, but 4 were provided

Quoted 5 lines
---
 app/test/test_hash_readwrite.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 4 lines

IF the "'" character is not supported, is there some other method to do thousands grouping in MSVC?

/Bruce

The problem exists with all compilers I tried on Windows:

1) MSVC logs the error I mentioned above

2) GCC and Clang don't complain at compile time, but don't honor the "'" as a special character. As an example,
printf("%'d\n", 1024);
results in
'd

It seems that for this syntax to work as you would expect, support needs to exist in both the compiler and the libraries used.

Back to your question: there's no equivalent syntax on Windows that provides the thousands grouping. If really needed (and I understand it is useful for large numbers), we could get the same result by calling a helper function that would convert the number in the formatted string and use that in the printf statement.

There is a Win32 API that does that. It takes a string as input though: GetNumberFormatA.
(https://learn.microsoft.com/en-us/windows/win32/api/winnls/nf-winnls-getnumberformata)

We could use ifdefs to keep the old logic on Linux and use new logic on Windows (for all compilers).

Let me know if this is something that would need to be done, or if the current output without thousands grouping is good enough.
--
Andre Muezerie

#3 lore raw
Quoted 40 lines · Andre Muezerie
Quoted 16 lines · Bruce Richardson
Quoted 10 lines · Andre Muezerie

Compiling with MSVC logs the warnings below, which result in build error:

../app/test/test_hash_readwrite.c(73): warning C4476: 'printf' :
unknown type field character ''' in format specifier
../app/test/test_hash_readwrite.c(75): warning C4474: 'printf' :
too many arguments passed for format string
../app/test/test_hash_readwrite.c(75): note: placeholders and
their parameters expect 2 variadic arguments, but 4 were provided

Quoted 5 lines
---
 app/test/test_hash_readwrite.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 4 lines

IF the "'" character is not supported, is there some other method to do thousands grouping in MSVC?

/Bruce

The problem exists with all compilers I tried on Windows:

1) MSVC logs the error I mentioned above

2) GCC and Clang don't complain at compile time, but don't honor the "'" as a special character. As an example,
printf("%'d\n", 1024);
results in
'd

It seems that for this syntax to work as you would expect, support needs to exist in both the compiler and the libraries used.

Back to your question: there's no equivalent syntax on Windows that provides the thousands grouping. If really needed (and I understand it is useful for large numbers), we could get the same result by calling a helper function that would convert the number in the formatted string and use that in the printf statement.

Quoted 8 lines

There is a Win32 API that does that. It takes a string as input though: GetNumberFormatA.
(https://learn.microsoft.com/en-us/windows/win32/api/winnls/nf-winnls-getnumberformata)

We could use ifdefs to keep the old logic on Linux and use new logic on Windows (for all compilers).

Let me know if this is something that would need to be done, or if the current output without thousands grouping is good enough.
--

The thousands grouping is incredibly helpful when working with large numbers, but given the lack of support for this on Windows, we'll just have to go without, I think.

/Bruce

#4 lore raw
Quoted 46 lines · On Mon, 10 Mar 2025 10:51:51 +0000 Bruce Richardson…
Quoted 45 lines · Andre Muezerie
Quoted 16 lines · Bruce Richardson
Quoted 10 lines · Andre Muezerie

Compiling with MSVC logs the warnings below, which result in build error:

../app/test/test_hash_readwrite.c(73): warning C4476: 'printf' :
unknown type field character ''' in format specifier
../app/test/test_hash_readwrite.c(75): warning C4474: 'printf' :
too many arguments passed for format string
../app/test/test_hash_readwrite.c(75): note: placeholders and
their parameters expect 2 variadic arguments, but 4 were provided

Quoted 5 lines
---
 app/test/test_hash_readwrite.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 4 lines

IF the "'" character is not supported, is there some other method to do thousands grouping in MSVC?

/Bruce

The problem exists with all compilers I tried on Windows:

1) MSVC logs the error I mentioned above

2) GCC and Clang don't complain at compile time, but don't honor the "'" as a special character. As an example,
printf("%'d\n", 1024);
results in
'd

It seems that for this syntax to work as you would expect, support needs to exist in both the compiler and the libraries used.

Back to your question: there's no equivalent syntax on Windows that provides the thousands grouping. If really needed (and I understand it is useful for large numbers), we could get the same result by calling a helper function that would convert the number in the formatted string and use that in the printf statement.

There is a Win32 API that does that. It takes a string as input though: GetNumberFormatA.
(https://learn.microsoft.com/en-us/windows/win32/api/winnls/nf-winnls-getnumberformata)

We could use ifdefs to keep the old logic on Linux and use new logic on Windows (for all compilers).

Quoted 8 lines
Quoted 3 lines

Let me know if this is something that would need to be done, or if the current output without thousands grouping is good enough.

The thousands grouping is incredibly helpful when working with large numbers, but given the lack of support for this on Windows, we'll just have to go without, I think.

/Bruce

Maybe some variation of the pretty printing code that iproute2 has (see print_num) would be useful. Feel free to reuse it. I wrote the initial version.

#5 lore raw
Quoted 56 lines · Stephen Hemminger
Quoted 53 lines · On Mon, 10 Mar 2025 10:51:51 +0000 Bruce Richardson…
Quoted 40 lines · Andre Muezerie
Quoted 16 lines · Bruce Richardson
Quoted 10 lines · Andre Muezerie

Compiling with MSVC logs the warnings below, which result in build error:

../app/test/test_hash_readwrite.c(73): warning C4476: 'printf' :
unknown type field character ''' in format specifier
../app/test/test_hash_readwrite.c(75): warning C4474: 'printf' :
too many arguments passed for format string
../app/test/test_hash_readwrite.c(75): note: placeholders and
their parameters expect 2 variadic arguments, but 4 were provided

Quoted 5 lines
---
 app/test/test_hash_readwrite.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 4 lines

IF the "'" character is not supported, is there some other method to do thousands grouping in MSVC?

/Bruce

The problem exists with all compilers I tried on Windows:

1) MSVC logs the error I mentioned above

2) GCC and Clang don't complain at compile time, but don't honor the "'" as a special character. As an example,
printf("%'d\n", 1024);
results in
'd

It seems that for this syntax to work as you would expect, support needs to exist in both the compiler and the libraries used.

Back to your question: there's no equivalent syntax on Windows that provides the thousands grouping. If really needed (and I understand it is useful for large numbers), we could get the same result by calling a helper function that would convert the number in the formatted string and use that in the printf statement.

Quoted 8 lines

There is a Win32 API that does that. It takes a string as input though: GetNumberFormatA.
(https://learn.microsoft.com/en-us/windows/win32/api/winnls/nf-winnls-getnumberformata)

We could use ifdefs to keep the old logic on Linux and use new logic on Windows (for all compilers).

Let me know if this is something that would need to be done, or if the current output without thousands grouping is good enough.

The thousands grouping is incredibly helpful when working with large numbers, but given the lack of support for this on Windows, we'll just have to go without, I think.

Quoted 5 lines
Quoted 1 line

/Bruce

Maybe some variation of the pretty printing code that iproute2 has (see print_num) would be useful. Feel free to reuse it. I wrote the initial version.

That's an interesting suggestion. I'll use that.

#6 lore raw
Quoted 75 lines

From: Andre Muezerie [mailto:[email protected]]
Sent: Tuesday, 11 March 2025 15.40

Quoted 10 lines · Stephen Hemminger
Quoted 7 lines · On Mon, 10 Mar 2025 10:51:51 +0000 Bruce Richardson…
Quoted 6 lines · Andre Muezerie
Quoted 5 lines · Bruce Richardson
Quoted 4 lines · Andre Muezerie

Compiling with MSVC logs the warnings below, which result in build error:

../app/test/test_hash_readwrite.c(73): warning C4476:

'printf' :

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

unknown type field character ''' in format specifier
../app/test/test_hash_readwrite.c(75): warning C4474:

'printf' :

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

too many arguments passed for format string
../app/test/test_hash_readwrite.c(75): note: placeholders and
their parameters expect 2 variadic arguments, but 4 were

provided

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 6 lines
---
 app/test/test_hash_readwrite.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

IF the "'" character is not supported, is there some other

method to do

Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 3 lines

thousands grouping in MSVC?

/Bruce

The problem exists with all compilers I tried on Windows:

1) MSVC logs the error I mentioned above

2) GCC and Clang don't complain at compile time, but don't honor

the "'" as a special

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines

character. As an example,
printf("%'d\n", 1024);
results in
'd

It seems that for this syntax to work as you would expect,

support needs to exist in both the

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

compiler and the libraries used.

Back to your question: there's no equivalent syntax on Windows

that provides the thousands grouping.

Quoted 1 line
Quoted 1 line
Quoted 1 line

If really needed (and I understand it is useful for large

numbers), we could get the same result

Quoted 1 line
Quoted 1 line
Quoted 1 line

by calling a helper function that would convert the number in the

formatted string and use that

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

in the printf statement.

There is a Win32 API that does that. It takes a string as input

though: GetNumberFormatA.

Quoted 1 line
Quoted 1 line
Quoted 1 line

(https://learn.microsoft.com/en-us/windows/win32/api/winnls/nf-

winnls-getnumberformata)

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

We could use ifdefs to keep the old logic on Linux and use new

logic on Windows (for all compilers).

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

Let me know if this is something that would need to be done, or

if the current output

Quoted 3 lines
Quoted 3 lines
Quoted 2 lines

without thousands grouping is good enough.
--

The thousands grouping is incredibly helpful when working with

large

Quoted 1 line
Quoted 1 line

numbers, but given the lack of support for this on Windows, we'll

just have

Quoted 2 lines
Quoted 2 lines

to go without, I think.

Quoted 7 lines
Quoted 5 lines
Quoted 1 line

/Bruce

Maybe some variation of the pretty printing code that iproute2 has (see print_num) would be useful. Feel free to reuse it. I wrote the initial version.

That's an interesting suggestion. I'll use that.

Often "123 M" is better than "123123123", but sometimes seeing the last digit move is important, e.g. when looking at packet counters.

Just mentioning, so you consider which format is better every time you use it.

#7 lore raw

PATCH v2 1/3 eal: add function rte_size_to_str

In reply to Andre Muezerie

It's common to use %' in the printf format specifier to make large numbers more easily readable by having the thousands grouped. However, this grouping does not work on Windows. Therefore, a function is needed to make uint64_t numbers more easily readable. There are at least two tests that can benefit from this new function.

---
 lib/eal/common/eal_common_string_fns.c | 44 ++++++++++++++++++++++++++
 lib/eal/include/rte_common.h           | 21 ++++++++++++
 2 files changed, 65 insertions(+)
lib/eal/common/eal_common_string_fns.c +44 -0
index 9ca2045b18..b658d68eac 100644
@@ -4,6 +4,7 @@
 
 #include <ctype.h>
 #include <errno.h>
+#include <inttypes.h>
 #include <stdio.h>
 #include <stdlib.h>
 
@@ -87,6 +88,12 @@ rte_str_to_size(const char *str)
 		endptr++; /* allow 1 space gap */
 
 	switch (*endptr) {
+	case 'E': case 'e':
+		size *= 1024; /* fall-through */
+	case 'P': case 'p':
+		size *= 1024; /* fall-through */
+	case 'T': case 't':
+		size *= 1024; /* fall-through */
 	case 'G': case 'g':
 		size *= 1024; /* fall-through */
 	case 'M': case 'm':
@@ -98,3 +105,40 @@ rte_str_to_size(const char *str)
 	}
 	return size;
 }
+
+int
+rte_size_to_str(char *buf, int buf_size,
+		uint64_t count, bool use_iec)
+{
+	const char *prefix = "kMGTPE";
+	const unsigned int base = use_iec ? 1024 : 1000;
+	uint64_t powi = 1;
+	uint16_t powj = 1;
+	uint8_t precision = 2;
+
+	if (count < base)
+		return snprintf(buf, buf_size, "%"PRIu64" ", count);
+
+	/* increase value by a factor of 1000/1024 and store
+	 * if result is something a human can read
+	 */
+	for (;;) {
+		powi *= base;
+		if (count / base < powi)
+			break;
+
+		if (!prefix[1])
+			break;
+		++prefix;
+	}
+
+	/* try to guess a good number of digits for precision */
+	for (; precision > 0; precision--) {
+		powj *= 10;
+		if (count / powi < powj)
+			break;
+	}
+
+	return snprintf(buf, buf_size, "%.*f%c%s", precision,
+			(double)count / powi, *prefix, use_iec ? "i" : "");
+}
lib/eal/include/rte_common.h +21 -0
index 386f11ae40..81fa4d3c34 100644
@@ -14,6 +14,7 @@
 
 #include <assert.h>
 #include <limits.h>
+#include <stdbool.h>
 #include <stdint.h>
 #include <stdalign.h>
 
@@ -919,6 +920,26 @@ __extension__ typedef uint64_t RTE_MARKER64[0];
 uint64_t
 rte_str_to_size(const char *str);
 
+/**
+ * Converts the uint64_t value provided to a human-readable string.
+ * It null-terminates the string, truncating the data if needed.
+ *
+ * @param buf
+ *     Buffer to write the string to.
+ * @param buf_size
+ *     Size of the buffer.
+ * @param count
+ *     Number to convert.
+ * @param use_iec
+ *     If true, use IEC units (1024-based), otherwise use SI units (1000-based).
+ * @return
+ *     Number of characters written (not including the null-terminator),
+ *     or that would have been required when the buffer is too small.
+ */
+int
+rte_size_to_str(char *buf, int buf_size,
+		uint64_t count, bool use_iec);
+
 /**
  * Function to terminate the application immediately, printing an error
  * message and returning the exit_code back to the shell.
Signature · 1 line
2.48.1.vfs.0.1
#8 lore raw

PATCH v2 2/3 hash_multiwriter_autotest: fix printf parameters

Compiling with MSVC logs the warnings below, which result in build error:

../app/test/test_hash_multiwriter.c(71): warning C4476: 'printf' :
unknown type field character ''' in format specifier
../app/test/test_hash_multiwriter.c(73): warning C4474: 'printf' :
too many arguments passed for format string
../app/test/test_hash_multiwriter.c(73): note: placeholders and their
parameters expect 2 variadic arguments, but 4 were provided

---
 app/test/test_hash_multiwriter.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)
app/test/test_hash_multiwriter.c +10 -3
index 33d3147bd8..5c1dd4c195 100644
@@ -32,6 +32,8 @@
 
 #define RTE_APP_TEST_HASH_MULTIWRITER_FAILED 0
 
+#define OFFSET_STR_LEN 16
+
 struct {
 	uint32_t *keys;
 	uint32_t *found;
@@ -51,11 +53,14 @@ static int use_htm;
 static int
 test_hash_multiwriter_worker(void *arg)
 {
+	char offset_start[OFFSET_STR_LEN];
+	char offset_end[OFFSET_STR_LEN];
 	uint64_t i, offset;
 	uint16_t pos_core;
 	uint32_t lcore_id = rte_lcore_id();
 	uint64_t begin, cycles;
 	uint16_t *enabled_core_ids = (uint16_t *)arg;
+	const bool use_iec = true;
 
 	for (pos_core = 0; pos_core < rte_lcore_count(); pos_core++) {
 		if (enabled_core_ids[pos_core] == lcore_id)
@@ -68,10 +73,12 @@ test_hash_multiwriter_worker(void *arg)
 	 */
 	offset = pos_core * tbl_multiwriter_test_params.nb_tsx_insertion;
 
-	printf("Core #%d inserting %d: %'"PRId64" - %'"PRId64"\n",
+	rte_size_to_str(offset_start, sizeof(offset_start), offset, use_iec);
+	rte_size_to_str(offset_end, sizeof(offset_end),
+			offset + tbl_multiwriter_test_params.nb_tsx_insertion - 1, use_iec);
+	printf("Core #%u inserting %u: %s - %s\n",
 	       lcore_id, tbl_multiwriter_test_params.nb_tsx_insertion,
-	       offset,
-	       offset + tbl_multiwriter_test_params.nb_tsx_insertion - 1);
+	       offset_start, offset_end);
 
 	begin = rte_rdtsc_precise();
 
Signature · 1 line
2.48.1.vfs.0.1
#9 lore raw

PATCH v2 3/3 hash_readwrite_autotest: fix printf parameters

In reply to Andre Muezerie

Compiling with MSVC logs the warnings below, which result in build error:

../app/test/test_hash_readwrite.c(73): warning C4476: 'printf' :
unknown type field character ''' in format specifier
../app/test/test_hash_readwrite.c(75): warning C4474: 'printf' :
too many arguments passed for format string
../app/test/test_hash_readwrite.c(75): note: placeholders and
their parameters expect 2 variadic arguments, but 4 were provided

---
 app/test/test_hash_readwrite.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)
app/test/test_hash_readwrite.c +11 -2
index 1867376ade..3fa7003bfb 100644
@@ -25,6 +25,8 @@
 #define NUM_TEST 3
 unsigned int core_cnt[NUM_TEST] = {2, 4, 8};
 
+#define OFFSET_STR_LEN 16
+
 unsigned int worker_core_ids[RTE_MAX_LCORE];
 struct perf {
 	uint32_t single_read;
@@ -57,10 +59,13 @@ static RTE_ATOMIC(uint64_t) gwrites;
 static int
 test_hash_readwrite_worker(__rte_unused void *arg)
 {
+	char offset_start[OFFSET_STR_LEN];
+	char offset_end[OFFSET_STR_LEN];
 	uint64_t i, offset;
 	uint32_t lcore_id = rte_lcore_id();
 	uint64_t begin, cycles;
 	int *ret;
+	const bool use_iec = true;
 
 	ret = rte_malloc(NULL, sizeof(int) *
 				tbl_rw_test_param.num_insert, 0);
@@ -70,9 +75,13 @@ test_hash_readwrite_worker(__rte_unused void *arg)
 	}
 	offset = tbl_rw_test_param.num_insert * i;
 
-	printf("Core #%d inserting and reading %d: %'"PRId64" - %'"PRId64"\n",
+	rte_size_to_str(offset_start, sizeof(offset_start), offset, use_iec);
+	rte_size_to_str(offset_end, sizeof(offset_end),
+			offset + tbl_rw_test_param.num_insert - 1, use_iec);
+
+	printf("Core #%u inserting and reading %u: %s - %s\n",
 	       lcore_id, tbl_rw_test_param.num_insert,
-	       offset, offset + tbl_rw_test_param.num_insert - 1);
+	       offset_start, offset_end);
 
 	begin = rte_rdtsc_precise();
 
Signature · 1 line
2.48.1.vfs.0.1
#10 lore raw

Re: [PATCH v2 1/3] eal: add function rte_size_to_str

In reply to Andre Muezerie
Quoted 38 lines · On Tue, 11 Mar 2025 08:33:13 -0700 Andre Muezerie…

It's common to use %' in the printf format specifier to make large numbers more easily readable by having the thousands grouped. However, this grouping does not work on Windows. Therefore, a function is needed to make uint64_t numbers more easily readable. There are at least two tests that can benefit from this new function.

---
 lib/eal/common/eal_common_string_fns.c | 44 ++++++++++++++++++++++++++
 lib/eal/include/rte_common.h           | 21 ++++++++++++
 2 files changed, 65 insertions(+)
lib/eal/common/eal_common_string_fns.c +7 -0
index 9ca2045b18..b658d68eac 100644
@@ -4,6 +4,7 @@
 
 #include <ctype.h>
 #include <errno.h>
+#include <inttypes.h>
 #include <stdio.h>
 #include <stdlib.h>
 
@@ -87,6 +88,12 @@ rte_str_to_size(const char *str)
 		endptr++; /* allow 1 space gap */
 
 	switch (*endptr) {
+	case 'E': case 'e':
+		size *= 1024; /* fall-through */
+	case 'P': case 'p':
+		size *= 1024; /* fall-through */
+	case 'T': case 't':
+		size *= 1024; /* fall-through */
 	case 'G': case 'g':
 		size *= 1024; /* fall-through */
 	case 'M': case 'm':

lib/eal/common/eal_common_string_fns.c rte_str_to_size(const char *str) · line 108

Quoted 4 lines
lib/eal/common/eal_common_string_fns.c +1 -0
@@ -98,3 +105,40 @@ rte_str_to_size(const char *str)
 	}
 	return size;
 }
+

Is this right? Looks like existing code is not using correct multiple

The standard for communication is K = 1000 and the standard for storage is K = 1024. That is why iproute2 has the use_iec flag.

https://en.wikipedia.org/wiki/Data-rate_units

#11 lore raw

Re: [PATCH v2 1/3] eal: add function rte_size_to_str

Quoted 48 lines · Stephen Hemminger
Quoted 38 lines · On Tue, 11 Mar 2025 08:33:13 -0700 Andre Muezerie…

It's common to use %' in the printf format specifier to make large numbers more easily readable by having the thousands grouped. However, this grouping does not work on Windows. Therefore, a function is needed to make uint64_t numbers more easily readable. There are at least two tests that can benefit from this new function.

---
 lib/eal/common/eal_common_string_fns.c | 44 ++++++++++++++++++++++++++
 lib/eal/include/rte_common.h           | 21 ++++++++++++
 2 files changed, 65 insertions(+)
lib/eal/common/eal_common_string_fns.c +7 -0
index 9ca2045b18..b658d68eac 100644
@@ -4,6 +4,7 @@
 
 #include <ctype.h>
 #include <errno.h>
+#include <inttypes.h>
 #include <stdio.h>
 #include <stdlib.h>
 
@@ -87,6 +88,12 @@ rte_str_to_size(const char *str)
 		endptr++; /* allow 1 space gap */
 
 	switch (*endptr) {
+	case 'E': case 'e':
+		size *= 1024; /* fall-through */
+	case 'P': case 'p':
+		size *= 1024; /* fall-through */
+	case 'T': case 't':
+		size *= 1024; /* fall-through */
 	case 'G': case 'g':
 		size *= 1024; /* fall-through */
 	case 'M': case 'm':

lib/eal/common/eal_common_string_fns.c rte_str_to_size(const char *str) · line 108

Quoted 4 lines
lib/eal/common/eal_common_string_fns.c +1 -0
@@ -98,3 +105,40 @@ rte_str_to_size(const char *str)
 	}
 	return size;
 }
+

Is this right? Looks like existing code is not using correct multiple

Quoted 5 lines

The standard for communication is K = 1000 and the standard for storage is K = 1024. That is why iproute2 has the use_iec flag.

https://en.wikipedia.org/wiki/Data-rate_units

Ideally, we should have that flag, but I believe this current function is for things like memory sizes, which means that using 1024 is correct.

/Bruce

#12 lore raw

RE: [PATCH v2 1/3] eal: add function rte_size_to_str

In reply to Andre Muezerie

lib/eal/common/eal_common_string_fns.c rte_str_to_size(const char *str) · line 143

Quoted 2 lines
lib/eal/common/eal_common_string_fns.c +2 -0
+	return snprintf(buf, buf_size, "%.*f%c%s", precision,
+			(double)count / powi, *prefix, use_iec ? "i" : "");

I prefer a space between the number and the postfix (if a postfix is present), e.g. "1.23 M", so when followed by the unit, it becomes "1.23 Mbit/s" rather than "1.23M bit/s".

Quoted 3 lines

+/**
+ * Converts the uint64_t value provided to a human-readable string.
+ * It null-terminates the string, truncating the data if needed.

Please add some examples showing the output formatting. E.g.: "123 M", "12.3 M",
"12 M", " 12 M" or "12.0 M",
"1.00 ", " 1.00", or " 1", or something else?

Quoted 19 lines

+ *
+ * @param buf
+ * Buffer to write the string to.
+ * @param buf_size
+ * Size of the buffer.
+ * @param count
+ * Number to convert.
+ * @param use_iec
+ * If true, use IEC units (1024-based), otherwise use SI units
(1000-based).
+ * @return
+ * Number of characters written (not including the null-
terminator),
+ * or that would have been required when the buffer is too small.
+ */

lib/eal/include/rte_common.h +4 -0
+int
+rte_size_to_str(char *buf, int buf_size,
+		uint64_t count, bool use_iec);
+
#13 lore raw

v3 cover fix how large numbers are printed by hash tests

In reply to Andre Muezerie

v3:
- Added rte_size_to_str to version.map (marked experimental for 25.07) so that symbol gets exported when building with -Ddefault_library=shared
- Added sample outputs
- Added a space between the number and the postfix (if a postfix is present)

Andre Muezerie (3):
eal: add function rte_size_to_str
hash_multiwriter_autotest: fix printf parameters
hash_readwrite_autotest: fix printf parameters

 app/test/test_hash_multiwriter.c       | 13 ++++++--
 app/test/test_hash_readwrite.c         | 13 ++++++--
 lib/eal/common/eal_common_string_fns.c | 44 ++++++++++++++++++++++++++
 lib/eal/include/rte_common.h           | 31 ++++++++++++++++++
 lib/eal/version.map                    |  3 ++
 5 files changed, 99 insertions(+), 5 deletions(-)

--
2.48.1.vfs.0.1

#14 lore raw

PATCH v3 1/3 eal: add function rte_size_to_str

It's common to use %' in the printf format specifier to make large numbers more easily readable by having the thousands grouped. However, this grouping does not work on Windows. Therefore, a function is needed to make uint64_t numbers more easily readable. There are at least two tests that can benefit from this new function.

---
 lib/eal/common/eal_common_string_fns.c | 44 ++++++++++++++++++++++++++
 lib/eal/include/rte_common.h           | 31 ++++++++++++++++++
 lib/eal/version.map                    |  3 ++
 3 files changed, 78 insertions(+)
lib/eal/common/eal_common_string_fns.c +44 -0
index 9ca2045b18..4cc7f35652 100644
@@ -4,6 +4,7 @@
 
 #include <ctype.h>
 #include <errno.h>
+#include <inttypes.h>
 #include <stdio.h>
 #include <stdlib.h>
 
@@ -87,6 +88,12 @@ rte_str_to_size(const char *str)
 		endptr++; /* allow 1 space gap */
 
 	switch (*endptr) {
+	case 'E': case 'e':
+		size *= 1024; /* fall-through */
+	case 'P': case 'p':
+		size *= 1024; /* fall-through */
+	case 'T': case 't':
+		size *= 1024; /* fall-through */
 	case 'G': case 'g':
 		size *= 1024; /* fall-through */
 	case 'M': case 'm':
@@ -98,3 +105,40 @@ rte_str_to_size(const char *str)
 	}
 	return size;
 }
+
+int
+rte_size_to_str(char *buf, int buf_size,
+		uint64_t count, bool use_iec)
+{
+	const char *prefix = "kMGTPE";
+	const unsigned int base = use_iec ? 1024 : 1000;
+	uint64_t powi = 1;
+	uint16_t powj = 1;
+	uint8_t precision = 2;
+
+	if (count < base)
+		return snprintf(buf, buf_size, "%"PRIu64" ", count);
+
+	/* increase value by a factor of 1000/1024 and store
+	 * if result is something a human can read
+	 */
+	for (;;) {
+		powi *= base;
+		if (count / base < powi)
+			break;
+
+		if (!prefix[1])
+			break;
+		++prefix;
+	}
+
+	/* try to guess a good number of digits for precision */
+	for (; precision > 0; precision--) {
+		powj *= 10;
+		if (count / powi < powj)
+			break;
+	}
+
+	return snprintf(buf, buf_size, "%.*f %c%s", precision,
+			(double)count / powi, *prefix, use_iec ? "i" : "");
+}
lib/eal/include/rte_common.h +31 -0
index 386f11ae40..781c56adcd 100644
@@ -14,9 +14,11 @@
 
 #include <assert.h>
 #include <limits.h>
+#include <stdbool.h>
 #include <stdint.h>
 #include <stdalign.h>
 
+#include <rte_compat.h>
 #include <rte_config.h>
 
 /* OS specific include */
@@ -919,6 +921,35 @@ __extension__ typedef uint64_t RTE_MARKER64[0];
 uint64_t
 rte_str_to_size(const char *str);
 
+/**
+ * Converts the uint64_t value provided to a human-readable string.
+ * It null-terminates the string, truncating the data if needed.
+ *
+ * Sample outputs with "use_iec" disabled and enabled:
+ * 0 : "0 ", "0 "
+ * 700 : "700 ", "700 "
+ * 1000 : "1.00 k", "1000 "
+ * 1024 : "1.02 k", "1.00 ki"
+ * 21474836480 : "21.5 G", "20.0 Gi"
+ * 109951162777600 : "110 T", "100 Ti"
+ *
+ * @param buf
+ *     Buffer to write the string to.
+ * @param buf_size
+ *     Size of the buffer.
+ * @param count
+ *     Number to convert.
+ * @param use_iec
+ *     If true, use IEC units (1024-based), otherwise use SI units (1000-based).
+ * @return
+ *     Number of characters written (not including the null-terminator),
+ *     or that would have been required when the buffer is too small.
+ */
+__rte_experimental
+int
+rte_size_to_str(char *buf, int buf_size,
+		uint64_t count, bool use_iec);
+
 /**
  * Function to terminate the application immediately, printing an error
  * message and returning the exit_code back to the shell.
lib/eal/version.map +3 -0
index a20c713eb1..01b6a7c190 100644
@@ -398,6 +398,9 @@ EXPERIMENTAL {
 	# added in 24.11
 	rte_bitset_to_str;
 	rte_lcore_var_alloc;
+
+	# added in 25.07
+	rte_size_to_str;
 };
 
 INTERNAL {
Signature · 1 line
2.48.1.vfs.0.1
#15 lore raw

Re: [PATCH v3 1/3] eal: add function rte_size_to_str

Quoted 8 lines · Andre Muezerie

It's common to use %' in the printf format specifier to make large numbers more easily readable by having the thousands grouped. However, this grouping does not work on Windows. Therefore, a function is needed to make uint64_t numbers more easily readable. There are at least two tests that can benefit from this new function.

---

Thanks Andre, comments inline below.

/Bruce

Quoted 31 lines
 lib/eal/common/eal_common_string_fns.c | 44 ++++++++++++++++++++++++++
 lib/eal/include/rte_common.h           | 31 ++++++++++++++++++
 lib/eal/version.map                    |  3 ++
 3 files changed, 78 insertions(+)
lib/eal/common/eal_common_string_fns.c +7 -0
index 9ca2045b18..4cc7f35652 100644
@@ -4,6 +4,7 @@
 
 #include <ctype.h>
 #include <errno.h>
+#include <inttypes.h>
 #include <stdio.h>
 #include <stdlib.h>
 
@@ -87,6 +88,12 @@ rte_str_to_size(const char *str)
 		endptr++; /* allow 1 space gap */
 
 	switch (*endptr) {
+	case 'E': case 'e':
+		size *= 1024; /* fall-through */
+	case 'P': case 'p':
+		size *= 1024; /* fall-through */
+	case 'T': case 't':
+		size *= 1024; /* fall-through */
 	case 'G': case 'g':
 		size *= 1024; /* fall-through */
 	case 'M': case 'm':

lib/eal/common/eal_common_string_fns.c rte_str_to_size(const char *str) · line 111

Quoted 7 lines
lib/eal/common/eal_common_string_fns.c +4 -0
@@ -98,3 +105,40 @@ rte_str_to_size(const char *str)
 	}
 	return size;
 }
+
+int
+rte_size_to_str(char *buf, int buf_size,
+		uint64_t count, bool use_iec)

Minor nit, I don't think you should need to wrap this, we can have lines up to 100 chars long.

lib/eal/common/eal_common_string_fns.c rte_str_to_size(const char *str) · line 113

Quoted 2 lines
lib/eal/common/eal_common_string_fns.c +2 -0
+{
+	const char *prefix = "kMGTPE";

Why is "k" in lower case compared to the others all in upper-case? Also, these are suffixes not prefixes. :-)

Quoted 4 lines
lib/eal/common/eal_common_string_fns.c +4 -0
+	const unsigned int base = use_iec ? 1024 : 1000;
+	uint64_t powi = 1;
+	uint16_t powj = 1;
+	uint8_t precision = 2;

lib/eal/common/eal_common_string_fns.c rte_str_to_size(const char *str) · line 127

Quoted 10 lines
lib/eal/common/eal_common_string_fns.c +10 -0
+
+	if (count < base)
+		return snprintf(buf, buf_size, "%"PRIu64" ", count);
+
+	/* increase value by a factor of 1000/1024 and store
+	 * if result is something a human can read
+	 */
+	for (;;) {
+		powi *= base;
+		if (count / base < powi)

This would seem more logical to me as "count / powi < base" since it would match the initial check for "count < base" (which is essentially the same check since powi == 1).

lib/eal/common/eal_common_string_fns.c rte_str_to_size(const char *str) · line 130

Quoted 3 lines
lib/eal/common/eal_common_string_fns.c +3 -0
+			break;
+
+		if (!prefix[1])

Since prefix is character string, the comparison should be against '\0', according to DPDK coding style. The "!" should only be used on bool values. So "if (prefix[1] == '\0')" or "if (*(prefix + 1) == '\0')".

Quoted 36 lines
lib/eal/common/eal_common_string_fns.c +14 -0
+			break;
+		++prefix;
+	}
+
+	/* try to guess a good number of digits for precision */
+	for (; precision > 0; precision--) {
+		powj *= 10;
+		if (count / powi < powj)
+			break;
+	}
+
+	return snprintf(buf, buf_size, "%.*f %c%s", precision,
+			(double)count / powi, *prefix, use_iec ? "i" : "");
+}
lib/eal/include/rte_common.h +4 -0
index 386f11ae40..781c56adcd 100644
@@ -14,9 +14,11 @@
 
 #include <assert.h>
 #include <limits.h>
+#include <stdbool.h>
 #include <stdint.h>
 #include <stdalign.h>
 
+#include <rte_compat.h>
 #include <rte_config.h>
 
 /* OS specific include */
@@ -919,6 +921,35 @@ __extension__ typedef uint64_t RTE_MARKER64[0];
 uint64_t
 rte_str_to_size(const char *str);
 
+/**
+ * Converts the uint64_t value provided to a human-readable string.

lib/eal/include/rte_common.h __extension__ typedef uint64_t RTE_MARKER64[0]; · line 935

Quoted 10 lines
lib/eal/include/rte_common.h +10 -0
+ * It null-terminates the string, truncating the data if needed.
+ *
+ * Sample outputs with "use_iec" disabled and enabled:
+ * 0 : "0 ", "0 "
+ * 700 : "700 ", "700 "
+ * 1000 : "1.00 k", "1000 "
+ * 1024 : "1.02 k", "1.00 ki"
+ * 21474836480 : "21.5 G", "20.0 Gi"
+ * 109951162777600 : "110 T", "100 Ti"
+ *

I would omit the space before the suffixes in the output. As well as looking better to me, it also solves the issue of the non-suffixed numbers having a trailing space.

Quoted 6 lines
lib/eal/include/rte_common.h +6 -0
+ * @param buf
+ *     Buffer to write the string to.
+ * @param buf_size
+ *     Size of the buffer.
+ * @param count
+ *     Number to convert.

lib/eal/include/rte_common.h __extension__ typedef uint64_t RTE_MARKER64[0]; · line 951

Quoted 10 lines
lib/eal/include/rte_common.h +10 -0
+ * @param use_iec
+ *     If true, use IEC units (1024-based), otherwise use SI units (1000-based).
+ * @return
+ *     Number of characters written (not including the null-terminator),
+ *     or that would have been required when the buffer is too small.
+ */
+__rte_experimental
+int
+rte_size_to_str(char *buf, int buf_size,
+		uint64_t count, bool use_iec);

Again, no need to wrap.

Quoted 21 lines
lib/eal/include/rte_common.h +1 -0
+
 /**
  * Function to terminate the application immediately, printing an error
  * message and returning the exit_code back to the shell.
lib/eal/version.map +3 -0
index a20c713eb1..01b6a7c190 100644
@@ -398,6 +398,9 @@ EXPERIMENTAL {
 	# added in 24.11
 	rte_bitset_to_str;
 	rte_lcore_var_alloc;
+
+	# added in 25.07
+	rte_size_to_str;
 };
 
 INTERNAL {
Signature · 1 line
2.48.1.vfs.0.1
#16 lore raw

RE: [PATCH v3 1/3] eal: add function rte_size_to_str

Quoted 4 lines

lib/eal/common/eal_common_string_fns.c rte_str_to_size(const char *str) · line 113

Quoted 2 lines
lib/eal/common/eal_common_string_fns.c +2 -0
+{
+	const char *prefix = "kMGTPE";

Why is "k" in lower case compared to the others all in upper-case?

That's what the standardization bodies decided. Let's stick with that!

Quoted 9 lines
Quoted 8 lines

+ * Sample outputs with "use_iec" disabled and enabled:
+ * 0 : "0 ", "0 "
+ * 700 : "700 ", "700 "
+ * 1000 : "1.00 k", "1000 "
+ * 1024 : "1.02 k", "1.00 ki"
+ * 21474836480 : "21.5 G", "20.0 Gi"
+ * 109951162777600 : "110 T", "100 Ti"
+ *

Quoted 4 lines

I would omit the space before the suffixes in the output. As well as looking better to me, it also solves the issue of the non-suffixed numbers
having a trailing space.

The space could be optional, like the "use_iec" parameter.
I would certainly expect the space when appending a unit, e.g. "bit/s":
"700 bit/s" or "1.00 kbit/s".

Or when the appended unit is seconds, e.g. for latency/jitter:
"12.3 ms" or "123 us".

With raw numbers (i.e. without a unit appended), it's probably a matter of personal preference, so I'm not going to insist here. I still prefer it, but only when a postfix is present.

In the SmartShare GUI, we output with the space, and accept input both with and without it.

#17 lore raw

Re: [PATCH v3 1/3] eal: add function rte_size_to_str

Quoted 7 lines · Morten Brørup
Quoted 4 lines

lib/eal/common/eal_common_string_fns.c rte_str_to_size(const char *str) · line 113

Quoted 2 lines
lib/eal/common/eal_common_string_fns.c +2 -0
+{
+	const char *prefix = "kMGTPE";

Why is "k" in lower case compared to the others all in upper-case?

That's what the standardization bodies decided. Let's stick with that!

Ok

Quoted 17 lines
Quoted 9 lines
Quoted 5 lines

+ * Sample outputs with "use_iec" disabled and enabled: + * 0 : "0 ", "0 " + * 700 : "700 ", "700 " + * 1000 : "1.00 k", "1000 " + * 1024 :
"1.02 k", "1.00 ki" + * 21474836480 : "21.5 G", "20.0 Gi" + * 109951162777600 : "110 T", "100 Ti" + *

I would omit the space before the suffixes in the output. As well as looking better to me, it also solves the issue of the non-suffixed numbers having a trailing space.

The space could be optional, like the "use_iec" parameter. I would certainly expect the space when appending a unit, e.g. "bit/s": "700 bit/s" or "1.00 kbit/s".

Or when the appended unit is seconds, e.g. for latency/jitter: "12.3 ms" or "123 us".

Quoted 7 lines

With raw numbers (i.e. without a unit appended), it's probably a matter of personal preference, so I'm not going to insist here. I still prefer it, but only when a postfix is present.

In the SmartShare GUI, we output with the space, and accept input both with and without it.

Ok to keep the space within the string or have it optional. Although it seems weird to me initially having a trailing space in some cases but not others, it does make sense in the case of appending something like "bits", or "B" to the string. Therefore, ok to keep things as now. Tend towards not having an optional parameter for it, for the sake of simplicity, but ok either way.

/Bruce

#18 lore raw

Re: [PATCH v3 1/3] eal: add function rte_size_to_str

In reply to Bruce Richardson
Quoted 56 lines · Bruce Richardson
Quoted 8 lines · Andre Muezerie

It's common to use %' in the printf format specifier to make large numbers more easily readable by having the thousands grouped. However, this grouping does not work on Windows. Therefore, a function is needed to make uint64_t numbers more easily readable. There are at least two tests that can benefit from this new function.

---

Thanks Andre, comments inline below.

/Bruce

Quoted 31 lines
  lib/eal/common/eal_common_string_fns.c | 44 ++++++++++++++++++++++++++
  lib/eal/include/rte_common.h           | 31 ++++++++++++++++++
  lib/eal/version.map                    |  3 ++
  3 files changed, 78 insertions(+)
lib/eal/common/eal_common_string_fns.c +7 -0
index 9ca2045b18..4cc7f35652 100644
@@ -4,6 +4,7 @@
 
  #include <ctype.h>
  #include <errno.h>
+#include <inttypes.h>
  #include <stdio.h>
  #include <stdlib.h>
 
@@ -87,6 +88,12 @@ rte_str_to_size(const char *str)
  		endptr++; /* allow 1 space gap */
 
  	switch (*endptr) {
+	case 'E': case 'e':
+		size *= 1024; /* fall-through */
+	case 'P': case 'p':
+		size *= 1024; /* fall-through */
+	case 'T': case 't':
+		size *= 1024; /* fall-through */
  	case 'G': case 'g':
  		size *= 1024; /* fall-through */
  	case 'M': case 'm':

lib/eal/common/eal_common_string_fns.c rte_str_to_size(const char *str) · line 111

Quoted 7 lines
lib/eal/common/eal_common_string_fns.c +4 -0
@@ -98,3 +105,40 @@ rte_str_to_size(const char *str)
  	}
  	return size;
  }
+
+int
+rte_size_to_str(char *buf, int buf_size,
+		uint64_t count, bool use_iec)

Minor nit, I don't think you should need to wrap this, we can have lines up to 100 chars long.

Quoted 4 lines

lib/eal/common/eal_common_string_fns.c rte_str_to_size(const char *str) · line 113

Quoted 2 lines
lib/eal/common/eal_common_string_fns.c +2 -0
+{
+	const char *prefix = "kMGTPE";

Why is "k" in lower case compared to the others all in upper-case?

That's how International Bureau of Weights and Measures likes it, although they keep them a factor 1000 and not 1024 apart.

https://www.bipm.org/documents/20126/41483022/SI-Brochure-9-EN.pdf

Page 143, table 7.

Quoted 120 lines

Also, these are suffixes not prefixes. :-)

Quoted 4 lines
lib/eal/common/eal_common_string_fns.c +4 -0
+	const unsigned int base = use_iec ? 1024 : 1000;
+	uint64_t powi = 1;
+	uint16_t powj = 1;
+	uint8_t precision = 2;

lib/eal/common/eal_common_string_fns.c rte_str_to_size(const char *str) · line 127

Quoted 10 lines
lib/eal/common/eal_common_string_fns.c +10 -0
+
+	if (count < base)
+		return snprintf(buf, buf_size, "%"PRIu64" ", count);
+
+	/* increase value by a factor of 1000/1024 and store
+	 * if result is something a human can read
+	 */
+	for (;;) {
+		powi *= base;
+		if (count / base < powi)

This would seem more logical to me as "count / powi < base" since it would match the initial check for "count < base" (which is essentially the same check since powi == 1).

lib/eal/common/eal_common_string_fns.c rte_str_to_size(const char *str) · line 130

Quoted 3 lines
lib/eal/common/eal_common_string_fns.c +3 -0
+			break;
+
+		if (!prefix[1])

Since prefix is character string, the comparison should be against '\0', according to DPDK coding style. The "!" should only be used on bool values. So "if (prefix[1] == '\0')" or "if (*(prefix + 1) == '\0')".

Quoted 36 lines
lib/eal/common/eal_common_string_fns.c +14 -0
+			break;
+		++prefix;
+	}
+
+	/* try to guess a good number of digits for precision */
+	for (; precision > 0; precision--) {
+		powj *= 10;
+		if (count / powi < powj)
+			break;
+	}
+
+	return snprintf(buf, buf_size, "%.*f %c%s", precision,
+			(double)count / powi, *prefix, use_iec ? "i" : "");
+}
lib/eal/include/rte_common.h +4 -0
index 386f11ae40..781c56adcd 100644
@@ -14,9 +14,11 @@
 
  #include <assert.h>
  #include <limits.h>
+#include <stdbool.h>
  #include <stdint.h>
  #include <stdalign.h>
 
+#include <rte_compat.h>
  #include <rte_config.h>
 
  /* OS specific include */
@@ -919,6 +921,35 @@ __extension__ typedef uint64_t RTE_MARKER64[0];
  uint64_t
  rte_str_to_size(const char *str);
 
+/**
+ * Converts the uint64_t value provided to a human-readable string.

lib/eal/include/rte_common.h __extension__ typedef uint64_t RTE_MARKER64[0]; · line 935

Quoted 10 lines
lib/eal/include/rte_common.h +10 -0
+ * It null-terminates the string, truncating the data if needed.
+ *
+ * Sample outputs with "use_iec" disabled and enabled:
+ * 0 : "0 ", "0 "
+ * 700 : "700 ", "700 "
+ * 1000 : "1.00 k", "1000 "
+ * 1024 : "1.02 k", "1.00 ki"
+ * 21474836480 : "21.5 G", "20.0 Gi"
+ * 109951162777600 : "110 T", "100 Ti"
+ *

I would omit the space before the suffixes in the output. As well as looking better to me, it also solves the issue of the non-suffixed numbers having a trailing space.

Quoted 6 lines
lib/eal/include/rte_common.h +6 -0
+ * @param buf
+ *     Buffer to write the string to.
+ * @param buf_size
+ *     Size of the buffer.
+ * @param count
+ *     Number to convert.

lib/eal/include/rte_common.h __extension__ typedef uint64_t RTE_MARKER64[0]; · line 951

Quoted 10 lines
lib/eal/include/rte_common.h +10 -0
+ * @param use_iec
+ *     If true, use IEC units (1024-based), otherwise use SI units (1000-based).
+ * @return
+ *     Number of characters written (not including the null-terminator),
+ *     or that would have been required when the buffer is too small.
+ */
+__rte_experimental
+int
+rte_size_to_str(char *buf, int buf_size,
+		uint64_t count, bool use_iec);

Again, no need to wrap.

Quoted 21 lines
lib/eal/include/rte_common.h +1 -0
+
  /**
   * Function to terminate the application immediately, printing an error
   * message and returning the exit_code back to the shell.
lib/eal/version.map +3 -0
index a20c713eb1..01b6a7c190 100644
@@ -398,6 +398,9 @@ EXPERIMENTAL {
  	# added in 24.11
  	rte_bitset_to_str;
  	rte_lcore_var_alloc;
+
+	# added in 25.07
+	rte_size_to_str;
  };
 
  INTERNAL {
Signature · 1 line
2.48.1.vfs.0.1
#19 lore raw

Re: [PATCH v3 1/3] eal: add function rte_size_to_str

In reply to Andre Muezerie
Quoted 112 lines · Andre Muezerie

It's common to use %' in the printf format specifier to make large numbers more easily readable by having the thousands grouped. However, this grouping does not work on Windows. Therefore, a function is needed to make uint64_t numbers more easily readable. There are at least two tests that can benefit from this new function.

---
 lib/eal/common/eal_common_string_fns.c | 44 ++++++++++++++++++++++++++
 lib/eal/include/rte_common.h           | 31 ++++++++++++++++++
 lib/eal/version.map                    |  3 ++
 3 files changed, 78 insertions(+)
lib/eal/common/eal_common_string_fns.c +44 -0
index 9ca2045b18..4cc7f35652 100644
@@ -4,6 +4,7 @@
 
 #include <ctype.h>
 #include <errno.h>
+#include <inttypes.h>
 #include <stdio.h>
 #include <stdlib.h>
 
@@ -87,6 +88,12 @@ rte_str_to_size(const char *str)
 		endptr++; /* allow 1 space gap */
 
 	switch (*endptr) {
+	case 'E': case 'e':
+		size *= 1024; /* fall-through */
+	case 'P': case 'p':
+		size *= 1024; /* fall-through */
+	case 'T': case 't':
+		size *= 1024; /* fall-through */
 	case 'G': case 'g':
 		size *= 1024; /* fall-through */
 	case 'M': case 'm':
@@ -98,3 +105,40 @@ rte_str_to_size(const char *str)
 	}
 	return size;
 }
+
+int
+rte_size_to_str(char *buf, int buf_size,
+		uint64_t count, bool use_iec)
+{
+	const char *prefix = "kMGTPE";
+	const unsigned int base = use_iec ? 1024 : 1000;
+	uint64_t powi = 1;
+	uint16_t powj = 1;
+	uint8_t precision = 2;
+
+	if (count < base)
+		return snprintf(buf, buf_size, "%"PRIu64" ", count);
+
+	/* increase value by a factor of 1000/1024 and store
+	 * if result is something a human can read
+	 */
+	for (;;) {
+		powi *= base;
+		if (count / base < powi)
+			break;
+
+		if (!prefix[1])
+			break;
+		++prefix;
+	}
+
+	/* try to guess a good number of digits for precision */
+	for (; precision > 0; precision--) {
+		powj *= 10;
+		if (count / powi < powj)
+			break;
+	}
+
+	return snprintf(buf, buf_size, "%.*f %c%s", precision,
+			(double)count / powi, *prefix, use_iec ? "i" : "");
+}
lib/eal/include/rte_common.h +15 -0
index 386f11ae40..781c56adcd 100644
@@ -14,9 +14,11 @@
 
 #include <assert.h>
 #include <limits.h>
+#include <stdbool.h>
 #include <stdint.h>
 #include <stdalign.h>
 
+#include <rte_compat.h>
 #include <rte_config.h>
 
 /* OS specific include */
@@ -919,6 +921,35 @@ __extension__ typedef uint64_t RTE_MARKER64[0];
 uint64_t
 rte_str_to_size(const char *str);
 
+/**
+ * Converts the uint64_t value provided to a human-readable string.
+ * It null-terminates the string, truncating the data if needed.
+ *
+ * Sample outputs with "use_iec" disabled and enabled:
+ * 0 : "0 ", "0 "
+ * 700 : "700 ", "700 "
+ * 1000 : "1.00 k", "1000 "
+ * 1024 : "1.02 k", "1.00 ki"
+ * 21474836480 : "21.5 G", "20.0 Gi"
+ * 109951162777600 : "110 T", "100 Ti"
+ *
+ * @param buf

lib/eal/include/rte_common.h __extension__ typedef uint64_t RTE_MARKER64[0]; · line 946

Quoted 10 lines
lib/eal/include/rte_common.h +10 -0
+ *     Buffer to write the string to.
+ * @param buf_size
+ *     Size of the buffer.
+ * @param count
+ *     Number to convert.
+ * @param use_iec
+ *     If true, use IEC units (1024-based), otherwise use SI units (1000-based).
+ * @return
+ *     Number of characters written (not including the null-terminator),
+ *     or that would have been required when the buffer is too small.

Just one further minor query on the API return value.

While this is a good return value for handling error cases, for convenience I wonder if it might be better to have the function return "buf" or NULL on error. For basic usage where the buffer is known to be sufficiently big, that would allow use inline in printfs/logs, rather than requiring a separate call e.g.:

printf("Value is %sB/s\n", rte_size_to_str(buf, RTE_DIM(buf), value, true));

WDYT?
/Bruce

#20 lore raw

Re: [PATCH v3 1/3] eal: add function rte_size_to_str

Quoted 132 lines · Bruce Richardson
Quoted 112 lines · Andre Muezerie

It's common to use %' in the printf format specifier to make large numbers more easily readable by having the thousands grouped. However, this grouping does not work on Windows. Therefore, a function is needed to make uint64_t numbers more easily readable. There are at least two tests that can benefit from this new function.

---
 lib/eal/common/eal_common_string_fns.c | 44 ++++++++++++++++++++++++++
 lib/eal/include/rte_common.h           | 31 ++++++++++++++++++
 lib/eal/version.map                    |  3 ++
 3 files changed, 78 insertions(+)
lib/eal/common/eal_common_string_fns.c +44 -0
index 9ca2045b18..4cc7f35652 100644
@@ -4,6 +4,7 @@
 
 #include <ctype.h>
 #include <errno.h>
+#include <inttypes.h>
 #include <stdio.h>
 #include <stdlib.h>
 
@@ -87,6 +88,12 @@ rte_str_to_size(const char *str)
 		endptr++; /* allow 1 space gap */
 
 	switch (*endptr) {
+	case 'E': case 'e':
+		size *= 1024; /* fall-through */
+	case 'P': case 'p':
+		size *= 1024; /* fall-through */
+	case 'T': case 't':
+		size *= 1024; /* fall-through */
 	case 'G': case 'g':
 		size *= 1024; /* fall-through */
 	case 'M': case 'm':
@@ -98,3 +105,40 @@ rte_str_to_size(const char *str)
 	}
 	return size;
 }
+
+int
+rte_size_to_str(char *buf, int buf_size,
+		uint64_t count, bool use_iec)
+{
+	const char *prefix = "kMGTPE";
+	const unsigned int base = use_iec ? 1024 : 1000;
+	uint64_t powi = 1;
+	uint16_t powj = 1;
+	uint8_t precision = 2;
+
+	if (count < base)
+		return snprintf(buf, buf_size, "%"PRIu64" ", count);
+
+	/* increase value by a factor of 1000/1024 and store
+	 * if result is something a human can read
+	 */
+	for (;;) {
+		powi *= base;
+		if (count / base < powi)
+			break;
+
+		if (!prefix[1])
+			break;
+		++prefix;
+	}
+
+	/* try to guess a good number of digits for precision */
+	for (; precision > 0; precision--) {
+		powj *= 10;
+		if (count / powi < powj)
+			break;
+	}
+
+	return snprintf(buf, buf_size, "%.*f %c%s", precision,
+			(double)count / powi, *prefix, use_iec ? "i" : "");
+}
lib/eal/include/rte_common.h +15 -0
index 386f11ae40..781c56adcd 100644
@@ -14,9 +14,11 @@
 
 #include <assert.h>
 #include <limits.h>
+#include <stdbool.h>
 #include <stdint.h>
 #include <stdalign.h>
 
+#include <rte_compat.h>
 #include <rte_config.h>
 
 /* OS specific include */
@@ -919,6 +921,35 @@ __extension__ typedef uint64_t RTE_MARKER64[0];
 uint64_t
 rte_str_to_size(const char *str);
 
+/**
+ * Converts the uint64_t value provided to a human-readable string.
+ * It null-terminates the string, truncating the data if needed.
+ *
+ * Sample outputs with "use_iec" disabled and enabled:
+ * 0 : "0 ", "0 "
+ * 700 : "700 ", "700 "
+ * 1000 : "1.00 k", "1000 "
+ * 1024 : "1.02 k", "1.00 ki"
+ * 21474836480 : "21.5 G", "20.0 Gi"
+ * 109951162777600 : "110 T", "100 Ti"
+ *
+ * @param buf

lib/eal/include/rte_common.h __extension__ typedef uint64_t RTE_MARKER64[0]; · line 946

Quoted 10 lines
lib/eal/include/rte_common.h +10 -0
+ *     Buffer to write the string to.
+ * @param buf_size
+ *     Size of the buffer.
+ * @param count
+ *     Number to convert.
+ * @param use_iec
+ *     If true, use IEC units (1024-based), otherwise use SI units (1000-based).
+ * @return
+ *     Number of characters written (not including the null-terminator),
+ *     or that would have been required when the buffer is too small.

Just one further minor query on the API return value.

While this is a good return value for handling error cases, for convenience I wonder if it might be better to have the function return "buf" or NULL on error. For basic usage where the buffer is known to be sufficiently big, that would allow use inline in printfs/logs, rather than requiring a separate call e.g.:

Quoted 4 lines

printf("Value is %sB/s\n", rte_size_to_str(buf, RTE_DIM(buf), value, true));

WDYT?
/Bruce

That's a good point. Given that this function is used specifically for converting numbers it's a valid assumption that the buffer space needed is always known and available. It's not like snprintf which has a more generic use.

Allowing this function to be used inline would be nice. I'll make this change.

#21 lore raw

PATCH v3 2/3 hash_multiwriter_autotest: fix printf parameters

In reply to Andre Muezerie

Compiling with MSVC logs the warnings below, which result in build error:

../app/test/test_hash_multiwriter.c(71): warning C4476: 'printf' :
unknown type field character ''' in format specifier
../app/test/test_hash_multiwriter.c(73): warning C4474: 'printf' :
too many arguments passed for format string
../app/test/test_hash_multiwriter.c(73): note: placeholders and their
parameters expect 2 variadic arguments, but 4 were provided

---
 app/test/test_hash_multiwriter.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)
app/test/test_hash_multiwriter.c +10 -3
index 33d3147bd8..5c1dd4c195 100644
@@ -32,6 +32,8 @@
 
 #define RTE_APP_TEST_HASH_MULTIWRITER_FAILED 0
 
+#define OFFSET_STR_LEN 16
+
 struct {
 	uint32_t *keys;
 	uint32_t *found;
@@ -51,11 +53,14 @@ static int use_htm;
 static int
 test_hash_multiwriter_worker(void *arg)
 {
+	char offset_start[OFFSET_STR_LEN];
+	char offset_end[OFFSET_STR_LEN];
 	uint64_t i, offset;
 	uint16_t pos_core;
 	uint32_t lcore_id = rte_lcore_id();
 	uint64_t begin, cycles;
 	uint16_t *enabled_core_ids = (uint16_t *)arg;
+	const bool use_iec = true;
 
 	for (pos_core = 0; pos_core < rte_lcore_count(); pos_core++) {
 		if (enabled_core_ids[pos_core] == lcore_id)
@@ -68,10 +73,12 @@ test_hash_multiwriter_worker(void *arg)
 	 */
 	offset = pos_core * tbl_multiwriter_test_params.nb_tsx_insertion;
 
-	printf("Core #%d inserting %d: %'"PRId64" - %'"PRId64"\n",
+	rte_size_to_str(offset_start, sizeof(offset_start), offset, use_iec);
+	rte_size_to_str(offset_end, sizeof(offset_end),
+			offset + tbl_multiwriter_test_params.nb_tsx_insertion - 1, use_iec);
+	printf("Core #%u inserting %u: %s - %s\n",
 	       lcore_id, tbl_multiwriter_test_params.nb_tsx_insertion,
-	       offset,
-	       offset + tbl_multiwriter_test_params.nb_tsx_insertion - 1);
+	       offset_start, offset_end);
 
 	begin = rte_rdtsc_precise();
 
Signature · 1 line
2.48.1.vfs.0.1
#22 lore raw

PATCH v3 3/3 hash_readwrite_autotest: fix printf parameters

In reply to Andre Muezerie

Compiling with MSVC logs the warnings below, which result in build error:

../app/test/test_hash_readwrite.c(73): warning C4476: 'printf' :
unknown type field character ''' in format specifier
../app/test/test_hash_readwrite.c(75): warning C4474: 'printf' :
too many arguments passed for format string
../app/test/test_hash_readwrite.c(75): note: placeholders and
their parameters expect 2 variadic arguments, but 4 were provided

---
 app/test/test_hash_readwrite.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)
app/test/test_hash_readwrite.c +11 -2
index 1867376ade..3fa7003bfb 100644
@@ -25,6 +25,8 @@
 #define NUM_TEST 3
 unsigned int core_cnt[NUM_TEST] = {2, 4, 8};
 
+#define OFFSET_STR_LEN 16
+
 unsigned int worker_core_ids[RTE_MAX_LCORE];
 struct perf {
 	uint32_t single_read;
@@ -57,10 +59,13 @@ static RTE_ATOMIC(uint64_t) gwrites;
 static int
 test_hash_readwrite_worker(__rte_unused void *arg)
 {
+	char offset_start[OFFSET_STR_LEN];
+	char offset_end[OFFSET_STR_LEN];
 	uint64_t i, offset;
 	uint32_t lcore_id = rte_lcore_id();
 	uint64_t begin, cycles;
 	int *ret;
+	const bool use_iec = true;
 
 	ret = rte_malloc(NULL, sizeof(int) *
 				tbl_rw_test_param.num_insert, 0);
@@ -70,9 +75,13 @@ test_hash_readwrite_worker(__rte_unused void *arg)
 	}
 	offset = tbl_rw_test_param.num_insert * i;
 
-	printf("Core #%d inserting and reading %d: %'"PRId64" - %'"PRId64"\n",
+	rte_size_to_str(offset_start, sizeof(offset_start), offset, use_iec);
+	rte_size_to_str(offset_end, sizeof(offset_end),
+			offset + tbl_rw_test_param.num_insert - 1, use_iec);
+
+	printf("Core #%u inserting and reading %u: %s - %s\n",
 	       lcore_id, tbl_rw_test_param.num_insert,
-	       offset, offset + tbl_rw_test_param.num_insert - 1);
+	       offset_start, offset_end);
 
 	begin = rte_rdtsc_precise();
 
Signature · 1 line
2.48.1.vfs.0.1
#23 lore raw

v4 cover fix how large numbers are printed by hash tests

In reply to Andre Muezerie

v4:
- Added parameter "unit", which allows rte_size_to_str() to be smarter about the need to append a space after the number. This keeps the function easy to use and avoids the need for complex explanations about the space that could be needed between number and multiple/unit.
- Changed the return type of rte_size_to_str() to be buf, or NULL on error, allowing the function to be inlined.
- Removed a line wrap.

v3:
- Added rte_size_to_str to version.map (marked experimental for 25.07) so that symbol gets exported when building with -Ddefault_library=shared
- Added sample outputs
- Added a space between the number and the postfix (if a postfix is present)

Andre Muezerie (3):
eal: add function rte_size_to_str
hash_multiwriter_autotest: fix printf parameters
hash_readwrite_autotest: fix printf parameters

 app/test/test_hash_multiwriter.c       | 14 +++++--
 app/test/test_hash_readwrite.c         | 14 ++++++-
 lib/eal/common/eal_common_string_fns.c | 53 ++++++++++++++++++++++++++
 lib/eal/include/rte_common.h           | 34 +++++++++++++++++
 lib/eal/version.map                    |  3 ++
 5 files changed, 113 insertions(+), 5 deletions(-)

--
2.48.1.vfs.0.1

#24 lore raw

PATCH v4 1/3 eal: add function rte_size_to_str

It's common to use %' in the printf format specifier to make large numbers more easily readable by having the thousands grouped. However, this grouping does not work on Windows. Therefore, a function is needed to make uint64_t numbers more easily readable. There are at least two tests that can benefit from this new function.

---
 lib/eal/common/eal_common_string_fns.c | 53 ++++++++++++++++++++++++++
 lib/eal/include/rte_common.h           | 34 +++++++++++++++++
 lib/eal/version.map                    |  3 ++
 3 files changed, 90 insertions(+)
lib/eal/common/eal_common_string_fns.c +53 -0
index 9ca2045b18..bda7d076c5 100644
@@ -4,6 +4,7 @@
 
 #include <ctype.h>
 #include <errno.h>
+#include <inttypes.h>
 #include <stdio.h>
 #include <stdlib.h>
 
@@ -87,6 +88,12 @@ rte_str_to_size(const char *str)
 		endptr++; /* allow 1 space gap */
 
 	switch (*endptr) {
+	case 'E': case 'e':
+		size *= 1024; /* fall-through */
+	case 'P': case 'p':
+		size *= 1024; /* fall-through */
+	case 'T': case 't':
+		size *= 1024; /* fall-through */
 	case 'G': case 'g':
 		size *= 1024; /* fall-through */
 	case 'M': case 'm':
@@ -98,3 +105,49 @@ rte_str_to_size(const char *str)
 	}
 	return size;
 }
+
+char*
+rte_size_to_str(char *buf, int buf_size, uint64_t count, bool use_iec, const char *unit)
+{
+	/* https://en.wikipedia.org/wiki/International_System_of_Units */
+	const char *prefix = "kMGTPE";
+	const unsigned int base = use_iec ? 1024 : 1000;
+	uint64_t powi = 1;
+	uint16_t powj = 1;
+	uint8_t precision = 2;
+	int result;
+
+	if (count < base) {
+		if (unit != NULL && *unit != '\0')
+			result = snprintf(buf, buf_size, "%"PRIu64" %s", count, unit);
+		else
+			result = snprintf(buf, buf_size, "%"PRIu64, count);
+
+		return result < buf_size ? buf : NULL;
+	}
+
+	/* increase value by a factor of 1000/1024 and store
+	 * if result is something a human can read
+	 */
+	for (;;) {
+		powi *= base;
+		if (count / powi < base)
+			break;
+
+		if (prefix[1] == '\0')
+			break;
+		++prefix;
+	}
+
+	/* try to guess a good number of digits for precision */
+	for (; precision > 0; precision--) {
+		powj *= 10;
+		if (count / powi < powj)
+			break;
+	}
+
+	result = snprintf(buf, buf_size, "%.*f %c%s%s", precision,
+			  (double)count / powi, *prefix, use_iec ? "i" : "",
+			  (unit != NULL) ? unit : "");
+	return result < buf_size ? buf : NULL;
+}
lib/eal/include/rte_common.h +34 -0
index 386f11ae40..a6085dce27 100644
@@ -14,9 +14,11 @@
 
 #include <assert.h>
 #include <limits.h>
+#include <stdbool.h>
 #include <stdint.h>
 #include <stdalign.h>
 
+#include <rte_compat.h>
 #include <rte_config.h>
 
 /* OS specific include */
@@ -919,6 +921,38 @@ __extension__ typedef uint64_t RTE_MARKER64[0];
 uint64_t
 rte_str_to_size(const char *str);
 
+/**
+ * Converts the uint64_t value provided to a human-readable string.
+ * It null-terminates the string, truncating the data if needed.
+ * An optional unit (like "B") can be provided as a string. It will be
+ * appended to the number, and a space will be inserted before the unit if needed.
+ *
+ * Sample outputs: (1) "use_iec" disabled, (2) "use_iec" enabled,
+ *                 (3) "use_iec" enabled and "B" as unit.
+ * 0 : "0", "0", "0 B"
+ * 700 : "700", "700", "700 B"
+ * 1000 : "1.00 k", "1000", "1000 B"
+ * 1024 : "1.02 k", "1.00 ki", "1.00 kiB"
+ * 21474836480 : "21.5 G", "20.0 Gi", "20.0 GiB"
+ * 109951162777600 : "110 T", "100 Ti", "100 TiB"
+ *
+ * @param buf
+ *     Buffer to write the string to.
+ * @param buf_size
+ *     Size of the buffer.
+ * @param count
+ *     Number to convert.
+ * @param use_iec
+ *     If true, use IEC units (1024-based), otherwise use SI units (1000-based).
+ * @param unit
+ *     Unit to append to the string (Like "B" for bytes). Can be NULL.
+ * @return
+ *     buf on success, NULL if the buffer is too small.
+ */
+__rte_experimental
+char *
+rte_size_to_str(char *buf, int buf_size, uint64_t count, bool use_iec, const char *unit);
+
 /**
  * Function to terminate the application immediately, printing an error
  * message and returning the exit_code back to the shell.
lib/eal/version.map +3 -0
index a20c713eb1..01b6a7c190 100644
@@ -398,6 +398,9 @@ EXPERIMENTAL {
 	# added in 24.11
 	rte_bitset_to_str;
 	rte_lcore_var_alloc;
+
+	# added in 25.07
+	rte_size_to_str;
 };
 
 INTERNAL {
Signature · 1 line
2.48.1.vfs.0.1
#26 lore raw

PATCH v4 2/3 hash_multiwriter_autotest: fix printf parameters

In reply to Andre Muezerie

Compiling with MSVC logs the warnings below, which result in build error:

../app/test/test_hash_multiwriter.c(71): warning C4476: 'printf' :
unknown type field character ''' in format specifier
../app/test/test_hash_multiwriter.c(73): warning C4474: 'printf' :
too many arguments passed for format string
../app/test/test_hash_multiwriter.c(73): note: placeholders and their
parameters expect 2 variadic arguments, but 4 were provided

---
 app/test/test_hash_multiwriter.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)
app/test/test_hash_multiwriter.c +11 -3
index 33d3147bd8..0af3eb2c33 100644
@@ -32,6 +32,8 @@
 
 #define RTE_APP_TEST_HASH_MULTIWRITER_FAILED 0
 
+#define OFFSET_STR_LEN 16
+
 struct {
 	uint32_t *keys;
 	uint32_t *found;
@@ -51,11 +53,15 @@ static int use_htm;
 static int
 test_hash_multiwriter_worker(void *arg)
 {
+	char offset_start[OFFSET_STR_LEN];
+	char offset_end[OFFSET_STR_LEN];
 	uint64_t i, offset;
 	uint16_t pos_core;
 	uint32_t lcore_id = rte_lcore_id();
 	uint64_t begin, cycles;
 	uint16_t *enabled_core_ids = (uint16_t *)arg;
+	const bool use_iec = true;
+	const char *unit = NULL;
 
 	for (pos_core = 0; pos_core < rte_lcore_count(); pos_core++) {
 		if (enabled_core_ids[pos_core] == lcore_id)
@@ -68,10 +74,12 @@ test_hash_multiwriter_worker(void *arg)
 	 */
 	offset = pos_core * tbl_multiwriter_test_params.nb_tsx_insertion;
 
-	printf("Core #%d inserting %d: %'"PRId64" - %'"PRId64"\n",
+	rte_size_to_str(offset_start, RTE_DIM(offset_start), offset, use_iec, unit);
+	rte_size_to_str(offset_end, RTE_DIM(offset_end),
+			offset + tbl_multiwriter_test_params.nb_tsx_insertion - 1, use_iec, unit);
+	printf("Core #%u inserting %u: %s - %s\n",
 	       lcore_id, tbl_multiwriter_test_params.nb_tsx_insertion,
-	       offset,
-	       offset + tbl_multiwriter_test_params.nb_tsx_insertion - 1);
+	       offset_start, offset_end);
 
 	begin = rte_rdtsc_precise();
 
Signature · 1 line
2.48.1.vfs.0.1
#27 lore raw

PATCH v4 3/3 hash_readwrite_autotest: fix printf parameters

In reply to Andre Muezerie

Compiling with MSVC logs the warnings below, which result in build error:

../app/test/test_hash_readwrite.c(73): warning C4476: 'printf' :
unknown type field character ''' in format specifier
../app/test/test_hash_readwrite.c(75): warning C4474: 'printf' :
too many arguments passed for format string
../app/test/test_hash_readwrite.c(75): note: placeholders and
their parameters expect 2 variadic arguments, but 4 were provided

---
 app/test/test_hash_readwrite.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)
app/test/test_hash_readwrite.c +12 -2
index 1867376ade..82fe03cc5a 100644
@@ -25,6 +25,8 @@
 #define NUM_TEST 3
 unsigned int core_cnt[NUM_TEST] = {2, 4, 8};
 
+#define OFFSET_STR_LEN 16
+
 unsigned int worker_core_ids[RTE_MAX_LCORE];
 struct perf {
 	uint32_t single_read;
@@ -57,10 +59,14 @@ static RTE_ATOMIC(uint64_t) gwrites;
 static int
 test_hash_readwrite_worker(__rte_unused void *arg)
 {
+	char offset_start[OFFSET_STR_LEN];
+	char offset_end[OFFSET_STR_LEN];
 	uint64_t i, offset;
 	uint32_t lcore_id = rte_lcore_id();
 	uint64_t begin, cycles;
 	int *ret;
+	const bool use_iec = true;
+	const char *unit = NULL;
 
 	ret = rte_malloc(NULL, sizeof(int) *
 				tbl_rw_test_param.num_insert, 0);
@@ -70,9 +76,13 @@ test_hash_readwrite_worker(__rte_unused void *arg)
 	}
 	offset = tbl_rw_test_param.num_insert * i;
 
-	printf("Core #%d inserting and reading %d: %'"PRId64" - %'"PRId64"\n",
+	rte_size_to_str(offset_start, RTE_DIM(offset_start), offset, use_iec, unit);
+	rte_size_to_str(offset_end, RTE_DIM(offset_end),
+			offset + tbl_rw_test_param.num_insert - 1, use_iec, unit);
+
+	printf("Core #%u inserting and reading %u: %s - %s\n",
 	       lcore_id, tbl_rw_test_param.num_insert,
-	       offset, offset + tbl_rw_test_param.num_insert - 1);
+	       offset_start, offset_end);
 
 	begin = rte_rdtsc_precise();
 
Signature · 1 line
2.48.1.vfs.0.1
#28 lore raw

Re: [PATCH v4 0/3] fix how large numbers are printed by hash tests

In reply to Andre Muezerie
Quoted 16 lines · Andre Muezerie

v4:
- Added parameter "unit", which allows rte_size_to_str() to be smarter about the need to append a space after the number. This keeps the function easy to use and avoids the need for complex explanations about the space that could be needed between number and multiple/unit.
- Changed the return type of rte_size_to_str() to be buf, or NULL on error, allowing the function to be inlined.
- Removed a line wrap.

v3:
- Added rte_size_to_str to version.map (marked experimental for 25.07) so that symbol gets exported when building with -Ddefault_library=shared
- Added sample outputs
- Added a space between the number and the postfix (if a postfix is present)

Quoted 5 lines

Andre Muezerie (3):
eal: add function rte_size_to_str
hash_multiwriter_autotest: fix printf parameters
hash_readwrite_autotest: fix printf parameters

#29 lore raw

v5 cover fix how large numbers are printed by hash tests

In reply to Andre Muezerie

v5:
- Rebased and updated to conform to new function versioning mechanism.

v4:
- Added parameter "unit", which allows rte_size_to_str() to be smarter about the need to append a space after the number. This keeps the function easy to use and avoids the need for complex explanations about the space that could be needed between number and multiple/unit.
- Changed the return type of rte_size_to_str() to be buf, or NULL on error, allowing the function to be inlined.
- Removed a line wrap.

v3:
- Added rte_size_to_str to version.map (marked experimental for 25.07) so that symbol gets exported when building with -Ddefault_library=shared
- Added sample outputs
- Added a space between the number and the postfix (if a postfix is present)

Andre Muezerie (3):
eal: add function rte_size_to_str
hash_multiwriter_autotest: fix printf parameters
hash_readwrite_autotest: fix printf parameters

 app/test/test_hash_multiwriter.c       | 14 +++++--
 app/test/test_hash_readwrite.c         | 14 ++++++-
 lib/eal/common/eal_common_string_fns.c | 54 ++++++++++++++++++++++++++
 lib/eal/include/rte_common.h           | 34 ++++++++++++++++
 4 files changed, 111 insertions(+), 5 deletions(-)

--
2.49.0.vfs.0.0

#30 lore raw

PATCH v5 1/3 eal: add function rte_size_to_str

It's common to use %' in the printf format specifier to make large numbers more easily readable by having the thousands grouped. However, this grouping does not work on Windows. Therefore, a function is needed to make uint64_t numbers more easily readable. There are at least two tests that can benefit from this new function.

---
 lib/eal/common/eal_common_string_fns.c | 54 ++++++++++++++++++++++++++
 lib/eal/include/rte_common.h           | 34 ++++++++++++++++
 2 files changed, 88 insertions(+)
lib/eal/common/eal_common_string_fns.c +54 -0
index 020603fe8b..ec17382487 100644
@@ -4,6 +4,7 @@
 
 #include <ctype.h>
 #include <errno.h>
+#include <inttypes.h>
 #include <stdio.h>
 #include <stdlib.h>
 
@@ -91,6 +92,12 @@ rte_str_to_size(const char *str)
 		endptr++; /* allow 1 space gap */
 
 	switch (*endptr) {
+	case 'E': case 'e':
+		size *= 1024; /* fall-through */
+	case 'P': case 'p':
+		size *= 1024; /* fall-through */
+	case 'T': case 't':
+		size *= 1024; /* fall-through */
 	case 'G': case 'g':
 		size *= 1024; /* fall-through */
 	case 'M': case 'm':
@@ -102,3 +109,50 @@ rte_str_to_size(const char *str)
 	}
 	return size;
 }
+
+RTE_EXPORT_EXPERIMENTAL_SYMBOL(rte_size_to_str, 25.07)
+char*
+rte_size_to_str(char *buf, int buf_size, uint64_t count, bool use_iec, const char *unit)
+{
+	/* https://en.wikipedia.org/wiki/International_System_of_Units */
+	const char *prefix = "kMGTPE";
+	const unsigned int base = use_iec ? 1024 : 1000;
+	uint64_t powi = 1;
+	uint16_t powj = 1;
+	uint8_t precision = 2;
+	int result;
+
+	if (count < base) {
+		if (unit != NULL && *unit != '\0')
+			result = snprintf(buf, buf_size, "%"PRIu64" %s", count, unit);
+		else
+			result = snprintf(buf, buf_size, "%"PRIu64, count);
+
+		return result < buf_size ? buf : NULL;
+	}
+
+	/* increase value by a factor of 1000/1024 and store
+	 * if result is something a human can read
+	 */
+	for (;;) {
+		powi *= base;
+		if (count / powi < base)
+			break;
+
+		if (prefix[1] == '\0')
+			break;
+		++prefix;
+	}
+
+	/* try to guess a good number of digits for precision */
+	for (; precision > 0; precision--) {
+		powj *= 10;
+		if (count / powi < powj)
+			break;
+	}
+
+	result = snprintf(buf, buf_size, "%.*f %c%s%s", precision,
+			  (double)count / powi, *prefix, use_iec ? "i" : "",
+			  (unit != NULL) ? unit : "");
+	return result < buf_size ? buf : NULL;
+}
lib/eal/include/rte_common.h +34 -0
index 386f11ae40..a6085dce27 100644
@@ -14,9 +14,11 @@
 
 #include <assert.h>
 #include <limits.h>
+#include <stdbool.h>
 #include <stdint.h>
 #include <stdalign.h>
 
+#include <rte_compat.h>
 #include <rte_config.h>
 
 /* OS specific include */
@@ -919,6 +921,38 @@ __extension__ typedef uint64_t RTE_MARKER64[0];
 uint64_t
 rte_str_to_size(const char *str);
 
+/**
+ * Converts the uint64_t value provided to a human-readable string.
+ * It null-terminates the string, truncating the data if needed.
+ * An optional unit (like "B") can be provided as a string. It will be
+ * appended to the number, and a space will be inserted before the unit if needed.
+ *
+ * Sample outputs: (1) "use_iec" disabled, (2) "use_iec" enabled,
+ *                 (3) "use_iec" enabled and "B" as unit.
+ * 0 : "0", "0", "0 B"
+ * 700 : "700", "700", "700 B"
+ * 1000 : "1.00 k", "1000", "1000 B"
+ * 1024 : "1.02 k", "1.00 ki", "1.00 kiB"
+ * 21474836480 : "21.5 G", "20.0 Gi", "20.0 GiB"
+ * 109951162777600 : "110 T", "100 Ti", "100 TiB"
+ *
+ * @param buf
+ *     Buffer to write the string to.
+ * @param buf_size
+ *     Size of the buffer.
+ * @param count
+ *     Number to convert.
+ * @param use_iec
+ *     If true, use IEC units (1024-based), otherwise use SI units (1000-based).
+ * @param unit
+ *     Unit to append to the string (Like "B" for bytes). Can be NULL.
+ * @return
+ *     buf on success, NULL if the buffer is too small.
+ */
+__rte_experimental
+char *
+rte_size_to_str(char *buf, int buf_size, uint64_t count, bool use_iec, const char *unit);
+
 /**
  * Function to terminate the application immediately, printing an error
  * message and returning the exit_code back to the shell.
Signature · 1 line
2.49.0.vfs.0.0
#31 lore raw

PATCH v5 2/3 hash_multiwriter_autotest: fix printf parameters

In reply to Andre Muezerie

Compiling with MSVC logs the warnings below, which result in build error:

../app/test/test_hash_multiwriter.c(71): warning C4476: 'printf' :
unknown type field character ''' in format specifier
../app/test/test_hash_multiwriter.c(73): warning C4474: 'printf' :
too many arguments passed for format string
../app/test/test_hash_multiwriter.c(73): note: placeholders and their
parameters expect 2 variadic arguments, but 4 were provided

---
 app/test/test_hash_multiwriter.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)
app/test/test_hash_multiwriter.c +11 -3
index 33d3147bd8..0af3eb2c33 100644
@@ -32,6 +32,8 @@
 
 #define RTE_APP_TEST_HASH_MULTIWRITER_FAILED 0
 
+#define OFFSET_STR_LEN 16
+
 struct {
 	uint32_t *keys;
 	uint32_t *found;
@@ -51,11 +53,15 @@ static int use_htm;
 static int
 test_hash_multiwriter_worker(void *arg)
 {
+	char offset_start[OFFSET_STR_LEN];
+	char offset_end[OFFSET_STR_LEN];
 	uint64_t i, offset;
 	uint16_t pos_core;
 	uint32_t lcore_id = rte_lcore_id();
 	uint64_t begin, cycles;
 	uint16_t *enabled_core_ids = (uint16_t *)arg;
+	const bool use_iec = true;
+	const char *unit = NULL;
 
 	for (pos_core = 0; pos_core < rte_lcore_count(); pos_core++) {
 		if (enabled_core_ids[pos_core] == lcore_id)
@@ -68,10 +74,12 @@ test_hash_multiwriter_worker(void *arg)
 	 */
 	offset = pos_core * tbl_multiwriter_test_params.nb_tsx_insertion;
 
-	printf("Core #%d inserting %d: %'"PRId64" - %'"PRId64"\n",
+	rte_size_to_str(offset_start, RTE_DIM(offset_start), offset, use_iec, unit);
+	rte_size_to_str(offset_end, RTE_DIM(offset_end),
+			offset + tbl_multiwriter_test_params.nb_tsx_insertion - 1, use_iec, unit);
+	printf("Core #%u inserting %u: %s - %s\n",
 	       lcore_id, tbl_multiwriter_test_params.nb_tsx_insertion,
-	       offset,
-	       offset + tbl_multiwriter_test_params.nb_tsx_insertion - 1);
+	       offset_start, offset_end);
 
 	begin = rte_rdtsc_precise();
 
Signature · 1 line
2.49.0.vfs.0.0
#32 lore raw

PATCH v5 3/3 hash_readwrite_autotest: fix printf parameters

In reply to Andre Muezerie

Compiling with MSVC logs the warnings below, which result in build error:

../app/test/test_hash_readwrite.c(73): warning C4476: 'printf' :
unknown type field character ''' in format specifier
../app/test/test_hash_readwrite.c(75): warning C4474: 'printf' :
too many arguments passed for format string
../app/test/test_hash_readwrite.c(75): note: placeholders and
their parameters expect 2 variadic arguments, but 4 were provided

---
 app/test/test_hash_readwrite.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)
app/test/test_hash_readwrite.c +12 -2
index 1867376ade..82fe03cc5a 100644
@@ -25,6 +25,8 @@
 #define NUM_TEST 3
 unsigned int core_cnt[NUM_TEST] = {2, 4, 8};
 
+#define OFFSET_STR_LEN 16
+
 unsigned int worker_core_ids[RTE_MAX_LCORE];
 struct perf {
 	uint32_t single_read;
@@ -57,10 +59,14 @@ static RTE_ATOMIC(uint64_t) gwrites;
 static int
 test_hash_readwrite_worker(__rte_unused void *arg)
 {
+	char offset_start[OFFSET_STR_LEN];
+	char offset_end[OFFSET_STR_LEN];
 	uint64_t i, offset;
 	uint32_t lcore_id = rte_lcore_id();
 	uint64_t begin, cycles;
 	int *ret;
+	const bool use_iec = true;
+	const char *unit = NULL;
 
 	ret = rte_malloc(NULL, sizeof(int) *
 				tbl_rw_test_param.num_insert, 0);
@@ -70,9 +76,13 @@ test_hash_readwrite_worker(__rte_unused void *arg)
 	}
 	offset = tbl_rw_test_param.num_insert * i;
 
-	printf("Core #%d inserting and reading %d: %'"PRId64" - %'"PRId64"\n",
+	rte_size_to_str(offset_start, RTE_DIM(offset_start), offset, use_iec, unit);
+	rte_size_to_str(offset_end, RTE_DIM(offset_end),
+			offset + tbl_rw_test_param.num_insert - 1, use_iec, unit);
+
+	printf("Core #%u inserting and reading %u: %s - %s\n",
 	       lcore_id, tbl_rw_test_param.num_insert,
-	       offset, offset + tbl_rw_test_param.num_insert - 1);
+	       offset_start, offset_end);
 
 	begin = rte_rdtsc_precise();
 
Signature · 1 line
2.49.0.vfs.0.0
#33 lore raw

Re: [PATCH v5 0/3] fix how large numbers are printed by hash tests

In reply to Andre Muezerie
Quoted 25 lines · Andre Muezerie

v5:
- Rebased and updated to conform to new function versioning mechanism.

v4:
- Added parameter "unit", which allows rte_size_to_str() to be smarter about the need to append a space after the number. This keeps the function easy to use and avoids the need for complex explanations about the space that could be needed between number and multiple/unit.
- Changed the return type of rte_size_to_str() to be buf, or NULL on error, allowing the function to be inlined.
- Removed a line wrap.

v3:
- Added rte_size_to_str to version.map (marked experimental for 25.07) so that symbol gets exported when building with -Ddefault_library=shared
- Added sample outputs
- Added a space between the number and the postfix (if a postfix is present)

Andre Muezerie (3):
eal: add function rte_size_to_str
hash_multiwriter_autotest: fix printf parameters
hash_readwrite_autotest: fix printf parameters

Quoted 5 lines
 app/test/test_hash_multiwriter.c       | 14 +++++--
 app/test/test_hash_readwrite.c         | 14 ++++++-
 lib/eal/common/eal_common_string_fns.c | 54 ++++++++++++++++++++++++++
 lib/eal/include/rte_common.h           | 34 ++++++++++++++++
 4 files changed, 111 insertions(+), 5 deletions(-)

It looks odd that string formatting helpers are declared in rte_common.h. I would move those declarations to rte_string_fns.h but this can wait a followup patch since other helper was already in rte_common.h.

Series applied, thanks.

Signature · 1 line
David Marchand