Re: [PATCH v3 03/12] net-shapers: implement NL get operation
From: Jiri Pirko <jiri@resnulli.us>
Date: 2024-08-01 13:42:36
Tue, Jul 30, 2024 at 10:39:46PM CEST, pabeni@redhat.com wrote:
quoted hunk ↗ jump to hunk
Introduce the basic infrastructure to implement the net-shaper core functionality. Each network devices carries a net-shaper cache, the NL get() operation fetches the data from such cache. The cache is initially empty, will be fill by the set()/group() operation implemented later and is destroyed at device cleanup time. Signed-off-by: Paolo Abeni <pabeni@redhat.com> --- RFC v2 -> RFC v3: - dev_put() -> netdev_put() --- Documentation/networking/kapi.rst | 3 + include/linux/netdevice.h | 17 ++ include/net/net_shaper.h | 158 +++++++++++++++++++ net/core/dev.c | 2 + net/core/dev.h | 6 + net/shaper/shaper.c | 251 +++++++++++++++++++++++++++++- 6 files changed, 435 insertions(+), 2 deletions(-) create mode 100644 include/net/net_shaper.hdiff --git a/Documentation/networking/kapi.rst b/Documentation/networking/kapi.rst index ea55f462cefa..98682b9a13ee 100644 --- a/Documentation/networking/kapi.rst +++ b/Documentation/networking/kapi.rst@@ -104,6 +104,9 @@ Driver Support.. kernel-doc:: include/linux/netdevice.h :internal: +.. kernel-doc:: include/net/net_shaper.h + :internal: + PHY Support -----------diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index 607009150b5f..d3d952be711c 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h@@ -81,6 +81,8 @@ struct xdp_frame;struct xdp_metadata_ops; struct xdp_md; struct ethtool_netdev_state; +struct net_shaper_ops; +struct net_shaper_data; typedef u32 xdp_features_t;@@ -1598,6 +1600,14 @@ 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 }; /**@@ -2408,6 +2418,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 u8 priv[] ____cacheline_aligned __counted_by(priv_len); } ____cacheline_aligned;diff --git a/include/net/net_shaper.h b/include/net/net_shaper.h new file mode 100644 index 000000000000..8cd65d727e52 --- /dev/null +++ b/include/net/net_shaper.h@@ -0,0 +1,158 @@ +/* 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 + * @children: Number of nested shapers, accounted only for DETACHED scope + */ +struct net_shaper_info { + u32 handle; + u32 parent; + enum net_shaper_metric metric; + u64 bw_min; + u64 bw_max; + u64 burst; + u32 priority; + u32 weight; + u32 children; +}; + +/** + * 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 + * + * 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 { + /** + * @group: create the specified shapers group + * + * Nest the specified @inputs shapers under the given @output shaper + * on the network device @dev. The @input shaper array size is specified + * by @nr_input. + * Create either the @inputs and the @output shaper as needed, + * otherwise move them as needed. Can't create @inputs shapers with + * NET_SHAPER_SCOPE_DETACHED scope, a separate @group call with such + * shaper as @output is needed. + * + * Returns 0 on group successfully created, otherwise an negative + * error value and set @extack to describe the failure's reason. + */ + int (*group)(struct net_device *dev, int nr_input, + const struct net_shaper_info *inputs, + const struct net_shaper_info *output, + struct netlink_ext_ack *extack); + + /** + * @set: Updates the specified shaper + * + * Updates or creates the specified @shaper on the given device + * @dev. Can't create NET_SHAPER_SCOPE_DETACHED shapers, use @group + * instead. + * + * Returns 0 on group successfully created, otherwise an negative + * error value and set @extack to describe the failure's reason. + */ + int (*set)(struct net_device *dev, + const struct net_shaper_info *shaper, + struct netlink_ext_ack *extack); + + /** + * @delete: Removes the specified shaper from the NIC + * + * Removes the shaper configuration as identified by the given @handle + * on the specified device @dev, restoring the default behavior. + * + * Returns 0 on group successfully created, otherwise an negative + * error value and set @extack to describe the failure's reason. + */ + int (*delete)(struct net_device *dev, u32 handle, + struct netlink_ext_ack *extack); +}; + +#define NET_SHAPER_SCOPE_SHIFT 26 +#define NET_SHAPER_ID_MASK GENMASK(NET_SHAPER_SCOPE_SHIFT - 1, 0) +#define NET_SHAPER_SCOPE_MASK GENMASK(31, NET_SHAPER_SCOPE_SHIFT) + +#define NET_SHAPER_ID_UNSPEC NET_SHAPER_ID_MASK + +/** + * 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);
Perhaps some scopes may find only part of u32 as limitting for id in the future? I find it elegant to have it in single u32 though. u64 may be nicer (I know, xarray) :)
quoted hunk ↗ jump to hunk
+} + +/** + * 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); +} + +#endif +diff --git a/net/core/dev.c b/net/core/dev.c index 6ea1d20676fb..3dc1dd428eda 100644 --- a/net/core/dev.c +++ b/net/core/dev.c@@ -11169,6 +11169,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..5d1d6e600a6a 100644 --- a/net/shaper/shaper.c +++ b/net/shaper/shaper.c@@ -1,19 +1,242 @@// SPDX-License-Identifier: GPL-2.0-or-later #include <linux/kernel.h> +#include <linux/idr.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 idr detached_ids;
Hmm, I wonder what this will be used for :) Anyway, could you move to patch which is using it?
+};
+
+struct net_shaper_nl_ctx {
+ u32 start_handle;
+};
+
+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_uint(msg, NET_SHAPER_A_BW_MIN, shaper->bw_min) ||
+ nla_put_uint(msg, NET_SHAPER_A_BW_MAX, shaper->bw_max) ||
+ nla_put_uint(msg, NET_SHAPER_A_BURST, shaper->burst) ||
+ 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);
+ netdev_put(dev, NULL);
+ return -EOPNOTSUPP;
+ }
+
+ *pdev = dev;
+ return 0;
+}
+
+static struct xarray *__sc_container(struct net_device *dev)
+{
+ return dev->net_shaper_data ? &dev->net_shaper_data->shapers : NULL;
+}
+
+/* lookup the given shaper inside the cache */
+static struct net_shaper_info *sc_lookup(struct net_device *dev, u32 handle)
+{
+ struct xarray *xa = __sc_container(dev);
+
+ return xa ? xa_load(xa, handle) : NULL;
+}
+
+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_attr, *id_attr;
+ enum net_shaper_scope scope;
+ u32 id = 0;
+ 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_attr = tb[NET_SHAPER_A_SCOPE];
+ if (!scope_attr) {
+ GENL_SET_ERR_MSG(info, "Missing 'scope' attribute for handle");
+ return -EINVAL;
+ }
+
+ scope = nla_get_u32(scope_attr);
+
+ /* the default id for detached scope shapers is an invalid one
+ * to help the 'group' operation discriminate between new
+ * detached shaper creation (ID_UNSPEC) and reuse of existing
+ * shaper (any other value)
+ */
+ id_attr = tb[NET_SHAPER_A_ID];
+ if (id_attr)
+ id = nla_get_u32(id_attr);
+ else if (scope == NET_SHAPER_SCOPE_DETACHED)
+ id = NET_SHAPER_ID_UNSPEC;
+
+ *handle = net_shaper_make_handle(scope, id);
+ 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);
This is quite net_device centric. Devlink rate shaper should be
eventually visible throught this api as well, won't they? How do you
imagine that?
Could we have various types of binding? Something like:
NET_SHAPER_A_BINDING nest
NET_SHAPER_A_BINDING_IFINDEX u32
or:
NET_SHAPER_A_BINDING nest
NET_SHAPER_A_BINDING_DEVLINK_PORT nest
DEVLINK_ATTR_BUS_NAME string
DEVLINK_ATTR_DEV_NAME string
DEVLINK_ATTR_PORT_INDEX u32
?
quoted hunk ↗ jump to hunk
+ 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; + + shaper = sc_lookup(dev, 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: + netdev_put(dev, NULL); + 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)); + + /* don't error out dumps performed before any set operation */ + 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: + netdev_put(dev, NULL); + return ret; } int net_shaper_nl_set_doit(struct sk_buff *skb, struct genl_info *info)@@ -26,6 +249,30 @@ int net_shaper_nl_delete_doit(struct sk_buff *skb, struct genl_info *info)return -EOPNOTSUPP; } +int net_shaper_nl_group_doit(struct sk_buff *skb, struct genl_info *info) +{ + return -EOPNOTSUPP; +} + +void dev_shaper_flush(struct net_device *dev) +{ + struct xarray *xa = __sc_container(dev); + struct net_shaper_info *cur; + unsigned long index; + + if (!xa) + return; + + xa_lock(xa); + xa_for_each(xa, index, cur) { + __xa_erase(xa, index); + kfree(cur); + } + idr_destroy(&dev->net_shaper_data->detached_ids); + xa_unlock(xa); + kfree(xa); +} + static int __init shaper_init(void)
fetch_dev fill_handle parse_handle sc_lookup __sc_container dev_shaper_flush shaper_init Could you perhaps maintain net_shaper_ prefix for all of there?
{
return genl_register_family(&net_shaper_nl_family);
--
2.45.2