On Mon, Dec 12, 2022 at 02:42:55PM +0800, Huisong Li wrote:
> Some lib telemetry interfaces add the 'u32' and 'u64' data by the
> rte_tel_data_add_dict/array_int API. This may cause data conversion error
> or data truncation.
> 
> The 'u32' data can not be assigned to signed 32-bit integer. However,
> assigning to u64 is very wasteful, after all, the buffer capacity of each
> transfer is limited. So it is necessary for 'u32' data to add usigned
> 32-bit integer type and a series of 'u32' operation APIs.
> 
> This patchset uses the new 'u32' API to resolve the problem of data
> conversion error, and use the 'u64' API to add 'u64' data.
> 
> In addition, this patchset introduces two APIs to store u32 and u64
> values as hexadecimal encoded strings in telemetry library. 
> 
> --- -v3: fix a misspelling mistake in commit log.  -v2: - fix ABI break
> warning.  - introduce two APIs to store u32 and u64 values as hexadecimal
> encoded strings. 
>
I'm not convinced about adding the u32 value generically to the telemetry
lib - except in the case of having explicit function calls for u32 vs u64
hex strings. Having a u32 type doesn't gain us any space internally over a
u64 value, since all values are in a union type. Also, for output as json,
the numeric values are all output as decimal values, meaning that the value
1 appears as the same size in the output string whether it is a u32 or u64
type. Now, it may save space in a future binary output format, but even
then it still may not do so.

Therefore, I'd tend to keep the existing u64 type as-is, and instead only
add the functions for outputting hex values. Those hex output functions
could take an additional parameter indicating the desired hex output
length, as there could well be cases where we want just 16-bit hex value
too.

/Bruce

Reply via email to