netdev - Re: [PATCH net-next v2 1/2] ethtool: qsfp transceiver reset, interrupt and presence pin control

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: <c6c33e70-267f-4433-95ca-93efca0dfbe8@lunn.ch>
Date: Thu, 15 May 2025 03:10:24 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Marek Pazdan <mpazdan@...sta.com>
Cc: aleksander.lobakin@...el.com, almasrymina@...gle.com,
	andrew+netdev@...n.ch, anthony.l.nguyen@...el.com,
	daniel.zahka@...il.com, davem@...emloft.net, ecree.xilinx@...il.com,
	edumazet@...gle.com, gal@...dia.com, horms@...nel.org,
	intel-wired-lan@...ts.osuosl.org, jianbol@...dia.com,
	kory.maincent@...tlin.com, kuba@...nel.org,
	linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
	pabeni@...hat.com, przemyslaw.kitszel@...el.com, willemb@...gle.com
Subject: Re: [PATCH net-next v2 1/2] ethtool: qsfp transceiver reset,
 interrupt and presence pin control

On Tue, May 13, 2025 at 10:40:00PM +0000, Marek Pazdan wrote:
> Common Management Interface Specification defines
> Management Signaling Layer (MSL) control and status signals. This change
> provides API for following signals status reading:
> - signal allowing the host to request module reset (Reset)
> - signal allowing the host to detect module presence (Presence)
> - signal allowing the host to detect module interrupt (Int)

What is missing from here is the use cases you are trying to
address. Why should user space want to reset the module? Why does user
spare care if there is a module inserted or not. What is user space
going to do with an interrupt?

> Additionally API allows for Reset signal assertion with
> following constraints:
> - reset cannot be asserted if firmware update is in progress
> - if reset is asserted, firmware update cannot be started
> - if reset is asserted, power mode cannot be get/set
> In all above constraint cases -EBUSY error is returned.

Seems like there should be one more condition. Reset cannot be
asserted if the interface is admin up. I assume a reset is disruptive
to the link, so you don't want it to happen when the link is in use.

> +static int module_mgmt_get(struct net_device *dev,
> +			   struct module_mgmt_reply_data *data,
> +			   const struct genl_info *info)
> +{
> +	const struct ethtool_ops *ops = dev->ethtool_ops;
> +	struct netlink_ext_ack *extack = info ? info->extack : NULL;
> +
> +	if (!ops->get_module_mgmt_signal)
> +		return -EOPNOTSUPP;
> +
> +	return ops->get_module_mgmt_signal(dev, &data->mgmt, extack);

Should there be a module_busy() check here? Can you get these
parameters if the module is in reset?

	Andrew