lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 31 Oct 2022 11:19:25 +0100 From: Jiri Pirko <jiri@...nulli.us> To: Michal Wilczynski <michal.wilczynski@...el.com> Cc: netdev@...r.kernel.org, alexandr.lobakin@...el.com, jacob.e.keller@...el.com, jesse.brandeburg@...el.com, przemyslaw.kitszel@...el.com, anthony.l.nguyen@...el.com, kuba@...nel.org, ecree.xilinx@...il.com Subject: Re: [PATCH net-next v8 3/9] devlink: Enable creation of the devlink-rate nodes from the driver Fri, Oct 28, 2022 at 12:51:37PM CEST, michal.wilczynski@...el.com wrote: >Intel 100G card internal firmware hierarchy for Hierarchicial QoS is very >rigid and can't be easily removed. This requires an ability to export >default hierarchy to allow user to modify it. Currently the driver is >only able to create the 'leaf' nodes, which usually represent the vport. >This is not enough for HQoS implemented in Intel hardware. > >Introduce new function devl_rate_node_create() that allows for creation >of the devlink-rate nodes from the driver. > >Signed-off-by: Michal Wilczynski <michal.wilczynski@...el.com> >--- > include/net/devlink.h | 4 ++++ > net/core/devlink.c | 49 +++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 53 insertions(+) > >diff --git a/include/net/devlink.h b/include/net/devlink.h >index 929cb72ef412..9d0a424712fd 100644 >--- a/include/net/devlink.h >+++ b/include/net/devlink.h >@@ -98,6 +98,8 @@ struct devlink_port_attrs { > }; > }; > >+#define DEVLINK_RATE_NAME_MAX_LEN 30 >+ > struct devlink_rate { > struct list_head list; > enum devlink_rate_type type; >@@ -1601,6 +1603,8 @@ void devlink_port_attrs_pci_sf_set(struct devlink_port *devlink_port, > u32 controller, u16 pf, u32 sf, > bool external); > int devl_rate_leaf_create(struct devlink_port *port, void *priv); >+int devl_rate_node_create(struct devlink *devlink, void *priv, char *node_name, >+ char *parent_name); > void devl_rate_leaf_destroy(struct devlink_port *devlink_port); > void devl_rate_nodes_destroy(struct devlink *devlink); > void devlink_port_linecard_set(struct devlink_port *devlink_port, >diff --git a/net/core/devlink.c b/net/core/devlink.c >index b97c077cf66e..08f1bbd54c43 100644 >--- a/net/core/devlink.c >+++ b/net/core/devlink.c >@@ -10270,6 +10270,55 @@ void devlink_port_attrs_pci_sf_set(struct devlink_port *devlink_port, u32 contro > } > EXPORT_SYMBOL_GPL(devlink_port_attrs_pci_sf_set); > >+/** >+ * devl_rate_node_create - create devlink rate node >+ * @devlink: devlink instance >+ * @priv: driver private data >+ * @node_name: name of the resulting node >+ * @parent_name: name of the parent node >+ * >+ * Create devlink rate object of type node >+ */ >+int devl_rate_node_create(struct devlink *devlink, void *priv, char *node_name, char *parent_name) Nope, this is certainly incorrect. Do not refer to kernel object by string. You also don't have internal kernel api based on ifname to refer to struct net_device instance. Please have "struct devlink_rate *parent" to refer to parent node and make this function return "struct devlink_rate *". >+{ >+ struct devlink_rate *rate_node; >+ struct devlink_rate *parent; >+ >+ rate_node = devlink_rate_node_get_by_name(devlink, node_name); >+ if (!IS_ERR(rate_node)) >+ return -EEXIST; >+ >+ rate_node = kzalloc(sizeof(*rate_node), GFP_KERNEL); >+ if (!rate_node) >+ return -ENOMEM; >+ >+ if (parent_name) { >+ parent = devlink_rate_node_get_by_name(devlink, parent_name); >+ if (IS_ERR(parent)) { >+ kfree(rate_node); >+ return -ENODEV; >+ } >+ rate_node->parent = parent; >+ refcount_inc(&rate_node->parent->refcnt); >+ } >+ >+ rate_node->type = DEVLINK_RATE_TYPE_NODE; >+ rate_node->devlink = devlink; >+ rate_node->priv = priv; >+ >+ rate_node->name = kstrndup(node_name, DEVLINK_RATE_NAME_MAX_LEN, GFP_KERNEL); Why do you limit the name length? We don't limit the length passed from user, I see no reason to do it for driver. >+ if (!rate_node->name) { >+ kfree(rate_node); >+ return -ENOMEM; >+ } >+ >+ refcount_set(&rate_node->refcnt, 1); >+ list_add(&rate_node->list, &devlink->rate_list); >+ devlink_rate_notify(rate_node, DEVLINK_CMD_RATE_NEW); >+ return 0; >+} >+EXPORT_SYMBOL_GPL(devl_rate_node_create); >+ > /** > * devl_rate_leaf_create - create devlink rate leaf > * @devlink_port: devlink port object to create rate object on >-- >2.37.2 >
Powered by blists - more mailing lists