netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 0/4] dpll: fix lies in the Netlink spec
@ 2026-09-04 19:10 Jakub Kicinski
  2026-09-04 19:10 ` [PATCH net-next 1/4] netlink: specs: dpll: add clock-quality-level to the device-get reply Jakub Kicinski
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Jakub Kicinski @ 2026-09-04 19:10 UTC (permalink / raw)
  To: davem
  Cc: netdev, edumazet, pabeni, andrew+netdev, horms, vadim.fedorenko,
	arkadiusz.kubalewski, ivecera, jiri, donald.hunter,
	Jakub Kicinski

Clashiko started accumulating pre-existing issues in specs so I did
a scan of the families. This set fixes issues discovered in dpll.

Jakub Kicinski (4):
  netlink: specs: dpll: add clock-quality-level to the device-get reply
  netlink: specs: dpll: drop the pin-set attributes nothing implements
  netlink: specs: dpll: pin-get has no dump filter
  MAINTAINERS: dpll: add the netlink spec

 MAINTAINERS                           |  1 +
 Documentation/netlink/specs/dpll.yaml | 15 ++++-----------
 drivers/dpll/dpll_nl.c                | 20 +++++---------------
 3 files changed, 10 insertions(+), 26 deletions(-)

-- 
2.55.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH net-next 1/4] netlink: specs: dpll: add clock-quality-level to the device-get reply
  2026-09-04 19:10 [PATCH net-next 0/4] dpll: fix lies in the Netlink spec Jakub Kicinski
@ 2026-09-04 19:10 ` Jakub Kicinski
  2026-09-05 11:19   ` Vadim Fedorenko
  2026-09-08  7:13   ` netdev-bot+sashiko
  2026-09-04 19:10 ` [PATCH net-next 2/4] netlink: specs: dpll: drop the pin-set attributes nothing implements Jakub Kicinski
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 12+ messages in thread
From: Jakub Kicinski @ 2026-09-04 19:10 UTC (permalink / raw)
  To: davem
  Cc: netdev, edumazet, pabeni, andrew+netdev, horms, vadim.fedorenko,
	arkadiusz.kubalewski, ivecera, jiri, donald.hunter,
	Jakub Kicinski

dpll_device_get_one() calls dpll_msg_add_clock_quality_level(), which
puts DPLL_A_CLOCK_QUALITY_LEVEL into every device-get reply, dump entry
and device-*-ntf of a device whose driver implements
clock_quality_level_get() - mlx5 does. The &dev-attrs anchor lists 12
attributes and not this one, so struct dpll_device_get_rsp has no field
for it, dpll_device_get_rsp_parse() drops it and dpll.rst does not
document it.

The attribute itself is correctly declared in the dpll attribute set,
only the reply list was missing an entry. Missing since commit
a1afb959add1 ("dpll: add clock quality level attribute and op").

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 Documentation/netlink/specs/dpll.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/netlink/specs/dpll.yaml b/Documentation/netlink/specs/dpll.yaml
index cdc8c7b456df..64a45088c027 100644
--- a/Documentation/netlink/specs/dpll.yaml
+++ b/Documentation/netlink/specs/dpll.yaml
@@ -632,6 +632,7 @@ doc: DPLL subsystem.
             - mode-supported
             - lock-status
             - lock-status-error
+            - clock-quality-level
             - temp
             - clock-id
             - type
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH net-next 2/4] netlink: specs: dpll: drop the pin-set attributes nothing implements
  2026-09-04 19:10 [PATCH net-next 0/4] dpll: fix lies in the Netlink spec Jakub Kicinski
  2026-09-04 19:10 ` [PATCH net-next 1/4] netlink: specs: dpll: add clock-quality-level to the device-get reply Jakub Kicinski
