* [PATCH net-next 0/5] net: introduce TX shaping H/W offload API
@ 2024-06-27 20:17 Paolo Abeni
2024-06-27 20:17 ` [PATCH net-next 1/5] netlink: spec: add shaper YAML spec Paolo Abeni
` (5 more replies)
0 siblings, 6 replies; 20+ messages in thread
From: Paolo Abeni @ 2024-06-27 20:17 UTC (permalink / raw)
To: netdev
Cc: Jakub Kicinski, Jiri Pirko, Madhu Chittim, Sridhar Samudrala,
Simon Horman, John Fastabend, Sunil Kovvuri Goutham,
Jamal Hadi Salim
We have a plurality of shaping-related drivers API, but none flexible
enough to meet existing demand from vendors[1].
This series introduces new device APIs to configure in a flexible way
TX shaping H/W offload. The new functionalities are exposed via a newly
defined generic netlink interface and include introspection
capabilities. Some basic self-tests are included, on top of a dummy
netdevsim implementation.
The ice driver support is currently a WIP, sharing the current status
earlier since some APIs details are still under discussion.
RFC: https://lore.kernel.org/netdev/3d1e2d945904a0fb55258559eb7322d7e11066b6.1715199358.git.pabeni@redhat.com/
[1] https://lore.kernel.org/netdev/20240405102313.GA310894@kernel.org/
Paolo Abeni (5):
netlink: spec: add shaper YAML spec
net: introduce HW Rate limiting driver API
netlink: spec: add shaper introspection support
net: shaper: implement introspection support
testing: net-drv: add basic shaper test
Documentation/netlink/specs/shaper.yaml | 276 +++++++
drivers/net/Kconfig | 1 +
drivers/net/netdevsim/netdev.c | 29 +
include/linux/netdevice.h | 16 +
include/net/net_shaper.h | 208 ++++++
include/uapi/linux/net_shaper.h | 90 +++
net/Kconfig | 3 +
net/Makefile | 1 +
net/core/dev.c | 2 +
net/core/dev.h | 6 +
net/shaper/Makefile | 9 +
net/shaper/shaper.c | 686 ++++++++++++++++++
net/shaper/shaper_nl_gen.c | 118 +++
net/shaper/shaper_nl_gen.h | 28 +
tools/testing/selftests/drivers/net/Makefile | 1 +
tools/testing/selftests/drivers/net/shaper.py | 198 +++++
.../testing/selftests/net/lib/py/__init__.py | 1 +
tools/testing/selftests/net/lib/py/ynl.py | 5 +
18 files changed, 1678 insertions(+)
create mode 100644 Documentation/netlink/specs/shaper.yaml
create mode 100644 include/net/net_shaper.h
create mode 100644 include/uapi/linux/net_shaper.h
create mode 100644 net/shaper/Makefile
create mode 100644 net/shaper/shaper.c
create mode 100644 net/shaper/shaper_nl_gen.c
create mode 100644 net/shaper/shaper_nl_gen.h
create mode 100755 tools/testing/selftests/drivers/net/shaper.py
--
2.45.1
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH net-next 1/5] netlink: spec: add shaper YAML spec
2024-06-27 20:17 [PATCH net-next 0/5] net: introduce TX shaping H/W offload API Paolo Abeni
@ 2024-06-27 20:17 ` Paolo Abeni
2024-06-29 2:12 ` Jakub Kicinski
2024-06-27 20:17 ` [PATCH net-next 2/5] net: introduce HW Rate limiting driver API Paolo Abeni
` (4 subsequent siblings)
5 siblings, 1 reply; 20+ messages in thread
From: Paolo Abeni @ 2024-06-27 20:17 UTC (permalink / raw)
To: netdev
Cc: Jakub Kicinski, Jiri Pirko, Madhu Chittim, Sridhar Samudrala,
Simon Horman, John Fastabend, Sunil Kovvuri Goutham,
Jamal Hadi Salim
Define the user-space visible interface to query, configure and delete
network shapers via yaml definition.
Add dummy implementations for the relevant NL callbacks.
set() and delete() operations allows touching multiple shapers with a
single operation, atomically.
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
v4 -> v5:
- fixed a few typos
- set() and get() ops reply with the number of affected handles
- re-ordered bps and pps
- added 'unspec' scope
v3 -> v4:
- dropped 'major'
- renamed 'minor' as 'id'
- rename 'bw_{max,min} as 'bw-{max,min}'
v2 -> v3:
- dropped 'move' op, use parent in 'set' instead
- expand 'handle' in 'scope', 'major', 'minor'
- rename 'queue_group' scope to 'detached'
- rename 'info' attr to 'shapers'
- added pad attribute (for 64 bits' sake)
v1 -> v2:
- reset -> delete
- added 'parent' and 'burst'
---
Documentation/netlink/specs/shaper.yaml | 202 ++++++++++++++++++++++++
include/uapi/linux/net_shaper.h | 73 +++++++++
net/Kconfig | 3 +
net/Makefile | 1 +
net/shaper/Makefile | 9 ++
net/shaper/shaper.c | 34 ++++
net/shaper/shaper_nl_gen.c | 93 +++++++++++
net/shaper/shaper_nl_gen.h | 25 +++
8 files changed, 440 insertions(+)
create mode 100644 Documentation/netlink/specs/shaper.yaml
create mode 100644 include/uapi/linux/net_shaper.h
create mode 100644 net/shaper/Makefile
create mode 100644 net/shaper/shaper.c
create mode 100644 net/shaper/shaper_nl_gen.c
create mode 100644 net/shaper/shaper_nl_gen.h
diff --git a/Documentation/netlink/specs/shaper.yaml b/Documentation/netlink/specs/shaper.yaml
new file mode 100644
index 000000000000..8563c85de68d
--- /dev/null
+++ b/Documentation/netlink/specs/shaper.yaml
@@ -0,0 +1,202 @@
+# SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause)
+
+name: net_shaper
+
+doc: Network HW Rate Limiting offload
+
+definitions:
+ -
+ type: enum
+ name: scope
+ doc: the different scopes where a shaper can be attached
+ entries:
+ - name: unspec
+ doc: The scope is not specified
+ -
+ name: port
+ doc: The root shaper for the whole H/W.
+ -
+ name: netdev
+ doc: The main shaper for the given network device.
+ -
+ name: queue
+ doc: The shaper is attached to the given device queue.
+ -
+ name: detached
+ doc: |
+ The shaper can be attached to port, netdev or other
+ detached shapers, allowing nesting and grouping of
+ netdev or queues.
+ render-max: true
+ -
+ type: enum
+ name: metric
+ doc: different metric each shaper can support
+ entries:
+ -
+ name: bps
+ doc: Shaper operates on a bits per second basis
+ -
+ name: pps
+ doc: Shaper operates on a packets per second basis
+
+attribute-sets:
+ -
+ name: net_shaper
+ attributes:
+ -
+ name: ifindex
+ type: u32
+ -
+ name: parent
+ type: nest
+ nested-attributes: handle
+ -
+ name: handle
+ type: nest
+ nested-attributes: handle
+ -
+ name: metric
+ type: u32
+ enum: metric
+ -
+ name: bw-min
+ type: u64
+ -
+ name: bw-max
+ type: u64
+ -
+ name: burst
+ type: u64
+ -
+ name: priority
+ type: u32
+ -
+ name: weight
+ type: u32
+ -
+ name: scope
+ type: u32
+ enum: scope
+ -
+ name: id
+ type: u32
+ -
+ name: handles
+ type: nest
+ multi-attr: true
+ nested-attributes: handle
+ -
+ name: shapers
+ type: nest
+ multi-attr: true
+ nested-attributes: ns-info
+ -
+ name: modified
+ type: u32
+ -
+ name: pad
+ type: pad
+ -
+ name: handle
+ subset-of: net_shaper
+ attributes:
+ -
+ name: scope
+ -
+ name: id
+ -
+ name: ns-info
+ subset-of: net_shaper
+ attributes:
+ -
+ name: parent
+ -
+ name: handle
+ -
+ name: metric
+ -
+ name: bw-min
+ -
+ name: bw-max
+ -
+ name: burst
+ -
+ name: priority
+ -
+ name: weight
+
+operations:
+ list:
+ -
+ name: get
+ doc: |
+ Get / Dump information about a/all the shaper for a given device
+ attribute-set: net_shaper
+ flags: [ admin-perm ]
+
+ do:
+ request:
+ attributes:
+ - ifindex
+ - handle
+ reply:
+ attributes: &ns-attrs
+ - parent
+ - handle
+ - metric
+ - bw-min
+ - bw-max
+ - burst
+ - priority
+ - weight
+
+ dump:
+ request:
+ attributes:
+ - ifindex
+ reply:
+ attributes: *ns-attrs
+ -
+ name: set
+ doc: |
+ Create or configures the specified shapers.
+ The update is atomic with respect to all shaper
+ affected by a single command, and is allowed to
+ affect a subset of the specified shapers, e.g.
+ due to H/W resources exhaustion. In such case
+ the update stops at the first failure, the extack
+ is set accordingly.
+ attribute-set: net_shaper
+ flags: [ admin-perm ]
+
+ do:
+ request:
+ attributes:
+ - ifindex
+ - shapers
+ reply:
+ attributes:
+ - modified
+
+ -
+ name: delete
+ doc: |
+ Clear (remove) the specified shaper.
+ The update is atomic with respect to all shaper
+ affected by a single command, and is allowed to
+ affect a subset of the specified shapers, e.g.
+ due to H/W resources exhaustion. In such case
+ the update stops at the first failure, the extack
+ is set accordingly.
+ attribute-set: net_shaper
+ flags: [ admin-perm ]
+
+ do:
+ request:
+ attributes:
+ - ifindex
+ - handles
+ reply:
+ attributes:
+ - modified
diff --git a/include/uapi/linux/net_shaper.h b/include/uapi/linux/net_shaper.h
new file mode 100644
index 000000000000..7e6b655e6c6d
--- /dev/null
+++ b/include/uapi/linux/net_shaper.h
@@ -0,0 +1,73 @@
+/* SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause) */
+/* Do not edit directly, auto-generated from: */
+/* Documentation/netlink/specs/shaper.yaml */
+/* YNL-GEN uapi header */
+
+#ifndef _UAPI_LINUX_NET_SHAPER_H
+#define _UAPI_LINUX_NET_SHAPER_H
+
+#define NET_SHAPER_FAMILY_NAME "net_shaper"
+#define NET_SHAPER_FAMILY_VERSION 1
+
+/**
+ * enum net_shaper_scope - the different scopes where a shaper can be attached
+ * @NET_SHAPER_SCOPE_UNSPEC: The scope is not specified
+ * @NET_SHAPER_SCOPE_PORT: The root shaper for the whole H/W.
+ * @NET_SHAPER_SCOPE_NETDEV: The main shaper for the given network device.
+ * @NET_SHAPER_SCOPE_QUEUE: The shaper is attached to the given device queue.
+ * @NET_SHAPER_SCOPE_DETACHED: The shaper can be attached to port, netdev or
+ * other detached shapers, allowing nesting and grouping of netdev or queues.
+ */
+enum net_shaper_scope {
+ NET_SHAPER_SCOPE_UNSPEC,
+ NET_SHAPER_SCOPE_PORT,
+ NET_SHAPER_SCOPE_NETDEV,
+ NET_SHAPER_SCOPE_QUEUE,
+ NET_SHAPER_SCOPE_DETACHED,
+
+ /* private: */
+ __NET_SHAPER_SCOPE_MAX,
+ NET_SHAPER_SCOPE_MAX = (__NET_SHAPER_SCOPE_MAX - 1)
+};
+
+/**
+ * enum net_shaper_metric - different metric each shaper can support
+ * @NET_SHAPER_METRIC_BPS: Shaper operates on a bits per second basis
+ * @NET_SHAPER_METRIC_PPS: Shaper operates on a packets per second basis
+ */
+enum net_shaper_metric {
+ NET_SHAPER_METRIC_BPS,
+ NET_SHAPER_METRIC_PPS,
+};
+
+enum {
+ NET_SHAPER_A_IFINDEX = 1,
+ NET_SHAPER_A_PARENT,
+ NET_SHAPER_A_HANDLE,
+ NET_SHAPER_A_METRIC,
+ NET_SHAPER_A_BW_MIN,
+ NET_SHAPER_A_BW_MAX,
+ NET_SHAPER_A_BURST,
+ NET_SHAPER_A_PRIORITY,
+ NET_SHAPER_A_WEIGHT,
+ NET_SHAPER_A_SCOPE,
+ NET_SHAPER_A_ID,
+ NET_SHAPER_A_HANDLES,
+ NET_SHAPER_A_SHAPERS,
+ NET_SHAPER_A_MODIFIED,
+ NET_SHAPER_A_PAD,
+
+ __NET_SHAPER_A_MAX,
+ NET_SHAPER_A_MAX = (__NET_SHAPER_A_MAX - 1)
+};
+
+enum {
+ NET_SHAPER_CMD_GET = 1,
+ NET_SHAPER_CMD_SET,
+ NET_SHAPER_CMD_DELETE,
+
+ __NET_SHAPER_CMD_MAX,
+ NET_SHAPER_CMD_MAX = (__NET_SHAPER_CMD_MAX - 1)
+};
+
+#endif /* _UAPI_LINUX_NET_SHAPER_H */
diff --git a/net/Kconfig b/net/Kconfig
index d27d0deac0bf..31fccfed04f7 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -66,6 +66,9 @@ config SKB_DECRYPTED
config SKB_EXTENSIONS
bool
+config NET_SHAPER
+ bool
+
menu "Networking options"
source "net/packet/Kconfig"
diff --git a/net/Makefile b/net/Makefile
index 65bb8c72a35e..60ed5190eda8 100644
--- a/net/Makefile
+++ b/net/Makefile
@@ -79,3 +79,4 @@ obj-$(CONFIG_XDP_SOCKETS) += xdp/
obj-$(CONFIG_MPTCP) += mptcp/
obj-$(CONFIG_MCTP) += mctp/
obj-$(CONFIG_NET_HANDSHAKE) += handshake/
+obj-$(CONFIG_NET_SHAPER) += shaper/
diff --git a/net/shaper/Makefile b/net/shaper/Makefile
new file mode 100644
index 000000000000..13375884d60e
--- /dev/null
+++ b/net/shaper/Makefile
@@ -0,0 +1,9 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# Makefile for the Generic HANDSHAKE service
+#
+# Copyright (c) 2024, Red Hat, Inc.
+#
+
+obj-y += shaper.o shaper_nl_gen.o
+
diff --git a/net/shaper/shaper.c b/net/shaper/shaper.c
new file mode 100644
index 000000000000..49de88c68e2f
--- /dev/null
+++ b/net/shaper/shaper.c
@@ -0,0 +1,34 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include <linux/kernel.h>
+#include <linux/skbuff.h>
+
+#include "shaper_nl_gen.h"
+
+int net_shaper_nl_get_doit(struct sk_buff *skb, struct genl_info *info)
+{
+ return -EOPNOTSUPP;
+}
+
+int net_shaper_nl_get_dumpit(struct sk_buff *skb,
+ struct netlink_callback *cb)
+{
+ return -EOPNOTSUPP;
+}
+
+int net_shaper_nl_set_doit(struct sk_buff *skb, struct genl_info *info)
+{
+ return -EOPNOTSUPP;
+}
+
+int net_shaper_nl_delete_doit(struct sk_buff *skb, struct genl_info *info)
+{
+ return -EOPNOTSUPP;
+}
+
+static int __init shaper_init(void)
+{
+ return genl_register_family(&net_shaper_nl_family);
+}
+
+subsys_initcall(shaper_init);
diff --git a/net/shaper/shaper_nl_gen.c b/net/shaper/shaper_nl_gen.c
new file mode 100644
index 000000000000..159b4cb6d2b8
--- /dev/null
+++ b/net/shaper/shaper_nl_gen.c
@@ -0,0 +1,93 @@
+// SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause)
+/* Do not edit directly, auto-generated from: */
+/* Documentation/netlink/specs/shaper.yaml */
+/* YNL-GEN kernel source */
+
+#include <net/netlink.h>
+#include <net/genetlink.h>
+
+#include "shaper_nl_gen.h"
+
+#include <uapi/linux/net_shaper.h>
+
+/* Common nested types */
+const struct nla_policy net_shaper_handle_nl_policy[NET_SHAPER_A_ID + 1] = {
+ [NET_SHAPER_A_SCOPE] = NLA_POLICY_MAX(NLA_U32, 4),
+ [NET_SHAPER_A_ID] = { .type = NLA_U32, },
+};
+
+const struct nla_policy net_shaper_ns_info_nl_policy[NET_SHAPER_A_WEIGHT + 1] = {
+ [NET_SHAPER_A_PARENT] = NLA_POLICY_NESTED(net_shaper_handle_nl_policy),
+ [NET_SHAPER_A_HANDLE] = NLA_POLICY_NESTED(net_shaper_handle_nl_policy),
+ [NET_SHAPER_A_METRIC] = NLA_POLICY_MAX(NLA_U32, 1),
+ [NET_SHAPER_A_BW_MIN] = { .type = NLA_U64, },
+ [NET_SHAPER_A_BW_MAX] = { .type = NLA_U64, },
+ [NET_SHAPER_A_BURST] = { .type = NLA_U64, },
+ [NET_SHAPER_A_PRIORITY] = { .type = NLA_U32, },
+ [NET_SHAPER_A_WEIGHT] = { .type = NLA_U32, },
+};
+
+/* NET_SHAPER_CMD_GET - do */
+static const struct nla_policy net_shaper_get_do_nl_policy[NET_SHAPER_A_HANDLE + 1] = {
+ [NET_SHAPER_A_IFINDEX] = { .type = NLA_U32, },
+ [NET_SHAPER_A_HANDLE] = NLA_POLICY_NESTED(net_shaper_handle_nl_policy),
+};
+
+/* NET_SHAPER_CMD_GET - dump */
+static const struct nla_policy net_shaper_get_dump_nl_policy[NET_SHAPER_A_IFINDEX + 1] = {
+ [NET_SHAPER_A_IFINDEX] = { .type = NLA_U32, },
+};
+
+/* NET_SHAPER_CMD_SET - do */
+static const struct nla_policy net_shaper_set_nl_policy[NET_SHAPER_A_SHAPERS + 1] = {
+ [NET_SHAPER_A_IFINDEX] = { .type = NLA_U32, },
+ [NET_SHAPER_A_SHAPERS] = NLA_POLICY_NESTED(net_shaper_ns_info_nl_policy),
+};
+
+/* NET_SHAPER_CMD_DELETE - do */
+static const struct nla_policy net_shaper_delete_nl_policy[NET_SHAPER_A_HANDLES + 1] = {
+ [NET_SHAPER_A_IFINDEX] = { .type = NLA_U32, },
+ [NET_SHAPER_A_HANDLES] = NLA_POLICY_NESTED(net_shaper_handle_nl_policy),
+};
+
+/* Ops table for net_shaper */
+static const struct genl_split_ops net_shaper_nl_ops[] = {
+ {
+ .cmd = NET_SHAPER_CMD_GET,
+ .doit = net_shaper_nl_get_doit,
+ .policy = net_shaper_get_do_nl_policy,
+ .maxattr = NET_SHAPER_A_HANDLE,
+ .flags = GENL_ADMIN_PERM | GENL_CMD_CAP_DO,
+ },
+ {
+ .cmd = NET_SHAPER_CMD_GET,
+ .dumpit = net_shaper_nl_get_dumpit,
+ .policy = net_shaper_get_dump_nl_policy,
+ .maxattr = NET_SHAPER_A_IFINDEX,
+ .flags = GENL_ADMIN_PERM | GENL_CMD_CAP_DUMP,
+ },
+ {
+ .cmd = NET_SHAPER_CMD_SET,
+ .doit = net_shaper_nl_set_doit,
+ .policy = net_shaper_set_nl_policy,
+ .maxattr = NET_SHAPER_A_SHAPERS,
+ .flags = GENL_ADMIN_PERM | GENL_CMD_CAP_DO,
+ },
+ {
+ .cmd = NET_SHAPER_CMD_DELETE,
+ .doit = net_shaper_nl_delete_doit,
+ .policy = net_shaper_delete_nl_policy,
+ .maxattr = NET_SHAPER_A_HANDLES,
+ .flags = GENL_ADMIN_PERM | GENL_CMD_CAP_DO,
+ },
+};
+
+struct genl_family net_shaper_nl_family __ro_after_init = {
+ .name = NET_SHAPER_FAMILY_NAME,
+ .version = NET_SHAPER_FAMILY_VERSION,
+ .netnsok = true,
+ .parallel_ops = true,
+ .module = THIS_MODULE,
+ .split_ops = net_shaper_nl_ops,
+ .n_split_ops = ARRAY_SIZE(net_shaper_nl_ops),
+};
diff --git a/net/shaper/shaper_nl_gen.h b/net/shaper/shaper_nl_gen.h
new file mode 100644
index 000000000000..663406224d62
--- /dev/null
+++ b/net/shaper/shaper_nl_gen.h
@@ -0,0 +1,25 @@
+/* SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause) */
+/* Do not edit directly, auto-generated from: */
+/* Documentation/netlink/specs/shaper.yaml */
+/* YNL-GEN kernel header */
+
+#ifndef _LINUX_NET_SHAPER_GEN_H
+#define _LINUX_NET_SHAPER_GEN_H
+
+#include <net/netlink.h>
+#include <net/genetlink.h>
+
+#include <uapi/linux/net_shaper.h>
+
+/* Common nested types */
+extern const struct nla_policy net_shaper_handle_nl_policy[NET_SHAPER_A_ID + 1];
+extern const struct nla_policy net_shaper_ns_info_nl_policy[NET_SHAPER_A_WEIGHT + 1];
+
+int net_shaper_nl_get_doit(struct sk_buff *skb, struct genl_info *info);
+int net_shaper_nl_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb);
+int net_shaper_nl_set_doit(struct sk_buff *skb, struct genl_info *info);
+int net_shaper_nl_delete_doit(struct sk_buff *skb, struct genl_info *info);
+
+extern struct genl_family net_shaper_nl_family;
+
+#endif /* _LINUX_NET_SHAPER_GEN_H */
--
2.45.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH net-next 2/5] net: introduce HW Rate limiting driver API
2024-06-27 20:17 [PATCH net-next 0/5] net: introduce TX shaping H/W offload API Paolo Abeni
2024-06-27 20:17 ` [PATCH net-next 1/5] netlink: spec: add shaper YAML spec Paolo Abeni
@ 2024-06-27 20:17 ` Paolo Abeni
2024-06-27 20:17 ` [PATCH net-next 3/5] netlink: spec: add shaper introspection support Paolo Abeni
` (3 subsequent siblings)
5 siblings, 0 replies; 20+ messages in thread
From: Paolo Abeni @ 2024-06-27 20:17 UTC (permalink / raw)
To: netdev
Cc: Jakub Kicinski, Jiri Pirko, Madhu Chittim, Sridhar Samudrala,
Simon Horman, John Fastabend, Sunil Kovvuri Goutham,
Jamal Hadi Salim
The network devices gain a new ops struct to directly manipulate the
H/W shapers implemented by the NIC.
The shapers can be attached to a pre-defined set of 'domains' - port,
device, etc. - and the overall shapers configuration pushed to the H/W is
maintained by the kernel.
Each shaper is identified by an unique integer id based on the domain
and additional domain-specific information - e.g. for the queue domain,
the queue id.
The shaper manipulation is exposed to user-space implementing
the NL operations described by the previous patch.
Note that an additional domain, not exposed to user-space, is defined
here (VF - to allow implementing the existing ndo_set_vf_rate() on top
of the API defined here). Such domain will allow implementing a generic
ndo_set_vf_rate() on top the shapers API.
Co-developed-by: Simon Horman <horms@kernel.org>
Signed-off-by: Simon Horman <horms@kernel.org>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
v4 -> v5:
- fixed a few typos
- set() operation commit changes in memory on success
- updated set() to handle positive return values for device op
- get() always fill the parent handle
v3 -> v4:
- fix build error with !NET_SHAPER
- fix skipping one item on shaper dump requiring multiple skbs
- add a bunch of comments
- clarified the return value for delete()
- rebased on yaml changes
v2 -> v3:
- completed the implementation for set/get/dump/delete NL ops
- plugged shaper cleanup at device dismatel
---
include/linux/netdevice.h | 16 ++
include/net/net_shaper.h | 195 +++++++++++++
net/core/dev.c | 2 +
net/core/dev.h | 6 +
net/shaper/shaper.c | 559 +++++++++++++++++++++++++++++++++++++-
5 files changed, 774 insertions(+), 4 deletions(-)
create mode 100644 include/net/net_shaper.h
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index cc18acd3c58b..ba0ad0159345 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -80,6 +80,8 @@ struct xdp_buff;
struct xdp_frame;
struct xdp_metadata_ops;
struct xdp_md;
+struct net_shaper_ops;
+struct net_shaper_data;
typedef u32 xdp_features_t;
@@ -1597,6 +1599,13 @@ struct net_device_ops {
int (*ndo_hwtstamp_set)(struct net_device *dev,
struct kernel_hwtstamp_config *kernel_config,
struct netlink_ext_ack *extack);
+
+#if IS_ENABLED(CONFIG_NET_SHAPER)
+ /** @net_shaper_ops: Device shaping offload operations
+ * see include/net/net_shapers.h
+ */
+ const struct net_shaper_ops *net_shaper_ops;
+#endif
};
/**
@@ -2405,6 +2414,13 @@ struct net_device {
/** @irq_moder: dim parameters used if IS_ENABLED(CONFIG_DIMLIB). */
struct dim_irq_moder *irq_moder;
+
+#if IS_ENABLED(CONFIG_NET_SHAPER)
+ /** @net_shaper_data: data tracking the current shaper status
+ * see include/net/net_shapers.h
+ */
+ struct net_shaper_data *net_shaper_data;
+#endif
};
#define to_net_dev(d) container_of(d, struct net_device, dev)
diff --git a/include/net/net_shaper.h b/include/net/net_shaper.h
new file mode 100644
index 000000000000..a4d8213f8594
--- /dev/null
+++ b/include/net/net_shaper.h
@@ -0,0 +1,195 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#ifndef _NET_SHAPER_H_
+#define _NET_SHAPER_H_
+
+#include <linux/types.h>
+#include <linux/bits.h>
+#include <linux/bitfield.h>
+#include <linux/netdevice.h>
+#include <linux/netlink.h>
+
+#include <uapi/linux/net_shaper.h>
+
+/**
+ * struct net_shaper_info - represents a shaping node on the NIC H/W
+ * zeroed field are considered not set.
+ * @handle: Unique identifier for the shaper, see @net_shaper_make_handle
+ * @parent: Unique identifier for the shaper parent, usually implied. Only
+ * NET_SHAPER_SCOPE_QUEUE, NET_SHAPER_SCOPE_NETDEV and NET_SHAPER_SCOPE_DETACHED
+ * can have the parent handle explicitly set, placing such shaper under
+ * the specified parent.
+ * @metric: Specify if the bw limits refers to PPS or BPS
+ * @bw_min: Minimum guaranteed rate for this shaper
+ * @bw_max: Maximum peak bw allowed for this shaper
+ * @burst: Maximum burst for the peek rate of this shaper
+ * @priority: Scheduling priority for this shaper
+ * @weight: Scheduling weight for this shaper
+ */
+struct net_shaper_info {
+ u32 handle;
+ u32 parent;
+ enum net_shaper_metric metric;
+ u64 bw_min; /* minimum guaranteed bandwidth, according to metric */
+ u64 bw_max; /* maximum allowed bandwidth */
+ u64 burst; /* maximum burst in bytes for bw_max */
+ u32 priority; /* scheduling strict priority */
+ u32 weight; /* scheduling WRR weight*/
+};
+
+/**
+ * define NET_SHAPER_SCOPE_VF - Shaper scope
+ *
+ * This shaper scope is not exposed to user-space; the shaper is attached to
+ * the given virtual function.
+ */
+#define NET_SHAPER_SCOPE_VF __NET_SHAPER_SCOPE_MAX
+
+/**
+ * struct net_shaper_ops - Operations on device H/W shapers
+ * @set: Modify the existing shaper.
+ * @delete: Delete the specified shaper.
+ *
+ * The initial shaping configuration ad device initialization is empty/
+ * a no-op/does not constraint the b/w in any way.
+ * The network core keeps track of the applied user-configuration in
+ * per device storage.
+ *
+ * Each shaper is uniquely identified within the device with an 'handle',
+ * dependent on the shaper scope and other data, see @shaper_make_handle()
+ */
+struct net_shaper_ops {
+ /** set - Update or create the specified shapers
+ * @dev: Netdevice to operate on.
+ * @nr: The number of items in the @shapers array
+ * @shapers: Configuration of shapers.
+ * @extack: Netlink extended ACK for reporting errors.
+ *
+ * Return:
+ * * The number of updated shapers - can be less then @nr, if so
+ * the driver must set @extack
+ * accordingly; only shapers in
+ * the [ret, nr) range are
+ * modified.
+ * * %-EOPNOTSUPP - Operation is not supported by hardware, driver,
+ * or core for any reason. @extack should be set to
+ * text describing the reason.
+ * * Other negative error values on failure.
+ */
+ int (*set)(struct net_device *dev, int nr,
+ const struct net_shaper_info *shapers,
+ struct netlink_ext_ack *extack);
+
+ /** delete - Removes the specified shapers from the NIC
+ * @dev: netdevice to operate on
+ * @nr: The number of entries in the @handles array
+ * @handles: The shapers identifier
+ * @extack: Netlink extended ACK for reporting errors.
+ *
+ * Removes the shapers configuration, restoring the default behavior
+ *
+ * Return:
+ * * The number of deleted shapers - can be less then @nr, if so
+ * the driver must set @extack
+ * accordingly; shapers in the
+ * [ret, nr) range are left
+ * unmodified.
+ * * %-EOPNOTSUPP - Operation is not supported by hardware, driver,
+ * or core for any reason. @extack should be set to
+ * text describing the reason.
+ * * Other negative error value on failure.
+ */
+ int (*delete)(struct net_device *dev, int nr, const u32 *handles,
+ struct netlink_ext_ack *extack);
+};
+
+#define NET_SHAPER_SCOPE_SHIFT 16
+#define NET_SHAPER_ID_MASK GENMASK(NET_SHAPER_SCOPE_SHIFT - 1, 0)
+#define NET_SHAPER_SCOPE_MASK GENMASK(31, NET_SHAPER_SCOPE_SHIFT)
+
+/**
+ * net_shaper_make_handle - creates an unique shaper identifier
+ * @scope: the shaper scope
+ * @id: the shaper id number
+ *
+ * Return: an unique identifier for the shaper
+ *
+ * Combines the specified arguments to create an unique identifier for
+ * the shaper. The @id argument semantic depends on the
+ * specified scope.
+ * For @NET_SHAPER_SCOPE_QUEUE_GROUP, @id is the queue group id
+ * For @NET_SHAPER_SCOPE_QUEUE, @id is the queue number.
+ * For @NET_SHAPER_SCOPE_VF, @id is virtual function number.
+ */
+static inline u32 net_shaper_make_handle(enum net_shaper_scope scope,
+ int id)
+{
+ return FIELD_PREP(NET_SHAPER_SCOPE_MASK, scope) |
+ FIELD_PREP(NET_SHAPER_ID_MASK, id);
+}
+
+/**
+ * net_shaper_handle_scope - extract the scope from the given handle
+ * @handle: the shaper handle
+ *
+ * Return: the corresponding scope
+ */
+static inline enum net_shaper_scope net_shaper_handle_scope(u32 handle)
+{
+ return FIELD_GET(NET_SHAPER_SCOPE_MASK, handle);
+}
+
+/**
+ * net_shaper_handle_id - extract the id number from the given handle
+ * @handle: the shaper handle
+ *
+ * Return: the corresponding id number
+ */
+static inline int net_shaper_handle_id(u32 handle)
+{
+ return FIELD_GET(NET_SHAPER_ID_MASK, handle);
+}
+
+/*
+ * Examples:
+ * - set shaping on a given queue
+ * struct shaper_info info = { }; // fill this
+ * info.handle = shaper_make_handle(NET_SHAPER_SCOPE_QUEUE, queue_id);
+ * dev->netdev_ops->net_shaper_ops->set(dev, 1, &info, NULL);
+ *
+ * - create a queue group with a queue group shaping limits.
+ * Assuming the following topology already exists:
+ * < netdev shaper >
+ * / \
+ * <queue 0 shaper> . . . <queue N shaper>
+ *
+ * struct shaper_info ginfo = { }; // fill this
+ * u32 ghandle = shaper_make_handle(NET_SHAPER_SCOPE_DETACHED, 0);
+ * ginfo.handle = ghandle;
+ * dev->netdev_ops->net_shaper_ops->set(dev, 1, &ginfo);
+ *
+ * // now topology is:
+ * // < netdev shaper >
+ * // / | \
+ * // / | < newly created shaper >
+ * // / |
+ * // <queue 0 shaper> . . . <queue N shaper>
+ *
+ * // move a shapers for queues 3..n out of such queue group
+ * for (i = 0; i <= 2; ++i) {
+ * struct shaper_info qinfo = {}; // fill this
+ *
+ * info.handle = net_shaper_make_handle(NET_SHAPER_SCOPE_QUEUE, i);
+ * info.parent = ghandle;
+ * dev->netdev_ops->shaper_ops->set(dev, &ginfo, NULL);
+ * }
+ *
+ * // now the topology is:
+ * // < netdev shaper >
+ * // / \
+ * // < newly created shaper> <queue 3 shaper> .. <queue n shaper>
+ * // / \
+ * // <queue 0 shaper> . . . <queue 2 shaper>
+ */
+#endif
+
diff --git a/net/core/dev.c b/net/core/dev.c
index b94fb4e63a28..3a5b2cb402b6 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -11174,6 +11174,8 @@ void free_netdev(struct net_device *dev)
/* Flush device addresses */
dev_addr_flush(dev);
+ dev_shaper_flush(dev);
+
list_for_each_entry_safe(p, n, &dev->napi_list, dev_list)
netif_napi_del(p);
diff --git a/net/core/dev.h b/net/core/dev.h
index 5654325c5b71..e376fc1c867b 100644
--- a/net/core/dev.h
+++ b/net/core/dev.h
@@ -35,6 +35,12 @@ void dev_addr_flush(struct net_device *dev);
int dev_addr_init(struct net_device *dev);
void dev_addr_check(struct net_device *dev);
+#if IS_ENABLED(CONFIG_NET_SHAPER)
+void dev_shaper_flush(struct net_device *dev);
+#else
+static inline void dev_shaper_flush(struct net_device *dev) {}
+#endif
+
/* sysctls not referred to from outside net/core/ */
extern int netdev_unregister_timeout_secs;
extern int weight_p;
diff --git a/net/shaper/shaper.c b/net/shaper/shaper.c
index 49de88c68e2f..b1c63cfa21c4 100644
--- a/net/shaper/shaper.c
+++ b/net/shaper/shaper.c
@@ -2,28 +2,579 @@
#include <linux/kernel.h>
#include <linux/skbuff.h>
+#include <linux/xarray.h>
+#include <net/net_shaper.h>
#include "shaper_nl_gen.h"
+#include "../core/dev.h"
+
+struct net_shaper_data {
+ struct xarray shapers;
+};
+
+struct net_shaper_nl_ctx {
+ u32 start_handle;
+};
+
+static u32 default_parent(u32 handle)
+{
+ enum net_shaper_scope parent, scope = net_shaper_handle_scope(handle);
+
+ switch (scope) {
+ case NET_SHAPER_SCOPE_DETACHED:
+ case NET_SHAPER_SCOPE_PORT:
+ case NET_SHAPER_SCOPE_UNSPEC:
+ parent = NET_SHAPER_SCOPE_UNSPEC;
+ break;
+
+ case NET_SHAPER_SCOPE_QUEUE:
+ parent = NET_SHAPER_SCOPE_NETDEV;
+ break;
+
+ case NET_SHAPER_SCOPE_NETDEV:
+ case NET_SHAPER_SCOPE_VF:
+ parent = NET_SHAPER_SCOPE_PORT;
+ break;
+ }
+
+ return net_shaper_make_handle(parent, 0);
+}
+
+static int fill_handle(struct sk_buff *msg, u32 handle, u32 type,
+ const struct genl_info *info)
+{
+ struct nlattr *handle_attr;
+
+ if (!handle)
+ return 0;
+
+ handle_attr = nla_nest_start_noflag(msg, type);
+ if (!handle_attr)
+ return -EMSGSIZE;
+
+ if (nla_put_u32(msg, NET_SHAPER_A_SCOPE,
+ net_shaper_handle_scope(handle)) ||
+ nla_put_u32(msg, NET_SHAPER_A_ID,
+ net_shaper_handle_id(handle)))
+ goto handle_nest_cancel;
+
+ nla_nest_end(msg, handle_attr);
+ return 0;
+
+handle_nest_cancel:
+ nla_nest_cancel(msg, handle_attr);
+ return -EMSGSIZE;
+}
+
+static int
+net_shaper_fill_one(struct sk_buff *msg, struct net_shaper_info *shaper,
+ const struct genl_info *info)
+{
+ void *hdr;
+
+ hdr = genlmsg_iput(msg, info);
+ if (!hdr)
+ return -EMSGSIZE;
+
+ if (fill_handle(msg, shaper->parent, NET_SHAPER_A_PARENT, info) ||
+ fill_handle(msg, shaper->handle, NET_SHAPER_A_HANDLE, info) ||
+ nla_put_u32(msg, NET_SHAPER_A_METRIC, shaper->metric) ||
+ nla_put_u64_64bit(msg, NET_SHAPER_A_BW_MIN, shaper->bw_min,
+ NET_SHAPER_A_PAD) ||
+ nla_put_u64_64bit(msg, NET_SHAPER_A_BW_MAX, shaper->bw_max,
+ NET_SHAPER_A_PAD) ||
+ nla_put_u64_64bit(msg, NET_SHAPER_A_BURST, shaper->burst,
+ NET_SHAPER_A_PAD) ||
+ nla_put_u32(msg, NET_SHAPER_A_PRIORITY, shaper->priority) ||
+ nla_put_u32(msg, NET_SHAPER_A_WEIGHT, shaper->weight))
+ goto nla_put_failure;
+
+ genlmsg_end(msg, hdr);
+
+ return 0;
+
+nla_put_failure:
+ genlmsg_cancel(msg, hdr);
+ return -EMSGSIZE;
+}
+
+/* On success sets pdev to the relevant device and acquires a reference
+ * to it
+ */
+static int fetch_dev(const struct genl_info *info, struct net_device **pdev)
+{
+ struct net *ns = genl_info_net(info);
+ struct net_device *dev;
+ int ifindex;
+
+ if (GENL_REQ_ATTR_CHECK(info, NET_SHAPER_A_IFINDEX))
+ return -EINVAL;
+
+ ifindex = nla_get_u32(info->attrs[NET_SHAPER_A_IFINDEX]);
+ dev = dev_get_by_index(ns, ifindex);
+ if (!dev) {
+ GENL_SET_ERR_MSG_FMT(info, "device %d not found", ifindex);
+ return -EINVAL;
+ }
+
+ if (!dev->netdev_ops->net_shaper_ops) {
+ GENL_SET_ERR_MSG_FMT(info, "device %s does not support H/W shaper",
+ dev->name);
+ dev_put(dev);
+ return -EOPNOTSUPP;
+ }
+
+ *pdev = dev;
+ return 0;
+}
+
+static int parse_handle(const struct nlattr *attr, const struct genl_info *info,
+ u32 *handle)
+{
+ struct nlattr *tb[NET_SHAPER_A_ID + 1];
+ struct nlattr *scope, *id;
+ int ret;
+
+ ret = nla_parse_nested(tb, NET_SHAPER_A_ID, attr,
+ net_shaper_handle_nl_policy, info->extack);
+ if (ret < 0)
+ return ret;
+
+ scope = tb[NET_SHAPER_A_SCOPE];
+ if (!scope) {
+ GENL_SET_ERR_MSG(info, "Missing 'scope' attribute for handle");
+ return -EINVAL;
+ }
+
+ id = tb[NET_SHAPER_A_ID];
+ *handle = net_shaper_make_handle(nla_get_u32(scope),
+ id ? nla_get_u32(id) : 0);
+ return 0;
+}
+
int net_shaper_nl_get_doit(struct sk_buff *skb, struct genl_info *info)
{
- return -EOPNOTSUPP;
+ struct net_shaper_info *shaper;
+ struct net_device *dev;
+ struct sk_buff *msg;
+ u32 handle;
+ int ret;
+
+ ret = fetch_dev(info, &dev);
+ if (ret)
+ return ret;
+
+ if (GENL_REQ_ATTR_CHECK(info, NET_SHAPER_A_HANDLE))
+ goto put;
+
+ ret = parse_handle(info->attrs[NET_SHAPER_A_HANDLE], info, &handle);
+ if (ret < 0)
+ goto put;
+
+ if (!dev->net_shaper_data) {
+ GENL_SET_ERR_MSG_FMT(info, "no shaper is initialized on device %s",
+ dev->name);
+ ret = -EINVAL;
+ goto put;
+ }
+
+ shaper = xa_load(&dev->net_shaper_data->shapers, handle);
+ if (!shaper) {
+ GENL_SET_ERR_MSG_FMT(info, "Can't find shaper for handle %x", handle);
+ ret = -EINVAL;
+ goto put;
+ }
+
+ msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+ if (!msg) {
+ ret = -ENOMEM;
+ goto put;
+ }
+
+ ret = net_shaper_fill_one(msg, shaper, info);
+ if (ret)
+ goto free_msg;
+
+ ret = genlmsg_reply(msg, info);
+ if (ret)
+ goto free_msg;
+
+put:
+ dev_put(dev);
+ return ret;
+
+free_msg:
+ nlmsg_free(msg);
+ goto put;
}
int net_shaper_nl_get_dumpit(struct sk_buff *skb,
struct netlink_callback *cb)
{
- return -EOPNOTSUPP;
+ struct net_shaper_nl_ctx *ctx = (struct net_shaper_nl_ctx *)cb->ctx;
+ const struct genl_info *info = genl_info_dump(cb);
+ struct net_shaper_info *shaper;
+ struct net_device *dev;
+ unsigned long handle;
+ int ret;
+
+ ret = fetch_dev(info, &dev);
+ if (ret)
+ return ret;
+
+ BUILD_BUG_ON(sizeof(struct net_shaper_nl_ctx) > sizeof(cb->ctx));
+
+ if (!dev->net_shaper_data) {
+ ret = 0;
+ goto put;
+ }
+
+ xa_for_each_range(&dev->net_shaper_data->shapers, handle, shaper,
+ ctx->start_handle, U32_MAX) {
+ ret = net_shaper_fill_one(skb, shaper, info);
+ if (ret)
+ goto put;
+
+ ctx->start_handle = handle;
+ }
+
+put:
+ dev_put(dev);
+ return ret;
+}
+
+/* count the number of [multi] attributes of the given type */
+static int attr_list_len(struct genl_info *info, int type)
+{
+ struct nlattr *attr;
+ int rem, cnt = 0;
+
+ nla_for_each_attr_type(attr, type, genlmsg_data(info->genlhdr),
+ genlmsg_len(info->genlhdr), rem)
+ cnt++;
+ return cnt;
+}
+
+/* fetch the cached shaper info and update them with the user-provided
+ * attributes
+ */
+static int fill_shaper(struct net_device *dev, const struct nlattr *attr,
+ const struct genl_info *info,
+ struct net_shaper_info *shaper)
+{
+ struct xarray *xa = &dev->net_shaper_data->shapers;
+ struct nlattr *tb[NET_SHAPER_A_MAX + 1];
+ int ret;
+
+ ret = nla_parse_nested(tb, NET_SHAPER_A_MAX, attr,
+ net_shaper_ns_info_nl_policy, info->extack);
+ if (ret < 0)
+ return ret;
+
+ /* the shaper handle is the only mandatory attribute */
+ if (NL_REQ_ATTR_CHECK(info->extack, NULL, tb, NET_SHAPER_A_HANDLE))
+ return -EINVAL;
+
+ ret = parse_handle(tb[NET_SHAPER_A_HANDLE], info, &shaper->handle);
+ if (ret)
+ return ret;
+
+ /* fetch existing data, if any, so that user provide info will
+ * incrementally update the existing shaper configuration
+ */
+ if (xa) {
+ struct net_shaper_info *old = xa_load(xa, shaper->handle);
+
+ if (old)
+ *shaper = *old;
+ }
+
+ if (tb[NET_SHAPER_A_PARENT]) {
+ ret = parse_handle(tb[NET_SHAPER_A_PARENT], info,
+ &shaper->parent);
+ if (ret)
+ return ret;
+ }
+
+ if (tb[NET_SHAPER_A_METRIC])
+ shaper->metric = nla_get_u32(tb[NET_SHAPER_A_METRIC]);
+
+ if (tb[NET_SHAPER_A_BW_MIN])
+ shaper->bw_min = nla_get_u64(tb[NET_SHAPER_A_BW_MIN]);
+
+ if (tb[NET_SHAPER_A_BW_MAX])
+ shaper->bw_max = nla_get_u64(tb[NET_SHAPER_A_BW_MAX]);
+
+ if (tb[NET_SHAPER_A_BURST])
+ shaper->burst = nla_get_u64(tb[NET_SHAPER_A_BURST]);
+
+ if (tb[NET_SHAPER_A_PRIORITY])
+ shaper->priority = nla_get_u32(tb[NET_SHAPER_A_PRIORITY]);
+
+ if (tb[NET_SHAPER_A_WEIGHT])
+ shaper->weight = nla_get_u32(tb[NET_SHAPER_A_WEIGHT]);
+
+ return 0;
+}
+
+/* Update the H/W and on success update the local cache, too */
+static int net_shaper_set(struct net_device *dev, int nr,
+ struct net_shaper_info *shapers,
+ struct netlink_ext_ack *extack)
+{
+ struct net_shaper_info *cur, *prev;
+ unsigned long index;
+ struct xarray *xa;
+ int i, ret;
+
+ /* allocate on demand the per device shaper's storage */
+ if (!dev->net_shaper_data) {
+ dev->net_shaper_data = kmalloc(sizeof(*dev->net_shaper_data),
+ GFP_KERNEL);
+ if (!dev->net_shaper_data) {
+ NL_SET_ERR_MSG(extack, "Can't allocate memory for shaper data");
+ return -ENOMEM;
+ }
+
+ xa_init(&dev->net_shaper_data->shapers);
+ }
+
+ /* allocate the memory for newly crated shapers. While at that,
+ * tentatively insert into the shaper store
+ */
+ ret = -ENOMEM;
+ xa = &dev->net_shaper_data->shapers;
+ for (i = 0; i < nr; ++i) {
+ /* ensure 'parent' is non zero only when the driver must move
+ * the shaper around
+ */
+ prev = xa_load(xa, shapers[i].handle);
+ if (prev) {
+ if (shapers[i].parent == prev->parent)
+ shapers[i].parent = 0;
+ continue;
+ }
+ if (shapers[i].parent == default_parent(shapers[i].handle))
+ shapers[i].parent = 0;
+
+ cur = kmalloc(sizeof(*cur), GFP_KERNEL);
+ if (!cur)
+ goto out;
+
+ *cur = shapers[i];
+ xa_lock(xa);
+ prev = __xa_store(xa, shapers[i].handle, cur, GFP_KERNEL);
+ __xa_set_mark(xa, shapers[i].handle, XA_MARK_0);
+ xa_unlock(xa);
+ if (xa_err(prev)) {
+ NL_SET_ERR_MSG(extack, "Can't update shaper store");
+ ret = xa_err(prev);
+ goto out;
+ }
+ }
+
+ ret = dev->netdev_ops->net_shaper_ops->set(dev, nr, shapers, extack);
+
+ /* be careful with possibly bugged drivers */
+ if (WARN_ON_ONCE(ret > nr))
+ ret = nr;
+
+out:
+ /* commit updated shapers and free failed tentative ones */
+ xa_lock(xa);
+ for (i = 0; i < ret; ++i) {
+ cur = xa_load(xa, shapers[i].handle);
+ if (WARN_ON_ONCE(!cur))
+ continue;
+
+ __xa_clear_mark(xa, shapers[i].handle, XA_MARK_0);
+ *cur = shapers[i];
+
+ /* ensure that get operation always specify the
+ * parent handle
+ */
+ if (net_shaper_handle_scope(cur->parent) ==
+ NET_SHAPER_SCOPE_UNSPEC)
+ cur->parent = default_parent(cur->handle);
+ }
+ xa_for_each_marked(xa, index, cur, XA_MARK_0) {
+ __xa_erase(xa, index);
+ kfree(cur);
+ }
+ xa_unlock(xa);
+ return ret;
+}
+
+static int modify_send_reply(struct genl_info *info, int modified)
+{
+ struct sk_buff *msg;
+ int ret = -EMSGSIZE;
+ void *hdr;
+
+ msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+ if (!msg)
+ return -ENOMEM;
+
+ hdr = genlmsg_iput(msg, info);
+ if (!hdr)
+ goto free_msg;
+
+ if (nla_put_u32(msg, NET_SHAPER_A_MODIFIED, modified))
+ goto cancel_msg;
+
+ genlmsg_end(msg, hdr);
+
+ ret = genlmsg_reply(msg, info);
+ if (ret)
+ goto free_msg;
+
+ return ret;
+
+cancel_msg:
+ genlmsg_cancel(msg, hdr);
+
+free_msg:
+ nlmsg_free(msg);
+ return ret;
}
int net_shaper_nl_set_doit(struct sk_buff *skb, struct genl_info *info)
{
- return -EOPNOTSUPP;
+ struct net_shaper_info *shapers;
+ int i, ret, nr_shapers, rem;
+ struct net_device *dev;
+ struct nlattr *attr;
+
+ ret = fetch_dev(info, &dev);
+ if (ret)
+ return ret;
+
+ nr_shapers = attr_list_len(info, NET_SHAPER_A_SHAPERS);
+ shapers = kcalloc(nr_shapers, sizeof(struct net_shaper_info), GFP_KERNEL);
+ if (!shapers) {
+ GENL_SET_ERR_MSG_FMT(info, "Can't allocate memory for %d shapers",
+ nr_shapers);
+ ret = -ENOMEM;
+ goto put;
+ }
+
+ i = 0;
+ nla_for_each_attr_type(attr, NET_SHAPER_A_SHAPERS,
+ genlmsg_data(info->genlhdr),
+ genlmsg_len(info->genlhdr), rem) {
+ if (WARN_ON_ONCE(i >= nr_shapers))
+ goto free_shapers;
+
+ ret = fill_shaper(dev, attr, info, &shapers[i++]);
+ if (ret)
+ goto free_shapers;
+ }
+
+ ret = net_shaper_set(dev, nr_shapers, shapers, info->extack);
+ if (ret < 0)
+ goto free_shapers;
+
+ ret = modify_send_reply(info, ret);
+
+free_shapers:
+ kfree(shapers);
+
+put:
+ dev_put(dev);
+ return ret;
+}
+
+static int net_shaper_delete(struct net_device *dev, int nr,
+ const u32 *handles,
+ struct netlink_ext_ack *extack)
+{
+ struct xarray *xa = &dev->net_shaper_data->shapers;
+ struct net_shaper_info *cur;
+ int i, ret;
+
+ ret = dev->netdev_ops->net_shaper_ops->delete(dev, nr, handles,
+ extack);
+ if (ret < 0 || !xa)
+ return ret;
+
+ /* be careful with possibly bugged drivers */
+ if (WARN_ON_ONCE(ret > nr))
+ ret = nr;
+
+ xa_lock(xa);
+ for (i = 0; i < ret; ++i) {
+ cur = xa_load(xa, handles[i]);
+ __xa_erase(xa, handles[i]);
+ kfree(cur);
+ }
+ xa_unlock(xa);
+ return ret;
}
int net_shaper_nl_delete_doit(struct sk_buff *skb, struct genl_info *info)
{
- return -EOPNOTSUPP;
+ int i, ret, nr_handles, rem;
+ struct net_device *dev;
+ struct nlattr *attr;
+ u32 *handles;
+
+ ret = fetch_dev(info, &dev);
+ if (ret)
+ return ret;
+
+ nr_handles = attr_list_len(info, NET_SHAPER_A_HANDLES);
+ handles = kmalloc_array(nr_handles, sizeof(u32), GFP_KERNEL);
+ if (!handles) {
+ ret = -ENOMEM;
+ GENL_SET_ERR_MSG_FMT(info, "Can't allocate memory for %d handles",
+ nr_handles);
+ goto put;
+ }
+
+ i = 0;
+ nla_for_each_attr_type(attr, NET_SHAPER_A_HANDLES,
+ genlmsg_data(info->genlhdr),
+ genlmsg_len(info->genlhdr), rem) {
+ if (WARN_ON_ONCE(i >= nr_handles))
+ goto free_handles;
+
+ ret = parse_handle(attr, info, &handles[i++]);
+ if (ret)
+ goto free_handles;
+ }
+
+ ret = net_shaper_delete(dev, nr_handles, handles, info->extack);
+ if (ret < 0)
+ goto free_handles;
+
+ ret = modify_send_reply(info, ret);
+
+free_handles:
+ kfree(handles);
+
+put:
+ dev_put(dev);
+ return ret;
+}
+
+void dev_shaper_flush(struct net_device *dev)
+{
+ struct net_shaper_info *cur;
+ unsigned long index;
+ struct xarray *xa;
+
+ if (!dev->net_shaper_data)
+ return;
+
+ xa = &dev->net_shaper_data->shapers;
+ xa_lock(xa);
+ xa_for_each(xa, index, cur) {
+ __xa_erase(xa, index);
+ kfree(cur);
+ }
+ xa_unlock(xa);
+ kfree(xa);
}
static int __init shaper_init(void)
--
2.45.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH net-next 3/5] netlink: spec: add shaper introspection support
2024-06-27 20:17 [PATCH net-next 0/5] net: introduce TX shaping H/W offload API Paolo Abeni
2024-06-27 20:17 ` [PATCH net-next 1/5] netlink: spec: add shaper YAML spec Paolo Abeni
2024-06-27 20:17 ` [PATCH net-next 2/5] net: introduce HW Rate limiting driver API Paolo Abeni
@ 2024-06-27 20:17 ` Paolo Abeni
2024-06-27 20:17 ` [PATCH net-next 4/5] net: shaper: implement " Paolo Abeni
` (2 subsequent siblings)
5 siblings, 0 replies; 20+ messages in thread
From: Paolo Abeni @ 2024-06-27 20:17 UTC (permalink / raw)
To: netdev
Cc: Jakub Kicinski, Jiri Pirko, Madhu Chittim, Sridhar Samudrala,
Simon Horman, John Fastabend, Sunil Kovvuri Goutham,
Jamal Hadi Salim
Allow the user-space to fine-grain query the shaping features
supported by the NIC on each domain.
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
Documentation/netlink/specs/shaper.yaml | 74 +++++++++++++++++++++++++
include/uapi/linux/net_shaper.h | 17 ++++++
net/shaper/shaper.c | 11 ++++
net/shaper/shaper_nl_gen.c | 25 +++++++++
net/shaper/shaper_nl_gen.h | 3 +
5 files changed, 130 insertions(+)
diff --git a/Documentation/netlink/specs/shaper.yaml b/Documentation/netlink/specs/shaper.yaml
index 8563c85de68d..2762aa9bc2bd 100644
--- a/Documentation/netlink/specs/shaper.yaml
+++ b/Documentation/netlink/specs/shaper.yaml
@@ -125,6 +125,51 @@ attribute-sets:
name: priority
-
name: weight
+ -
+ name: capabilities
+ attributes:
+ -
+ name: ifindex
+ type: u32
+ -
+ name: scope
+ type: u32
+ enum: scope
+ -
+ name: support-metric-bps
+ doc: the device accepts 'bps' metric for bw-min, bw-max and burst
+ type: flag
+ -
+ name: support-metric-pps
+ doc: the device accepts 'pps' metric for bw-min, bw-max and burst
+ type: flag
+ -
+ name: support-nesting
+ doc: |
+ the device supports nesting shaper belonging to this scope
+ below 'detached' scoped shapers. Only 'queue', 'netdev' and
+ 'detached' scope and flag 'support-nesting'.
+ type: flag
+ -
+ name: support-bw-min
+ doc: the device supports a minimum guaranteed bw
+ type: flag
+ -
+ name: support-bw-max
+ doc: the device supports maximum bw shaping
+ type: flag
+ -
+ name: support-burst
+ doc: the device supports a maximum burst size
+ type: flag
+ -
+ name: support-priority
+ doc: the device supports priority scheduling
+ type: flag
+ -
+ name: support-weight
+ doc: the device supports weighted round robin scheduling
+ type: flag
operations:
list:
@@ -200,3 +245,32 @@ operations:
reply:
attributes:
- modified
+
+ -
+ name: cap-get
+ doc: |
+ Get / Dump the shaper capabilities supported by the given device
+ attribute-set: capabilities
+
+ do:
+ request:
+ attributes:
+ - ifindex
+ - scope
+ reply:
+ attributes: &cap-attrs
+ - support-metric-bps
+ - support-metric-pps
+ - support-nesting
+ - support-bw-min
+ - support-bw-max
+ - support-burst
+ - support-priority
+ - support-weight
+
+ dump:
+ request:
+ attributes:
+ - ifindex
+ reply:
+ attributes: *cap-attrs
diff --git a/include/uapi/linux/net_shaper.h b/include/uapi/linux/net_shaper.h
index 7e6b655e6c6d..83cf94fe4a2f 100644
--- a/include/uapi/linux/net_shaper.h
+++ b/include/uapi/linux/net_shaper.h
@@ -61,10 +61,27 @@ enum {
NET_SHAPER_A_MAX = (__NET_SHAPER_A_MAX - 1)
};
+enum {
+ NET_SHAPER_A_CAPABILITIES_IFINDEX = 1,
+ NET_SHAPER_A_CAPABILITIES_SCOPE,
+ NET_SHAPER_A_CAPABILITIES_SUPPORT_METRIC_BPS,
+ NET_SHAPER_A_CAPABILITIES_SUPPORT_METRIC_PPS,
+ NET_SHAPER_A_CAPABILITIES_SUPPORT_NESTING,
+ NET_SHAPER_A_CAPABILITIES_SUPPORT_BW_MIN,
+ NET_SHAPER_A_CAPABILITIES_SUPPORT_BW_MAX,
+ NET_SHAPER_A_CAPABILITIES_SUPPORT_BURST,
+ NET_SHAPER_A_CAPABILITIES_SUPPORT_PRIORITY,
+ NET_SHAPER_A_CAPABILITIES_SUPPORT_WEIGHT,
+
+ __NET_SHAPER_A_CAPABILITIES_MAX,
+ NET_SHAPER_A_CAPABILITIES_MAX = (__NET_SHAPER_A_CAPABILITIES_MAX - 1)
+};
+
enum {
NET_SHAPER_CMD_GET = 1,
NET_SHAPER_CMD_SET,
NET_SHAPER_CMD_DELETE,
+ NET_SHAPER_CMD_CAP_GET,
__NET_SHAPER_CMD_MAX,
NET_SHAPER_CMD_MAX = (__NET_SHAPER_CMD_MAX - 1)
diff --git a/net/shaper/shaper.c b/net/shaper/shaper.c
index b1c63cfa21c4..74da98aaa078 100644
--- a/net/shaper/shaper.c
+++ b/net/shaper/shaper.c
@@ -558,6 +558,17 @@ int net_shaper_nl_delete_doit(struct sk_buff *skb, struct genl_info *info)
return ret;
}
+int net_shaper_nl_cap_get_doit(struct sk_buff *skb, struct genl_info *info)
+{
+ return 0;
+}
+
+int net_shaper_nl_cap_get_dumpit(struct sk_buff *skb,
+ struct netlink_callback *cb)
+{
+ return 0;
+}
+
void dev_shaper_flush(struct net_device *dev)
{
struct net_shaper_info *cur;
diff --git a/net/shaper/shaper_nl_gen.c b/net/shaper/shaper_nl_gen.c
index 159b4cb6d2b8..38dd6900d2c6 100644
--- a/net/shaper/shaper_nl_gen.c
+++ b/net/shaper/shaper_nl_gen.c
@@ -50,6 +50,17 @@ static const struct nla_policy net_shaper_delete_nl_policy[NET_SHAPER_A_HANDLES
[NET_SHAPER_A_HANDLES] = NLA_POLICY_NESTED(net_shaper_handle_nl_policy),
};
+/* NET_SHAPER_CMD_CAP_GET - do */
+static const struct nla_policy net_shaper_cap_get_do_nl_policy[NET_SHAPER_A_CAPABILITIES_SCOPE + 1] = {
+ [NET_SHAPER_A_CAPABILITIES_IFINDEX] = { .type = NLA_U32, },
+ [NET_SHAPER_A_CAPABILITIES_SCOPE] = NLA_POLICY_MAX(NLA_U32, 4),
+};
+
+/* NET_SHAPER_CMD_CAP_GET - dump */
+static const struct nla_policy net_shaper_cap_get_dump_nl_policy[NET_SHAPER_A_CAPABILITIES_IFINDEX + 1] = {
+ [NET_SHAPER_A_CAPABILITIES_IFINDEX] = { .type = NLA_U32, },
+};
+
/* Ops table for net_shaper */
static const struct genl_split_ops net_shaper_nl_ops[] = {
{
@@ -80,6 +91,20 @@ static const struct genl_split_ops net_shaper_nl_ops[] = {
.maxattr = NET_SHAPER_A_HANDLES,
.flags = GENL_ADMIN_PERM | GENL_CMD_CAP_DO,
},
+ {
+ .cmd = NET_SHAPER_CMD_CAP_GET,
+ .doit = net_shaper_nl_cap_get_doit,
+ .policy = net_shaper_cap_get_do_nl_policy,
+ .maxattr = NET_SHAPER_A_CAPABILITIES_SCOPE,
+ .flags = GENL_CMD_CAP_DO,
+ },
+ {
+ .cmd = NET_SHAPER_CMD_CAP_GET,
+ .dumpit = net_shaper_nl_cap_get_dumpit,
+ .policy = net_shaper_cap_get_dump_nl_policy,
+ .maxattr = NET_SHAPER_A_CAPABILITIES_IFINDEX,
+ .flags = GENL_CMD_CAP_DUMP,
+ },
};
struct genl_family net_shaper_nl_family __ro_after_init = {
diff --git a/net/shaper/shaper_nl_gen.h b/net/shaper/shaper_nl_gen.h
index 663406224d62..aa28ddc0a812 100644
--- a/net/shaper/shaper_nl_gen.h
+++ b/net/shaper/shaper_nl_gen.h
@@ -19,6 +19,9 @@ int net_shaper_nl_get_doit(struct sk_buff *skb, struct genl_info *info);
int net_shaper_nl_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb);
int net_shaper_nl_set_doit(struct sk_buff *skb, struct genl_info *info);
int net_shaper_nl_delete_doit(struct sk_buff *skb, struct genl_info *info);
+int net_shaper_nl_cap_get_doit(struct sk_buff *skb, struct genl_info *info);
+int net_shaper_nl_cap_get_dumpit(struct sk_buff *skb,
+ struct netlink_callback *cb);
extern struct genl_family net_shaper_nl_family;
--
2.45.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH net-next 4/5] net: shaper: implement introspection support
2024-06-27 20:17 [PATCH net-next 0/5] net: introduce TX shaping H/W offload API Paolo Abeni
` (2 preceding siblings ...)
2024-06-27 20:17 ` [PATCH net-next 3/5] netlink: spec: add shaper introspection support Paolo Abeni
@ 2024-06-27 20:17 ` Paolo Abeni
2024-06-27 20:17 ` [PATCH net-next 5/5] testing: net-drv: add basic shaper test Paolo Abeni
2024-06-29 2:03 ` [PATCH net-next 0/5] net: introduce TX shaping H/W offload API Jakub Kicinski
5 siblings, 0 replies; 20+ messages in thread
From: Paolo Abeni @ 2024-06-27 20:17 UTC (permalink / raw)
To: netdev
Cc: Jakub Kicinski, Jiri Pirko, Madhu Chittim, Sridhar Samudrala,
Simon Horman, John Fastabend, Sunil Kovvuri Goutham,
Jamal Hadi Salim
The netlink op is a simple wrapper around the device callback.
Extend the existing fetch_dev() helper adding an attribute argument
for the requested device. Reuse such helper in the newly implemented
operation.
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
v4 -> v5:
- doc fix
---
include/net/net_shaper.h | 13 +++++
net/shaper/shaper.c | 108 +++++++++++++++++++++++++++++++++++----
2 files changed, 112 insertions(+), 9 deletions(-)
diff --git a/include/net/net_shaper.h b/include/net/net_shaper.h
index a4d8213f8594..aac43812f684 100644
--- a/include/net/net_shaper.h
+++ b/include/net/net_shaper.h
@@ -49,6 +49,7 @@ struct net_shaper_info {
* struct net_shaper_ops - Operations on device H/W shapers
* @set: Modify the existing shaper.
* @delete: Delete the specified shaper.
+ * @capabilities: Introspect the device shaper-related features
*
* The initial shaping configuration ad device initialization is empty/
* a no-op/does not constraint the b/w in any way.
@@ -101,6 +102,18 @@ struct net_shaper_ops {
*/
int (*delete)(struct net_device *dev, int nr, const u32 *handles,
struct netlink_ext_ack *extack);
+
+ /** capabilities - get the shaper features supported by the NIC
+ * @dev: netdevice to operate on
+ * @scope: the queried scope
+ * @flags: bitfield of supported features for the given scope
+ *
+ * Return:
+ * * %0 - Success, @flags is set according to the supported features
+ * * %-EOPNOTSUPP - the H/W does not support the specified scope
+ */
+ int (*capabilities)(struct net_device *dev, enum net_shaper_scope,
+ unsigned long *flags);
};
#define NET_SHAPER_SCOPE_SHIFT 16
diff --git a/net/shaper/shaper.c b/net/shaper/shaper.c
index 74da98aaa078..a9ac013a148c 100644
--- a/net/shaper/shaper.c
+++ b/net/shaper/shaper.c
@@ -102,16 +102,17 @@ net_shaper_fill_one(struct sk_buff *msg, struct net_shaper_info *shaper,
/* On success sets pdev to the relevant device and acquires a reference
* to it
*/
-static int fetch_dev(const struct genl_info *info, struct net_device **pdev)
+static int fetch_dev(const struct genl_info *info, int type,
+ struct net_device **pdev)
{
struct net *ns = genl_info_net(info);
struct net_device *dev;
int ifindex;
- if (GENL_REQ_ATTR_CHECK(info, NET_SHAPER_A_IFINDEX))
+ if (GENL_REQ_ATTR_CHECK(info, type))
return -EINVAL;
- ifindex = nla_get_u32(info->attrs[NET_SHAPER_A_IFINDEX]);
+ ifindex = nla_get_u32(info->attrs[type]);
dev = dev_get_by_index(ns, ifindex);
if (!dev) {
GENL_SET_ERR_MSG_FMT(info, "device %d not found", ifindex);
@@ -161,7 +162,7 @@ int net_shaper_nl_get_doit(struct sk_buff *skb, struct genl_info *info)
u32 handle;
int ret;
- ret = fetch_dev(info, &dev);
+ ret = fetch_dev(info, NET_SHAPER_A_IFINDEX, &dev);
if (ret)
return ret;
@@ -219,7 +220,7 @@ int net_shaper_nl_get_dumpit(struct sk_buff *skb,
unsigned long handle;
int ret;
- ret = fetch_dev(info, &dev);
+ ret = fetch_dev(info, NET_SHAPER_A_IFINDEX, &dev);
if (ret)
return ret;
@@ -446,7 +447,7 @@ int net_shaper_nl_set_doit(struct sk_buff *skb, struct genl_info *info)
struct net_device *dev;
struct nlattr *attr;
- ret = fetch_dev(info, &dev);
+ ret = fetch_dev(info, NET_SHAPER_A_IFINDEX, &dev);
if (ret)
return ret;
@@ -519,7 +520,7 @@ int net_shaper_nl_delete_doit(struct sk_buff *skb, struct genl_info *info)
struct nlattr *attr;
u32 *handles;
- ret = fetch_dev(info, &dev);
+ ret = fetch_dev(info, NET_SHAPER_A_IFINDEX, &dev);
if (ret)
return ret;
@@ -558,15 +559,104 @@ int net_shaper_nl_delete_doit(struct sk_buff *skb, struct genl_info *info)
return ret;
}
-int net_shaper_nl_cap_get_doit(struct sk_buff *skb, struct genl_info *info)
+static int
+net_shaper_cap_fill_one(struct sk_buff *msg, unsigned long flags,
+ const struct genl_info *info)
{
+ unsigned long cur;
+ void *hdr;
+
+ hdr = genlmsg_iput(msg, info);
+ if (!hdr)
+ return -EMSGSIZE;
+
+ for (cur = NET_SHAPER_A_CAPABILITIES_SUPPORT_METRIC_BPS;
+ cur <= NET_SHAPER_A_CAPABILITIES_MAX; ++cur) {
+ if (flags & BIT(cur) && nla_put_flag(msg, cur))
+ goto nla_put_failure;
+ }
+
+ genlmsg_end(msg, hdr);
+
return 0;
+
+nla_put_failure:
+ genlmsg_cancel(msg, hdr);
+ return -EMSGSIZE;
+}
+
+int net_shaper_nl_cap_get_doit(struct sk_buff *skb, struct genl_info *info)
+{
+ const struct net_shaper_ops *ops;
+ enum net_shaper_scope scope;
+ struct net_device *dev;
+ struct sk_buff *msg;
+ unsigned long flags;
+ int ret;
+
+ ret = fetch_dev(info, NET_SHAPER_A_CAPABILITIES_IFINDEX, &dev);
+ if (ret)
+ return ret;
+
+ if (GENL_REQ_ATTR_CHECK(info, NET_SHAPER_A_CAPABILITIES_SCOPE)) {
+ ret = -EINVAL;
+ goto put;
+ }
+
+ scope = nla_get_u32(info->attrs[NET_SHAPER_A_CAPABILITIES_SCOPE]);
+ ops = dev->netdev_ops->net_shaper_ops;
+ ret = ops->capabilities(dev, scope, &flags);
+ if (ret)
+ goto put;
+
+ msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+ if (!msg)
+ goto put;
+
+ ret = net_shaper_cap_fill_one(msg, flags, info);
+ if (ret)
+ goto free_msg;
+
+ ret = genlmsg_reply(msg, info);
+ if (ret)
+ goto free_msg;
+
+put:
+ dev_put(dev);
+ return ret;
+
+free_msg:
+ nlmsg_free(msg);
+ goto put;
}
int net_shaper_nl_cap_get_dumpit(struct sk_buff *skb,
struct netlink_callback *cb)
{
- return 0;
+ const struct genl_info *info = genl_info_dump(cb);
+ const struct net_shaper_ops *ops;
+ enum net_shaper_scope scope;
+ struct net_device *dev;
+ unsigned long flags;
+ int ret;
+
+ ret = fetch_dev(info, NET_SHAPER_A_CAPABILITIES_IFINDEX, &dev);
+ if (ret)
+ return ret;
+
+ ops = dev->netdev_ops->net_shaper_ops;
+ for (scope = 0; scope <= NET_SHAPER_SCOPE_MAX; ++scope) {
+ if (ops->capabilities(dev, scope, &flags))
+ continue;
+
+ ret = net_shaper_cap_fill_one(skb, flags, info);
+ if (ret)
+ goto put;
+ }
+
+put:
+ dev_put(dev);
+ return ret;
}
void dev_shaper_flush(struct net_device *dev)
--
2.45.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH net-next 5/5] testing: net-drv: add basic shaper test
2024-06-27 20:17 [PATCH net-next 0/5] net: introduce TX shaping H/W offload API Paolo Abeni
` (3 preceding siblings ...)
2024-06-27 20:17 ` [PATCH net-next 4/5] net: shaper: implement " Paolo Abeni
@ 2024-06-27 20:17 ` Paolo Abeni
2024-06-29 2:03 ` [PATCH net-next 0/5] net: introduce TX shaping H/W offload API Jakub Kicinski
5 siblings, 0 replies; 20+ messages in thread
From: Paolo Abeni @ 2024-06-27 20:17 UTC (permalink / raw)
To: netdev
Cc: Jakub Kicinski, Jiri Pirko, Madhu Chittim, Sridhar Samudrala,
Simon Horman, John Fastabend, Sunil Kovvuri Goutham,
Jamal Hadi Salim
Leverage a basic/dummy netdevsim implementation to do functional
coverage for NL interface.
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
v4 -> v5:
- fixed a few typos
---
drivers/net/Kconfig | 1 +
drivers/net/netdevsim/netdev.c | 29 +++
tools/testing/selftests/drivers/net/Makefile | 1 +
tools/testing/selftests/drivers/net/shaper.py | 198 ++++++++++++++++++
.../testing/selftests/net/lib/py/__init__.py | 1 +
tools/testing/selftests/net/lib/py/ynl.py | 5 +
6 files changed, 235 insertions(+)
create mode 100755 tools/testing/selftests/drivers/net/shaper.py
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 9920b3a68ed1..1fd5acdc73c6 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -641,6 +641,7 @@ config NETDEVSIM
depends on PTP_1588_CLOCK_MOCK || PTP_1588_CLOCK_MOCK=n
select NET_DEVLINK
select PAGE_POOL
+ select NET_SHAPER
help
This driver is a developer testing tool and software model that can
be used to test various control path networking APIs, especially
diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
index 017a6102be0a..60dfb7167975 100644
--- a/drivers/net/netdevsim/netdev.c
+++ b/drivers/net/netdevsim/netdev.c
@@ -22,6 +22,7 @@
#include <net/netdev_queues.h>
#include <net/page_pool/helpers.h>
#include <net/netlink.h>
+#include <net/net_shaper.h>
#include <net/pkt_cls.h>
#include <net/rtnetlink.h>
#include <net/udp_tunnel.h>
@@ -475,6 +476,33 @@ static int nsim_stop(struct net_device *dev)
return 0;
}
+static int nsim_shaper_set(struct net_device *dev, int nr,
+ const struct net_shaper_info *shapers,
+ struct netlink_ext_ack *extack)
+{
+ return nr;
+}
+
+static int nsim_shaper_del(struct net_device *dev, int nr,
+ const u32 *handles,
+ struct netlink_ext_ack *extack)
+{
+ return nr;
+}
+
+static int nsim_shaper_cap(struct net_device *dev, enum net_shaper_scope scope,
+ unsigned long *flags)
+{
+ *flags = ULONG_MAX;
+ return 0;
+}
+
+static const struct net_shaper_ops nsim_shaper_ops = {
+ .set = nsim_shaper_set,
+ .delete = nsim_shaper_del,
+ .capabilities = nsim_shaper_cap,
+};
+
static const struct net_device_ops nsim_netdev_ops = {
.ndo_start_xmit = nsim_start_xmit,
.ndo_set_rx_mode = nsim_set_rx_mode,
@@ -496,6 +524,7 @@ static const struct net_device_ops nsim_netdev_ops = {
.ndo_bpf = nsim_bpf,
.ndo_open = nsim_open,
.ndo_stop = nsim_stop,
+ .net_shaper_ops = &nsim_shaper_ops,
};
static const struct net_device_ops nsim_vf_netdev_ops = {
diff --git a/tools/testing/selftests/drivers/net/Makefile b/tools/testing/selftests/drivers/net/Makefile
index e54f382bcb02..432306f11664 100644
--- a/tools/testing/selftests/drivers/net/Makefile
+++ b/tools/testing/selftests/drivers/net/Makefile
@@ -6,6 +6,7 @@ TEST_PROGS := \
ping.py \
queues.py \
stats.py \
+ shaper.py
# end of TEST_PROGS
include ../../lib.mk
diff --git a/tools/testing/selftests/drivers/net/shaper.py b/tools/testing/selftests/drivers/net/shaper.py
new file mode 100755
index 000000000000..a5f1c0607dff
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/shaper.py
@@ -0,0 +1,198 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0
+
+from lib.py import ksft_run, ksft_exit, ksft_eq, ksft_true, KsftSkipEx
+from lib.py import ShaperFamily
+from lib.py import NetDrvEnv
+from lib.py import NlError
+from lib.py import cmd
+import glob
+import sys
+
+def get_shapers(cfg, nl_shaper) -> None:
+ try:
+ shapers = nl_shaper.get({'ifindex': cfg.ifindex}, dump=True)
+ except NlError as e:
+ if e.error == 95:
+ raise KsftSkipEx("shapers not supported by the device")
+ raise
+
+ # default configuration, no shapers configured
+ ksft_eq(len(shapers), 0)
+
+def get_caps(cfg, nl_shaper) -> None:
+ try:
+ caps = nl_shaper.cap_get({'ifindex': cfg.ifindex}, dump=True)
+ except NlError as e:
+ if e.error == 95:
+ raise KsftSkipEx("shapers not supported by the device")
+ raise
+
+ # each device implementing shaper support must support some
+ # features in at least a scope
+ ksft_true(len(caps)> 0)
+
+
+def set_qshapers(cfg, nl_shaper) -> None:
+ try:
+ caps = nl_shaper.cap_get({'ifindex': cfg.ifindex, 'scope':'queue'})
+ except NlError as e:
+ if e.error == 95:
+ cfg.queues = False;
+ raise KsftSkipEx("shapers not supported by the device")
+ raise
+ if not 'support-bw-max' in caps or not 'support-metric-bps' in caps:
+ raise KsftSkipEx("device does not support queue scope shapers with bw_max and metric bps")
+
+ nl_shaper.set({'ifindex': cfg.ifindex,
+ 'shapers': [{ 'handle': { 'scope': 'queue', 'id': 1 }, 'metric': 'bps', 'bw-max': 10000 },
+ { 'handle': { 'scope': 'queue', 'id': 2 }, 'metric': 'bps', 'bw-max': 20000 }]})
+
+ # querying a specific shaper not yet configure must fail
+ raised = False
+ try:
+ shaper_q0 = nl_shaper.get({'ifindex': cfg.ifindex, 'handle': { 'scope': 'queue', 'id': 0}})
+ except (NlError):
+ raised = True
+ ksft_eq(raised, True)
+
+ shaper_q1 = nl_shaper.get({'ifindex': cfg.ifindex, 'handle': { 'scope': 'queue', 'id': 1 }})
+ ksft_eq(shaper_q1, { 'parent': { 'scope': 'netdev', 'id': 0 },
+ 'handle': { 'scope': 'queue', 'id': 1 },
+ 'metric': 'bps',
+ 'bw-min': 0,
+ 'bw-max': 10000,
+ 'burst': 0,
+ 'priority': 0,
+ 'weight': 0 })
+
+ shapers = nl_shaper.get({'ifindex': cfg.ifindex}, dump=True)
+ ksft_eq(shapers, [{'parent': { 'scope': 'netdev', 'id': 0 },
+ 'handle': { 'scope': 'queue', 'id': 1 },
+ 'metric': 'bps',
+ 'bw-min': 0,
+ 'bw-max': 10000,
+ 'burst': 0,
+ 'priority': 0,
+ 'weight': 0 },
+ {'parent': { 'scope': 'netdev', 'id': 0 },
+ 'handle': { 'scope': 'queue', 'id': 2 },
+ 'metric': 'bps',
+ 'bw-min': 0,
+ 'bw-max': 20000,
+ 'burst': 0,
+ 'priority': 0,
+ 'weight': 0 }])
+
+
+def del_qshapers(cfg, nl_shaper) -> None:
+ if not cfg.queues:
+ raise KsftSkipEx("queue shapers not supported by device, skipping delete")
+
+ raised = False
+ try:
+ nl_shaper.delete({'ifindex': cfg.ifindex, 'handles': [ { 'scope': 'queue', 'id': 0}]})
+ except (NlError):
+ if e.error == 95:
+ raise KsftSkipEx("shapers not supported by the device")
+ raised = True
+ ksft_eq(raised, False)
+
+ nl_shaper.delete({'ifindex': cfg.ifindex,
+ 'handles': [{ 'scope': 'queue', 'id': 2},
+ { 'scope': 'queue', 'id': 1}]})
+ shapers = nl_shaper.get({'ifindex': cfg.ifindex}, dump=True)
+ ksft_eq(len(shapers), 0)
+
+def set_nshapers(cfg, nl_shaper) -> None:
+ # check required features
+ try:
+ caps = nl_shaper.cap_get({'ifindex': cfg.ifindex, 'scope':'netdev'})
+ except NlError as e:
+ if e.error == 95:
+ cfg.groups = False;
+ raise KsftSkipEx("shapers not supported by the device")
+ raise
+ if not 'support-bw-max' in caps or not 'support-metric-bps' in caps:
+ raise KsftSkipEx("device does not support nested netdev scope shapers with weight")
+
+ nl_shaper.set({'ifindex': cfg.ifindex,
+ 'shapers': [{ 'handle': { 'scope': 'netdev', 'id': 0 }, 'bw-max': 100000 }]})
+
+ shapers = nl_shaper.get({'ifindex': cfg.ifindex}, dump=True)
+ ksft_eq(shapers, [{'parent': { 'scope': 'port', 'id': 0 },
+ 'handle': { 'scope': 'netdev', 'id': 0 },
+ 'metric': 'bps',
+ 'bw-min': 0,
+ 'bw-max': 100000,
+ 'burst': 0,
+ 'priority': 0,
+ 'weight': 0 }])
+
+def del_nshapers(cfg, nl_shaper) -> None:
+ if not cfg.netdev:
+ raise KsftSkipEx("netdev shaper not supported by device, skipping delete")
+
+ nl_shaper.delete({'ifindex': cfg.ifindex,
+ 'handles': [{ 'scope': 'netdev', 'id': 0}]})
+ shapers = nl_shaper.get({'ifindex': cfg.ifindex}, dump=True)
+ ksft_eq(len(shapers), 0)
+
+def qgroups(cfg, nl_shaper) -> None:
+ try:
+ caps = nl_shaper.cap_get({'ifindex': cfg.ifindex, 'scope':'detached'})
+ except NlError as e:
+ if e.error == 95:
+ cfg.groups = False;
+ raise KsftSkipEx("shapers not supported by the device")
+ raise
+ if not 'support-bw-max' in caps or not 'support-metric-bps' in caps:
+ raise KsftSkipEx("device does not support detached scope shapers with bw_max and metric bps")
+ try:
+ caps = nl_shaper.cap_get({'ifindex': cfg.ifindex, 'scope':'queue'})
+ except NlError as e:
+ if e.error == 95:
+ cfg.groups = False;
+ raise KsftSkipEx("shapers not supported by the device")
+ raise
+ if not 'support-nesting' in caps or not 'support-weight' in caps or not 'support-metric-bps' in caps:
+ raise KsftSkipEx("device does not support nested queue scope shapers with weight")
+
+ nl_shaper.set({'ifindex': cfg.ifindex,
+ 'shapers': [{ 'parent': {'scope': 'netdev'}, 'handle': {'scope':'detached', 'id':0}, 'metric': 'bps', 'bw-max': 10000},
+ { 'parent': {'scope': 'detached', 'id':0}, 'handle': { 'scope': 'queue', 'id': 1 }, 'metric': 'bps', 'weight': 3 },
+ { 'parent': {'scope': 'detached', 'id':0}, 'handle': { 'scope': 'queue', 'id': 2 }, 'metric': 'bps', 'weight': 2 }]})
+
+ shaper = nl_shaper.get({'ifindex': cfg.ifindex, 'handle': { 'scope': 'queue', 'id': 1 }})
+ ksft_eq(shaper, {'parent': { 'scope': 'detached', 'id': 0 },
+ 'handle': { 'scope': 'queue', 'id': 1 },
+ 'metric': 'bps',
+ 'bw-min': 0,
+ 'bw-max': 0,
+ 'burst': 0,
+ 'priority': 0,
+ 'weight': 3 })
+
+ nl_shaper.delete({'ifindex': cfg.ifindex,
+ 'handles': [{ 'scope': 'queue', 'id': 2},
+ { 'scope': 'queue', 'id': 1},
+ { 'scope': 'detached', 'id': 0}]})
+ shapers = nl_shaper.get({'ifindex': cfg.ifindex}, dump=True)
+ ksft_eq(len(shapers), 0)
+
+def main() -> None:
+ with NetDrvEnv(__file__, queue_count=3) as cfg:
+ cfg.queues = True
+ cfg.netdev = True
+ ksft_run([get_shapers,
+ get_caps,
+ set_qshapers,
+ del_qshapers,
+ qgroups,
+ set_nshapers,
+ del_nshapers], args=(cfg, ShaperFamily()))
+ ksft_exit()
+
+
+if __name__ == "__main__":
+ main()
diff --git a/tools/testing/selftests/net/lib/py/__init__.py b/tools/testing/selftests/net/lib/py/__init__.py
index b6d498d125fe..ef1aa52f4761 100644
--- a/tools/testing/selftests/net/lib/py/__init__.py
+++ b/tools/testing/selftests/net/lib/py/__init__.py
@@ -6,3 +6,4 @@ from .netns import NetNS
from .nsim import *
from .utils import *
from .ynl import NlError, YnlFamily, EthtoolFamily, NetdevFamily, RtnlFamily
+from .ynl import ShaperFamily
diff --git a/tools/testing/selftests/net/lib/py/ynl.py b/tools/testing/selftests/net/lib/py/ynl.py
index 1ace58370c06..42e825905ade 100644
--- a/tools/testing/selftests/net/lib/py/ynl.py
+++ b/tools/testing/selftests/net/lib/py/ynl.py
@@ -47,3 +47,8 @@ class NetdevFamily(YnlFamily):
def __init__(self):
super().__init__((SPEC_PATH / Path('netdev.yaml')).as_posix(),
schema='')
+
+class ShaperFamily(YnlFamily):
+ def __init__(self):
+ super().__init__((SPEC_PATH / Path('shaper.yaml')).as_posix(),
+ schema='')
--
2.45.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH net-next 0/5] net: introduce TX shaping H/W offload API
2024-06-27 20:17 [PATCH net-next 0/5] net: introduce TX shaping H/W offload API Paolo Abeni
` (4 preceding siblings ...)
2024-06-27 20:17 ` [PATCH net-next 5/5] testing: net-drv: add basic shaper test Paolo Abeni
@ 2024-06-29 2:03 ` Jakub Kicinski
5 siblings, 0 replies; 20+ messages in thread
From: Jakub Kicinski @ 2024-06-29 2:03 UTC (permalink / raw)
To: Paolo Abeni
Cc: netdev, Jiri Pirko, Madhu Chittim, Sridhar Samudrala,
Simon Horman, John Fastabend, Sunil Kovvuri Goutham,
Jamal Hadi Salim
On Thu, 27 Jun 2024 22:17:17 +0200 Paolo Abeni wrote:
> The ice driver support is currently a WIP, sharing the current status
> earlier since some APIs details are still under discussion.
Let's stick to RFC until it's ready? Quoting documentation:
netdevsim
~~~~~~~~~
``netdevsim`` is a test driver which can be used to exercise driver
configuration APIs without requiring capable hardware.
Mock-ups and tests based on ``netdevsim`` are strongly encouraged when
vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
adding new APIs, but ``netdevsim`` in itself is **not** considered
a use case/user. You must also implement the new APIs in a real driver.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
See: https://www.kernel.org/doc/html/next/process/maintainer-netdev.html#netdevsim
--
pw-bot: rfc
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next 1/5] netlink: spec: add shaper YAML spec
2024-06-27 20:17 ` [PATCH net-next 1/5] netlink: spec: add shaper YAML spec Paolo Abeni
@ 2024-06-29 2:12 ` Jakub Kicinski
2024-07-01 10:14 ` Paolo Abeni
2024-07-01 14:50 ` Paolo Abeni
0 siblings, 2 replies; 20+ messages in thread
From: Jakub Kicinski @ 2024-06-29 2:12 UTC (permalink / raw)
To: Paolo Abeni
Cc: netdev, Jiri Pirko, Madhu Chittim, Sridhar Samudrala,
Simon Horman, John Fastabend, Sunil Kovvuri Goutham,
Jamal Hadi Salim
On Thu, 27 Jun 2024 22:17:18 +0200 Paolo Abeni wrote:
> + -
> + name: port
> + doc: The root shaper for the whole H/W.
> + -
> + name: netdev
> + doc: The main shaper for the given network device.
> + -
> + name: queue
> + doc: The shaper is attached to the given device queue.
> + -
> + name: detached
> + doc: |
> + The shaper can be attached to port, netdev or other
> + detached shapers, allowing nesting and grouping of
> + netdev or queues.
> + render-max: true
nit: move other properties before the list
> +attribute-sets:
> + -
> + name: net_shaper
s/_/-/ on all names
> + attributes:
> + -
> + name: ifindex
> + type: u32
> + -
> + name: parent
> + type: nest
> + nested-attributes: handle
> + -
> + name: handle
> + type: nest
> + nested-attributes: handle
> + -
> + name: metric
> + type: u32
> + enum: metric
> + -
> + name: bw-min
> + type: u64
s/u64/uint/
> + -
> + name: handles
> + type: nest
> + multi-attr: true
> + nested-attributes: handle
oh both handle and handles...
it'd be good to have more doc: for the attributes, all these things get
auto-rendered on docs.kernel.org
> + -
> + name: shapers
> + type: nest
> + multi-attr: true
> + nested-attributes: ns-info
How do shapers differ from shaping attrs in this scope? :S
> + -
> + name: modified
> + type: u32
> + -
> + name: pad
> + type: pad
after using uint this can go
> +operations:
> + list:
> + -
> + name: get
> + doc: |
> + Get / Dump information about a/all the shaper for a given device
> + attribute-set: net_shaper
> + flags: [ admin-perm ]
Any reason why get is admin-perm ?
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next 1/5] netlink: spec: add shaper YAML spec
2024-06-29 2:12 ` Jakub Kicinski
@ 2024-07-01 10:14 ` Paolo Abeni
2024-07-02 2:54 ` Jakub Kicinski
2024-07-01 14:50 ` Paolo Abeni
1 sibling, 1 reply; 20+ messages in thread
From: Paolo Abeni @ 2024-07-01 10:14 UTC (permalink / raw)
To: Jakub Kicinski
Cc: netdev, Jiri Pirko, Madhu Chittim, Sridhar Samudrala,
Simon Horman, John Fastabend, Sunil Kovvuri Goutham,
Jamal Hadi Salim
On Fri, 2024-06-28 at 19:12 -0700, Jakub Kicinski wrote:
> On Thu, 27 Jun 2024 22:17:18 +0200 Paolo Abeni wrote:
>
> > + -
> > + name: shapers
> > + type: nest
> > + multi-attr: true
> > + nested-attributes: ns-info
>
> How do shapers differ from shaping attrs in this scope? :S
the set() operation must configure multiple shapers with a single
command - to allow the 'atomic configuration changes' need for Andrew's
use-case.
Out-of-sheer ignorance on my side, the above was the most straight-
forward way to provide set() with an array of shapers.
Do you mean there are better way to achieve the goal, or "just" that
the documentation here is missing and _necessary_?
> > +operations:
> > + list:
> > + -
> > + name: get
> > + doc: |
> > + Get / Dump information about a/all the shaper for a given device
> > + attribute-set: net_shaper
> > + flags: [ admin-perm ]
>
> Any reason why get is admin-perm ?
Mostly a "better safe then sorry" approach and cargo-cult form other
recent yaml changes the hard reasons. Fine to drop it, if there is
agreement.
Side note: ack to everything else noted in your reply.
Thanks,
Paolo
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next 1/5] netlink: spec: add shaper YAML spec
2024-06-29 2:12 ` Jakub Kicinski
2024-07-01 10:14 ` Paolo Abeni
@ 2024-07-01 14:50 ` Paolo Abeni
2024-07-01 15:50 ` Paolo Abeni
1 sibling, 1 reply; 20+ messages in thread
From: Paolo Abeni @ 2024-07-01 14:50 UTC (permalink / raw)
To: Jakub Kicinski
Cc: netdev, Jiri Pirko, Madhu Chittim, Sridhar Samudrala,
Simon Horman, John Fastabend, Sunil Kovvuri Goutham,
Jamal Hadi Salim
On Fri, 2024-06-28 at 19:12 -0700, Jakub Kicinski wrote:
> On Thu, 27 Jun 2024 22:17:18 +0200 Paolo Abeni wrote:
>
> > +attribute-sets:
> > + -
> > + name: net_shaper
>
> s/_/-/ on all names
It looks like the initial
name: net_shaper
would require some patching to ynl-gen-c.py to handle the s/_/-/
conversion in the generated file names, too.
Cheers,
Paolo
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next 1/5] netlink: spec: add shaper YAML spec
2024-07-01 14:50 ` Paolo Abeni
@ 2024-07-01 15:50 ` Paolo Abeni
2024-07-02 0:37 ` Jakub Kicinski
0 siblings, 1 reply; 20+ messages in thread
From: Paolo Abeni @ 2024-07-01 15:50 UTC (permalink / raw)
To: Jakub Kicinski
Cc: netdev, Jiri Pirko, Madhu Chittim, Sridhar Samudrala,
Simon Horman, John Fastabend, Sunil Kovvuri Goutham,
Jamal Hadi Salim
On Mon, 2024-07-01 at 16:50 +0200, Paolo Abeni wrote:
> On Fri, 2024-06-28 at 19:12 -0700, Jakub Kicinski wrote:
> > On Thu, 27 Jun 2024 22:17:18 +0200 Paolo Abeni wrote:
> >
> > > +attribute-sets:
> > > + -
> > > + name: net_shaper
> >
> > s/_/-/ on all names
>
> It looks like the initial
>
> name: net_shaper
>
> would require some patching to ynl-gen-c.py to handle the s/_/-/
> conversion in the generated file names, too.
I mean something alike the following, to be explicit:
---
diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py
index 374ca5e86e24..51529fabd517 100755
--- a/tools/net/ynl/ynl-gen-c.py
+++ b/tools/net/ynl/ynl-gen-c.py
@@ -59,9 +59,9 @@ class Type(SpecAttr):
if 'nested-attributes' in attr:
self.nested_attrs = attr['nested-attributes']
if self.nested_attrs == family.name:
- self.nested_render_name = c_lower(f"{family.name}")
+ self.nested_render_name = c_lower(f"{family.ident_name}")
else:
- self.nested_render_name = c_lower(f"{family.name}_{self.nested_attrs}")
+ self.nested_render_name = c_lower(f"{family.ident_name}_{self.nested_attrs}")
if self.nested_attrs in self.family.consts:
self.nested_struct_type = 'struct ' + self.nested_render_name + '_'
@@ -693,9 +693,9 @@ class Struct:
self.nested = type_list is None
if family.name == c_lower(space_name):
- self.render_name = c_lower(family.name)
+ self.render_name = c_lower(family.ident_name)
else:
- self.render_name = c_lower(family.name + '-' + space_name)
+ self.render_name = c_lower(family.ident_name + '-' + space_name)
self.struct_name = 'struct ' + self.render_name
if self.nested and space_name in family.consts:
self.struct_name += '_'
@@ -761,7 +761,7 @@ class EnumEntry(SpecEnumEntry):
class EnumSet(SpecEnumSet):
def __init__(self, family, yaml):
- self.render_name = c_lower(family.name + '-' + yaml['name'])
+ self.render_name = c_lower(family.ident_name + '-' + yaml['name'])
if 'enum-name' in yaml:
if yaml['enum-name']:
@@ -777,7 +777,7 @@ class EnumSet(SpecEnumSet):
else:
self.user_type = 'int'
- self.value_pfx = yaml.get('name-prefix', f"{family.name}-{yaml['name']}-")
+ self.value_pfx = yaml.get('name-prefix', f"{family.ident_name}-{yaml['name']}-")
super().__init__(family, yaml)
@@ -802,9 +802,9 @@ class AttrSet(SpecAttrSet):
if 'name-prefix' in yaml:
pfx = yaml['name-prefix']
elif self.name == family.name:
- pfx = family.name + '-a-'
+ pfx = family.ident_name + '-a-'
else:
- pfx = f"{family.name}-a-{self.name}-"
+ pfx = f"{family.ident_name}-a-{self.name}-"
self.name_prefix = c_upper(pfx)
self.max_name = c_upper(self.yaml.get('attr-max-name', f"{self.name_prefix}max"))
self.cnt_name = c_upper(self.yaml.get('attr-cnt-name', f"__{self.name_prefix}max"))
@@ -861,7 +861,7 @@ class Operation(SpecOperation):
def __init__(self, family, yaml, req_value, rsp_value):
super().__init__(family, yaml, req_value, rsp_value)
- self.render_name = c_lower(family.name + '_' + self.name)
+ self.render_name = c_lower(family.ident_name + '_' + self.name)
self.dual_policy = ('do' in yaml and 'request' in yaml['do']) and \
('dump' in yaml and 'request' in yaml['dump'])
@@ -911,11 +911,11 @@ class Family(SpecFamily):
if 'uapi-header' in self.yaml:
self.uapi_header = self.yaml['uapi-header']
else:
- self.uapi_header = f"linux/{self.name}.h"
+ self.uapi_header = f"linux/{self.ident_name}.h"
if self.uapi_header.startswith("linux/") and self.uapi_header.endswith('.h'):
self.uapi_header_name = self.uapi_header[6:-2]
else:
- self.uapi_header_name = self.name
+ self.uapi_header_name = self.ident_name
def resolve(self):
self.resolve_up(super())
@@ -923,7 +923,7 @@ class Family(SpecFamily):
if self.yaml.get('protocol', 'genetlink') not in {'genetlink', 'genetlink-c', 'genetlink-legacy'}:
raise Exception("Codegen only supported for genetlink")
- self.c_name = c_lower(self.name)
+ self.c_name = c_lower(self.ident_name)
if 'name-prefix' in self.yaml['operations']:
self.op_prefix = c_upper(self.yaml['operations']['name-prefix'])
else:
@@ -2173,7 +2173,7 @@ def print_kernel_op_table_fwd(family, cw, terminate):
exported = not kernel_can_gen_family_struct(family)
if not terminate or exported:
- cw.p(f"/* Ops table for {family.name} */")
+ cw.p(f"/* Ops table for {family.ident_name} */")
pol_to_struct = {'global': 'genl_small_ops',
'per-op': 'genl_ops',
@@ -2225,12 +2225,12 @@ def print_kernel_op_table_fwd(family, cw, terminate):
continue
if 'do' in op:
- name = c_lower(f"{family.name}-nl-{op_name}-doit")
+ name = c_lower(f"{family.ident_name}-nl-{op_name}-doit")
cw.write_func_prot('int', name,
['struct sk_buff *skb', 'struct genl_info *info'], suffix=';')
if 'dump' in op:
- name = c_lower(f"{family.name}-nl-{op_name}-dumpit")
+ name = c_lower(f"{family.ident_name}-nl-{op_name}-dumpit")
cw.write_func_prot('int', name,
['struct sk_buff *skb', 'struct netlink_callback *cb'], suffix=';')
cw.nl()
@@ -2256,13 +2256,13 @@ def print_kernel_op_table(family, cw):
for x in op['dont-validate']])), )
for op_mode in ['do', 'dump']:
if op_mode in op:
- name = c_lower(f"{family.name}-nl-{op_name}-{op_mode}it")
+ name = c_lower(f"{family.ident_name}-nl-{op_name}-{op_mode}it")
members.append((op_mode + 'it', name))
if family.kernel_policy == 'per-op':
struct = Struct(family, op['attribute-set'],
type_list=op['do']['request']['attributes'])
- name = c_lower(f"{family.name}-{op_name}-nl-policy")
+ name = c_lower(f"{family.ident_name}-{op_name}-nl-policy")
members.append(('policy', name))
members.append(('maxattr', struct.attr_max_val.enum_name))
if 'flags' in op:
@@ -2294,7 +2294,7 @@ def print_kernel_op_table(family, cw):
members.append(('validate',
' | '.join([c_upper('genl-dont-validate-' + x)
for x in dont_validate])), )
- name = c_lower(f"{family.name}-nl-{op_name}-{op_mode}it")
+ name = c_lower(f"{family.ident_name}-nl-{op_name}-{op_mode}it")
if 'pre' in op[op_mode]:
members.append((cb_names[op_mode]['pre'], c_lower(op[op_mode]['pre'])))
members.append((op_mode + 'it', name))
@@ -2305,9 +2305,9 @@ def print_kernel_op_table(family, cw):
type_list=op[op_mode]['request']['attributes'])
if op.dual_policy:
- name = c_lower(f"{family.name}-{op_name}-{op_mode}-nl-policy")
+ name = c_lower(f"{family.ident_name}-{op_name}-{op_mode}-nl-policy")
else:
- name = c_lower(f"{family.name}-{op_name}-nl-policy")
+ name = c_lower(f"{family.ident_name}-{op_name}-nl-policy")
members.append(('policy', name))
members.append(('maxattr', struct.attr_max_val.enum_name))
flags = (op['flags'] if 'flags' in op else []) + ['cmd-cap-' + op_mode]
@@ -2326,7 +2326,7 @@ def print_kernel_mcgrp_hdr(family, cw):
cw.block_start('enum')
for grp in family.mcgrps['list']:
- grp_id = c_upper(f"{family.name}-nlgrp-{grp['name']},")
+ grp_id = c_upper(f"{family.ident_name}-nlgrp-{grp['name']},")
cw.p(grp_id)
cw.block_end(';')
cw.nl()
@@ -2339,7 +2339,7 @@ def print_kernel_mcgrp_src(family, cw):
cw.block_start('static const struct genl_multicast_group ' + family.c_name + '_nl_mcgrps[] =')
for grp in family.mcgrps['list']:
name = grp['name']
- grp_id = c_upper(f"{family.name}-nlgrp-{name}")
+ grp_id = c_upper(f"{family.ident_name}-nlgrp-{name}")
cw.p('[' + grp_id + '] = { "' + name + '", },')
cw.block_end(';')
cw.nl()
@@ -2361,7 +2361,7 @@ def print_kernel_family_struct_src(family, cw):
if not kernel_can_gen_family_struct(family):
return
- cw.block_start(f"struct genl_family {family.name}_nl_family __ro_after_init =")
+ cw.block_start(f"struct genl_family {family.ident_name}_nl_family __ro_after_init =")
cw.p('.name\t\t= ' + family.fam_key + ',')
cw.p('.version\t= ' + family.ver_key + ',')
cw.p('.netnsok\t= true,')
@@ -2429,7 +2429,7 @@ def render_uapi(family, cw):
cw.p(' */')
uapi_enum_start(family, cw, const, 'name')
- name_pfx = const.get('name-prefix', f"{family.name}-{const['name']}-")
+ name_pfx = const.get('name-prefix', f"{family.ident_name}-{const['name']}-")
for entry in enum.entries.values():
suffix = ','
if entry.value_change:
@@ -2451,7 +2451,7 @@ def render_uapi(family, cw):
cw.nl()
elif const['type'] == 'const':
defines.append([c_upper(family.get('c-define-name',
- f"{family.name}-{const['name']}")),
+ f"{family.ident_name}-{const['name']}")),
const['value']])
if defines:
@@ -2529,7 +2529,7 @@ def render_uapi(family, cw):
defines = []
for grp in family.mcgrps['list']:
name = grp['name']
- defines.append([c_upper(grp.get('c-define-name', f"{family.name}-mcgrp-{name}")),
+ defines.append([c_upper(grp.get('c-define-name', f"{family.ident_name}-mcgrp-{name}")),
f'{name}'])
cw.nl()
if defines:
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH net-next 1/5] netlink: spec: add shaper YAML spec
2024-07-01 15:50 ` Paolo Abeni
@ 2024-07-02 0:37 ` Jakub Kicinski
0 siblings, 0 replies; 20+ messages in thread
From: Jakub Kicinski @ 2024-07-02 0:37 UTC (permalink / raw)
To: Paolo Abeni
Cc: netdev, Jiri Pirko, Madhu Chittim, Sridhar Samudrala,
Simon Horman, John Fastabend, Sunil Kovvuri Goutham,
Jamal Hadi Salim
On Mon, 01 Jul 2024 17:50:17 +0200 Paolo Abeni wrote:
> > would require some patching to ynl-gen-c.py to handle the s/_/-/
> > conversion in the generated file names, too.
>
> I mean something alike the following, to be explicit:
Great, thanks! I wasn't sure we're actually ready to make the family
name use dashes, since we don't have any precedent. But since you have
the patch - please submit!
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next 1/5] netlink: spec: add shaper YAML spec
2024-07-01 10:14 ` Paolo Abeni
@ 2024-07-02 2:54 ` Jakub Kicinski
2024-07-02 14:21 ` Paolo Abeni
0 siblings, 1 reply; 20+ messages in thread
From: Jakub Kicinski @ 2024-07-02 2:54 UTC (permalink / raw)
To: Paolo Abeni
Cc: netdev, Jiri Pirko, Madhu Chittim, Sridhar Samudrala,
Simon Horman, John Fastabend, Sunil Kovvuri Goutham,
Jamal Hadi Salim
On Mon, 01 Jul 2024 12:14:32 +0200 Paolo Abeni wrote:
> > > + -
> > > + name: shapers
> > > + type: nest
> > > + multi-attr: true
> > > + nested-attributes: ns-info
> >
> > How do shapers differ from shaping attrs in this scope? :S
>
> the set() operation must configure multiple shapers with a single
> command - to allow the 'atomic configuration changes' need for Andrew's
> use-case.
>
> Out-of-sheer ignorance on my side, the above was the most straight-
> forward way to provide set() with an array of shapers.
>
> Do you mean there are better way to achieve the goal, or "just" that
> the documentation here is missing and _necessary_?
I see, I had a look at patch 2 now.
But that's really "Andrew's use-case" it doesn't cover deletion, right?
Sorry that I don't have a perfect suggestion either but it seems like
a half-measure. It's a partial support for transactions. If we want
transactions we should group ops like nftables. Have normal ops (add,
delete, modify) and control ops (start, commit) which clone the entire
tree, then ops change it, and commit presents new tree to the device.
Alternative would be to, instead of supporting transactions have some
form of a "complex instruction set". Most transformations will take a
set of inputs (+weights / prios), shaping params, and where to attach.
> > > +operations:
> > > + list:
> > > + -
> > > + name: get
> > > + doc: |
> > > + Get / Dump information about a/all the shaper for a given device
> > > + attribute-set: net_shaper
> > > + flags: [ admin-perm ]
> >
> > Any reason why get is admin-perm ?
>
> Mostly a "better safe then sorry" approach and cargo-cult form other
> recent yaml changes the hard reasons. Fine to drop it, if there is
> agreement.
I thought we default to GET being non-privileged.
I think that's better, monitoring shouldn't require admin perm
and presumably those shapers may grow stats at some stage.
But no strong feelings.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next 1/5] netlink: spec: add shaper YAML spec
2024-07-02 2:54 ` Jakub Kicinski
@ 2024-07-02 14:21 ` Paolo Abeni
2024-07-02 15:04 ` Jakub Kicinski
0 siblings, 1 reply; 20+ messages in thread
From: Paolo Abeni @ 2024-07-02 14:21 UTC (permalink / raw)
To: Jakub Kicinski
Cc: netdev, Jiri Pirko, Madhu Chittim, Sridhar Samudrala,
Simon Horman, John Fastabend, Sunil Kovvuri Goutham,
Jamal Hadi Salim
On Mon, 2024-07-01 at 19:54 -0700, Jakub Kicinski wrote:
> On Mon, 01 Jul 2024 12:14:32 +0200 Paolo Abeni wrote:
> > > > + -
> > > > + name: shapers
> > > > + type: nest
> > > > + multi-attr: true
> > > > + nested-attributes: ns-info
> > >
> > > How do shapers differ from shaping attrs in this scope? :S
> >
> > the set() operation must configure multiple shapers with a single
> > command - to allow the 'atomic configuration changes' need for Andrew's
> > use-case.
> >
> > Out-of-sheer ignorance on my side, the above was the most straight-
> > forward way to provide set() with an array of shapers.
> >
> > Do you mean there are better way to achieve the goal, or "just" that
> > the documentation here is missing and _necessary_?
>
> I see, I had a look at patch 2 now.
> But that's really "Andrew's use-case" it doesn't cover deletion, right?
> Sorry that I don't have a perfect suggestion either but it seems like
> a half-measure. It's a partial support for transactions. If we want
> transactions we should group ops like nftables. Have normal ops (add,
> delete, modify) and control ops (start, commit) which clone the entire
> tree, then ops change it, and commit presents new tree to the device.
Yes, it does not cover deletion _and_ update/add/move within the same
atomic operation.
Still any configuration could be reached from default/initial state
with set(<possibly many shapers>). Additionally, given any arbitrary
configuration, the default/initial state could be restored with a
single delete(<possibly many handlers>).
The above covers any possible limitation enforced by the H/W, not just
the DSA use-case.
Do you have a strong feeling for atomic transactions from any arbitrary
state towards any other? If so, I’d like to understand why?
Dealing with transactions allowing arbitrary any state <> any state
atomic changes will involve some complex logic that seems better
assigned to user-space.
Thanks,
Paolo
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next 1/5] netlink: spec: add shaper YAML spec
2024-07-02 14:21 ` Paolo Abeni
@ 2024-07-02 15:04 ` Jakub Kicinski
[not found] ` <CAF6piCLnrDWo70ZgXLtdmRkr+w5TMtuXPMW9=JKSSN2fvw1HMA@mail.gmail.com>
0 siblings, 1 reply; 20+ messages in thread
From: Jakub Kicinski @ 2024-07-02 15:04 UTC (permalink / raw)
To: Paolo Abeni
Cc: netdev, Jiri Pirko, Madhu Chittim, Sridhar Samudrala,
Simon Horman, John Fastabend, Sunil Kovvuri Goutham,
Jamal Hadi Salim
On Tue, 02 Jul 2024 16:21:38 +0200 Paolo Abeni wrote:
> > I see, I had a look at patch 2 now.
> > But that's really "Andrew's use-case" it doesn't cover deletion, right?
> > Sorry that I don't have a perfect suggestion either but it seems like
> > a half-measure. It's a partial support for transactions. If we want
> > transactions we should group ops like nftables. Have normal ops (add,
> > delete, modify) and control ops (start, commit) which clone the entire
> > tree, then ops change it, and commit presents new tree to the device.
>
> Yes, it does not cover deletion _and_ update/add/move within the same
> atomic operation.
>
> Still any configuration could be reached from default/initial state
> with set(<possibly many shapers>). Additionally, given any arbitrary
> configuration, the default/initial state could be restored with a
> single delete(<possibly many handlers>).
From:
q0 -. RR \
q1 / > SP
q2 -. RR /
q3 /
To:
q0 ------\
q1 -------> SP
q2 -. RR /
q3 /
You have to both delete an RR node, and set SP params on Q0 and Q1.
> The above covers any possible limitation enforced by the H/W, not just
> the DSA use-case.
>
> Do you have a strong feeling for atomic transactions from any arbitrary
> state towards any other? If so, I’d like to understand why?
I don't believe this is covers all cases.
> Dealing with transactions allowing arbitrary any state <> any state
> atomic changes will involve some complex logic that seems better
> assigned to user-space.
Complex logic in which part of the code?
It's just a full clone of the xarray, then do whatever ops user is
asking to do, then tree walk to render diff as a set of ops.
If you mean the tree walk to convert tree diff into ops, I think we
need that anyway, otherwise we may get into a situation where there's
a dependency between the user space implementation and driver
expectations.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Fwd: [PATCH net-next 1/5] netlink: spec: add shaper YAML spec
[not found] ` <CAF6piCLnrDWo70ZgXLtdmRkr+w5TMtuXPMW9=JKSSN2fvw1HMA@mail.gmail.com>
@ 2024-07-02 19:51 ` Paolo Abeni
[not found] ` <20240702140830.2890f77b@kernel.org>
1 sibling, 0 replies; 20+ messages in thread
From: Paolo Abeni @ 2024-07-02 19:51 UTC (permalink / raw)
To: Network Development, Jiri Pirko, Madhu Chittim, Sridhar Samudrala,
Simon Horman, John Fastabend, Sunil Kovvuri Goutham,
Jamal Hadi Salim
Oops, I unintentionally dropped most recipients, re-adding them, sorry
for the duplicate.
On Tue, Jul 2, 2024 at 5:05 PM Jakub Kicinski <kuba@kernel.org> wrote:
> On Tue, 02 Jul 2024 16:21:38 +0200 Paolo Abeni wrote:
> > > I see, I had a look at patch 2 now.
> > > But that's really "Andrew's use-case" it doesn't cover deletion, right?
> > > Sorry that I don't have a perfect suggestion either but it seems like
> > > a half-measure. It's a partial support for transactions. If we want
> > > transactions we should group ops like nftables. Have normal ops (add,
> > > delete, modify) and control ops (start, commit) which clone the entire
> > > tree, then ops change it, and commit presents new tree to the device.
> >
> > Yes, it does not cover deletion _and_ update/add/move within the same
> > atomic operation.
> >
> > Still any configuration could be reached from default/initial state
> > with set(<possibly many shapers>). Additionally, given any arbitrary
> > configuration, the default/initial state could be restored with a
> > single delete(<possibly many handlers>).
>
> From:
>
> q0 -. RR \
> q1 / > SP
> q2 -. RR /
> q3 /
Call this C1
> To:
>
> q0 ------\
> q1 -------> SP
> q2 -. RR /
> q3 /
Call this C2
> You have to both delete an RR node, and set SP params on Q0 and Q1.
default -> C1:
./tools/net/ynl/cli.py --spec Documentation/netlink/specs/shaper.yaml \
--do set --json '{ "ifindex":2, "shapers": [ \
{ "parent": { "scope": "netdev"}, "handle": {
"scope": "detached", "id": 1 }, "priority": 1 },
{ "parent": { "scope": "netdev"}, "handle": {
"scope": "detached", "id": 2 }, "priority": 2 },
{ "parent": { "scope": "detached", "id":1},
"handle": { "scope": "queue", "id": 1 }, "weight": 1 },
{ "parent": { "scope": "detached", "id":1},
"handle": { "scope": "queue", "id": 2 }, "weight": 2 },
{ "parent": { "scope": "detached" "id":2},
"handle": { "scope": "queue", "id": 3 }, "weight": 1 },
{ "parent": { "scope": "detached" "id":2},
"handle": { "scope": "queue", "id": 4 }, "weight": 2 }]}
C1 -> C2:
./tools/net/ynl/cli.py --spec Documentation/netlink/specs/shaper.yaml \
--do delete --json '{ "ifindex":2, "handles": [ \
{ "scope": "queue", "id": 1 },
{ "scope": "queue", "id": 2 },
{ "scope": "queue", "id": 3 },
{ "scope": "queue", "id": 4 },
{ "scope": "detached", "id": 1 },
{ "scope": "detached", "id": 2 }]}
./tools/net/ynl/cli.py --spec Documentation/netlink/specs/shaper.yaml \
--do set --json '{ "ifindex":2, "shapers": [ \
{ "parent": { "scope": "netdev"}, "handle": {
"scope": "detached", "id": 1 }, "priority": 1 },
{ "parent": { "scope": "netdev"}, "handle": {
"scope": "queue", "id": 1 }, "priority": 2 },
{ "parent": { "scope": "netdev"}, "handle": {
"scope": "queue", "id": 2 }, "priorirty": 3 },
{ "parent": { "scope": "detached" "id":1},
"handle": { "scope": "queue", "id": 3 }, "weight": 1 },
{ "parent": { "scope": "detached" "id":1},
"handle": { "scope": "queue", "id": 4 }, "weight": 2 },
The goal is to allow the system to reach the final configuration, even
with the assumption the H/W does not support any configuration except
the starting one and the final one.
But the infra requires that the system _must_ support at least a 3rd
configuration, the default one.
> > The above covers any possible limitation enforced by the H/W, not just
> > the DSA use-case.
> >
> > Do you have a strong feeling for atomic transactions from any arbitrary
> > state towards any other? If so, I’d like to understand why?
>
> I don't believe this is covers all cases.
Given any pair of configurations C1, C2 we can reach C2 via C1 ->
default, default -> C2. respecting any H/W constraint.
> > Dealing with transactions allowing arbitrary any state <> any state
> > atomic changes will involve some complex logic that seems better
> > assigned to user-space.
>
> Complex logic in which part of the code?
IIRC in a past iteration you pointed out that the complexity of
computing the delta between 2 arbitrary configurations is
significantly higher than going through the empty/default
configuration.
In any case I think that the larger complexity to implement a full
transactional model. nft had proven that to be very hard and bug
prone. I really would avoid that option, if possible.
Thanks,
Paolo
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next 1/5] netlink: spec: add shaper YAML spec
[not found] ` <20240702140830.2890f77b@kernel.org>
@ 2024-07-03 14:53 ` Paolo Abeni
2024-07-03 21:20 ` Jakub Kicinski
0 siblings, 1 reply; 20+ messages in thread
From: Paolo Abeni @ 2024-07-03 14:53 UTC (permalink / raw)
To: Jakub Kicinski
Cc: netdev, Jiri Pirko, Madhu Chittim, Sridhar Samudrala,
Simon Horman, John Fastabend, Sunil Kovvuri Goutham,
Jamal Hadi Salim
On Tue, 2024-07-02 at 14:08 -0700, Jakub Kicinski wrote:
> Not sure if dropping CCs was on purpose ?
Nope, just PEBKAC here. Re-adding them and include your reply verbatim,
to avoid losing track on the ML
> On Tue, 2 Jul 2024 21:49:09 +0200 Paolo Abeni wrote:
> > On Tue, Jul 2, 2024 at 5:05 PM Jakub Kicinski <kuba@kernel.org> wrote:
> > > On Tue, 02 Jul 2024 16:21:38 +0200 Paolo Abeni wrote:
> > > > Yes, it does not cover deletion _and_ update/add/move within the same
> > > > atomic operation.
> > > >
> > > > Still any configuration could be reached from default/initial state
> > > > with set(<possibly many shapers>). Additionally, given any arbitrary
> > > > configuration, the default/initial state could be restored with a
> > > > single delete(<possibly many handlers>).
> > >
> > > From:
> > >
> > > q0 -. RR \
> > > q1 / > SP
> > > q2 -. RR /
> > > q3 /
> >
> > Call this C1
> >
> > > To:
> > >
> > > q0 ------\
> > > q1 -------> SP
> > > q2 -. RR /
> > > q3 /
> >
> > Call this C2
> >
> > > You have to both delete an RR node, and set SP params on Q0 and Q1.
> >
> > default -> C1:
> >
> > ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/shaper.yaml \
> > --do set --json '{ "ifindex":2, "shapers": [ \
> > { "parent": { "scope": "netdev"}, "handle": {
> > "scope": "detached", "id": 1 }, "priority": 1 },
> > { "parent": { "scope": "netdev"}, "handle": {
> > "scope": "detached", "id": 2 }, "priority": 2 },
> > { "parent": { "scope": "detached", "id":1},
> > "handle": { "scope": "queue", "id": 1 }, "weight": 1 },
> > { "parent": { "scope": "detached", "id":1},
> > "handle": { "scope": "queue", "id": 2 }, "weight": 2 },
> > { "parent": { "scope": "detached" "id":2},
> > "handle": { "scope": "queue", "id": 3 }, "weight": 1 },
> > { "parent": { "scope": "detached" "id":2},
> > "handle": { "scope": "queue", "id": 4 }, "weight": 2 }]}
> > C1 -> C2:
> >
> > ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/shaper.yaml \
> > --do delete --json '{ "ifindex":2, "handles": [ \
> > { "scope": "queue", "id": 1 },
> > { "scope": "queue", "id": 2 },
> > { "scope": "queue", "id": 3 },
> > { "scope": "queue", "id": 4 },
> > { "scope": "detached", "id": 1 },
> > { "scope": "detached", "id": 2 }]}
> >
> > ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/shaper.yaml \
> > --do set --json '{ "ifindex":2, "shapers": [ \
> > { "parent": { "scope": "netdev"}, "handle": {
> > "scope": "detached", "id": 1 }, "priority": 1 },
> > { "parent": { "scope": "netdev"}, "handle": {
> > "scope": "queue", "id": 1 }, "priority": 2 },
> > { "parent": { "scope": "netdev"}, "handle": {
> > "scope": "queue", "id": 2 }, "priorirty": 3 },
> > { "parent": { "scope": "detached" "id":1},
> > "handle": { "scope": "queue", "id": 3 }, "weight": 1 },
> > { "parent": { "scope": "detached" "id":1},
> > "handle": { "scope": "queue", "id": 4 }, "weight": 2 },
> >
> > The goal is to allow the system to reach the final configuration, even
> > with the assumption the H/W does not support any configuration except
> > the starting one and the final one.
> > But the infra requires that the system _must_ support at least a 3rd
> > configuration, the default one.
>
> This assumes there is a single daemon responsible for control of all of
> the shaping, which I think the endless BPF multi-program back and forth
> proves to be incorrect.
>
> We'd also lose stats each time, and make reconfiguration disruptive to
> running workloads.
Note there is no stats support for the shapers, nor is planned, nor the
H/W I know of have any support for it.
The destructive operations will be needed only when the configuration
change is inherently destructive. Nothing prevents the user-space to
push a direct configuration change when possible - which should be the
most frequent case, in practice.
Regarding the entity responsible for control, I had in mind a single
one, yes. I read the above as you are looking forward to e.g. different
applications configuring their own shaping accessing directly the NL
interface, am I correct? Why can’t such applications talk to that
daemon, instead?
Anyway different applications must touch disjoint resources (e.g.
disjoint queues sets) right? In such a case even multiple destructive
configuration changes (on disjoint resources set) will not be
problematic.
Still if we want to allow somewhat consistent, concurrent, destructive
configuration changes on shared resource (why? It sounds a bit of
overdesign at this point), we could extend the set() operation to
additional support shapers deletion, e.g. adding an additional ‘delete’
flag attribute to the ‘handle’ sub-set.
> > > > The above covers any possible limitation enforced by the H/W, not just
> > > > the DSA use-case.
> > > >
> > > > Do you have a strong feeling for atomic transactions from any arbitrary
> > > > state towards any other? If so, I’d like to understand why?
> > >
> > > I don't believe this is covers all cases.
> >
> > Given any pair of configurations C1, C2 we can reach C2 via C1 ->
> > default, default -> C2. respecting any H/W constraint.
> >
> > > > Dealing with transactions allowing arbitrary any state <> any state
> > > > atomic changes will involve some complex logic that seems better
> > > > assigned to user-space.
> > >
> > > Complex logic in which part of the code?
> >
> > IIRC in a past iteration you pointed out that the complexity of
> > computing the delta between 2 arbitrary configurations is
> > significantly higher than going through the empty/default
> > configuration.
>
> For a fixed-layout scheduler where HW blocks have a fixed mapping
> with user's hierarchy - it's easier to program the shapers from
> the hierarchy directly. Since node maps to the same set of registers
> reprogramming will be writing to a register a value it already has
> - a noop. That's different than doing a full reset and reprogram
> each time, as two separate calls from user space.
>
> For Intel's cases OTOH, when each command is a separate FW call
> we can't actually enforce the atomic transitions, right?
> Your code seems to handle returns smaller the number of commands,
> from which I infer that we may execute half of the modification?
Yes, the code and the NL API allows the NIC to do the update
incrementally, and AFAICS Intel ICE has no support for complex
transactions.
Somewhat enforcing atomic transitions will be quite complex at best and
is not need to accomplish the stated goal - allow reconfiguration even
when the H/W does not support intermediate states.
Do we need to enforce atomicity? Why? NFT has proven that a
transational model implementation is hard, and should be avoided if
possible.
> IOW for Andrew's HW - he'd probably prefer to look at the resulting
> tree, no matter what previous state we were in. For Intel we _can't_
> support atomic commands, if they span multiple cycles of FW exchanges?
My understanding is that we can’t have atomic updates on Intel, from
firmare perspective. As said, I don’t think it’s necessary to support
them.
WRT the DSA H/W, do you mean the core should always set() the whole
known tree to the driver, regardless of the specific changes asked by
the user-space? If so, what about letting the driver expose some
capability (or private flag) asking the core for such behavior? So that
the driver will do the largish set() only with the H/W requiring that.
Anyway I'm not sure the mentioned DSA H/W would benefit from always
receiving the whole configuration. e.g. changing the weight for a
single queue shaper would not need the whole data-set.
> > In any case I think that the larger complexity to implement a full
> > transactional model. nft had proven that to be very hard and bug
> > prone. I really would avoid that option, if possible.
>
> Maybe instead of discussing the user space API it'd be more beneficial
> to figure out a solid way of translating the existing APIs into the new
> model?
Could you please rephrase? I think all the arguments discussed here are
related to the model - at some point that impact the user space API,
too.
Thanks,
Paolo
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next 1/5] netlink: spec: add shaper YAML spec
2024-07-03 14:53 ` Paolo Abeni
@ 2024-07-03 21:20 ` Jakub Kicinski
2024-07-08 19:42 ` Paolo Abeni
0 siblings, 1 reply; 20+ messages in thread
From: Jakub Kicinski @ 2024-07-03 21:20 UTC (permalink / raw)
To: Paolo Abeni
Cc: netdev, Jiri Pirko, Madhu Chittim, Sridhar Samudrala,
Simon Horman, John Fastabend, Sunil Kovvuri Goutham,
Jamal Hadi Salim
On Wed, 03 Jul 2024 16:53:38 +0200 Paolo Abeni wrote:
> Note there is no stats support for the shapers, nor is planned, nor the
> H/W I know of have any support for it.
>
> The destructive operations will be needed only when the configuration
> change is inherently destructive. Nothing prevents the user-space to
> push a direct configuration change when possible - which should be the
> most frequent case, in practice.
>
> Regarding the entity responsible for control, I had in mind a single
> one, yes. I read the above as you are looking forward to e.g. different
> applications configuring their own shaping accessing directly the NL
> interface, am I correct? Why can’t such applications talk to that
> daemon, instead?
We know such daemon did not materialize in other cases.
We can assume there is a daemon if we think that's a good design.
We shouldn't use a mirage of a third-party daemon to wave away
problems we didn't manage to solve.
> Anyway different applications must touch disjoint resources (e.g.
> disjoint queues sets) right? In such a case even multiple destructive
> configuration changes (on disjoint resources set) will not be
> problematic.
>
> Still if we want to allow somewhat consistent, concurrent, destructive
> configuration changes on shared resource (why? It sounds a bit of
> overdesign at this point), we could extend the set() operation to
> additional support shapers deletion, e.g. adding an additional ‘delete’
> flag attribute to the ‘handle’ sub-set.
To judge whether it's an over-design we'd need to know what the user
scenarios are, those are not included in the series AFAICS.
> > For a fixed-layout scheduler where HW blocks have a fixed mapping
> > with user's hierarchy - it's easier to program the shapers from
> > the hierarchy directly. Since node maps to the same set of registers
> > reprogramming will be writing to a register a value it already has
> > - a noop. That's different than doing a full reset and reprogram
> > each time, as two separate calls from user space.
> >
> > For Intel's cases OTOH, when each command is a separate FW call
> > we can't actually enforce the atomic transitions, right?
> > Your code seems to handle returns smaller the number of commands,
> > from which I infer that we may execute half of the modification?
>
> Yes, the code and the NL API allows the NIC to do the update
> incrementally, and AFAICS Intel ICE has no support for complex
> transactions.
> Somewhat enforcing atomic transitions will be quite complex at best and
> is not need to accomplish the stated goal - allow reconfiguration even
> when the H/W does not support intermediate states.
>
> Do we need to enforce atomicity? Why? NFT has proven that a
> transational model implementation is hard, and should be avoided if
> possible.
>
> > IOW for Andrew's HW - he'd probably prefer to look at the resulting
> > tree, no matter what previous state we were in. For Intel we _can't_
> > support atomic commands, if they span multiple cycles of FW exchanges?
>
> My understanding is that we can’t have atomic updates on Intel, from
> firmare perspective. As said, I don’t think it’s necessary to support
> them.
>
> WRT the DSA H/W, do you mean the core should always set() the whole
> known tree to the driver, regardless of the specific changes asked by
> the user-space? If so, what about letting the driver expose some
> capability (or private flag) asking the core for such behavior? So that
> the driver will do the largish set() only with the H/W requiring that.
>
> Anyway I'm not sure the mentioned DSA H/W would benefit from always
> receiving the whole configuration. e.g. changing the weight for a
> single queue shaper would not need the whole data-set.
To be blunt - what I'm getting at is that the API mirrors Intel's FW
API with an extra kludge to support the DSA H/W - in the end matching
neither what the DSA wants nor what Intel can do.
> > > In any case I think that the larger complexity to implement a full
> > > transactional model. nft had proven that to be very hard and bug
> > > prone. I really would avoid that option, if possible.
> >
> > Maybe instead of discussing the user space API it'd be more beneficial
> > to figure out a solid way of translating the existing APIs into the new
> > model?
>
> Could you please rephrase? I think all the arguments discussed here are
> related to the model - at some point that impact the user space API,
> too.
I was hoping that implementing the code that mirrors the existing APIs
into tree operations would teach us something about the primitives
that operate on the tree. The proposed primitives are really low level,
which is why we need to "fuse" them into multi-change operations.
More specifically what I described a few emails up was a
group+schedule+limit paradigm. Instead of describing single moves
and parameter setting - transformation would describe inputs,
scheduling across them, and rate limit to apply. But one transformation
would always operate on at most one MUX node.
IOW I'm trying to explore whether we can find a language of
transformations which will be more complex than single micro-operations
on the tree, but sufficiently expressive to provide atomic
transformations without transactions of micro-ops.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next 1/5] netlink: spec: add shaper YAML spec
2024-07-03 21:20 ` Jakub Kicinski
@ 2024-07-08 19:42 ` Paolo Abeni
2024-07-09 2:54 ` Jakub Kicinski
0 siblings, 1 reply; 20+ messages in thread
From: Paolo Abeni @ 2024-07-08 19:42 UTC (permalink / raw)
To: Jakub Kicinski
Cc: netdev, Jiri Pirko, Madhu Chittim, Sridhar Samudrala,
Simon Horman, John Fastabend, Sunil Kovvuri Goutham,
Jamal Hadi Salim
On Wed, 2024-07-03 at 14:20 -0700, Jakub Kicinski wrote:
> On Wed, 03 Jul 2024 16:53:38 +0200 Paolo Abeni wrote:
>
> > Anyway different applications must touch disjoint resources (e.g.
> > disjoint queues sets) right? In such a case even multiple destructive
> > configuration changes (on disjoint resources set) will not be
> > problematic.
> >
> > Still if we want to allow somewhat consistent, concurrent, destructive
> > configuration changes on shared resource (why? It sounds a bit of
> > overdesign at this point), we could extend the set() operation to
> > additional support shapers deletion, e.g. adding an additional ‘delete’
> > flag attribute to the ‘handle’ sub-set.
>
> To judge whether it's an over-design we'd need to know what the user
> scenarios are, those are not included in the series AFAICS.
My bad, in the cover I referred to the prior discussions without
explicitly quoting the contents.
The initial goal here was to allow the user-space to configure per-
queue, H/W offloaded, TX shaping.
That later evolved in introducing an in-kernel H/W offload TX shaping
API capable of replacing the many existing similar in-kernel APIs and
supporting the foreseeable H/W capabilities.
> To be blunt - what I'm getting at is that the API mirrors Intel's FW
> API with an extra kludge to support the DSA H/W - in the end matching
> neither what the DSA wants nor what Intel can do.
The API is similar to Intel’s FW API because to my understanding the
underlying design - an arbitrary tree - is the most complete
representation possible for shaping H/W. AFAICT is also similar to what
other NIC vendors’ offer.
I don’t see why the APIs don’t match what Intel can do, could you
please elaborate on that?
> IOW I'm trying to explore whether we can find a language of
> transformations which will be more complex than single micro-operations
> on the tree, but sufficiently expressive to provide atomic
> transformations without transactions of micro-ops.
I personally find it straight-forward describing the scenario you
proposed in terms of the simple operations as allowed by this series. I
also think it’s easier to build arbitrarily complex scenarios in terms
of simple operations instead of trying to put enough complexity in the
language to describe everything. It will easily lose flexibility or
increase complexity for unclear gain. Why do you think that would be a
better approach?
Also the simple building blocks approach is IMHO closer to the original
use-case.
Are there any other reasons for atomic operations, beyond addressing
low-end H/W? Note that WRT the specific limitations we are aware of,
the APIs allows atomic conf changes without resorting to transition
into the default configuration.
My biggest concern is that from my perspective we are incrementally
adding requisites, with increasing complexity, further and further away
from the initial use-case.
Thanks,
Paolo
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next 1/5] netlink: spec: add shaper YAML spec
2024-07-08 19:42 ` Paolo Abeni
@ 2024-07-09 2:54 ` Jakub Kicinski
0 siblings, 0 replies; 20+ messages in thread
From: Jakub Kicinski @ 2024-07-09 2:54 UTC (permalink / raw)
To: Paolo Abeni
Cc: netdev, Jiri Pirko, Madhu Chittim, Sridhar Samudrala,
Simon Horman, John Fastabend, Sunil Kovvuri Goutham,
Jamal Hadi Salim
On Mon, 08 Jul 2024 21:42:00 +0200 Paolo Abeni wrote:
> > To judge whether it's an over-design we'd need to know what the user
> > scenarios are, those are not included in the series AFAICS.
>
> My bad, in the cover I referred to the prior discussions without
> explicitly quoting the contents.
>
> The initial goal here was to allow the user-space to configure per-
> queue, H/W offloaded, TX shaping.
Per-queue Tx shaping is already supported.
I mean "user scenarios" in the agile programming sense as in something
that gets closer to production use cases. "Set rate limit on a queue"
is a suggested solution not a statement of a problem.
> That later evolved in introducing an in-kernel H/W offload TX shaping
> API capable of replacing the many existing similar in-kernel APIs and
> supporting the foreseeable H/W capabilities.
>
> > To be blunt - what I'm getting at is that the API mirrors Intel's FW
> > API with an extra kludge to support the DSA H/W - in the end matching
> > neither what the DSA wants nor what Intel can do.
>
> The API is similar to Intel’s FW API because to my understanding the
> underlying design - an arbitrary tree - is the most complete
> representation possible for shaping H/W. AFAICT is also similar to what
> other NIC vendors’ offer.
>
> I don’t see why the APIs don’t match what Intel can do, could you
> please elaborate on that?
That's not the main point I was making, I was complaining about how
the "extension" to support DSA HW was bolted onto this API.
Undeniably the implementation must be stored as a tree (with some
max depth). That doesn't imply that, for example, arbitrary re-parenting
of non-leaf nodes is an operation that makes sense for all devices.
From memory devlink rate doesn't allow mixing some node types after one
parent, too.
> > IOW I'm trying to explore whether we can find a language of
> > transformations which will be more complex than single micro-operations
> > on the tree, but sufficiently expressive to provide atomic
> > transformations without transactions of micro-ops.
>
> I personally find it straight-forward describing the scenario you
> proposed in terms of the simple operations as allowed by this series. I
> also think it’s easier to build arbitrarily complex scenarios in terms
> of simple operations instead of trying to put enough complexity in the
> language to describe everything. It will easily lose flexibility or
> increase complexity for unclear gain. Why do you think that would be a
> better approach?
I strongly dislike the operation grouping/batching as it stands in v1.
Do you think that part of the design is clean?
I also disagree with the assertion that having a language of
transformations more advanced that "add/move/delete" increases
complexity. Language gives you properties you can reason about.
That's why rbtree, B-tree and other algos define a language of
transformations. Naive ops make arbitrary trees easy but I put
it to you that allowing arbitrary trees without any enforced
invariants will breed far more complexity and bugs than a properly
designed language :)
The primary operation of interest is, in fact: given a set of resources
(queues or netdevs) which currently feed one mux node - build
a sub-hierarchy.
Use case 1 - container b/w sharing. Container manager will want to
group a set of queues, and feed a higher layer RR node (so that number
of queues doesn't impact load sharing).
Say we have two containers (c1, c2 represent queues assigned to them);
before container 3 starts:
c1 - \
c1 - >RR
c1 - / > RR(root)
c2 - \ RR
c2 - /
allocate 2 queues:
c1 - \
c1 - ->RR
c1 - / > RR(root)
c2 - \ RR
c2 - / '
qX /
qY /
hierarchize ("group([qX, qY], type="rr")):
c1 - \
c1 - >RR
c1 - / \
c2 - \ RR -> RR(root)
c2 - / /
c3 - \ RR
c3 - /
The container manager just wants so say "take the new queues (X, Y),
put them under an RR node". If the language is build around creating
mux nodes - that's a single call.
Note that the RR(root) node is implicit (in your API it's not visible
but it is implied).
Users case 2 - delegation - the neat thing about using such construct is
that as you can see we never referred to output, i.e. RR(root).
The output can be implied by whatever node the queues already output to.
So say container 1 (c1) wants to set a b/w limit on two of it's queues
(let's call its queues A B C):
rr_id = group([B C], type="rr")
rate_limit(rr_id, 1Gbps)
and end up with:
cA ----- \
cB - \RR*/ RR
cC - / \
c2 - \ RR -> RR(root)
c2 - / /
c3 - \ RR
c3 - /
* new node, also has rate limit set
You don't have to worry about parentage permissions. Container can only
add nodes (which is always safe) or delete nodes (and we can trivially
enforce it only deletes node it has created itself).
> Also the simple building blocks approach is IMHO closer to the original
> use-case.
>
> Are there any other reasons for atomic operations, beyond addressing
> low-end H/W?
I think atomic changes are convenient to match what the user wants to
do. And the second use case is that I do believe there's a real need
to allow uncoordinated agents to modify sections on the hierarchy.
Neither of those are hard requirements, but I think any application
driven requirement should come before "that's the FW API for vendor X"
I hope this we can agree on.
Thinking about it (for longer than I care to admit), one concern I have
about the "mux creation" API I described above is that it forces
existence of leaf and non-leaf nodes at the same parent, at least
transiently.
Can we go back to an API with an explicit create/modify/delete?
All we need for Andrew's use case, I believe, is to be able to
"somewhat atomically" move leaf nodes.
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2024-07-09 2:54 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-27 20:17 [PATCH net-next 0/5] net: introduce TX shaping H/W offload API Paolo Abeni
2024-06-27 20:17 ` [PATCH net-next 1/5] netlink: spec: add shaper YAML spec Paolo Abeni
2024-06-29 2:12 ` Jakub Kicinski
2024-07-01 10:14 ` Paolo Abeni
2024-07-02 2:54 ` Jakub Kicinski
2024-07-02 14:21 ` Paolo Abeni
2024-07-02 15:04 ` Jakub Kicinski
[not found] ` <CAF6piCLnrDWo70ZgXLtdmRkr+w5TMtuXPMW9=JKSSN2fvw1HMA@mail.gmail.com>
2024-07-02 19:51 ` Fwd: " Paolo Abeni
[not found] ` <20240702140830.2890f77b@kernel.org>
2024-07-03 14:53 ` Paolo Abeni
2024-07-03 21:20 ` Jakub Kicinski
2024-07-08 19:42 ` Paolo Abeni
2024-07-09 2:54 ` Jakub Kicinski
2024-07-01 14:50 ` Paolo Abeni
2024-07-01 15:50 ` Paolo Abeni
2024-07-02 0:37 ` Jakub Kicinski
2024-06-27 20:17 ` [PATCH net-next 2/5] net: introduce HW Rate limiting driver API Paolo Abeni
2024-06-27 20:17 ` [PATCH net-next 3/5] netlink: spec: add shaper introspection support Paolo Abeni
2024-06-27 20:17 ` [PATCH net-next 4/5] net: shaper: implement " Paolo Abeni
2024-06-27 20:17 ` [PATCH net-next 5/5] testing: net-drv: add basic shaper test Paolo Abeni
2024-06-29 2:03 ` [PATCH net-next 0/5] net: introduce TX shaping H/W offload API Jakub Kicinski
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).