diff mbox series

[net-next,3/6] Documentation: netlink: add a YAML spec for mptcp

Message ID 20231010-upstream-net-next-20231006-mptcp-ynl-v1-3-18dd117e8f50@kernel.org (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series mptcp: convert Netlink code to use YAML spec | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 9 this patch: 9
netdev/cc_maintainers success CCed 8 of 8 maintainers
netdev/build_clang success Errors and warnings before: 9 this patch: 9
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 9 this patch: 9
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 401 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Matthieu Baerts Oct. 10, 2023, 7:21 p.m. UTC 
From: Davide Caratti <dcaratti@redhat.com>

it describes most of the current netlink interface (uAPI definitions,
doit/dumpit operations and attributes)

Link: https://github.com/multipath-tcp/mptcp_net-next/issues/340
Acked-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
Signed-off-by: Matthieu Baerts <matttbe@kernel.org>
---
 Documentation/netlink/specs/mptcp.yaml | 394 +++++++++++++++++++++++++++++++++
 MAINTAINERS                            |   1 +
 2 files changed, 395 insertions(+)

Comments

Jakub Kicinski Oct. 11, 2023, 1:08 a.m. UTC | #1 
On Tue, 10 Oct 2023 21:21:44 +0200 Matthieu Baerts wrote:
> +definitions:
> +  -
> +    type: enum
> +    name: event-type
> +    enum-name: mptcp_event_type
> +    name-prefix: mptcp_event_

I think you can use - instead of _ here.
For consistency with other families?

> +    entries:
> +     -
> +      name: unspec
> +      value: 0

90% sure enums still start at 0, only attrs and msgs now default to 1.

> +     -
> +      name: announced
> +      value: 6
> +      doc:
> +        token, rem_id, family, daddr4 | daddr6 [, dport]
> +        A new address has been announced by the peer.
> +     -
> +      name: removed
> +      value: 7

Follows 6 so no need for value?

> +      doc:
> +        token, rem_id
> +        An address has been lost by the peer.
> +     -
> +      name: sub_established

Similarly for names we generally recommend - as a separator.
Looks more natural in places where it's used as a string, eg Python.
Well, I guess at least to me it does :)

> +      value: 10
> +      doc:
> +        token, family, loc_id, rem_id, saddr4 | saddr6, daddr4 | daddr6, sport,
> +        dport, backup, if_idx [, error]
> +        A new subflow has been established. 'error' should not be set.
> +     -
> +      name: sub_closed
> +      value: 11

and here, /value/d, s/_/-/

> +      doc:
> +        token, family, loc_id, rem_id, saddr4 | saddr6, daddr4 | daddr6, sport,
> +        dport, backup, if_idx [, error]
> +        A subflow has been closed. An error (copy of sk_err) could be set if an
> +        error has been detected for this subflow.

> +attribute-sets:
> +  -
> +    name: address
> +    name-prefix: mptcp_pm_addr_attr_
> +    attributes:
> +      -
> +        name: unspec
> +        type: unused
> +        value: 0
> +      -
> +        name: family
> +        type: u16
> +      -
> +        name: id
> +        type: u8
> +      -
> +        name: addr4
> +        type: u32
> +        byte-order: big-endian
> +      -
> +        name: addr6
> +        type: binary
> +        checks:
> +          min-len: 16

Do you not want the exact length for this?
If YNL doesn't support something just LMK, we add stuff as needed..
Davide Caratti Oct. 11, 2023, 10:13 a.m. UTC | #2 
hello, Jakub, thanks for looking at this!

On Wed, Oct 11, 2023 at 3:08 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Tue, 10 Oct 2023 21:21:44 +0200 Matthieu Baerts wrote:
> > +definitions:
> > +  -
> > +    type: enum
> > +    name: event-type
> > +    enum-name: mptcp_event_type
> > +    name-prefix: mptcp_event_
>
> I think you can use - instead of _ here.
> For consistency with other families?

right, I will convert the whole spec.

>
> > +    entries:
> > +     -
> > +      name: unspec
> > +      value: 0
>
> 90% sure enums still start at 0, only attrs and msgs now default to 1.

Just checked, value:0 is not needed for enums: I will remove it

> > +     -
> > +      name: announced
> > +      value: 6
> > +      doc:
> > +        token, rem_id, family, daddr4 | daddr6 [, dport]
> > +        A new address has been announced by the peer.
> > +     -
> > +      name: removed
> > +      value: 7
>
> Follows 6 so no need for value?

correct, will fix this too

> > +      -
> > +        name: addr6
> > +        type: binary
> > +        checks:
> > +          min-len: 16
>
> Do you not want the exact length for this?
> If YNL doesn't support something just LMK, we add stuff as needed..

ohh yes, we had NLA_POLICY_EXACT_LEN before but ynl doesn't seem to
support it. I can try to add the support and include another patch at
the beginning of the series, is that ok?
Jakub Kicinski Oct. 11, 2023, 4:22 p.m. UTC | #3 
On Wed, 11 Oct 2023 12:13:04 +0200 Davide Caratti wrote:
> > Do you not want the exact length for this?
> > If YNL doesn't support something just LMK, we add stuff as needed..  
> 
> ohh yes, we had NLA_POLICY_EXACT_LEN before but ynl doesn't seem to
> support it. I can try to add the support and include another patch at
> the beginning of the series, is that ok?