@ 2026-09-04 19:10 ` Jakub Kicinski
  2026-09-06 11:44   ` Vadim Fedorenko
  2026-09-04 19:10 ` [PATCH net-next 3/4] netlink: specs: dpll: pin-get has no dump filter Jakub Kicinski
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Jakub Kicinski @ 2026-09-04 19:10 UTC (permalink / raw)
  To: davem
  Cc: netdev, edumazet, pabeni, andrew+netdev, horms, vadim.fedorenko,
	arkadiusz.kubalewski, ivecera, jiri, donald.hunter,
	Jakub Kicinski

pin-set advertises top level direction, prio and state, so the
generated policy accepts them and the generated user space API grows
setters for them. dpll_pin_set_from_nlattr() has cases only for
FREQUENCY, PHASE_ADJUST, PARENT_DEVICE, PARENT_PIN, ESYNC_FREQUENCY and
REFERENCE_SYNC - the request succeeds, does nothing and produces no
extack. The three values are only actionable inside the parent-device
nest, where dpll_pin_parent_device_set() handles them next to
DPLL_A_PIN_PARENT_ID, which is what makes them meaningful: a pin can be
registered with more than one device.

Never implemented - "git log -S 'case DPLL_A_PIN_PRIO'" on
dpll_netlink.c comes up empty - the spec has advertised them since
commit 3badff3a25d8 ("dpll: spec: Add Netlink spec in YAML").

Note that this does change what the kernel accepts: sending one of the
three now fails validation instead of being ignored.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 Documentation/netlink/specs/dpll.yaml | 3 ---
 drivers/dpll/dpll_nl.c                | 3 ---
 2 files changed, 6 deletions(-)

diff --git a/Documentation/netlink/specs/dpll.yaml b/Documentation/netlink/specs/dpll.yaml
index 64a45088c027..93e546ba1f56 100644
--- a/Documentation/netlink/specs/dpll.yaml
+++ b/Documentation/netlink/specs/dpll.yaml
@@ -760,9 +760,6 @@ doc: DPLL subsystem.
           attributes:
             - id
             - frequency
-            - direction
-            - prio
-            - state
             - parent-device
             - parent-pin
             - phase-adjust
diff --git a/drivers/dpll/dpll_nl.c b/drivers/dpll/dpll_nl.c
index b1ba490e72b0..3bdabbf2b931 100644
--- a/drivers/dpll/dpll_nl.c
+++ b/drivers/dpll/dpll_nl.c
@@ -78,9 +78,6 @@ static const struct nla_policy dpll_pin_get_dump_nl_policy[DPLL_A_PIN_ID + 1] =
 static const struct nla_policy dpll_pin_set_nl_policy[DPLL_A_PIN_REFERENCE_SYNC + 1] = {
 	[DPLL_A_PIN_ID] = { .type = NLA_U32, },
 	[DPLL_A_PIN_FREQUENCY] = { .type = NLA_U64, },
-	[DPLL_A_PIN_DIRECTION] = NLA_POLICY_RANGE(NLA_U32, 1, 2),
-	[DPLL_A_PIN_PRIO] = { .type = NLA_U32, },
-	[DPLL_A_PIN_STATE] = NLA_POLICY_RANGE(NLA_U32, 1, 3),
 	[DPLL_A_PIN_PARENT_DEVICE] = NLA_POLICY_NESTED(dpll_pin_parent_device_nl_policy),
 	[DPLL_A_PIN_PARENT_PIN] = NLA_POLICY_NESTED(dpll_pin_parent_pin_nl_policy),
 	[DPLL_A_PIN_PHASE_ADJUST] = { .type = NLA_S32, },
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH net-next 3/4] netlink: specs: dpll: pin-get has no dump filter
  2026-09-04 19:10 [PATCH net-next 0/4] dpll: fix lies in the Netlink spec Jakub Kicinski
  2026-09-04 19:10 ` [PATCH net-next 1/4] netlink: specs: dpll: add clock-quality-level to the device-get reply Jakub Kicinski
  2026-09-04 19:10 ` [PATCH net-next 2/4] netlink: specs: dpll: drop the pin-set attributes nothing implements Jakub Kicinski
@ 2026-09-04 19:10 ` Jakub Kicinski
  2026-09-06 11:44   ` Vadim Fedorenko
  2026-09-08  7:13   ` netdev-bot+sashiko
  2026-09-04 19:10 ` [PATCH net-next 4/4] MAINTAINERS: dpll: add the netlink spec Jakub Kicinski
  2026-09-09 21:00 ` [PATCH net-next 0/4] dpll: fix lies in the Netlink spec patchwork-bot+netdevbpf
  4 siblings, 2 replies; 12+ messages in thread
From: Jakub Kicinski @ 2026-09-04 19:10 UTC (permalink / raw)
  To: davem
  Cc: netdev, edumazet, pabeni, andrew+netdev, horms, vadim.fedorenko,
	arkadiusz.kubalewski, ivecera, jiri, donald.hunter,
	Jakub Kicinski

The op doc promises "dump request with target dpll - list all the pins
registered with a given dpll device" and "do request with target dpll
and target pin". Neither exists. pin-get uses attribute-set: pin, so
the id in both requests is DPLL_A_PIN_ID, not a device id; there is no
reachable "target dpll" attribute at all (DPLL_A_PIN_PARENT_ID is in
neither list). dpll_nl_pin_get_dumpit() does not look at the request,
it walks dpll_pin_xa from ctx->idx and emits every registered,
available pin, and there is no dump start callback to consume an id
either.

