netdev - Re: [patch net-next 01/15] devlink: introduce port ops placeholder

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: <ZG77DAskJxpDYbdi@nanopsycho>
Date: Thu, 25 May 2023 08:07:08 +0200
From: Jiri Pirko <jiri@...nulli.us>
To: Jakub Kicinski <kuba@...nel.org>
Cc: netdev@...r.kernel.org, pabeni@...hat.com, davem@...emloft.net,
	edumazet@...gle.com, leon@...nel.org, saeedm@...dia.com,
	moshe@...dia.com, jesse.brandeburg@...el.com,
	anthony.l.nguyen@...el.com, tariqt@...dia.com, idosch@...dia.com,
	petrm@...dia.com, simon.horman@...igine.com, ecree.xilinx@...il.com,
	habetsm.xilinx@...il.com, michal.wilczynski@...el.com,
	jacob.e.keller@...el.com
Subject: Re: [patch net-next 01/15] devlink: introduce port ops placeholder

Thu, May 25, 2023 at 06:48:11AM CEST, kuba@...nel.org wrote:
>On Wed, 24 May 2023 14:18:22 +0200 Jiri Pirko wrote:
>> @@ -6799,6 +6799,7 @@ EXPORT_SYMBOL_GPL(devlink_port_fini);
>>   * @devlink: devlink
>>   * @devlink_port: devlink port
>>   * @port_index: driver-specific numerical identifier of the port
>> + * @ops: port ops
>>   *
>>   * Register devlink port with provided port index. User can use
>>   * any indexing, even hw-related one. devlink_port structure
>> @@ -6806,9 +6807,10 @@ EXPORT_SYMBOL_GPL(devlink_port_fini);
>>   * Note that the caller should take care of zeroing the devlink_port
>>   * structure.
>>   */
>> -int devl_port_register(struct devlink *devlink,
>> -		       struct devlink_port *devlink_port,
>> -		       unsigned int port_index)
>> +int devl_port_register_with_ops(struct devlink *devlink,
>> +				struct devlink_port *devlink_port,
>> +				unsigned int port_index,
>> +				const struct devlink_port_ops *ops)
>>  {
>>  	int err;
>
>function name in kdoc needs an update

Ah, will fix.


>
>> @@ -6819,6 +6821,7 @@ int devl_port_register(struct devlink *devlink,
>>  	devlink_port_init(devlink, devlink_port);
>>  	devlink_port->registered = true;
>>  	devlink_port->index = port_index;
>> +	devlink_port->ops = ops;
>>  	spin_lock_init(&devlink_port->type_lock);
>>  	INIT_LIST_HEAD(&devlink_port->reporter_list);
>>  	err = xa_insert(&devlink->ports, port_index, devlink_port, GFP_KERNEL);
>> @@ -6830,7 +6833,7 @@ int devl_port_register(struct devlink *devlink,
>>  	devlink_port_notify(devlink_port, DEVLINK_CMD_PORT_NEW);
>>  	return 0;
>>  }
>> -EXPORT_SYMBOL_GPL(devl_port_register);
>> +EXPORT_SYMBOL_GPL(devl_port_register_with_ops);
>>  
>>  /**
>>   *	devlink_port_register - Register devlink port
>> @@ -6838,6 +6841,7 @@ EXPORT_SYMBOL_GPL(devl_port_register);
>>   *	@devlink: devlink
>>   *	@devlink_port: devlink port
>>   *	@port_index: driver-specific numerical identifier of the port
>> + *	@ops: port ops
>>   *
>>   *	Register devlink port with provided port index. User can use
>>   *	any indexing, even hw-related one. devlink_port structure
>> @@ -6847,18 +6851,20 @@ EXPORT_SYMBOL_GPL(devl_port_register);
>>   *
>>   *	Context: Takes and release devlink->lock <mutex>.
>>   */
>> -int devlink_port_register(struct devlink *devlink,
>> -			  struct devlink_port *devlink_port,
>> -			  unsigned int port_index)
>> +int devlink_port_register_with_ops(struct devlink *devlink,
>> +				   struct devlink_port *devlink_port,
>> +				   unsigned int port_index,
>> +				   const struct devlink_port_ops *ops)
>
>same here.

Yeah, will fix.

>
>BTW do we need to provide the "devlink_*" form of this API or can we
>use this as an opportunity to move everyone to devl_*. Even if the
>driver just wraps the call with devl_lock(), sooner or later people
>will coalesce the locking in the drivers, I hope.

It is on my list, stay tuned, will address in a follow-up.


>-- 
>pw-bot: cr