netdev - Re: [RFC net-next 10/17] net/dibs: Define dibs_client_ops and dibs_dev

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: <aJiye8W_giWiWWpI@linux.alibaba.com>
Date: Sun, 10 Aug 2025 22:53:47 +0800
From: Dust Li <dust.li@...ux.alibaba.com>
To: Alexandra Winter <wintera@...ux.ibm.com>,
	David Miller <davem@...emloft.net>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Eric Dumazet <edumazet@...gle.com>,
	Andrew Lunn <andrew+netdev@...n.ch>,
	"D. Wythe" <alibuda@...ux.alibaba.com>,
	Sidraya Jayagond <sidraya@...ux.ibm.com>,
	Wenjia Zhang <wenjia@...ux.ibm.com>,
	Julian Ruess <julianr@...ux.ibm.com>
Cc: netdev@...r.kernel.org, linux-s390@...r.kernel.org,
	Heiko Carstens <hca@...ux.ibm.com>,
	Vasily Gorbik <gor@...ux.ibm.com>,
	Alexander Gordeev <agordeev@...ux.ibm.com>,
	Christian Borntraeger <borntraeger@...ux.ibm.com>,
	Sven Schnelle <svens@...ux.ibm.com>,
	Thorsten Winkler <twinkler@...ux.ibm.com>,
	Simon Horman <horms@...nel.org>,
	Mahanta Jambigi <mjambigi@...ux.ibm.com>,
	Tony Lu <tonylu@...ux.alibaba.com>,
	Wen Gu <guwen@...ux.alibaba.com>, Halil Pasic <pasic@...ux.ibm.com>,
	linux-rdma@...r.kernel.org
Subject: Re: [RFC net-next 10/17] net/dibs: Define dibs_client_ops and
 dibs_dev_ops

On 2025-08-06 17:41:15, Alexandra Winter wrote:
>Move the device add() and remove() functions from ism_client to
>dibs_client_ops and call add_dev()/del_dev() for ism devices and
>dibs_loopback devices. dibs_client_ops->add_dev() = smcd_register_dev() for
>the smc_dibs_client. This is the first step to handle ism and loopback
>devices alike (as dibs devices) in the smc dibs client.
>
>Define dibs_dev->ops and move smcd_ops->get_chid to
>dibs_dev_ops->get_fabric_id() for ism and loopback devices. See below for
>why this needs to be in the same patch as dibs_client_ops->add_dev().
>
>The following changes contain intermediate steps, that will be obsoleted by
>follow-on patches, once more functionality has been moved to dibs:
>
>Use different smcd_ops and max_dmbs for ism and loopback. Follow-on patches
>will change SMC-D to directly use dibs_ops instead of smcd_ops.
>
>In smcd_register_dev() it is now necessary to identify a dibs_loopback
>device before smcd_dev and smcd_ops->get_chid() are available. So provide
>dibs_dev_ops->get_fabric_id() in this patch and evaluate it in
>smc_ism_is_loopback().
>
>Call smc_loopback_init() in smcd_register_dev() and call
>smc_loopback_exit() in smcd_unregister_dev() to handle the functionality
>that is still in smc_loopback. Follow-on patches will move all smc_loopback
>code to dibs_loopback.
>
>In smcd_unregister_dev() use only ism device name, this will be replaced by
>dibs device name by a follow-on patch.
>
>End of changes with intermediate parts.
>
>Allocate an smcd event workqueue for all dibs devices, although
>dibs_loopback does not generate events.
>
>Use kernel memory instead of devres memory for smcd_dev and smcd->conn.
>Since commit a72178cfe855 ("net/smc: Fix dependency of SMC on ISM") an ism
>device and its driver can have a longer lifetime than the smc module, so
>smc should not rely on devres to free its resources [1]. It is now the
>responsibility of the smc client to free smcd and smcd->conn for all dibs
>devices, ism devices as well as loopback. Call client->ops->del_dev() for
>all existing dibs devices in dibs_unregister_client(), so all device
>related structures can be freed in the client.
>
>When dibs_unregister_client() is called in the context of smc_exit() or
>smc_core_reboot_event(), these functions have already called
>smc_lgrs_shutdown() which calls smc_smcd_terminate_all(smcd) and sets
>going_away. This is done a second time in smcd_unregister_dev(). This is
>analogous to how smcr is handled in these functions, by calling first
>smc_lgrs_shutdown() and then smc_ib_unregister_client() >
>smc_ib_remove_dev(), so leave it that way. It may be worth investigating,
>whether smc_lgrs_shutdown() is still required or useful.
>
>Remove CONFIG_SMC_LO. CONFIG_DIBS_LO now controls whether a dibs loopback
>device exists or not.
>
>Link: https://www.kernel.org/doc/Documentation/driver-model/devres.txt [1]
>Signed-off-by: Alexandra Winter <wintera@...ux.ibm.com>
>Reviewed-by: Mahanta Jambigi <mjambigi@...ux.ibm.com>

Hi Winter,

I feel a bit hard for me to review the code especially with so many
intermediate parts. I may need more time to review these.

Seperate such a big refine patch is hard. Maybe put the
small parts in the front and the final one in the last to reduce
the intermediate part as much as possible ? I'm not sure.

Best regards,
Dust