So the generated dpll_pin_get_dump() sends an id, gets the full list
back and reports no error. Drop the id from the dump request and the
two targeting claims from the doc.

Present since commit 3badff3a25d8 ("dpll: spec: Add Netlink spec in
YAML").
Same caveat as for pin-set: sending DPLL_A_PIN_ID in a pin-get dump now
fails validation instead of being ignored.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 Documentation/netlink/specs/dpll.yaml | 11 +++--------
 drivers/dpll/dpll_nl.c                | 17 +++++------------
 2 files changed, 8 insertions(+), 20 deletions(-)

diff --git a/Documentation/netlink/specs/dpll.yaml b/Documentation/netlink/specs/dpll.yaml
index 93e546ba1f56..e2ca4df5699a 100644
--- a/Documentation/netlink/specs/dpll.yaml
+++ b/Documentation/netlink/specs/dpll.yaml
@@ -701,11 +701,9 @@ doc: DPLL subsystem.
       doc: |
         Get list of pins and its attributes.
 
-        - dump request without any attributes given - list all the pins in the
-          system
-        - dump request with target dpll - list all the pins registered with
-          a given dpll device
-        - do request with target dpll and target pin - single pin attributes
+        - dump request - list all the registered, available pins in the
+          system, the request takes no attributes
+        - do request with target pin - single pin attributes
       attribute-set: pin
       flags: [admin-perm]
 
@@ -742,9 +740,6 @@ doc: DPLL subsystem.
             - measured-frequency
 
       dump:
-        request:
-          attributes:
-            - id
         reply: *pin-attrs
 
     -
diff --git a/drivers/dpll/dpll_nl.c b/drivers/dpll/dpll_nl.c
index 3bdabbf2b931..fa09b524a4a0 100644
--- a/drivers/dpll/dpll_nl.c
+++ b/drivers/dpll/dpll_nl.c
@@ -65,12 +65,7 @@ static const struct nla_policy dpll_pin_id_get_nl_policy[DPLL_A_PIN_TYPE + 1] =
 };
 
 /* DPLL_CMD_PIN_GET - do */
