[PATCH net-next] net: shaper: Reject zero weight in shaper config

Mohsin Bashir posted 1 patch 2 months ago
Documentation/netlink/specs/net_shaper.yaml | 2 ++
net/shaper/shaper_nl_gen.c                  | 6 +++---
2 files changed, 5 insertions(+), 3 deletions(-)
[PATCH net-next] net: shaper: Reject zero weight in shaper config
Posted by Mohsin Bashir 2 months ago
A zero weight is meaningless for DWRR scheduling and can cause
starvation of the affected node. Add a min-value constraint to
the weight attribute in the net_shaper netlink spec so that zero
is rejected at the netlink policy level.

Found while prototyping a new driver, existing drivers are not
affected.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Mohsin Bashir <hmohsin@meta.com>
---
 Documentation/netlink/specs/net_shaper.yaml | 2 ++
 net/shaper/shaper_nl_gen.c                  | 6 +++---
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/Documentation/netlink/specs/net_shaper.yaml b/Documentation/netlink/specs/net_shaper.yaml
index 3f2ad772b64b..7216568fcfc5 100644
--- a/Documentation/netlink/specs/net_shaper.yaml
+++ b/Documentation/netlink/specs/net_shaper.yaml
@@ -106,6 +106,8 @@ attribute-sets:
       -
         name: weight
         type: u32
+        checks:
+          min: 1
         doc: |
           Relative weight for round robin scheduling of the
           given shaper.
diff --git a/net/shaper/shaper_nl_gen.c b/net/shaper/shaper_nl_gen.c
index 9b29be3ef19a..0cad1a355350 100644
--- a/net/shaper/shaper_nl_gen.c
+++ b/net/shaper/shaper_nl_gen.c
@@ -20,7 +20,7 @@ const struct nla_policy net_shaper_handle_nl_policy[NET_SHAPER_A_HANDLE_ID + 1]
 const struct nla_policy net_shaper_leaf_info_nl_policy[NET_SHAPER_A_WEIGHT + 1] = {
 	[NET_SHAPER_A_HANDLE] = NLA_POLICY_NESTED(net_shaper_handle_nl_policy),
 	[NET_SHAPER_A_PRIORITY] = { .type = NLA_U32, },
-	[NET_SHAPER_A_WEIGHT] = { .type = NLA_U32, },
+	[NET_SHAPER_A_WEIGHT] = NLA_POLICY_MIN(NLA_U32, 1),
 };
 
 /* NET_SHAPER_CMD_GET - do */
@@ -43,7 +43,7 @@ static const struct nla_policy net_shaper_set_nl_policy[NET_SHAPER_A_IFINDEX + 1
 	[NET_SHAPER_A_BW_MAX] = { .type = NLA_UINT, },
 	[NET_SHAPER_A_BURST] = { .type = NLA_UINT, },
 	[NET_SHAPER_A_PRIORITY] = { .type = NLA_U32, },
-	[NET_SHAPER_A_WEIGHT] = { .type = NLA_U32, },
+	[NET_SHAPER_A_WEIGHT] = NLA_POLICY_MIN(NLA_U32, 1),
 };
 
 /* NET_SHAPER_CMD_DELETE - do */
@@ -62,7 +62,7 @@ static const struct nla_policy net_shaper_group_nl_policy[NET_SHAPER_A_LEAVES +
 	[NET_SHAPER_A_BW_MAX] = { .type = NLA_UINT, },
 	[NET_SHAPER_A_BURST] = { .type = NLA_UINT, },
 	[NET_SHAPER_A_PRIORITY] = { .type = NLA_U32, },
-	[NET_SHAPER_A_WEIGHT] = { .type = NLA_U32, },
+	[NET_SHAPER_A_WEIGHT] = NLA_POLICY_MIN(NLA_U32, 1),
 	[NET_SHAPER_A_LEAVES] = NLA_POLICY_NESTED(net_shaper_leaf_info_nl_policy),
 };
 
-- 
2.52.0
Re: [PATCH net-next] net: shaper: Reject zero weight in shaper config
Posted by Jakub Kicinski 2 months ago
On Fri, 10 Apr 2026 15:51:23 -0700 Mohsin Bashir wrote:
> A zero weight is meaningless for DWRR scheduling and can cause
> starvation of the affected node. Add a min-value constraint to
> the weight attribute in the net_shaper netlink spec so that zero
> is rejected at the netlink policy level.
> 
> Found while prototyping a new driver, existing drivers are not
> affected.

AI review points out that if the netlink attr is not present core will
leave the DWRR weight as 0 in the struct. I guess we need to think this
thru a little more carefully. What should the "default" weight be?
What if user specifies weights only for subset of leaves?

This part of the uAPI seems under-defined.

Maybe a better adjustment would be to make core set the weight to 1
automatically if the user has not defined it? Only when sending it to
the driver tho, because we'd still want it to not be reported back to
user space. Not sure how hairy it'd get code-wise.
Re: [PATCH net-next] net: shaper: Reject zero weight in shaper config
Posted by Mohsin Bashir 2 months ago

On 4/13/26 2:50 PM, Jakub Kicinski wrote:
> On Fri, 10 Apr 2026 15:51:23 -0700 Mohsin Bashir wrote:
>> A zero weight is meaningless for DWRR scheduling and can cause
>> starvation of the affected node. Add a min-value constraint to
>> the weight attribute in the net_shaper netlink spec so that zero
>> is rejected at the netlink policy level.
>>
>> Found while prototyping a new driver, existing drivers are not
>> affected.
> 
> AI review points out that if the netlink attr is not present core will
> leave the DWRR weight as 0 in the struct. I guess we need to think this
> thru a little more carefully. What should the "default" weight be?
> What if user specifies weights only for subset of leaves?
> 
> This part of the uAPI seems under-defined.
> 
> Maybe a better adjustment would be to make core set the weight to 1
> automatically if the user has not defined it? Only when sending it to
> the driver tho, because we'd still want it to not be reported back to
> user space. Not sure how hairy it'd get code-wise.

Interesting!!
Let me look at the big picture here and re-spin.