From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Ahern Subject: Re: [patch net-next RFC v2 02/11] devlink: Add support for resource abstraction Date: Sat, 18 Nov 2017 11:34:09 -0700 Message-ID: References: <20171114161852.6633-1-jiri@resnulli.us> <20171114161852.6633-3-jiri@resnulli.us> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: davem@davemloft.net, mlxsw@mellanox.com, andrew@lunn.ch, vivien.didelot@savoirfairelinux.com, f.fainelli@gmail.com, michael.chan@broadcom.com, ganeshgr@chelsio.com, saeedm@mellanox.com, matanb@mellanox.com, leonro@mellanox.com, idosch@mellanox.com, jakub.kicinski@netronome.com, ast@kernel.org, daniel@iogearbox.net, simon.horman@netronome.com, pieter.jansenvanvuuren@netronome.com, john.hurley@netronome.com, alexander.h.duyck@intel.com, linville@tuxdriver.com, gospo@broadcom.com, steven.lin1@broadcom.com, yuvalm@mellanox.com, ogerlitz@mellanox.com, roopa@cumulusnetworks.com To: Jiri Pirko , netdev@vger.kernel.org Return-path: Received: from mail-pf0-f194.google.com ([209.85.192.194]:38128 "EHLO mail-pf0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753752AbdKRSeO (ORCPT ); Sat, 18 Nov 2017 13:34:14 -0500 Received: by mail-pf0-f194.google.com with SMTP id r62so4289702pfd.5 for ; Sat, 18 Nov 2017 10:34:14 -0800 (PST) In-Reply-To: <20171114161852.6633-3-jiri@resnulli.us> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 11/14/17 9:18 AM, Jiri Pirko wrote: > diff --git a/include/net/devlink.h b/include/net/devlink.h > index 4d2c6fc..960e80a 100644 > --- a/include/net/devlink.h > +++ b/include/net/devlink.h ... > @@ -469,6 +523,32 @@ devlink_dpipe_match_put(struct sk_buff *skb, > return 0; > } > > +static inline int > +devlink_resource_register(struct devlink *devlink, > + const char *resource_name, > + bool top_hierarchy, > + bool reload_required, > + u64 resource_size, > + u64 resource_id, > + u64 parent_resource_id, > + struct devlink_resource_ops *resource_ops) > +{ > + return 0; > +} > + > +static inline void > +devlink_resources_unregister(struct devlink *devlink, > + struct devlink_resource *resource) > +{ > +} > + > +static inline int > +devlink_resource_size_get(struct devlink *devlink, u64 resource_id, > + u64 *p_resource_size) > +{ > + return -EINVAL; It's compiled out so -EOPNOTSUPP seems more appropriate. > diff --git a/net/core/devlink.c b/net/core/devlink.c > index 0114dfc..6ae644f 100644 > --- a/net/core/devlink.c > +++ b/net/core/devlink.c > +static int devlink_nl_cmd_resource_set(struct sk_buff *skb, > + struct genl_info *info) > +{ > + struct devlink *devlink = info->user_ptr[0]; > + struct devlink_resource *resource; > + u64 resource_id; > + u64 size; > + int err; > + > + if (!info->attrs[DEVLINK_ATTR_RESOURCE_ID] || > + !info->attrs[DEVLINK_ATTR_RESOURCE_SIZE]) > + return -EINVAL; several of the of the DEVLINK_ATTR_RESOURCE attributes are kernel to user only (e.g., DEVLINK_ATTR_RESOURCE_SIZE_NEW and DEVLINK_ATTR_RESOURCE_RELOAD_REQUIRED), so if they are given by the user that should be an error too right? > + resource_id = nla_get_u64(info->attrs[DEVLINK_ATTR_RESOURCE_ID]); I don't see where these attributes are validated for proper size. > + > + resource = devlink_resource_find(devlink, NULL, resource_id); > + if (!resource) > + return -EINVAL; > + > + if (!resource->resource_ops->size_validate) > + return -EINVAL; genl_info has extack; please add user messages for the above failures. > + > + size = nla_get_u64(info->attrs[DEVLINK_ATTR_RESOURCE_SIZE]); > + err = resource->resource_ops->size_validate(devlink, size, > + &resource->resource_list, > + info->extack); > + if (err) > + return err; > + > + resource->size_new = size; > + return 0; > +} > +