-static const struct nla_policy dpll_pin_get_do_nl_policy[DPLL_A_PIN_ID + 1] = {
-	[DPLL_A_PIN_ID] = { .type = NLA_U32, },
-};
-
-/* DPLL_CMD_PIN_GET - dump */
-static const struct nla_policy dpll_pin_get_dump_nl_policy[DPLL_A_PIN_ID + 1] = {
+static const struct nla_policy dpll_pin_get_nl_policy[DPLL_A_PIN_ID + 1] = {
 	[DPLL_A_PIN_ID] = { .type = NLA_U32, },
 };
 
@@ -133,16 +128,14 @@ static const struct genl_split_ops dpll_nl_ops[] = {
 		.pre_doit	= dpll_pin_pre_doit,
 		.doit		= dpll_nl_pin_get_doit,
 		.post_doit	= dpll_pin_post_doit,
-		.policy		= dpll_pin_get_do_nl_policy,
+		.policy		= dpll_pin_get_nl_policy,
 		.maxattr	= DPLL_A_PIN_ID,
 		.flags		= GENL_ADMIN_PERM | GENL_CMD_CAP_DO,
 	},
 	{
-		.cmd		= DPLL_CMD_PIN_GET,
-		.dumpit		= dpll_nl_pin_get_dumpit,
-		.policy		= dpll_pin_get_dump_nl_policy,
-		.maxattr	= DPLL_A_PIN_ID,
-		.flags		= GENL_ADMIN_PERM | GENL_CMD_CAP_DUMP,
+		.cmd	= DPLL_CMD_PIN_GET,
+		.dumpit	= dpll_nl_pin_get_dumpit,
+		.flags	= GENL_ADMIN_PERM | GENL_CMD_CAP_DUMP,
 	},
 	{
 		.cmd		= DPLL_CMD_PIN_SET,
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH net-next 4/4] MAINTAINERS: dpll: add the netlink spec
  2026-09-04 19:10 [PATCH net-next 0/4] dpll: fix lies in the Netlink spec Jakub Kicinski
                   ` (2 preceding siblings ...)
  2026-09-04 19:10 ` [PATCH net-next 3/4] netlink: specs: dpll: pin-get has no dump filter Jakub Kicinski
@ 2026-09-04 19:10 ` Jakub Kicinski
  2026-09-06 11:45   ` Vadim Fedorenko
  2026-09-09 21:00 ` [PATCH net-next 0/4] dpll: fix lies in the Netlink spec patchwork-bot+netdevbpf
  4 siblings, 1 reply; 12+ messages in thread
From: Jakub Kicinski @ 2026-09-04 19:10 UTC (permalink / raw)
  To: davem
  Cc: netdev, edumazet, pabeni, andrew+netdev, horms, vadim.fedorenko,
	arkadiusz.kubalewski, ivecera, jiri, donald.hunter,
	Jakub Kicinski

get_maintainer.pl on Documentation/netlink/specs/dpll.yaml returns only
the YNL and NETWORKING [GENERAL] entries, so a change to the DPLL uAPI
description does not CC any of the DPLL maintainers. Ten other families
(binder, dev-energymodel, ethtool, handshake, mptcp_pm, ovpn, psp,
team, wireguard, rt-*) already list their spec explicitly.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index b23fb6f2f4ef..b000853829b5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7919,6 +7919,7 @@ S:	Supported
 F:	Documentation/devicetree/bindings/dpll/dpll-device.yaml
 F:	Documentation/devicetree/bindings/dpll/dpll-pin.yaml
 F:	Documentation/driver-api/dpll.rst
+F:	Documentation/netlink/specs/dpll.yaml
 F:	drivers/dpll/
 F:	include/linux/dpll.h
 F:	include/uapi/linux/dpll.h
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next 1/4] netlink: specs: dpll: add clock-quality-level to the device-get reply
  2026-09-04 19:10 ` [PATCH net-next 1/4] netlink: specs: dpll: add clock-quality-level to the device-get reply Jakub Kicinski
@ 2026-09-05 11:19   ` Vadim Fedorenko
  2026-09-08  7:13   ` netdev-bot+sashiko
  1 sibling, 0 replies; 12+ messages in thread
From: Vadim Fedorenko @ 2026-09-05 11:19 UTC (permalink / raw)
  To: Jakub Kicinski, davem
  Cc: netdev, edumazet, pabeni, andrew+netdev, horms,
	arkadiusz.kubalewski, ivecera, jiri, donald.hunter

On 04/09/2026 20:10, Jakub Kicinski wrote:
> dpll_device_get_one() calls dpll_msg_add_clock_quality_level(), which
> puts DPLL_A_CLOCK_QUALITY_LEVEL into every device-get reply, dump entry
> and device-*-ntf of a device whose driver implements
> clock_quality_level_get() - mlx5 does. The &dev-attrs anchor lists 12
> attributes and not this one, so struct dpll_device_get_rsp has no field
> for it, dpll_device_get_rsp_parse() drops it and dpll.rst does not
> document it.
> 
> The attribute itself is correctly declared in the dpll attribute set,
> only the reply list was missing an entry. Missing since commit
> a1afb959add1 ("dpll: add clock quality level attribute and op").
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
>   Documentation/netlink/specs/dpll.yaml | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/netlink/specs/dpll.yaml b/Documentation/netlink/specs/dpll.yaml
> index cdc8c7b456df..64a45088c027 100644
> --- a/Documentation/netlink/specs/dpll.yaml
> +++ b/Documentation/netlink/specs/dpll.yaml
> @@ -632,6 +632,7 @@ doc: DPLL subsystem.
>               - mode-supported
>               - lock-status
>               - lock-status-error
> +            - clock-quality-level
>               - temp
>               - clock-id
>               - type

Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next 2/4] netlink: specs: dpll: drop the pin-set attributes nothing implements
  2026-09-04 19:10 ` [PATCH net-next 2/4] netlink: specs: dpll: drop the pin-set attributes nothing implements Jakub Kicinski
@ 2026-09-06 11:44   ` Vadim Fedorenko
  0 siblings, 0 replies; 12+ messages in thread
From: Vadim Fedorenko @ 2026-09-06 11:44 UTC (permalink / raw)
  To: Jakub Kicinski, davem, arkadiusz.kubalewski
  Cc: netdev, edumazet, pabeni, andrew+netdev, horms, ivecera, jiri,
	donald.hunter

On 04/09/2026 20:10, Jakub Kicinski wrote:
> pin-set advertises top level direction, prio and state, so the
> generated policy accepts them and the generated user space API grows
> setters for them. dpll_pin_set_from_nlattr() has cases only for
> FREQUENCY, PHASE_ADJUST, PARENT_DEVICE, PARENT_PIN, ESYNC_FREQUENCY and
> REFERENCE_SYNC - the request succeeds, does nothing and produces no
> extack. The three values are only actionable inside the parent-device
> nest, where dpll_pin_parent_device_set() handles them next to
> DPLL_A_PIN_PARENT_ID, which is what makes them meaningful: a pin can be
> registered with more than one device.
> 
> Never implemented - "git log -S 'case DPLL_A_PIN_PRIO'" on
> dpll_netlink.c comes up empty - the spec has advertised them since
> commit 3badff3a25d8 ("dpll: spec: Add Netlink spec in YAML").
> 
> Note that this does change what the kernel accepts: sending one of the
> three now fails validation instead of being ignored.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

