On 2025-01-21 15:55:28+0100, Roberto Sassu wrote:

On Tue, 2025-01-21 at 14:29 +0100, Thomas Weißschuh wrote:

quoted

On 2024-11-19 11:49:08+0100, Roberto Sassu wrote:

[..]

quoted

quoted

+typedef int (*callback)(void *callback_data, __u16 field,
+			const __u8 *field_data, __u32 field_len);

No need for __underscore types in kernel-only signatures.

It is just for convenience. I'm reusing the same file for the userspace
counterpart digest-cache-tools. In that case, the parser is used for
example to show the content of the digest list.

This reuse leads to quite some ugly constructs.
Given that the single function will be really simple after removing the
unnecessary parts, maybe two clean copies are easier.
One copy is needed for Frama-C anyways.

quoted

quoted

+
+int tlv_parse(callback callback, void *callback_data, const __u8 *data,
+	      size_t data_len, const char **fields, __u32 num_fields);
+
+#endif /* _LINUX_TLV_PARSER_H */

diff --git a/include/uapi/linux/tlv_parser.h b/include/uapi/linux/tlv_parser.h
new file mode 100644
index 000000000000..171d0cfd2c4c
--- /dev/null
+++ b/include/uapi/linux/tlv_parser.h

@@ -0,0 +1,41 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*
+ * Copyright (C) 2023-2024 Huawei Technologies Duesseldorf GmbH
+ *
+ * Author: Roberto Sassu <roberto.sassu@huawei.com>
+ *
+ * Implement the user space interface for the TLV parser.
+ */

Can you explain in the commit message where this will be exposed to
userspace as binary?

I see that my explanation is not ideal.

This is the format for data exchange between user space and kernel
space, but it is still the kernel that reads and parses the TLV-
formatted file for extracting the digests and adding them to the digest
cache.

I figured that out :-)
It should be clear from the commit itself, though.

quoted

quoted

+
+#ifndef _UAPI_LINUX_TLV_PARSER_H
+#define _UAPI_LINUX_TLV_PARSER_H
+
+#include <linux/types.h>
+
+/*
+ * TLV format:
+ *
+ * +--------------+--+---------+--------+---------+
+ * | field1 (u16) | len1 (u32) | value1 (u8 len1) |
+ * +--------------+------------+------------------+
+ * |     ...      |    ...     |        ...       |
+ * +--------------+------------+------------------+
+ * | fieldN (u16) | lenN (u32) | valueN (u8 lenN) |
+ * +--------------+------------+------------------+
+ */
+
+/**
+ * struct tlv_entry - Entry of TLV format
+ * @field: Field identifier
+ * @length: Data length
+ * @data: Data
+ *
+ * This structure represents an entry of the TLV format.
+ */
+struct tlv_entry {
+	__u16 field;
+	__u32 length;

Looking at this again, the "length" field is unaligned by default.

Also FYI there is already a TLV implementation in
include/uapi/linux/tipc_config.

quoted

quoted

+} __attribute__((packed));

[..]

quoted

Some kunit tests would be great.

I implemented kselftests also injecting errors (patch 13). If it is not
enough, I implement kunit tests too.

These selftests are for the digest_cache.
If the TLV library is meant to be used alone, some dedicated tests would
be nice. kunit has the advantage that it can directly call kernel
functions with arbitrary parameters and does not require any userspace
setup.