From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jiri Pirko Subject: Re: [RFC net-next 2/6] devlink: add version reporting API Date: Tue, 15 Jan 2019 11:12:17 +0100 Message-ID: <20190115101217.GC2290@nanopsycho> References: <20190115005009.16025-1-jakub.kicinski@netronome.com> <20190115005009.16025-3-jakub.kicinski@netronome.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: davem@davemloft.net, netdev@vger.kernel.org, oss-drivers@netronome.com To: Jakub Kicinski Return-path: Received: from mail-wr1-f67.google.com ([209.85.221.67]:40663 "EHLO mail-wr1-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726594AbfAOKUx (ORCPT ); Tue, 15 Jan 2019 05:20:53 -0500 Received: by mail-wr1-f67.google.com with SMTP id p4so2272882wrt.7 for ; Tue, 15 Jan 2019 02:20:50 -0800 (PST) Content-Disposition: inline In-Reply-To: <20190115005009.16025-3-jakub.kicinski@netronome.com> Sender: netdev-owner@vger.kernel.org List-ID: Tue, Jan 15, 2019 at 01:50:04AM CET, jakub.kicinski@netronome.com wrote: [...] >@@ -3701,6 +3732,40 @@ static int devlink_nl_cmd_info_get_dumpit(struct sk_buff *msg, > return msg->len; > } > >+int devlink_versions_report(struct sk_buff *skb, enum devlink_attr attr, >+ const char *version_name, const char *version_value) Hmm, so this is a helper to construct the msg. I think it would be good to have this somehow separated from the netlink. Meaning a separate enum just for fixed/running/stored and perhaps a "context structure" which you can use just as a pointer with layout invisible for the driver (containing skb). Passing skb itself is a bit confusing and gives the driver opportunity to put in some weird crap. Better to disallow it by the api design. >+{ >+ struct nlattr *nest; >+ int err; >+ >+ if (attr < DEVLINK_ATTR_INFO_VERSIONS_FIXED || >+ attr > DEVLINK_ATTR_INFO_VERSIONS_STORED) >+ return -EINVAL; >+ >+ nest = nla_nest_start(skb, attr); >+ if (!nest) >+ return -EMSGSIZE; >+ >+ err = nla_put_string(skb, DEVLINK_ATTR_INFO_VERSIONS_NAME, >+ version_name); >+ if (err) >+ goto nla_put_failure; >+ >+ err = nla_put_string(skb, DEVLINK_ATTR_INFO_VERSIONS_VALUE, >+ version_value); >+ if (err) >+ goto nla_put_failure; >+ >+ nla_nest_end(skb, nest); >+ >+ return 0; >+ >+nla_put_failure: >+ nla_nest_cancel(skb, nest); >+ return err; >+} >+EXPORT_SYMBOL_GPL(devlink_versions_report); >+ > static const struct nla_policy devlink_nl_policy[DEVLINK_ATTR_MAX + 1] = { > [DEVLINK_ATTR_BUS_NAME] = { .type = NLA_NUL_STRING }, > [DEVLINK_ATTR_DEV_NAME] = { .type = NLA_NUL_STRING }, >-- >2.19.2 >