netdev - Re: [PATCH net-next v7 1/8] devlink: Add devlink_param for port register and unregister

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]

Date:   Wed, 23 Jan 2019 09:25:13 +0100
From:   Jiri Pirko <jiri@...nulli.us>
To:     Vasundhara Volam <vasundhara-v.volam@...adcom.com>
Cc:     davem@...emloft.net, michael.chan@...adcom.com, jiri@...lanox.com,
        jakub.kicinski@...ronome.com, mkubecek@...e.cz,
        netdev@...r.kernel.org
Subject: Re: [PATCH net-next v7 1/8] devlink: Add devlink_param for port
 register and unregister

Fri, Jan 18, 2019 at 08:09:38AM CET, vasundhara-v.volam@...adcom.com wrote:
>Add functions to register and unregister for the driver supported
>configuration parameters table per port.
>
>v2->v3:
>- Add a helper __devlink_params_register() with common code used by
>  both devlink_params_register() and devlink_port_params_register().
>

[...]	
	
	
>+
>+static int __devlink_params_register(struct devlink *devlink,
>+				     struct list_head *param_list,
>+				     const struct devlink_param *params,
>+				     size_t params_count)
> {
> 	const struct devlink_param *param = params;
> 	int i;
>@@ -4490,20 +4493,11 @@ int devlink_params_register(struct devlink *devlink,
> 
> 	mutex_lock(&devlink->lock);
> 	for (i = 0; i < params_count; i++, param++) {
>-		if (!param || !param->name || !param->supported_cmodes) {
>-			err = -EINVAL;
>+		err = devlink_param_verify(param);
>+		if (err)
> 			goto rollback;
>-		}
>-		if (param->generic) {
>-			err = devlink_param_generic_verify(param);
>-			if (err)
>-				goto rollback;
>-		} else {
>-			err = devlink_param_driver_verify(param);
>-			if (err)
>-				goto rollback;
>-		}
>-		err = devlink_param_register_one(devlink, param);
>+
>+		err = devlink_param_register_one(devlink, param_list, param);
> 		if (err)
> 			goto rollback;
> 	}
>@@ -4515,31 +4509,57 @@ int devlink_params_register(struct devlink *devlink,
> 	if (!i)
> 		goto unlock;
> 	for (param--; i > 0; i--, param--)
>-		devlink_param_unregister_one(devlink, param);
>+		devlink_param_unregister_one(devlink, param_list, param);
> unlock:
> 	mutex_unlock(&devlink->lock);
> 	return err;
> }
>-EXPORT_SYMBOL_GPL(devlink_params_register);
> 
> /**
>- *	devlink_params_unregister - unregister configuration parameters
>+ *	devlink_params_register - register configuration parameters
>+ *
>  *	@devlink: devlink
>- *	@params: configuration parameters to unregister
>+ *	@params: configuration parameters array
>  *	@params_count: number of parameters provided
>+ *
>+ *	Register the configuration parameters supported by the driver.
>  */
>-void devlink_params_unregister(struct devlink *devlink,
>-			       const struct devlink_param *params,
>-			       size_t params_count)
>+int devlink_params_register(struct devlink *devlink,

The order of the functions should be:
__devlink_params_register
__devlink_params_unregister
devlink_params_register
devlink_params_unregister
devlink_port_params_register
devlink_port_params_unregister

Your order is a bit confusing.

Other than that:
Acked-by: Jiri Pirko <jiri@...lanox.com>


[...]