@Arkadiusz was this a plan that never happened?

Anyways, LGMT
Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next 3/4] netlink: specs: dpll: pin-get has no dump filter
  2026-09-04 19:10 ` [PATCH net-next 3/4] netlink: specs: dpll: pin-get has no dump filter Jakub Kicinski
@ 2026-09-06 11:44   ` Vadim Fedorenko
  2026-09-08  7:13   ` netdev-bot+sashiko
  1 sibling, 0 replies; 12+ messages in thread
From: Vadim Fedorenko @ 2026-09-06 11:44 UTC (permalink / raw)
  To: Jakub Kicinski, davem
  Cc: netdev, edumazet, pabeni, andrew+netdev, horms,
	arkadiusz.kubalewski, ivecera, jiri, donald.hunter

On 04/09/2026 20:10, Jakub Kicinski wrote:
> The op doc promises "dump request with target dpll - list all the pins
> registered with a given dpll device" and "do request with target dpll
> and target pin". Neither exists. pin-get uses attribute-set: pin, so
> the id in both requests is DPLL_A_PIN_ID, not a device id; there is no
> reachable "target dpll" attribute at all (DPLL_A_PIN_PARENT_ID is in
> neither list). dpll_nl_pin_get_dumpit() does not look at the request,
> it walks dpll_pin_xa from ctx->idx and emits every registered,
> available pin, and there is no dump start callback to consume an id
> either.
> 
> So the generated dpll_pin_get_dump() sends an id, gets the full list
> back and reports no error. Drop the id from the dump request and the
> two targeting claims from the doc.
> 
> Present since commit 3badff3a25d8 ("dpll: spec: Add Netlink spec in
> YAML").
> Same caveat as for pin-set: sending DPLL_A_PIN_ID in a pin-get dump now
> fails validation instead of being ignored.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next 4/4] MAINTAINERS: dpll: add the netlink spec
  2026-09-04 19:10 ` [PATCH net-next 4/4] MAINTAINERS: dpll: add the netlink spec Jakub Kicinski
@ 2026-09-06 11:45   ` Vadim Fedorenko
  0 siblings, 0 replies; 12+ messages in thread
From: Vadim Fedorenko @ 2026-09-06 11:45 UTC (permalink / raw)
  To: Jakub Kicinski, davem
  Cc: netdev, edumazet, pabeni, andrew+netdev, horms,
	arkadiusz.kubalewski, ivecera, jiri, donald.hunter

On 04/09/2026 20:10, Jakub Kicinski wrote:
> get_maintainer.pl on Documentation/netlink/specs/dpll.yaml returns only
> the YNL and NETWORKING [GENERAL] entries, so a change to the DPLL uAPI
> description does not CC any of the DPLL maintainers. Ten other families
> (binder, dev-energymodel, ethtool, handshake, mptcp_pm, ovpn, psp,
> team, wireguard, rt-*) already list their spec explicitly.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
>   MAINTAINERS | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b23fb6f2f4ef..b000853829b5 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7919,6 +7919,7 @@ S:	Supported
>   F:	Documentation/devicetree/bindings/dpll/dpll-device.yaml
>   F:	Documentation/devicetree/bindings/dpll/dpll-pin.yaml
>   F:	Documentation/driver-api/dpll.rst
> +F:	Documentation/netlink/specs/dpll.yaml
>   F:	drivers/dpll/
>   F:	include/linux/dpll.h
>   F:	include/uapi/linux/dpll.h

Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next 1/4] netlink: specs: dpll: add clock-quality-level to the device-get reply
  2026-09-04 19:10 ` [PATCH net-next 1/4] netlink: specs: dpll: add clock-quality-level to the device-get reply Jakub Kicinski
  2026-09-05 11:19   ` Vadim Fedorenko
@ 2026-09-08  7:13   ` netdev-bot+sashiko
  1 sibling, 0 replies; 12+ messages in thread
From: netdev-bot+sashiko @ 2026-09-08  7:13 UTC (permalink / raw)
  To: kuba
  Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms,
	vadim.fedorenko, arkadiusz.kubalewski, ivecera, jiri,
	donald.hunter

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 0 · Low: 1 (1)

