| 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
| ||
|
Date: Sat, 1 May 2021 14:25:21 +0200
From: Hannes Reinecke <hare@...e.de>
To: Shai Malin <smalin@...vell.com>, netdev@...r.kernel.org,
linux-nvme@...ts.infradead.org, sagi@...mberg.me, hch@....de,
axboe@...com, kbusch@...nel.org
Cc: "David S . Miller davem @ davemloft . net --cc=Jakub Kicinski"
<kuba@...nel.org>, aelior@...vell.com, mkalderon@...vell.com,
okulkarni@...vell.com, pkushwaha@...vell.com, malin1024@...il.com,
Dean Balandin <dbalandin@...vell.com>
Subject: Re: [RFC PATCH v4 10/27] nvme-tcp-offload: Add device scan
implementation
On 4/29/21 9:09 PM, Shai Malin wrote:
> From: Dean Balandin <dbalandin@...vell.com>
>
> As part of create_ctrl(), it scans the registered devices and calls
> the claim_dev op on each of them, to find the first devices that matches
> the connection params. Once the correct devices is found (claim_dev
> returns true), we raise the refcnt of that device and return that device
> as the device to be used for ctrl currently being created.
>
> Acked-by: Igor Russkikh <irusskikh@...vell.com>
> Signed-off-by: Dean Balandin <dbalandin@...vell.com>
> Signed-off-by: Prabhakar Kushwaha <pkushwaha@...vell.com>
> Signed-off-by: Omkar Kulkarni <okulkarni@...vell.com>
> Signed-off-by: Michal Kalderon <mkalderon@...vell.com>
> Signed-off-by: Ariel Elior <aelior@...vell.com>
> Signed-off-by: Shai Malin <smalin@...vell.com>
> ---
> drivers/nvme/host/tcp-offload.c | 94 +++++++++++++++++++++++++++++++++
> 1 file changed, 94 insertions(+)
>
> diff --git a/drivers/nvme/host/tcp-offload.c b/drivers/nvme/host/tcp-offload.c
> index 711232eba339..aa7cc239abf2 100644
> --- a/drivers/nvme/host/tcp-offload.c
> +++ b/drivers/nvme/host/tcp-offload.c
> @@ -13,6 +13,11 @@
> static LIST_HEAD(nvme_tcp_ofld_devices);
> static DECLARE_RWSEM(nvme_tcp_ofld_devices_rwsem);
>
> +static inline struct nvme_tcp_ofld_ctrl *to_tcp_ofld_ctrl(struct nvme_ctrl *nctrl)
> +{
> + return container_of(nctrl, struct nvme_tcp_ofld_ctrl, nctrl);
> +}
> +
> /**
> * nvme_tcp_ofld_register_dev() - NVMeTCP Offload Library registration
> * function.
> @@ -98,6 +103,94 @@ void nvme_tcp_ofld_req_done(struct nvme_tcp_ofld_req *req,
> /* Placeholder - complete request with/without error */
> }
>
> +struct nvme_tcp_ofld_dev *
> +nvme_tcp_ofld_lookup_dev(struct nvme_tcp_ofld_ctrl *ctrl)
> +{
> + struct nvme_tcp_ofld_dev *dev;
> +
> + down_read(&nvme_tcp_ofld_devices_rwsem);
> + list_for_each_entry(dev, &nvme_tcp_ofld_devices, entry) {
> + if (dev->ops->claim_dev(dev, &ctrl->conn_params)) {
> + /* Increase driver refcnt */
> + if (!try_module_get(dev->ops->module)) {
> + pr_err("try_module_get failed\n");
> + dev = NULL;
> + }
> +
> + goto out;
> + }
> + }
> +
> + dev = NULL;
> +out:
> + up_read(&nvme_tcp_ofld_devices_rwsem);
> +
> + return dev;
> +}
> +
> +static int nvme_tcp_ofld_setup_ctrl(struct nvme_ctrl *nctrl, bool new)
> +{
> + /* Placeholder - validates inputs and creates admin and IO queues */
> +
> + return 0;
> +}
> +
> +static struct nvme_ctrl *
> +nvme_tcp_ofld_create_ctrl(struct device *ndev, struct nvmf_ctrl_options *opts)
> +{
> + struct nvme_tcp_ofld_ctrl *ctrl;
> + struct nvme_tcp_ofld_dev *dev;
> + struct nvme_ctrl *nctrl;
> + int rc = 0;
> +
> + ctrl = kzalloc(sizeof(*ctrl), GFP_KERNEL);
> + if (!ctrl)
> + return ERR_PTR(-ENOMEM);
> +
> + nctrl = &ctrl->nctrl;
> +
> + /* Init nvme_tcp_ofld_ctrl and nvme_ctrl params based on received opts */
> +
> + /* Find device that can reach the dest addr */
> + dev = nvme_tcp_ofld_lookup_dev(ctrl);
> + if (!dev) {
> + pr_info("no device found for addr %s:%s.\n",
> + opts->traddr, opts->trsvcid);
> + rc = -EINVAL;
> + goto out_free_ctrl;
> + }
> +
> + ctrl->dev = dev;
> +
> + if (ctrl->dev->ops->max_hw_sectors)
> + nctrl->max_hw_sectors = ctrl->dev->ops->max_hw_sectors;
> + if (ctrl->dev->ops->max_segments)
> + nctrl->max_segments = ctrl->dev->ops->max_segments;
> +
> + /* Init queues */
> +
> + /* Call nvme_init_ctrl */
> +
> + rc = ctrl->dev->ops->setup_ctrl(ctrl, true);
> + if (rc)
> + goto out_module_put;
> +
> + rc = nvme_tcp_ofld_setup_ctrl(nctrl, true);
> + if (rc)
> + goto out_uninit_ctrl;
> +
> + return nctrl;
> +
> +out_uninit_ctrl:
> + ctrl->dev->ops->release_ctrl(ctrl);
> +out_module_put:
> + module_put(dev->ops->module);
> +out_free_ctrl:
> + kfree(ctrl);
> +
> + return ERR_PTR(rc);
> +}
> +
> static struct nvmf_transport_ops nvme_tcp_ofld_transport = {
> .name = "tcp_offload",
> .module = THIS_MODULE,
> @@ -107,6 +200,7 @@ static struct nvmf_transport_ops nvme_tcp_ofld_transport = {
> NVMF_OPT_RECONNECT_DELAY | NVMF_OPT_HDR_DIGEST |
> NVMF_OPT_DATA_DIGEST | NVMF_OPT_NR_POLL_QUEUES |
> NVMF_OPT_TOS,
> + .create_ctrl = nvme_tcp_ofld_create_ctrl,
> };
>
> static int __init nvme_tcp_ofld_init_module(void)
>
I wonder if we shouldn't take the approach from Martin Belanger, and
introduce a new option 'host_iface' to select the interface to use.
That is, _if_ the nvme-tcp offload driver would present itself as a
network interface; one might argue that it would put too much
restriction on the implementations.
But if it does not present itself as a network interface, how do we
address it? And if it does, wouldn't we be better off to specify the
interface directly, and not try to imply the interface from the IP address?
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@...e.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
Powered by blists - more mailing lists