github-mirror/linux
1
0
Fork 0
mirror of https://github.com/torvalds/linux.git synced 2026-05-24 07:03:03 +02:00
Code Issues Actions Packages Projects Releases Wiki Activity Graph

Merge branch 'net-shaper-fix-various-minor-bugs'

Browse Source 
Jakub Kicinski says:

====================
net: shaper: fix various minor bugs

Fix various minor bugs in the net shaper API.

First 2 patches deal with ordering issues around inserting
and publishing new shapers. Shapers are inserted "tentatively"
and marked valid only after HW op succeeded, this used to
be slightly racy.

Only other patch of note is patch 8. We want to add a Netlink
policy check on the handle ID. This necessitates patch 7.

The rest are simple and self-explanatory.

v1: https://lore.kernel.org/20260506000628.1501691-1-kuba@kernel.org
====================

Link: https://patch.msgid.link/20260510192904.3987113-1-kuba@kernel.org
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
This commit is contained in:
Paolo Abeni 2026-05-12 16:15:03 +02:00
commit 9988931df9
10 changed files with 200 additions and 43 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
Documentation/netlink/genetlink-c.yaml
View File

@ -69,6 +69,15 @@ properties:
header:
description: For C-compatible languages, header which already defines this value.
type: string
scope:
description: |
Visibility of this definition. "uapi" (default) renders into
the uAPI header, "kernel" renders into the kernel-side
generated header, "user" renders into the user-side
generated header. When combined with `header:`, the
definition is not rendered, and the named header is
included only by code matching the scope.
enum: [ uapi, kernel, user ]
type:
enum: [ const, enum, flags ]
doc:

9
Documentation/netlink/genetlink-legacy.yaml
View File

@ -83,6 +83,15 @@ properties:
header:
description: For C-compatible languages, header which already defines this value.
type: string
scope:
description: |
Visibility of this definition. "uapi" (default) renders into
the uAPI header, "kernel" renders into the kernel-side
generated header, "user" renders into the user-side
generated header. When combined with `header:`, the
definition is not rendered, and the named header is
included only by code matching the scope.
enum: [ uapi, kernel, user ]
type:
enum: [ const, enum, flags, struct ] # Trim
doc:

9
Documentation/netlink/genetlink.yaml
View File

@ -55,6 +55,15 @@ properties:
header:
description: For C-compatible languages, header which already defines this value.
type: string
scope:
description: |
Visibility of this definition. "uapi" (default) renders into
the uAPI header, "kernel" renders into the kernel-side
generated header, "user" renders into the user-side
generated header. When combined with `header:`, the
definition is not rendered, and the named header is
included only by code matching the scope.
enum: [ uapi, kernel, user ]
type:
enum: [ const, enum, flags ]
doc:

9
Documentation/netlink/netlink-raw.yaml
View File

@ -87,6 +87,15 @@ properties:
header:
description: For C-compatible languages, header which already defines this value.
type: string
scope:
description: |
Visibility of this definition. "uapi" (default) renders into
the uAPI header, "kernel" renders into the kernel-side
generated header, "user" renders into the user-side
generated header. When combined with `header:`, the
definition is not rendered, and the named header is
included only by code matching the scope.
enum: [ uapi, kernel, user ]
type:
enum: [ const, enum, flags, struct ] # Trim
doc:

7
Documentation/netlink/specs/net_shaper.yaml
View File

@ -33,6 +33,11 @@ doc: |
@cap-get operation.
definitions:
-
type: const
name: max-handle-id
value: 0x3fffffe
scope: kernel
-
type: enum
name: scope
@ -140,6 +145,8 @@ attribute-sets:
-
name: id
type: u32
checks:
max: max-handle-id
doc: |
Numeric identifier of a shaper. The id semantic depends on
the scope. For @queue scope it's the queue id and for @node

136
net/shaper/shaper.c
View File