- [Low] Documentation/netlink/specs/dpll.yaml: the patch repairs a user-
  visible uAPI-description defect (the &dev-attrs reply anchor of…

Pre-existing issues:
- [Low] The hand-written DPLL ABI table in Documentation/driver-
  api/dpll.rst (``DPLL_CMD_DEVICE_GET`` section, lines ~382-395) still…

--

netlink: specs: dpll: add clock-quality-level to the device-get reply

The device-get reply anchor (&dev-attrs) in the dpll YAML spec was missing
clock-quality-level, even though dpll_device_get_one() puts
DPLL_A_CLOCK_QUALITY_LEVEL into every device-get reply, dump entry and
device-*-ntf for drivers implementing clock_quality_level_get(). The patch
adds the missing entry to the reply list.

> diff --git a/Documentation/netlink/specs/dpll.yaml b/Documentation/netlink/specs/dpll.yaml
> index cdc8c7b456df8..64a45088c0273 100644
> --- a/Documentation/netlink/specs/dpll.yaml
> +++ b/Documentation/netlink/specs/dpll.yaml
> @@ -632,6 +632,7 @@ operations:
>              - mode-supported
>              - lock-status
>              - lock-status-error
> +            - clock-quality-level

[Severity: Low]
The commit message names the culprit in prose:

  "Missing since commit a1afb959add1 ("dpll: add clock quality level
  attribute and op")"

but there is no Fixes: tag. Since this changes what the generated ynl
bindings parse (struct dpll_device_get_rsp gains a field for
DPLL_A_CLOCK_QUALITY_LEVEL that dpll_device_get_rsp_parse() previously
dropped), would a machine-readable tag be preferable here so backport
tooling picks it up?

  Fixes: a1afb959add1 ("dpll: add clock quality level attribute and op")

[Severity: Low]
This isn't something introduced by this patch, but there is a second,
hand-written description of the same reply that stays narrower than what
the kernel emits. The ``DPLL_CMD_DEVICE_GET`` table in
Documentation/driver-api/dpll.rst lists only:

    ``DPLL_A_ID``
    ``DPLL_A_MODULE_NAME``
    ``DPLL_A_CLOCK_ID``
    ``DPLL_A_MODE``
    ``DPLL_A_MODE_SUPPORTED``
    ``DPLL_A_LOCK_STATUS``
    ``DPLL_A_TEMP``
    ``DPLL_A_TYPE``

while dpll_device_get_one() also puts DPLL_A_CLOCK_QUALITY_LEVEL,
DPLL_A_LOCK_STATUS_ERROR, DPLL_A_PHASE_OFFSET_MONITOR,
DPLL_A_PHASE_OFFSET_AVG_FACTOR and DPLL_A_FREQUENCY_MONITOR into the
message.

The commit message says "dpll.rst does not document it", which reads as the
ynl-generated Documentation/networking/netlink_spec/dpll.rst and is indeed
fixed by this YAML change.

Should the driver-api table get the same treatment, or is it intended to
be superseded by the generated spec documentation?

>              - temp
>              - clock-id
>              - type

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260904191023.3869690-1-kuba%40kernel.org

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next 3/4] netlink: specs: dpll: pin-get has no dump filter
  2026-09-04 19:10 ` [PATCH net-next 3/4] netlink: specs: dpll: pin-get has no dump filter Jakub Kicinski
  2026-09-06 11:44   ` Vadim Fedorenko
