From 16812d9674d4991ebbae80769b15f7342dbfa988 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Fri, 24 Jul 2026 14:07:54 -0700 Subject: [PATCH 1/3] net_shaper: remove incorrect comment about group leaves It is true that the user-facing group() operation can only be invoked with queues as leaves (see net_shaper_parse_leaf()), but the driver facing op is also called when we delete a node. When we delete a node we conceptually call group(parent, node.list_of_leaves) to add node's leaves to the parent. Node deletion "mid-hierarchy" is supported so some of the leaves may themselves be nodes. Therefore the driver facing group() may be called with nodes. Remove the incorrect comment, and add a comment about differences between the Netlink API and driver facing API. Link: https://patch.msgid.link/20260724210756.1553565-2-kuba@kernel.org Signed-off-by: Jakub Kicinski --- include/net/net_shaper.h | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/include/net/net_shaper.h b/include/net/net_shaper.h index 3939b816b001..0fcca29207ac 100644 --- a/include/net/net_shaper.h +++ b/include/net/net_shaper.h @@ -72,6 +72,18 @@ struct net_shaper { * * Each shaper is uniquely identified within the device with a 'handle' * comprising the shaper scope and a scope-specific id. + * + * Driver ops vs uAPI + * ------------------ + * Members of the driver ops mirror the Netlink uAPI but driver calls do not + * map 1:1 to user calls. Drivers need to be careful when assuming that calls + * disallowed at the uAPI level will never be made at the driver level. + * The shaper core performs automatic reparenting and cleanup, generating + * additional calls. Notably: + * - @group calls in the driver facing API may have nodes as leaves (user is + * only allowed to construct groups with queues as leaves) + * - @group calls may update leaf's parent if the parent is about + * to be removed (re-parenting nodes explicitly is not supported in the uAPI) */ struct net_shaper_ops { /** @@ -82,7 +94,6 @@ struct net_shaper_ops { * The @leaves arrays size is specified by @leaves_count. * Create either the @leaves and the @node shaper; or if they already * exists, links them together in the desired way. - * @leaves scope must be NET_SHAPER_SCOPE_QUEUE. */ int (*group)(struct net_shaper_binding *binding, int leaves_count, const struct net_shaper *leaves, From 26bc4cfb17374f69717970699b4ccbdb1fd2a027 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Fri, 24 Jul 2026 14:07:55 -0700 Subject: [PATCH 2/3] net_shaper: clarify the kernel API / comments The shaper API takes some getting used to. Try to improve the doc on struct net_shaper_ops to help driver developers. Link: https://patch.msgid.link/20260724210756.1553565-3-kuba@kernel.org Signed-off-by: Jakub Kicinski --- include/net/net_shaper.h | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/include/net/net_shaper.h b/include/net/net_shaper.h index 0fcca29207ac..c14eb87efe5e 100644 --- a/include/net/net_shaper.h +++ b/include/net/net_shaper.h @@ -68,7 +68,7 @@ struct net_shaper { * The operations are serialized via a per device lock. * * Device not supporting any kind of nesting should not provide the - * group operation. + * @group operation. * * Each shaper is uniquely identified within the device with a 'handle' * comprising the shaper scope and a scope-specific id. @@ -84,16 +84,30 @@ struct net_shaper { * only allowed to construct groups with queues as leaves) * - @group calls may update leaf's parent if the parent is about * to be removed (re-parenting nodes explicitly is not supported in the uAPI) + * + * Implicit creation + * ----------------- + * Shapers are created implicitly, meaning that @set and @group operations + * are called both for existing and new shapers. The driver has to infer + * whether the operation is an update or a creation by tracking the handles. + * Removal of shapers is explicit and done with a @delete call. + * + * The @set operation implicitly creates NET_SHAPER_SCOPE_NETDEV and + * NET_SHAPER_SCOPE_QUEUE shapers. + * The @group operation implicitly creates NET_SHAPER_SCOPE_NETDEV and + * NET_SHAPER_SCOPE_NODE shapers (the group shaper itself), as well as + * NET_SHAPER_SCOPE_QUEUE shapers (leaves). */ struct net_shaper_ops { /** - * @group: create the specified shapers scheduling group + * @group: create a scheduling group or add leaves * - * Nest the @leaves shapers identified under the * @node shaper. + * Nest the @leaves shapers identified under the @node shaper. * All the shapers belong to the device specified by @binding. - * The @leaves arrays size is specified by @leaves_count. - * Create either the @leaves and the @node shaper; or if they already - * exists, links them together in the desired way. + * The @leaves array's size is specified by @leaves_count. + * + * @node and @leaves may or may not already exist + * (see the "Implicit creation" note). */ int (*group)(struct net_shaper_binding *binding, int leaves_count, const struct net_shaper *leaves, From ff6461c1483420d0da542ff085dbd94e841afc1a Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Fri, 24 Jul 2026 14:07:56 -0700 Subject: [PATCH 3/3] net_shaper: add some notes on re-parenting Clarify the re-parenting expectations. Specifically that @delete on a queue removes it from the hierarchy which is a bit unusual in the overall API structure. IIRC the implicit delete behavior was introduced because otherwise it would not be possible to remove a queue from the hierarchy without changing at least one handle of the shapers. Normally "removal" is done by "adding" to the new parent, but "outside the hierarchy" does not have a parent we can point at. Link: https://patch.msgid.link/20260724210756.1553565-4-kuba@kernel.org Signed-off-by: Jakub Kicinski --- include/net/net_shaper.h | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/include/net/net_shaper.h b/include/net/net_shaper.h index c14eb87efe5e..05cb625b0fe5 100644 --- a/include/net/net_shaper.h +++ b/include/net/net_shaper.h @@ -107,7 +107,12 @@ struct net_shaper_ops { * The @leaves array's size is specified by @leaves_count. * * @node and @leaves may or may not already exist - * (see the "Implicit creation" note). + * (see the "Implicit creation" note). If @node already exists, + * the @leaves should be *added* to its children. In this case, + * the @leaves array only holds new/modified leaves, not the full list. + * + * Re-parenting @leaves is implemented by a @group call on a new parent. + * There's no explicit call to remove the children from the old parent. */ int (*group)(struct net_shaper_binding *binding, int leaves_count, const struct net_shaper *leaves, @@ -128,6 +133,13 @@ struct net_shaper_ops { * * Removes the shaper configuration as identified by the given @handle * on the device specified by @binding, restoring the default behavior. + * + * Note that a @delete call on a NET_SHAPER_SCOPE_QUEUE shaper also + * implicitly removes the associated queue from the scheduling + * hierarchy. The driver must take care of that step. + * @delete calls on NET_SHAPER_SCOPE_NODE should not require any + * implicit re-parenting in the driver as core will re-parent the leaves + * first, before deleting the SCOPE_NODE shaper. */ int (*delete)(struct net_shaper_binding *binding, const struct net_shaper_handle *handle,