netdev - Re: [PATCH net-next v4 01/10] devlink: Add permanent config parameter get/set operations

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 linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20171027210455.GC1980@nanopsycho.orion>
Date:   Fri, 27 Oct 2017 23:04:55 +0200
From:   Jiri Pirko <jiri@...nulli.us>
To:     Steve Lin <steven.lin1@...adcom.com>
Cc:     netdev@...r.kernel.org, jiri@...lanox.com, davem@...emloft.net,
        michael.chan@...adcom.com, linville@...driver.com,
        gospo@...adcom.com, yuvalm@...lanox.com
Subject: Re: [PATCH net-next v4 01/10] devlink: Add permanent config
 parameter get/set operations

Fri, Oct 27, 2017 at 10:54:05PM CEST, steven.lin1@...adcom.com wrote:
>Add support for permanent config parameter get/set commands. Used
>for persistent device configuration parameters.
>
>Signed-off-by: Steve Lin <steven.lin1@...adcom.com>
>Acked-by: Andy Gospodarek <gospo@...adcom.com>
>---
> include/net/devlink.h        |   7 ++
> include/uapi/linux/devlink.h |  25 ++++
> net/core/devlink.c           | 287 +++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 319 insertions(+)
>
>diff --git a/include/net/devlink.h b/include/net/devlink.h
>index b9654e1..c7dd912 100644
>--- a/include/net/devlink.h
>+++ b/include/net/devlink.h
>@@ -270,6 +270,13 @@ struct devlink_ops {
> 	int (*eswitch_inline_mode_set)(struct devlink *devlink, u8 inline_mode);
> 	int (*eswitch_encap_mode_get)(struct devlink *devlink, u8 *p_encap_mode);
> 	int (*eswitch_encap_mode_set)(struct devlink *devlink, u8 encap_mode);
>+	int (*perm_config_get)(struct devlink *devlink,
>+			       enum devlink_perm_config_param param, u8 type,
>+			       union devlink_perm_config_value *value);
>+	int (*perm_config_set)(struct devlink *devlink,
>+			       enum devlink_perm_config_param param, u8 type,
>+			       union devlink_perm_config_value *value,
>+			       u8 *restart_reqd);

type should be enum and restart_reqd should be bool.

[...]	
	
	
>+static int devlink_nl_single_param_set(struct sk_buff *msg,
>+				       struct devlink *devlink,
>+				       u32 param, u8 type,
>+				       union devlink_perm_config_value *value)
>+{
>+	const struct devlink_ops *ops = devlink->ops;
>+	struct nlattr *cfgparam_attr;
>+	u8 need_restart;
>+	int err;
>+
>+	/* Now set parameter */
>+	err = ops->perm_config_set(devlink, param, type, value, &need_restart);
>+	if (err)
>+		return err;
>+
>+	cfgparam_attr = nla_nest_start(msg, DEVLINK_ATTR_PERM_CONFIG);
>+	/* Update restart reqd - if any param needs restart, should be set */
>+	if (need_restart) {
>+		err = nla_put_u8(msg,
>+				 DEVLINK_ATTR_PERM_CONFIG_RESTART_REQUIRED, 1);

Why don't you just put this flag always? Otherwise it could be NLA_FLAG


>+		if (err)
>+			goto nest_fail;
>+	}
>+