@ 2026-09-08  7:13   ` netdev-bot+sashiko
  1 sibling, 0 replies; 12+ messages in thread
From: netdev-bot+sashiko @ 2026-09-08  7:13 UTC (permalink / raw)
  To: kuba
  Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms,
	vadim.fedorenko, arkadiusz.kubalewski, ivecera, jiri,
	donald.hunter

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 · Low: 0 (1)

- [Medium] Dropping the dump policy for DPLL_CMD_PIN_GET turns a
  previously accepted (and silently ignored) attribute into a hard…

Pre-existing issues:
- [Low] After this patch the two in-tree descriptions of the
  DPLL_CMD_PIN_GET dump disagree.

--

netlink: specs: dpll: pin-get has no dump filter

The pin-get op doc claimed a dump could be filtered by a target dpll and
that a do request takes a target dpll plus a target pin, neither of which
exists in the implementation. The patch drops the id attribute from the
pin-get dump request and rewords the doc, which also regenerates
drivers/dpll/dpll_nl.c so the dump op has no policy and no maxattr.

> diff --git a/Documentation/netlink/specs/dpll.yaml b/Documentation/netlink/specs/dpll.yaml
> index 93e546ba1f56d..e2ca4df5699ae 100644
> --- a/Documentation/netlink/specs/dpll.yaml
> +++ b/Documentation/netlink/specs/dpll.yaml
> @@ -701,11 +701,9 @@ operations:
>        doc: |
>          Get list of pins and its attributes.
>  
> -        - dump request without any attributes given - list all the pins in the
> -          system
> -        - dump request with target dpll - list all the pins registered with
> -          a given dpll device
> -        - do request with target dpll and target pin - single pin attributes
> +        - dump request - list all the registered, available pins in the
> +          system, the request takes no attributes
> +        - do request with target pin - single pin attributes

[Severity: Low]
This isn't a bug introduced by this patch, but with the YAML wording now
saying "registered, available pins", does the prose in
Documentation/driver-api/dpll.rst end up contradicting it?

In the "Pin object" section it still says:

  It is also possible to list all the pins that were registered in the
  system with `dump` request of ``DPLL_CMD_PIN_GET`` command.

and in the "Netlink dump requests" section:

  The ``DPLL_CMD_DEVICE_GET`` and ``DPLL_CMD_PIN_GET`` commands are
  capable of dump type netlink requests, in which case the response is in
  the same format as for their ``do`` request, but every device or pin
  registered in the system is returned.

dpll_nl_pin_get_dumpit() does not emit every registered pin:

	xa_for_each_marked_start(&dpll_pin_xa, i, pin, DPLL_REGISTERED,
				 ctx->idx) {
		if (!dpll_pin_available(pin))
			continue;

The same file is already inconsistent with itself elsewhere, where it
describes the command as "command to get pin info or dump list of
available pins". Would it make sense to fix the availability wording in
dpll.rst here too, since this series is about doc accuracy?

>        attribute-set: pin
>        flags: [admin-perm]
>  
> @@ -742,9 +740,6 @@ operations:
>              - measured-frequency
>  
>        dump:
> -        request:
> -          attributes:
> -            - id
>          reply: *pin-attrs
>  
>      -
> diff --git a/drivers/dpll/dpll_nl.c b/drivers/dpll/dpll_nl.c
> index 3bdabbf2b9319..fa09b524a4a0f 100644
> --- a/drivers/dpll/dpll_nl.c
> +++ b/drivers/dpll/dpll_nl.c
> @@ -65,12 +65,7 @@ static const struct nla_policy dpll_pin_id_get_nl_policy[DPLL_A_PIN_TYPE + 1] =
>  };
>  
>  /* DPLL_CMD_PIN_GET - do */
> -static const struct nla_policy dpll_pin_get_do_nl_policy[DPLL_A_PIN_ID + 1] = {
> -	[DPLL_A_PIN_ID] = { .type = NLA_U32, },
> -};
> -
> -/* DPLL_CMD_PIN_GET - dump */
> -static const struct nla_policy dpll_pin_get_dump_nl_policy[DPLL_A_PIN_ID + 1] = {
> +static const struct nla_policy dpll_pin_get_nl_policy[DPLL_A_PIN_ID + 1] = {
>  	[DPLL_A_PIN_ID] = { .type = NLA_U32, },
>  };
>  

[ ... ]

> @@ -133,16 +128,14 @@ static const struct genl_split_ops dpll_nl_ops[] = {
>  		.pre_doit	= dpll_pin_pre_doit,
>  		.doit		= dpll_nl_pin_get_doit,
>  		.post_doit	= dpll_pin_post_doit,
> -		.policy		= dpll_pin_get_do_nl_policy,
> +		.policy		= dpll_pin_get_nl_policy,
>  		.maxattr	= DPLL_A_PIN_ID,
>  		.flags		= GENL_ADMIN_PERM | GENL_CMD_CAP_DO,
>  	},
>  	{
> -		.cmd		= DPLL_CMD_PIN_GET,
> -		.dumpit		= dpll_nl_pin_get_dumpit,
> -		.policy		= dpll_pin_get_dump_nl_policy,
> -		.maxattr	= DPLL_A_PIN_ID,
> -		.flags		= GENL_ADMIN_PERM | GENL_CMD_CAP_DUMP,
> +		.cmd	= DPLL_CMD_PIN_GET,
> +		.dumpit	= dpll_nl_pin_get_dumpit,
> +		.flags	= GENL_ADMIN_PERM | GENL_CMD_CAP_DUMP,
>  	},

[Severity: Medium]
Is this a uAPI regression for callers generated from the previously
published spec?

With no .policy and no .maxattr on this dump entry of dpll_nl_ops[],
genl_get_cmd_split() -> genl_op_fill_in_reject_policy_split() substitutes
the reject-all policy:

net/netlink/genetlink.c:genl_op_fill_in_reject_policy_split() {
	if (op->policy)
		return;

	op->policy = genl_policy_reject_all;
}

and genl_start() -> genl_family_rcv_msg_attrs_parse() then takes the
maxattr == 0 path:

	} else {
		/* Reject all policy, __nlmsg_parse() will just validate */
		attrbuf = NULL;
	}

	err = __nlmsg_parse(nlh, hdrlen, attrbuf, ops->maxattr, ops->policy,
			    validate, extack);

dpll's generated ops do not set GENL_DONT_VALIDATE_DUMP_STRICT, so any
attribute present in a pin-get dump request is now rejected with -EINVAL.
Before this patch the dump op carried dpll_pin_get_dump_nl_policy with
maxattr DPLL_A_PIN_ID, so a request carrying DPLL_A_PIN_ID parsed fine and
dpll_nl_pin_get_dumpit() returned the whole pin list.

The commit message acknowledges this:

  Same caveat as for pin-set: sending DPLL_A_PIN_ID in a pin-get dump now
  fails validation instead of being ignored.

Any existing binary built from the earlier dpll.yaml that fills in the id
field of the pin-get dump request stops working after a kernel upgrade.
Would keeping the dump policy (so the attribute stays accepted and
ignored) while dropping it from the published spec avoid breaking those
callers? The command does require CAP_NET_ADMIN, so the exposure is
limited to privileged callers issuing a dump that never filtered anything.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260904191023.3869690-1-kuba%40kernel.org

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next 0/4] dpll: fix lies in the Netlink spec
  2026-09-04 19:10 [PATCH net-next 0/4] dpll: fix lies in the Netlink spec Jakub Kicinski
                   ` (3 preceding siblings ...)
  2026-09-04 19:10 ` [PATCH net-next 4/4] MAINTAINERS: dpll: add the netlink spec Jakub Kicinski
@ 2026-09-09 21:00 ` patchwork-bot+netdevbpf
  4 siblings, 0 replies; 12+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-09-09 21:00 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms,
	vadim.fedorenko, arkadiusz.kubalewski, ivecera, jiri,
	donald.hunter

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Fri,  4 Sep 2026 12:10:19 -0700 you wrote:
> Clashiko started accumulating pre-existing issues in specs so I did
> a scan of the families. This set fixes issues discovered in dpll.
> 
> Jakub Kicinski (4):
>   netlink: specs: dpll: add clock-quality-level to the device-get reply
>   netlink: specs: dpll: drop the pin-set attributes nothing implements
>   netlink: specs: dpll: pin-get has no dump filter
>   MAINTAINERS: dpll: add the netlink spec
> 
> [...]

