Re: [RFC PATCH 02/12] drm/fabric: add query uAPI and generated headers

[email protected]
Newsgroups org.freedesktop.lists.dri-devel
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The Netlink YAML specification uses fixed-width integer types (`u32`, `u64`) and `pad` attributes instead of the preferred variable-width types (`uint`, `sint`).
- [Medium] The `port-peer-create-ntf` and `port-peer-delete-ntf` notifications use custom `event` subsets instead of reusing the `GET` operation shape via `notify`.
--

--- Patch [2]: [RFC PATCH 02/12] drm/fabric: add query uAPI and generated headers ---
commit 180af4ca3a8c57e81df2225599a4c339598edf21
Author: Ilia Levi <[email protected]>

drm/fabric: add query uAPI and generated headers
    
This commit introduces the drm-fabric Generic Netlink family using a YNL
specification, along with generated userspace and kernel headers. It defines
the core object model attributes, operations, and multicast notifications.
The read-only ABI supports lookup, dump, and change notifications for
fabrics, endpoints, and ports.

> diff --git a/Documentation/netlink/specs/drm_fabric.yaml b/Documentation/netlink/specs/drm_fabric.yaml
> new file mode 100644
> index 0000000000000..778443d92a0d4
> --- /dev/null
> +++ b/Documentation/netlink/specs/drm_fabric.yaml

[ ... ]

> +  -
> +    name: fabric
> +    name-prefix: drm-fabric-a-fabric-attrs-
> +    enum-name: drm-fabric-a-fabric-attrs
> +    doc: Attributes describing a fabric object.
> +    attributes:
> +      -
> +        name: pad
> +        type: pad
> +      -
> +        name: fabric-id
> +        type: u32

[Severity: Medium]
Since this is a new Netlink family, should we prefer the variable-width uint
or sint types instead of fixed-width u32 and u64 integers?

The pad attribute is also generally restricted to legacy fixed structs.
Changing this before the uAPI is finalized might be a good idea to maintain
consistency with modern Netlink design guidelines.

[ ... ]

> +    -
> +      name: port-peer-create-ntf
> +      doc: A port's neighbor has been set.
> +      attribute-set: drm-fabric
> +      event:
> +        attributes:
> +          - endpoint-id
> +          - port-index
> +          - peer
> +          - topology-generation
> +
> +    -
> +      name: port-peer-delete-ntf
> +      doc: A port's neighbor has been unset.
> +      attribute-set: drm-fabric
> +      event:
> +        attributes:
> +          - endpoint-id
> +          - port-index
> +          - peer
> +          - topology-generation

[Severity: Medium]
Could we use notify: port-get for these notifications instead of custom event
subsets?

Events are considered less idiomatic than notify operations, which share
contents with the GET op. Since peer information is queryable via port-get,
reusing the port-get shape here would avoid forcing userspace to implement
separate parsing logic for these notifications.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.