Yes, definitely, thanks!
diff mbox series

Patch

diff --git a/Documentation/netlink/specs/mptcp.yaml b/Documentation/netlink/specs/mptcp.yaml
new file mode 100644
index 000000000000..f8ebcd009592
--- /dev/null
+++ b/Documentation/netlink/specs/mptcp.yaml
@@ -0,0 +1,394 @@ 
+# SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause)
+
+name: mptcp_pm
+protocol: genetlink-legacy
+doc: Multipath TCP.
+
+c-family-name: mptcp_pm_name
+c-version-name: mptcp_pm_ver
+max-by-define: true
+kernel-policy: per-op
+
+definitions:
+  -
+    type: enum
+    name: event-type
+    enum-name: mptcp_event_type
+    name-prefix: mptcp_event_
+    entries:
+     -
+      name: unspec
+      value: 0
+     -
+      name: created
+      doc:
+        token, family, saddr4 | saddr6, daddr4 | daddr6, sport, dport
+        A new MPTCP connection has been created. It is the good time to
+        allocate memory and send ADD_ADDR if needed. Depending on the
+        traffic-patterns it can take a long time until the
+        MPTCP_EVENT_ESTABLISHED is sent.
+     -
+      name: established
+      doc:
+        token, family, saddr4 | saddr6, daddr4 | daddr6, sport, dport
+        A MPTCP connection is established (can start new subflows).
+     -
+      name: closed
+      doc:
+        token
+        A MPTCP connection has stopped.
+     -
+      name: announced
+      value: 6
+      doc:
+        token, rem_id, family, daddr4 | daddr6 [, dport]
+        A new address has been announced by the peer.
+     -
+      name: removed
+      value: 7
+      doc:
+        token, rem_id
+        An address has been lost by the peer.
+     -
+      name: sub_established
+      value: 10
+      doc:
+        token, family, loc_id, rem_id, saddr4 | saddr6, daddr4 | daddr6, sport,
+        dport, backup, if_idx [, error]
+        A new subflow has been established. 'error' should not be set.
+     -
+      name: sub_closed
+      value: 11
+      doc:
+        token, family, loc_id, rem_id, saddr4 | saddr6, daddr4 | daddr6, sport,
+        dport, backup, if_idx [, error]
+        A subflow has been closed. An error (copy of sk_err) could be set if an
+        error has been detected for this subflow.
+     -
+      name: sub_priority
+      value: 13
+      doc:
+        token, family, loc_id, rem_id, saddr4 | saddr6, daddr4 | daddr6, sport,
+        dport, backup, if_idx [, error]
+        The priority of a subflow has changed. 'error' should not be set.
+     -
+      name: listener_created
+      value: 15
+      doc:
+        family, sport, saddr4 | saddr6
+        A new PM listener is created.
+     -
+      name: listener_closed
+      value: 16
+      doc:
+        family, sport, saddr4 | saddr6
+        A PM listener is closed.
+
+attribute-sets:
+  -
+    name: address
+    name-prefix: mptcp_pm_addr_attr_
+    attributes:
+      -
+        name: unspec
+        type: unused
+        value: 0
+      -
+        name: family
+        type: u16
+      -
+        name: id
+        type: u8
+      -
+        name: addr4
+        type: u32
+        byte-order: big-endian
+      -
+        name: addr6
+        type: binary
+        checks:
+          min-len: 16
+      -
+        name: port
+        type: u16
+        byte-order: big-endian
+      -
+        name: flags
+        type: u32
+      -
+        name: if_idx
+        type: s32
+  -
+    name: subflow-attribute
+    name-prefix: mptcp_subflow_attr_
+    attributes:
+      -
+        name: unspec
+        type: unused
+        value: 0
+      -
+        name: token_rem
+        type: u32
+      -
+        name: token_loc
+        type: u32
+      -
+        name: relwrite_seq
+        type: u32
+      -
+        name: map_seq
+        type: u64
+      -
+        name: map_sfseq
+        type: u32
+      -
+        name: ssn_offset
+        type: u32
+      -
+        name: map_datalen
+        type: u16
+      -
+        name: flags
+        type: u32
+      -
+        name: id_rem
+        type: u8
+      -
+        name: id_loc
+        type: u8
+      -
+        name: pad
+        type: pad
+  -
+    name: endpoint
+    name-prefix: mptcp_pm_endpoint_
+    attributes:
+      -
+        name: addr
+        type: nest
+        nested-attributes: address
+  -
+    name: attr
+    name-prefix: mptcp_pm_attr_
+    attributes:
+      -
+        name: unspec
+        type: unused
+        value: 0
+      -
+        name: addr
+        type: nest
+        nested-attributes: address
+      -
+        name: rcv_add_addrs
+        type: u32
+      -
+        name: subflows
+        type: u32
+      -
+        name: token
+        type: u32
+      -
+        name: loc_id
+        type: u8
+      -
+        name: addr_remote
+        type: nest
+        nested-attributes: address
+  -
+    name: event_attr
+    enum-name: mptcp_event_attr
+    name-prefix: mptcp_attr_
+    attributes:
+      -
+        name: UNSPEC
+        type: unused
+        value: 0
+      -
+        name: token
+        type: u32
+      -
+        name: family
+        type: u16
+      -
+        name: loc_id
+        type: u8
+      -
+        name: rem_id
+        type: u8
+      -
+        name: saddr4
+        type: u32
+        byte-order: big-endian
+      -
+        name: saddr6
+        type: binary
+        checks:
+          min-len: 16
+      -
+        name: daddr4
+        type: u32
+        byte-order: big-endian
+      -
+        name: daddr6
+        type: binary
+        checks:
+          min-len: 16
+      -
+        name: sport
+        type: u16
+        byte-order: big-endian
+      -
+        name: dport
+        type: u16
+        byte-order: big-endian
+      -
+        name: backup
+        type: u8
+      -
+        name: error
+        type: u8
+      -
+        name: flags
+        type: u16
+      -
+        name: timeout
+        type: u32
+      -
+        name: if_idx
+        type: u32
+      -
+        name: reset_reason
+        type: u32
+      -
+        name: reset_flags
+        type: u32
+      -
+        name: server_side
+        type: u8
+
+operations:
+  list:
+    -
+      name: unspec
+      doc: unused
+      value: 0
+    -
+      name: add_addr
+      doc: Add endpoint
+      attribute-set: endpoint
+      dont-validate: [ strict ]
+      flags: [ uns-admin-perm ]
+      do: &add_addr_attrs
+        request:
+          attributes:
+            - addr
+    -
+      name: del_addr
+      doc: Delete endpoint
+      attribute-set: endpoint
+      dont-validate: [ strict ]
+      flags: [ uns-admin-perm ]
+      do: *add_addr_attrs
+    -
+      name: get_addr
+      doc: Get endpoint information
+      attribute-set: endpoint
+      dont-validate: [ strict ]
+      flags: [ uns-admin-perm ]
+      do: &get_addr_attrs
+        request:
+          attributes:
+           - addr
+        reply:
+          attributes:
+           - addr
+      dump:
+        reply:
+         attributes:
+           - addr
+    -
+      name:  flush_addrs
+      doc: flush addresses
+      attribute-set: endpoint
+      dont-validate: [ strict ]
+      flags: [ uns-admin-perm ]
+      do: *add_addr_attrs
+    -
+      name: set_limits
+      doc: Set protocol limits
+      attribute-set: attr
+      dont-validate: [ strict ]
+      flags: [ uns-admin-perm ]
+      do: &mptcp_limits
+        request:
+          attributes:
+            - rcv_add_addrs
+            - subflows
+    -
+      name: get_limits
+      doc: Get protocol limits
+      attribute-set: attr
+      dont-validate: [ strict ]
+      do: &mptcp_get_limits
+        request:
+           attributes:
+            - rcv_add_addrs
+            - subflows
+        reply:
+          attributes:
+            - rcv_add_addrs
+            - subflows
+    -
+      name: set_flags
+      doc: Change endpoint flags
+      attribute-set: attr
+      dont-validate: [ strict ]
+      flags: [ uns-admin-perm ]
+      do: &mptcp_set_flags
+        request:
+          attributes:
+            - addr
+            - token
+            - addr_remote
+    -
+      name: announce
+      doc: announce new sf
+      attribute-set: attr
+      dont-validate: [ strict ]
+      flags: [ uns-admin-perm ]
+      do: &announce_add
+        request:
+          attributes:
+            - addr
+            - token
+    -
+      name: remove
+      doc: announce removal
+      attribute-set: attr
+      dont-validate: [ strict ]
+      flags: [ uns-admin-perm ]
+      do:
+        request:
+         attributes:
+           - token
+           - loc_id
+    -
+      name: subflow_create
+      doc: todo
+      attribute-set: attr
+      dont-validate: [ strict ]
+      flags: [ uns-admin-perm ]
+      do: &sf_create
+        request:
+          attributes:
+            - addr
+            - token
+            - addr_remote
+    -
+      name: subflow_destroy
+      doc: todo
+      attribute-set: attr
+      dont-validate: [ strict ]
+      flags: [ uns-admin-perm ]
+      do: *sf_create
diff --git a/MAINTAINERS b/MAINTAINERS
index 1bd96045beb8..ab699a697616 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14960,6 +14960,7 @@  W:	https://github.com/multipath-tcp/mptcp_net-next/wiki
 B:	https://github.com/multipath-tcp/mptcp_net-next/issues
 T:	git https://github.com/multipath-tcp/mptcp_net-next.git export-net
 T:	git https://github.com/multipath-tcp/mptcp_net-next.git export
+F:	Documentation/netlink/specs/mptcp.yaml
 F:	Documentation/networking/mptcp-sysctl.rst
 F:	include/net/mptcp.h
 F:	include/trace/events/mptcp.h