@ -21,6 +21,8 @@
#define NET_SHAPER_ID_UNSPEC NET_SHAPER_ID_MASK
static_assert(NET_SHAPER_ID_UNSPEC == NET_SHAPER_MAX_HANDLE_ID + 1);
struct net_shaper_hierarchy {
struct xarray shapers;
};
@ -90,6 +92,12 @@ static int net_shaper_handle_size(void)
nla_total_size(sizeof(u32)));
}
static int net_shaper_group_reply_size(void)
{
return nla_total_size(sizeof(u32)) + /* NET_SHAPER_A_IFINDEX */
net_shaper_handle_size(); /* NET_SHAPER_A_HANDLE */
}
static int net_shaper_fill_binding(struct sk_buff *msg,
const struct net_shaper_binding *binding,
u32 type)
@ -275,11 +283,13 @@ static void net_shaper_default_parent(const struct net_shaper_handle *handle,
parent->id = 0;
}
/*
* MARK_0 is already in use due to XA_FLAGS_ALLOC, can't reuse such flag as
* it's cleared by xa_store().
/* MARK_0 is already in use due to XA_FLAGS_ALLOC. The VALID mark is set on
* an entry only after the device-side configuration has completed
* successfully (see net_shaper_commit()). Lookups and dumps must filter on
* this mark to avoid exposing tentative entries inserted by
* net_shaper_pre_insert() while the driver call is still in flight.
*/
#define NET_SHAPER_NOT_VALID XA_MARK_1
#define NET_SHAPER_VALID XA_MARK_1
static struct net_shaper *
net_shaper_lookup(struct net_shaper_binding *binding,
@ -289,10 +299,14 @@ net_shaper_lookup(struct net_shaper_binding *binding,
struct net_shaper_hierarchy *hierarchy;
hierarchy = net_shaper_hierarchy_rcu(binding);
if (!hierarchy || xa_get_mark(&hierarchy->shapers, index,
NET_SHAPER_NOT_VALID))
if (!hierarchy || !xa_get_mark(&hierarchy->shapers, index,
NET_SHAPER_VALID))
return NULL;
/* Pairs with smp_wmb() in net_shaper_commit(): if the entry is
* valid, its contents must be visible too.
*/
smp_rmb();
return xa_load(&hierarchy->shapers, index);
}
@ -348,7 +362,7 @@ static int net_shaper_pre_insert(struct net_shaper_binding *binding,
handle->id == NET_SHAPER_ID_UNSPEC) {
u32 min, max;
handle->id = NET_SHAPER_ID_MASK - 1;
handle->id = NET_SHAPER_MAX_HANDLE_ID;
max = net_shaper_handle_to_index(handle);
handle->id = 0;
min = net_shaper_handle_to_index(handle);
@ -370,13 +384,10 @@ static int net_shaper_pre_insert(struct net_shaper_binding *binding,
goto free_id;
}
/* Mark 'tentative' shaper inside the hierarchy container.
* xa_set_mark is a no-op if the previous store fails.
/* Insert as 'tentative' (no VALID mark). The mark will be set by
* net_shaper_commit() once the driver-side configuration succeeds.
*/
xa_lock(&hierarchy->shapers);
prev = __xa_store(&hierarchy->shapers, index, cur, GFP_KERNEL);
__xa_set_mark(&hierarchy->shapers, index, NET_SHAPER_NOT_VALID);
xa_unlock(&hierarchy->shapers);
prev = xa_store(&hierarchy->shapers, index, cur, GFP_KERNEL);
if (xa_err(prev)) {
NL_SET_ERR_MSG(extack, "Can't insert shaper into device store");
kfree_rcu(cur, rcu);
@ -413,9 +424,9 @@ static void net_shaper_commit(struct net_shaper_binding *binding,
/* Successful update: drop the tentative mark
* and update the hierarchy container.
*/
__xa_clear_mark(&hierarchy->shapers, index,
NET_SHAPER_NOT_VALID);
*cur = shapers[i];
smp_wmb();
__xa_set_mark(&hierarchy->shapers, index, NET_SHAPER_VALID);
}
xa_unlock(&hierarchy->shapers);
}
@ -431,8 +442,9 @@ static void net_shaper_rollback(struct net_shaper_binding *binding)
return;
xa_lock(&hierarchy->shapers);
xa_for_each_marked(&hierarchy->shapers, index, cur,
NET_SHAPER_NOT_VALID) {
xa_for_each(&hierarchy->shapers, index, cur) {
if (xa_get_mark(&hierarchy->shapers, index, NET_SHAPER_VALID))
continue;
__xa_erase(&hierarchy->shapers, index);
kfree(cur);
}
@ -465,10 +477,21 @@ static int net_shaper_parse_handle(const struct nlattr *attr,
* shaper (any other value).
*/
id_attr = tb[NET_SHAPER_A_HANDLE_ID];
if (id_attr)
if (id_attr) {
id = nla_get_u32(id_attr);
else if (handle->scope == NET_SHAPER_SCOPE_NODE)
} else if (handle->scope == NET_SHAPER_SCOPE_NODE) {
id = NET_SHAPER_ID_UNSPEC;
} else if (handle->scope == NET_SHAPER_SCOPE_QUEUE) {
NL_SET_ERR_ATTR_MISS(info->extack, attr,
NET_SHAPER_A_HANDLE_ID);
return -EINVAL;
}
if (id && handle->scope == NET_SHAPER_SCOPE_NETDEV) {
NL_SET_ERR_MSG_ATTR(info->extack, id_attr,
"Netdev scope is a singleton, must use ID 0");
return -EINVAL;
}
handle->id = id;
return 0;
@ -836,7 +859,12 @@ int net_shaper_nl_get_dumpit(struct sk_buff *skb,
goto out_unlock;
for (; (shaper = xa_find(&hierarchy->shapers, &ctx->start_index,
U32_MAX, XA_PRESENT)); ctx->start_index++) {
U32_MAX, NET_SHAPER_VALID));
ctx->start_index++) {
/* Pairs with smp_wmb() in net_shaper_commit(): the entry
* is marked VALID, so its contents must be visible too.
*/
smp_rmb();
ret = net_shaper_fill_one(skb, binding, shaper, info);
if (ret)
break;
@ -932,6 +960,46 @@ static int net_shaper_handle_cmp(const struct net_shaper_handle *a,
return memcmp(a, b, sizeof(*a));
}
static int net_shaper_parse_leaves(struct net_shaper_binding *binding,
struct genl_info *info,
const struct net_shaper *node,
struct net_shaper *leaves,
int leaves_count)
{
struct nlattr *attr;
int i, j, ret, rem;
i = 0;
nla_for_each_attr_type(attr, NET_SHAPER_A_LEAVES,
genlmsg_data(info->genlhdr),
genlmsg_len(info->genlhdr), rem) {
if (WARN_ON_ONCE(i >= leaves_count))
return -EINVAL;
ret = net_shaper_parse_leaf(binding, attr, info,
node, &leaves[i]);
if (ret)
return ret;
/* Reject duplicates */
for (j = 0; j < i; j++) {
if (net_shaper_handle_cmp(&leaves[i].handle,
&leaves[j].handle))
continue;
NL_SET_ERR_MSG_ATTR_FMT(info->extack, attr,
"Duplicate leaf shaper %d:%d",
leaves[i].handle.scope,
leaves[i].handle.id);
return -EINVAL;
}
i++;
}
return 0;
}
static int net_shaper_parent_from_leaves(int leaves_count,
const struct net_shaper *leaves,
struct net_shaper *node,
@ -1178,7 +1246,7 @@ static int net_shaper_group_send_reply(struct net_shaper_binding *binding,
free_msg:
/* Should never happen as msg is pre-allocated with enough space. */
WARN_ONCE(true, "calculated message payload length (%d)",
net_shaper_handle_size());
net_shaper_group_reply_size());
nlmsg_free(msg);
return -EMSGSIZE;
}
@ -1188,10 +1256,9 @@ int net_shaper_nl_group_doit(struct sk_buff *skb, struct genl_info *info)
struct net_shaper **old_nodes, *leaves, node = {};
struct net_shaper_hierarchy *hierarchy;
struct net_shaper_binding *binding;
int i, ret, rem, leaves_count;
int i, ret, leaves_count;
int old_nodes_count = 0;
struct sk_buff *msg;
struct nlattr *attr;
if (GENL_REQ_ATTR_CHECK(info, NET_SHAPER_A_LEAVES))
return -EINVAL;
@ -1219,26 +1286,19 @@ int net_shaper_nl_group_doit(struct sk_buff *skb, struct genl_info *info)
if (ret)
goto free_leaves;
i = 0;
nla_for_each_attr_type(attr, NET_SHAPER_A_LEAVES,
genlmsg_data(info->genlhdr),
genlmsg_len(info->genlhdr), rem) {
if (WARN_ON_ONCE(i >= leaves_count))
goto free_leaves;
ret = net_shaper_parse_leaf(binding, attr, info,
&node, &leaves[i]);
if (ret)
goto free_leaves;
i++;
}
ret = net_shaper_parse_leaves(binding, info, &node,
leaves, leaves_count);
if (ret)
goto free_leaves;
/* Prepare the msg reply in advance, to avoid device operation
* rollback on allocation failure.
*/
msg = genlmsg_new(net_shaper_handle_size(), GFP_KERNEL);
if (!msg)
msg = genlmsg_new(net_shaper_group_reply_size(), GFP_KERNEL);
if (!msg) {
ret = -ENOMEM;
goto free_leaves;
}
hierarchy = net_shaper_hierarchy_setup(binding);
if (!hierarchy) {

7
net/shaper/shaper_nl_gen.c
View File

@ -11,10 +11,15 @@
#include <uapi/linux/net_shaper.h>
/* Integer value ranges */
static const struct netlink_range_validation net_shaper_a_handle_id_range = {
.max = NET_SHAPER_MAX_HANDLE_ID,
};
/* Common nested types */
const struct nla_policy net_shaper_handle_nl_policy[NET_SHAPER_A_HANDLE_ID + 1] = {
[NET_SHAPER_A_HANDLE_SCOPE] = NLA_POLICY_MAX(NLA_U32, 3),
[NET_SHAPER_A_HANDLE_ID] = { .type = NLA_U32, },
[NET_SHAPER_A_HANDLE_ID] = NLA_POLICY_FULL_RANGE(NLA_U32, &net_shaper_a_handle_id_range),
};
const struct nla_policy net_shaper_leaf_info_nl_policy[NET_SHAPER_A_WEIGHT + 1] = {

2
net/shaper/shaper_nl_gen.h
View File

@ -12,6 +12,8 @@
#include <uapi/linux/net_shaper.h>
#define NET_SHAPER_MAX_HANDLE_ID 67108862
/* Common nested types */
extern const struct nla_policy net_shaper_handle_nl_policy[NET_SHAPER_A_HANDLE_ID + 1];
extern const struct nla_policy net_shaper_leaf_info_nl_policy[NET_SHAPER_A_WEIGHT + 1];

31
tools/net/ynl/pyynl/ynl_gen_c.py
View File

@ -3212,6 +3212,8 @@ def render_uapi(family, cw):
for const in family['definitions']:
if const.get('header'):
continue
if const.get('scope', 'uapi') != 'uapi':
continue
if const['type'] != 'const':
cw.writes_defines(defines)
@ -3339,6 +3341,25 @@ def render_uapi(family, cw):
cw.p(f'#endif /* {hdr_prot} */')
def render_scoped_consts(family, cw, scope):
defines = []
for const in family['definitions']:
if const['type'] != 'const':
continue
if const.get('header'):
continue
if const.get('scope') != scope:
continue
name_pfx = const.get('name-prefix', f"{family.ident_name}-")
defines.append([
c_upper(family.get('c-define-name',
f"{name_pfx}{const['name']}")),
const['value']])
if defines:
cw.writes_defines(defines)
cw.nl()
def _render_user_ntf_entry(ri, op):
if not ri.family.is_classic():
ri.cw.block_start(line=f"[{op.enum_name}] = ")
@ -3504,8 +3525,12 @@ def main():
cw.p('#include "ynl.h"')
headers = []
for definition in parsed['definitions'] + parsed['attribute-sets']:
if 'header' in definition:
headers.append(definition['header'])
if 'header' not in definition:
continue
scope = definition.get('scope', 'uapi')
if scope != 'uapi' and scope != args.mode:
continue
headers.append(definition['header'])
if args.mode == 'user':
headers.append(parsed.uapi_header)
seen_header = []
@ -3522,6 +3547,7 @@ def main():
for one in args.user_header:
cw.p(f'#include "{one}"')
else:
render_scoped_consts(parsed, cw, 'user')
cw.p('struct ynl_sock;')
cw.nl()
render_user_family(parsed, cw, True)
@ -3529,6 +3555,7 @@ def main():
if args.mode == "kernel":
if args.header:
render_scoped_consts(parsed, cw, 'kernel')
for _, struct in sorted(parsed.pure_nested_structs.items()):
if struct.request:
cw.p('/* Common nested types */')

24
tools/testing/selftests/drivers/net/shaper.py
View File

@ -1,7 +1,10 @@
#!/usr/bin/env python3
# SPDX-License-Identifier: GPL-2.0
from lib.py import ksft_run, ksft_exit, ksft_eq, ksft_true, KsftSkipEx
import errno
from lib.py import ksft_run, ksft_exit
from lib.py import ksft_eq, ksft_raises, ksft_true, KsftSkipEx
from lib.py import EthtoolFamily, NetshaperFamily
from lib.py import NetDrvEnv
from lib.py import NlError
@ -438,6 +441,21 @@ def queue_update(cfg, nl_shaper) -> None:
nl_shaper.delete({'ifindex': cfg.ifindex,
'handle': {'scope': 'queue', 'id': i}})
def dup_leaves(cfg, nl_shaper) -> None:
""" Ensure that the kernel rejects duplicate leaves. """
if not cfg.groups:
raise KsftSkipEx("device does not support node scope")
with ksft_raises(NlError) as cm:
nl_shaper.group({
'ifindex': cfg.ifindex,
'leaves':[{'handle': {'scope': 'queue', 'id': 0}},
{'handle': {'scope': 'queue', 'id': 0}}],
'handle': {'scope':'node'},
'metric': 'bps',
'bw-max': 10000})
ksft_eq(cm.exception.error, errno.EINVAL)
def main() -> None:
with NetDrvEnv(__file__, queue_count=4) as cfg:
cfg.queues = False
@ -453,7 +471,9 @@ def main() -> None:
basic_groups,
qgroups,
delegation,
queue_update], args=(cfg, NetshaperFamily()))
dup_leaves,
queue_update],
args=(cfg, NetshaperFamily()))
ksft_exit()