Here is the summary with links:
  - [net-next,1/4] netlink: specs: dpll: add clock-quality-level to the device-get reply
    https://git.kernel.org/netdev/net-next/c/0274de8b9fdc
  - [net-next,2/4] netlink: specs: dpll: drop the pin-set attributes nothing implements
    https://git.kernel.org/netdev/net-next/c/28f363015aea
  - [net-next,3/4] netlink: specs: dpll: pin-get has no dump filter
    https://git.kernel.org/netdev/net-next/c/f4ac7c7a846d
  - [net-next,4/4] MAINTAINERS: dpll: add the netlink spec
    https://git.kernel.org/netdev/net-next/c/356635525f5e

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2026-09-09 21:01 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-04 19:10 [PATCH net-next 0/4] dpll: fix lies in the Netlink spec Jakub Kicinski
2026-09-04 19:10 ` [PATCH net-next 1/4] netlink: specs: dpll: add clock-quality-level to the device-get reply Jakub Kicinski
2026-09-05 11:19   ` Vadim Fedorenko
2026-09-08  7:13   ` netdev-bot+sashiko
2026-09-04 19:10 ` [PATCH net-next 2/4] netlink: specs: dpll: drop the pin-set attributes nothing implements Jakub Kicinski
2026-09-06 11:44   ` Vadim Fedorenko
2026-09-04 19:10 ` [PATCH net-next 3/4] netlink: specs: dpll: pin-get has no dump filter Jakub Kicinski
2026-09-06 11:44   ` Vadim Fedorenko
2026-09-08  7:13   ` netdev-bot+sashiko
2026-09-04 19:10 ` [PATCH net-next 4/4] MAINTAINERS: dpll: add the netlink spec Jakub Kicinski
2026-09-06 11:45   ` Vadim Fedorenko
2026-09-09 21:00 ` [PATCH net-next 0/4] dpll: fix lies in the Netlink spec patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).