Re: [PATCH] netlink: specs: add specification for NBD

[Donald Hunter <donald.hunter@gmail.com>]

Hristo Venev <hristo@venev.name> writes:

> This patch adds an initial YNL specification for NBD. The specification
> can be used to produce a UAPI header that is equivalent to the one
> currently shipped in the kernel.
>
> The spec appears to be accurate enough so that commands can be issued
> with pyynl.
>
> Signed-off-by: Hristo Venev <hristo@venev.name>
> ---
>  Documentation/netlink/specs/nbd.yaml | 206 +++++++++++++++++++++++++++
>  1 file changed, 206 insertions(+)
>  create mode 100644 Documentation/netlink/specs/nbd.yaml

There are yamllint errors:

make -C tools/net/ynl lint
make: Entering directory '/home/donaldh/net-next/tools/net/ynl'
yamllint ../../../Documentation/netlink/specs
../../../Documentation/netlink/specs/nbd.yaml
  159:81    error    line too long (104 > 80 characters)  (line-length)
  169:6     error    syntax error: expected <block end>, but found '<block mapping start>' (syntax)
  170:7     error    wrong indentation: expected 5 but found 6  (indentation)

>
> diff --git a/Documentation/netlink/specs/nbd.yaml b/Documentation/netlink/specs/nbd.yaml
> new file mode 100644
> index 0000000000000..a84912a867d91
> --- /dev/null
> +++ b/Documentation/netlink/specs/nbd.yaml
> @@ -0,0 +1,206 @@
> +# SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause)
> +---
> +name: nbd
> +protocol: genetlink-c
> +uapi-header: linux/nbd-netlink.h
> +doc: See :file:`drivers/block/nbd.c`

Prefer to see a meaningful doc string here.

> +
> +c-family-name: nbd-genl-family-name
> +c-version-name: nbd-genl-version
> +max-by-define: true
> +
> +attribute-sets:
> +  -
> +    name: nbd-attrs
> +    name-prefix: nbd-attr-
> +    doc: Configuration policy attributes, used for CONNECT
> +    attributes:
> +      -
> +        name: unspec
> +        value: 0
> +        type: unused

No need for unspec in the attribute list, ynl codegen does the right thing.

> +      -
> +        name: index
> +        type: u32
> +      -
> +        name: size-bytes
> +        type: u64
> +      -
> +        name: block-size-bytes
> +        type: u64
> +      -
> +        name: timeout
> +        type: u64
> +      -
> +        name: server-flags
> +        type: u64
> +      -
> +        name: client-flags
> +        type: u64
> +      -
> +        name: sockets
> +        type: nest
> +        nested-attributes: sock-item-attrs
> +      -
> +        name: dead-conn-timeout
> +        type: u64
> +      -
> +        name: device-list
> +        type: nest
> +        nested-attributes: device-item-attrs
> +      -
> +        name: backend-identifier
> +        type: string
> +  -
> +    name: device-item-attrs
> +    name-prefix: nbd-device-item-
> +    doc: |
> +      This is the format for multiple devices with :code:`NBD_ATTR_DEVICE_LIST`
> +
> +      .. code-block::
> +
> +        [NBD_ATTR_DEVICE_LIST]
> +          [NBD_DEVICE_ITEM]
> +            [NBD_DEVICE_INDEX]
> +            [NBD_DEVICE_CONNECTED]
> +    attributes:
> +      -
> +        name: unspec
> +        value: 0
> +        type: unused

Not required.

> +      -
> +        name: item
> +        name-prefix: nbd-device-
> +        type: nest
> +        nested-attributes: device-attrs
> +        multi-attr: true
> +  -
> +    name: device-attrs
> +    name-prefix: nbd-device-
> +    attr-max-name: nbd-device-attr-max
> +    attributes:
> +      -
> +        name: unspec
> +        value: 0
> +        type: unused

Not required.

> +      -
> +        name: index
> +        type: u32
> +      -
> +        name: connected
> +        type: u8
> +  -
> +    name: sock-item-attrs
> +    name-prefix: nbd-sock-item-
> +    doc: |
> +      This is the format for multiple sockets with :code:`NBD_ATTR_SOCKETS`
> +
> +      .. code-block::
> +
> +        [NBD_ATTR_SOCKETS]
> +          [NBD_SOCK_ITEM]
> +            [NBD_SOCK_FD]
> +          [NBD_SOCK_ITEM]
> +            [NBD_SOCK_FD]
> +    attributes:
> +      -
> +        name: unspec
> +        value: 0
> +        type: unused

Not required.

> +      -
> +        name: item
> +        name-prefix: nbd-sock-
> +        type: nest
> +        nested-attributes: sock-attrs
> +        multi-attr: true
> +  -
> +    name: sock-attrs
> +    name-prefix: nbd-sock-
> +    attributes:
> +      -
> +        name: unspec
> +        value: 0
> +        type: unused

Not required.

> +      -
> +        name: fd
> +        type: u32
> +
> +operations:
> +  enum-model: unified
> +  name-prefix: nbd-cmd-
> +  list:
> +    -
> +      name: unspec
> +      value: 0
> +      doc: NBD_CMD_UNSPEC

Not required.

> +    -
> +      name: connect
> +      doc: See :file:`drivers/block/nbd.c`, :code:`nbd_genl_connect()`

Prefer to see meaningful doc string (same for other ops)

> +      attribute-set: nbd-attrs
> +      dont-validate: [strict]

nbd.c has strict & dump, should this be [strict, dump] ? (same for other ops)

> +      do:
> +        request:
> +          attributes:
> +            - index
> +            - size-bytes
> +            - block-size-bytes
> +            - timeout
> +            - server-flags
> +            - client-flags
> +            - sockets
> +            - dead-conn-timeout
> +            - backend-identifier
> +        reply:
> +          attributes:
> +            - index
> +    -
> +      name: disconnect
> +      doc: See :file:`drivers/block/nbd.c`, :code:`nbd_genl_disconnect()`
> +      attribute-set: nbd-attrs
> +      dont-validate: [strict]
> +      do:
> +        request:
> +          attributes:
> +            - index
> +    -
> +      name: reconfigure
> +      doc: See :file:`drivers/block/nbd.c`, :code:`nbd_genl_reconfigure()`
> +      attribute-set: nbd-attrs
> +      dont-validate: [strict]
> +      do:
> +        request:
> +          attributes:
> +            - index
> +            - size-bytes
> +            - block-size-bytes
> +            - timeout
> +            - client-flags
> +            - sockets
> +            - dead-conn-timeout
> +            - backend-identifier
> +    -
> +      name: link-dead
> +      doc: See :file:`drivers/block/nbd.c`, :code:`nbd_mark_nsock_dead()` / :code:`nbd_dead_link_work()`
> +      attribute-set: nbd-attrs
> +      event:
> +        attributes:
> +          - index
> +      mcgrp: nbd_mc_group
> +    -
> +      name: status
> +      doc: See :file:`drivers/block/nbd.c`, :code:`nbd_genl_status()`
> +      attribute-set: nbd-attrs
> +      dont-validate: [strict]
> +      do:
> +        request:
> +          attributes:
> +            - index
> +        reply:
> +          attributes:
> +            - device-list
> +
> +mcast-groups:
> +  list:
> +    -
> +      name: nbd_mc_group
> +      c-define-name: nbd-genl-mcast-group-name