| Message ID | 20240626201133.2572487-1-kuba@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] tcp_metrics: add netlink protocol spec in YAML | expand |
Jakub Kicinski <kuba@kernel.org> writes: > Add a protocol spec for tcp_metrics, so that it's accessible via YNL. > Useful at the very least for testing fixes. > > In this episode of "10,000 ways to complicate netlink" the metric > nest has defines which are off by 1. iproute2 does: > > struct rtattr *m[TCP_METRIC_MAX + 1 + 1]; > > parse_rtattr_nested(m, TCP_METRIC_MAX + 1, a); > > for (i = 0; i < TCP_METRIC_MAX + 1; i++) { > // ... > attr = m[i + 1]; > > This is too weird to support in YNL, add a new set of defines > with _correct_ values to the official kernel header. I had to add tcp_metrics.h to Makefile.deps to get the generated code to complile. diff --git a/tools/net/ynl/Makefile.deps b/tools/net/ynl/Makefile.deps index f4e8eb79c1b8..2f05f8ec2324 100644 --- a/tools/net/ynl/Makefile.deps +++ b/tools/net/ynl/Makefile.deps @@ -25,3 +25,4 @@ CFLAGS_nfsd:=$(call get_hdr_inc,_LINUX_NFSD_NETLINK_H,nfsd_netlink.h) CFLAGS_ovs_datapath:=$(call get_hdr_inc,__LINUX_OPENVSWITCH_H,openvswitch.h) CFLAGS_ovs_flow:=$(call get_hdr_inc,__LINUX_OPENVSWITCH_H,openvswitch.h) CFLAGS_ovs_vport:=$(call get_hdr_inc,__LINUX_OPENVSWITCH_H,openvswitch.h) +CFLAGS_tcp_metrics:=$(call get_hdr_inc,__LINUX_TCP_METRICS_H,tcp_metrics.h) > + > + - > + name: metrics > + # Intentially don't define the name-prefix to match the kernel, see below. Typo: Intentionally > + doc: | > + Attributes with metrics. Note that the values here do not match > + the TCP_METRIC_* defines in the kernel, because kernel defines > + are off-by one (e.g. rtt is defined as enum 0, while netlink carries > + attribute type 1). > + attributes: > + - > + name: rtt > + type: u32 > + doc: | > + Round Trip Time (RTT), in msecs with 3 bits fractional > + (left-shift by 3 to get the msec value). > + - > + name: rttvar > + type: u32 > + doc: | > + Round Trip Time VARiance (RTT), in msecs with 2 bits fractional > + (left-shift by 2 to get the msec value). > + - > + name: ssthresh > + type: u32 > + doc: Slow Start THRESHold. > + - > + name: cwnd > + type: u32 > + doc: Congestion Window. > + - > + name: reodering > + type: u32 > + doc: Reodering metric. > + - > + name: rtt_us Should this be a dash, 'rtt-us' ? > + type: u32 > + doc: | > + Round Trip Time (RTT), in usecs, with 3 bits fractional > + (left-shift by 3 to get the msec value). > + - > + name: rttvar_us Same here? > + type: u32 > + doc: | > + Round Trip Time (RTT), in usecs, with 3 bits fractional > + (left-shift by 3 to get the msec value). > +
On Wed, 26 Jun 2024 at 21:11, Jakub Kicinski <kuba@kernel.org> wrote: > > +operations: > + list: > + - > + name: unspec > + doc: unused > + value: 0 Is this needed? It shows up in generated docs as: unspec unused value: 0 The spec works with it removed.
On Thu, 27 Jun 2024 10:20:15 +0100 Donald Hunter wrote: > I had to add tcp_metrics.h to Makefile.deps to get the generated code to > complile. > > diff --git a/tools/net/ynl/Makefile.deps b/tools/net/ynl/Makefile.deps > index f4e8eb79c1b8..2f05f8ec2324 100644 > --- a/tools/net/ynl/Makefile.deps > +++ b/tools/net/ynl/Makefile.deps > @@ -25,3 +25,4 @@ CFLAGS_nfsd:=$(call get_hdr_inc,_LINUX_NFSD_NETLINK_H,nfsd_netlink.h) > CFLAGS_ovs_datapath:=$(call get_hdr_inc,__LINUX_OPENVSWITCH_H,openvswitch.h) > CFLAGS_ovs_flow:=$(call get_hdr_inc,__LINUX_OPENVSWITCH_H,openvswitch.h) > CFLAGS_ovs_vport:=$(call get_hdr_inc,__LINUX_OPENVSWITCH_H,openvswitch.h) > +CFLAGS_tcp_metrics:=$(call get_hdr_inc,__LINUX_TCP_METRICS_H,tcp_metrics.h) Indeed, and I think we need to add _UAPI to the guard. Or just include the file without calling get_hdr_inc. I'll send v2 as a (tiny) series.
diff --git a/Documentation/netlink/specs/tcp_metrics.yaml b/Documentation/netlink/specs/tcp_metrics.yaml new file mode 100644 index 000000000000..cc3aa66f915a --- /dev/null +++ b/Documentation/netlink/specs/tcp_metrics.yaml @@ -0,0 +1,178 @@ +# SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause) + +name: tcp_metrics + +protocol: genetlink-legacy + +doc: | + Management interface for TCP metrics. + +c-family-name: tcp-metrics-genl-name +c-version-name: tcp-metrics-genl-version +max-by-define: true +kernel-policy: global + +definitions: + - + name: tcp-fastopen-cookie-max + type: const + value: 16 + +attribute-sets: + - + name: tcp-metrics + name-prefix: tcp-metrics-attr- + attributes: + - + name: unspec + type: unused + value: 0 + - + name: addr-ipv4 + type: u32 + byte-order: big-endian + display-hint: ipv4 + - + name: addr-ipv6 + type: binary + checks: + min-len: 16 + byte-order: big-endian + display-hint: ipv6 + - + name: age + type: u64 + - + name: tw-tsval + type: u32 + doc: unused + - + name: tw-ts-stamp + type: s32 + doc: unused + - + name: vals + type: nest + nested-attributes: metrics + - + name: fopen-mss + type: u16 + - + name: fopen-syn-drops + type: u16 + - + name: fopen-syn-drop-ts + type: u64 + - + name: fopen-cookie + type: binary + checks: + min-len: tcp-fastopen-cookie-max + - + name: saddr-ipv4 + type: u32 + byte-order: big-endian + display-hint: ipv4 + - + name: saddr-ipv6 + type: binary + checks: + min-len: 16 + byte-order: big-endian + display-hint: ipv6 + - + name: pad + type: pad + + - + name: metrics + # Intentially don't define the name-prefix to match the kernel, see below. + doc: | + Attributes with metrics. Note that the values here do not match + the TCP_METRIC_* defines in the kernel, because kernel defines + are off-by one (e.g. rtt is defined as enum 0, while netlink carries + attribute type 1). + attributes: + - + name: rtt + type: u32 + doc: | + Round Trip Time (RTT), in msecs with 3 bits fractional + (left-shift by 3 to get the msec value). + - + name: rttvar + type: u32 + doc: | + Round Trip Time VARiance (RTT), in msecs with 2 bits fractional + (left-shift by 2 to get the msec value). + - + name: ssthresh + type: u32 + doc: Slow Start THRESHold. + - + name: cwnd + type: u32 + doc: Congestion Window. + - + name: reodering + type: u32 + doc: Reodering metric. + - + name: rtt_us + type: u32 + doc: | + Round Trip Time (RTT), in usecs, with 3 bits fractional + (left-shift by 3 to get the msec value). + - + name: rttvar_us + type: u32 + doc: | + Round Trip Time (RTT), in usecs, with 3 bits fractional + (left-shift by 3 to get the msec value). + +operations: + list: + - + name: unspec + doc: unused + value: 0 + + - + name: get + doc: Retrieve metrics. + attribute-set: tcp-metrics + + dont-validate: [ strict, dump ] + + do: + request: &sel_attrs + attributes: + - addr-ipv4 + - addr-ipv6 + - saddr-ipv4 + - saddr-ipv6 + reply: &all_attrs + attributes: + - addr-ipv4 + - addr-ipv6 + - saddr-ipv4 + - saddr-ipv6 + - age + - vals + - fopen-mss + - fopen-syn-drops + - fopen-syn-drop-ts + - fopen-cookie + dump: + reply: *all_attrs + + - + name: del + doc: Delete metrics. + attribute-set: tcp-metrics + + dont-validate: [ strict, dump ] + flags: [ admin-perm ] + + do: + request: *sel_attrs diff --git a/include/uapi/linux/tcp_metrics.h b/include/uapi/linux/tcp_metrics.h index 7cb4a172feed..d93d32f163f0 100644 --- a/include/uapi/linux/tcp_metrics.h +++ b/include/uapi/linux/tcp_metrics.h @@ -27,6 +27,22 @@ enum tcp_metric_index { #define TCP_METRIC_MAX (__TCP_METRIC_MAX - 1) +/* Re-define enum tcp_metric_index, again, using the values carried + * as netlink attribute types. + */ +enum { + TCP_METRICS_A_METRICS_RTT = 1, + TCP_METRICS_A_METRICS_RTTVAR, + TCP_METRICS_A_METRICS_SSTHRESH, + TCP_METRICS_A_METRICS_CWND, + TCP_METRICS_A_METRICS_REODERING, + TCP_METRICS_A_METRICS_RTT_US, + TCP_METRICS_A_METRICS_RTTVAR_US, + + __TCP_METRICS_A_METRICS_MAX +}; +#define TCP_METRICS_A_METRICS_MAX (__TCP_METRICS_A_METRICS_MAX - 1) + enum { TCP_METRICS_ATTR_UNSPEC, TCP_METRICS_ATTR_ADDR_IPV4, /* u32 */
Add a protocol spec for tcp_metrics, so that it's accessible via YNL. Useful at the very least for testing fixes. In this episode of "10,000 ways to complicate netlink" the metric nest has defines which are off by 1. iproute2 does: struct rtattr *m[TCP_METRIC_MAX + 1 + 1]; parse_rtattr_nested(m, TCP_METRIC_MAX + 1, a); for (i = 0; i < TCP_METRIC_MAX + 1; i++) { // ... attr = m[i + 1]; This is too weird to support in YNL, add a new set of defines with _correct_ values to the official kernel header. Signed-off-by: Jakub Kicinski <kuba@kernel.org> --- CC: donald.hunter@gmail.com --- Documentation/netlink/specs/tcp_metrics.yaml | 178 +++++++++++++++++++ include/uapi/linux/tcp_metrics.h | 16 ++ 2 files changed, 194 insertions(+) create mode 100644 Documentation/netlink/specs/tcp_metrics.yaml