Subject: Re: [PATCH net-next v4 02/12] doc/netlink: Add a schema for netlink-raw families

From: Donald Hunter <[email-masked]>
Date: Wed, 23 Aug 2023 22:19:53 +0100

Jacob Keller <[email-masked]> writes:

> On 8/23/2023 4:41 AM, Donald Hunter wrote:
>> This schema is largely a copy of the genetlink-legacy schema with the
>> following additions:
>> 
>>  - a top-level protonum property, e.g. 0 (for NETLINK_ROUTE)
>>  - add netlink-raw to the list of protocols supported by the schema
>>  - add a value property to mcast-group definitions
>> 
>> This schema is very similar to genetlink-legacy and I considered
>> making the changes there and symlinking to it. On balance I felt that
>> might be problematic for accurate schema validation.
>> 
>
> Ya, I think they have to be distinct to properly validate.
>
>> Signed-off-by: Donald Hunter <[email-masked]>
>> ---
>>  Documentation/netlink/netlink-raw.yaml | 414 +++++++++++++++++++++++++
>>  1 file changed, 414 insertions(+)
>>  create mode 100644 Documentation/netlink/netlink-raw.yaml
>> 
>> diff --git a/Documentation/netlink/netlink-raw.yaml b/Documentation/netlink/netlink-raw.yaml
>> new file mode 100644
>> index 000000000000..ef7bd07eab62
>> --- /dev/null
>> +++ b/Documentation/netlink/netlink-raw.yaml
>> @@ -0,0 +1,414 @@
>> +# SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://kernel.org/schemas/netlink/genetlink-legacy.yaml#
>> +$schema: https://json-schema.org/draft-07/schema
>> +
>> +# Common defines
>> +$defs:
>> +  uint:
>> +    type: integer
>> +    minimum: 0
>> +  len-or-define:
>> +    type: [ string, integer ]
>> +    pattern: ^[0-9A-Za-z_]+( - 1)?$
>> +    minimum: 0
>> +
>> +# Schema for specs
>> +title: Protocol
>> +description: Specification of a genetlink protocol
>
> If this is for netlink-raw, shouldn't this not say genetlink? Same
> elsewhere? or am I misunderstanding something?

It's a good question. The schema definitions are currently strict
supersets of genetlink:

genetlink <= genetlink-c <= genetlink-legacy <= netlink-raw

As you noted below, there's only 2 additions needed for the netlink raw
families, protonum and mcast-group value.

I would be happy to change the description and other references to
genetlink in this spec, but I'd like to hear Jakub's thoughts about
minimal modification vs a more thorough rewording. Perhaps a middle
ground would be to extend the top-level description to say "genetlink or
raw netlink" and qualify that all mention of genetlink also applies to
raw netlink.

Either way, I just noticed that the schema $id does need to be updated.

>> +type: object
>> +required: [ name, doc, attribute-sets, operations ]
>> +additionalProperties: False
>> +properties:
>> +  name:
>> +    description: Name of the genetlink family.
>> +    type: string
>> +  doc:
>> +    type: string
>> +  version:
>> +    description: Generic Netlink family version. Default is 1.
>> +    type: integer
>> +    minimum: 1
>> +  protocol:
>> +    description: Schema compatibility level. Default is "genetlink".
>> +    enum: [ genetlink, genetlink-c, genetlink-legacy, netlink-raw ] # Trim
>> +  # Start netlink-raw
>
> I guess the netlink raw part is only below this? Or does netlink raw
> share more of the generic netlink code than I thought?

Raw netlink is, so far, the same as genetlink-legacy with the addition
of hard-coded protocol ids.

>> +  protonum:
>> +    description: Protocol number to use for netlink-raw
>> +    type: integer
>> +  # End netlink-raw

[...]

>> +            # Start netlink-raw
>> +            value:
>> +              description: Value of the netlink multicast group in the uAPI.
>> +              type: integer
>> +            # End netlink-raw