Re: [PATCH v6 01/15] lib: Add TLV parser

["Thomas Weißschuh" <linux@weissschuh.net>]

On 2025-01-21 14:48:09+0100, Roberto Sassu wrote:
> On Tue, 2025-01-21 at 14:29 +0100, Thomas Weißschuh wrote:
> > Hi Robert,
> > 
> > On 2024-11-19 11:49:08+0100, Roberto Sassu wrote:
> > > From: Roberto Sassu <roberto.sassu@huawei.com>
> > > 
> > > Add a parser of a generic Type-Length-Value (TLV) format:
> > > 
> > > +--------------+--+---------+--------+---------+
> > > > field1 (u16) | len1 (u32) | value1 (u8 len1) |
> > > +--------------+------------+------------------+
> > > >     ...      |    ...     |        ...       |
> > > +--------------+------------+------------------+
> > > > fieldN (u16) | lenN (u32) | valueN (u8 lenN) |
> > > +--------------+------------+------------------+
> > 
> > Should mention that its big endian.
> 
> Ok.
> 
> > > Each adopter can define its own fields. The TLV parser does not need to be
> > > aware of those, but lets the adopter obtain the data and decide how to
> > 
> > "adopter" -> "user".
> 
> Ok.
> 
> > > continue.
> > > 
> > > After processing a TLV entry, call the callback function also with the
> > > callback data provided by the adopter. The latter can decide how to
> > > interpret the TLV entry depending on the field ID.
> > > 
> > > Nesting TLVs is also possible, the callback function can call tlv_parse()
> > > to parse the inner structure.
> > 
> > Given that we already have the netlink data structures, helpers and
> > infrastructure, what is the advantage over those?
> 
> Sorry, I'm not too familiar on how netlink works, so I might not
> understand your point.

Netlink is a TLV format used by the Linux networking subsystem:

struct nlmsghdr {
    __u32 nlmsg_len;    /* Length of message including header */
    __u16 nlmsg_type;   /* Type of message content */
    __u16 nlmsg_flags;  /* Additional flags */
    __u32 nlmsg_seq;    /* Sequence number */
    __u32 nlmsg_pid;    /* Sender port ID */
};

https://man.archlinux.org/man/core/man-pages/netlink.7.en

There are both userspace and in-kernel infrastructures to handle it.
Looking at it again however it has some unnecessary fields, wasting
space and uses "host" byteorder which is a problem for an on-disk
datastructure.
So maybe not a good alternative after all.

> I think the benefit of this data structure is the retrocompatibility.
> If you add new data fields, you don't need to introduce a v2, v3 data
> format.
> 
> New versions of the parser can consume the new information, while the
> older can still take the ones they are able to understand.

This is also exactly how netlink is used.

FYI there were also some review comments inside the patch diff itself.


Thomas