[PATCH 0/2] nvme: support I/O passthrough for multipath

[PATCH 0/2] nvme: support I/O passthrough for multipath

[Minwoo Im]

Hello,

This series is to support I/O passthrough for multipath I/O.

Currently driver exposes block device for the head namespace.  Also, the
path will be decided by the driver with the I/O policy defined to the
actual hidden device based on ANA state.  This means that userspace is not
allowed to I/O passthrough to a specified namespace target which is hidden
by the driver.

This series is to support passthrough to the namespace attached to a
controller regardless to ANA state (e.g., inaccessible) and decided path
by the driver.

Please review.

Thanks,

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

[PATCH] nvme: introduce passthrough ioctl for multipath

[Minwoo Im]

We don't allow NVME_IOCTL_IO_CMD ioctl in case that a controller has
multiple namespaces attached.  Also, I/O request to the controller
character device has not been recommended and deprecated because we have
block device to I/O with where the multipath consideration is taken.

Once kernel decided a path to I/O for a namespace based on the I/O
policy of a NVMe subsystem, userspace is not allowed to choose a path to
I/O.  If a path is broken(inaccessible state in ANA), then it will not
try to I/O to that path.

This patch introduced NVME_IOCTL_MPATH_IO command for controller
device(e.g., /dev/nvme0) to support multipath I/O passthrough for
userspace.  Regardless driver's path decision, userspace can target a
namespace to I/O.  In this case, `cmd.nsid` will be used to find out the
namespace instance target which is hidden(e.g., nvmeXcYnZ).

Signed-off-by: Minwoo Im <minwoo.im.dev@gmail.com>
---
 drivers/nvme/host/core.c        | 31 +++++++++++++++++++++++++++++++
 include/uapi/linux/nvme_ioctl.h |  1 +
 2 files changed, 32 insertions(+)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index d77f3f26d8d3..fed9fc41b021 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1707,6 +1707,35 @@ static int nvme_user_cmd64(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
 	return status;
 }
 
+#ifdef CONFIG_NVME_MULTIPATH
+static int nvme_dev_mpath_cmd(struct nvme_ctrl *ctrl,
+		struct nvme_passthru_cmd __user *ucmd)
+{
+	struct nvme_passthru_cmd cmd;
+	struct nvme_ns *ns;
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EACCES;
+	if (copy_from_user(&cmd, ucmd, sizeof(cmd)))
+		return -EFAULT;
+
+	ns = nvme_find_get_ns(ctrl, cmd.nsid);
+	if (unlikely(!ns))
+		return -EINVAL;
+	if (WARN_ON_ONCE(!(ns->disk->flags & GENHD_FL_HIDDEN)))
+		return -EINVAL;
+
+	return nvme_user_cmd(ctrl, ns, ucmd);
+}
+#else
+static int nvme_dev_mpath_cmd(struct nvme_ctrl *ctrl,
+		struct nvme_passthru_cmd __user *ucmd)
+{
+	dev_warn(ctrl->device, "CONFIG_NVME_MULTIPATH should be enabled\n");
+	return -EINVAL;
+}
+#endif
+
 /*
  * Issue ioctl requests on the first available path.  Note that unlike normal
  * block layer requests we will not retry failed request on another controller.
@@ -3329,6 +3358,8 @@ static long nvme_dev_ioctl(struct file *file, unsigned int cmd,
 		return nvme_user_cmd64(ctrl, NULL, argp);
 	case NVME_IOCTL_IO_CMD:
 		return nvme_dev_user_cmd(ctrl, argp);
+	case NVME_IOCTL_MPATH_IO:
+		return nvme_dev_mpath_cmd(ctrl, argp);
 	case NVME_IOCTL_RESET:
 		dev_warn(ctrl->device, "resetting controller\n");
 		return nvme_reset_ctrl_sync(ctrl);
diff --git a/include/uapi/linux/nvme_ioctl.h b/include/uapi/linux/nvme_ioctl.h
index d99b5a772698..38bd330e5416 100644
--- a/include/uapi/linux/nvme_ioctl.h
+++ b/include/uapi/linux/nvme_ioctl.h
@@ -78,5 +78,6 @@ struct nvme_passthru_cmd64 {
 #define NVME_IOCTL_RESCAN	_IO('N', 0x46)
 #define NVME_IOCTL_ADMIN64_CMD	_IOWR('N', 0x47, struct nvme_passthru_cmd64)
 #define NVME_IOCTL_IO64_CMD	_IOWR('N', 0x48, struct nvme_passthru_cmd64)
+#define NVME_IOCTL_MPATH_IO	_IOWR('N', 0x49, struct nvme_passthru_cmd)
 
 #endif /* _UAPI_LINUX_NVME_IOCTL_H */
-- 
2.17.1


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

[nvme-cli PATCH] nvme: add [--mpath|-M] option to io-passthru

[Minwoo Im]

`io-passthru` command is to submit a I/O command to controller with a
namespace specified.  Kernel driver does not allow userspace to I/O
passthrough in case that controller has multiple namespaces attached.
NVME_IOCTL_IO_CMD will return -EINVAL error from the kernel driver.

This patch added an option `--mpath|-M` option to the `io-passthru`
command to support specifying device namespace id to I/O.  This option
will make `--namespace-id` option to be used to find out the namespae
instance to the kernel side even it's a hidden blkdev (e.g.,
/dev/nvmeXcYnZ).

This feature can be used to debug or test the broken path for ANA case
by specifying the target namespace of a controller to I/O.

  nvme io-passthru /dev/nvme0 --namespace-id=123 --opcode=0x2 --data-len=512 --read --mpath

Signed-off-by: Minwoo Im <minwoo.im.dev@gmail.com>
---
 linux/nvme_ioctl.h |  1 +
 nvme.c             | 20 ++++++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/linux/nvme_ioctl.h b/linux/nvme_ioctl.h
index d569414211d1..578faaee62b6 100644
--- a/linux/nvme_ioctl.h
+++ b/linux/nvme_ioctl.h
@@ -87,5 +87,6 @@ struct nvme_passthru_cmd64 {
 #define NVME_IOCTL_RESCAN	_IO('N', 0x46)
 #define NVME_IOCTL_ADMIN64_CMD  _IOWR('N', 0x47, struct nvme_passthru_cmd64)
 #define NVME_IOCTL_IO64_CMD _IOWR('N', 0x48, struct nvme_passthru_cmd64)
+#define NVME_IOCTL_MPATH_IO	_IOWR('N', 0x49, struct nvme_passthru_cmd)
 
 #endif /* _UAPI_LINUX_NVME_IOCTL_H */
diff --git a/nvme.c b/nvme.c
index c79edbda17ed..9dea66edcabe 100644
--- a/nvme.c
+++ b/nvme.c
@@ -5109,6 +5109,7 @@ static int passthru(int argc, char **argv, int ioctl_cmd, const char *desc, stru
 		int   read;
 		int   write;
 		__u8  prefill;
+		bool  mpath;
 	};
 
 	struct config cfg = {
@@ -5129,6 +5130,7 @@ static int passthru(int argc, char **argv, int ioctl_cmd, const char *desc, stru
 		.cdw15        = 0,
 		.input_file   = "",
 		.prefill      = 0,
+		.mpath        = false,
 	};
 
 	const char *opcode = "opcode (required)";
@@ -5153,6 +5155,7 @@ static int passthru(int argc, char **argv, int ioctl_cmd, const char *desc, stru
 	const char *re = "set dataflow direction to receive";
 	const char *wr = "set dataflow direction to send";
 	const char *prefill = "prefill buffers with known byte-value, default 0";
+	const char *mpath = "multipath I/O (valid only if io-passthru)";
 
 	OPT_ARGS(opts) = {
 		OPT_BYTE("opcode",       'o', &cfg.opcode,       opcode),
@@ -5177,6 +5180,7 @@ static int passthru(int argc, char **argv, int ioctl_cmd, const char *desc, stru
 		OPT_FLAG("dry-run",      'd', &cfg.dry_run,      dry),
 		OPT_FLAG("read",         'r', &cfg.read,         re),
 		OPT_FLAG("write",        'w', &cfg.write,        wr),
+		OPT_FLAG("mpath",        'M', &cfg.mpath,        mpath),
 		OPT_END()
 	};
 
@@ -5184,6 +5188,22 @@ static int passthru(int argc, char **argv, int ioctl_cmd, const char *desc, stru
 	if (fd < 0)
 		goto ret;
 
+	if (cfg.mpath) {
+		if (!cfg.namespace_id) {
+			fprintf(stderr, "mpath should be with --namespace-id\n");
+			err = -EINVAL;
+			goto close_fd;
+		}
+
+		if (!is_chardev()) {
+			fprintf(stderr, "mpath should be with chardev (e.g., /dev/nvme0)\n");
+			err = -EINVAL;
+			goto close_fd;
+		}
+
+		ioctl_cmd = NVME_IOCTL_MPATH_IO;
+	}
+
 	if (strlen(cfg.input_file)){
 		wfd = open(cfg.input_file, O_RDONLY,
 			   S_IRUSR | S_IRGRP | S_IROTH);
-- 
2.17.1


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH] nvme: introduce passthrough ioctl for multipath

[Keith Busch]

On Sun, Feb 14, 2021 at 08:01:25PM +0900, Minwoo Im wrote:
> We don't allow NVME_IOCTL_IO_CMD ioctl in case that a controller has
> multiple namespaces attached.  Also, I/O request to the controller
> character device has not been recommended and deprecated because we have
> block device to I/O with where the multipath consideration is taken.
> 
> Once kernel decided a path to I/O for a namespace based on the I/O
> policy of a NVMe subsystem, userspace is not allowed to choose a path to
> I/O.  If a path is broken(inaccessible state in ANA), then it will not
> try to I/O to that path.
> 
> This patch introduced NVME_IOCTL_MPATH_IO command for controller
> device(e.g., /dev/nvme0) to support multipath I/O passthrough for
> userspace.  Regardless driver's path decision, userspace can target a
> namespace to I/O.  In this case, `cmd.nsid` will be used to find out the
> namespace instance target which is hidden(e.g., nvmeXcYnZ).

IO commands are not allowed through the character handle with the
existing ioctls. A new ioctl doesn't make it okay. If it was okay, then
we could just remove the limitation in the current ones.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH] nvme: introduce passthrough ioctl for multipath

[Minwoo Im]

On 21-02-15 09:02:33, Keith Busch wrote:
> On Sun, Feb 14, 2021 at 08:01:25PM +0900, Minwoo Im wrote:
> > We don't allow NVME_IOCTL_IO_CMD ioctl in case that a controller has
> > multiple namespaces attached.  Also, I/O request to the controller
> > character device has not been recommended and deprecated because we have
> > block device to I/O with where the multipath consideration is taken.
> > 
> > Once kernel decided a path to I/O for a namespace based on the I/O
> > policy of a NVMe subsystem, userspace is not allowed to choose a path to
> > I/O.  If a path is broken(inaccessible state in ANA), then it will not
> > try to I/O to that path.
> > 
> > This patch introduced NVME_IOCTL_MPATH_IO command for controller
> > device(e.g., /dev/nvme0) to support multipath I/O passthrough for
> > userspace.  Regardless driver's path decision, userspace can target a
> > namespace to I/O.  In this case, `cmd.nsid` will be used to find out the
> > namespace instance target which is hidden(e.g., nvmeXcYnZ).
> 
> IO commands are not allowed through the character handle with the
> existing ioctls. A new ioctl doesn't make it okay. If it was okay, then
> we could just remove the limitation in the current ones.

Thanks for your feedback, Keith.  If you don't mind, may I ask why it's
been entirely unsafe and deprecated in the exsiting
ioctl(NVME_IOCTL_IO_CMD)?  I've seen a patch bfd8947194b2 ("nvme: fixes
for NVME_IOCTL_IO_CMD on the char device), but have no idea why it's
been really depreacted :)

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH] nvme: introduce passthrough ioctl for multipath

[Keith Busch]

On Tue, Feb 16, 2021 at 06:51:47PM +0900, Minwoo Im wrote:
> On 21-02-15 09:02:33, Keith Busch wrote:
> > On Sun, Feb 14, 2021 at 08:01:25PM +0900, Minwoo Im wrote:
> > > We don't allow NVME_IOCTL_IO_CMD ioctl in case that a controller has
> > > multiple namespaces attached.  Also, I/O request to the controller
> > > character device has not been recommended and deprecated because we have
> > > block device to I/O with where the multipath consideration is taken.
> > > 
> > > Once kernel decided a path to I/O for a namespace based on the I/O
> > > policy of a NVMe subsystem, userspace is not allowed to choose a path to
> > > I/O.  If a path is broken(inaccessible state in ANA), then it will not
> > > try to I/O to that path.
> > > 
> > > This patch introduced NVME_IOCTL_MPATH_IO command for controller
> > > device(e.g., /dev/nvme0) to support multipath I/O passthrough for
> > > userspace.  Regardless driver's path decision, userspace can target a
> > > namespace to I/O.  In this case, `cmd.nsid` will be used to find out the
> > > namespace instance target which is hidden(e.g., nvmeXcYnZ).
> > 
> > IO commands are not allowed through the character handle with the
> > existing ioctls. A new ioctl doesn't make it okay. If it was okay, then
> > we could just remove the limitation in the current ones.
> 
> Thanks for your feedback, Keith.  If you don't mind, may I ask why it's
> been entirely unsafe and deprecated in the exsiting
> ioctl(NVME_IOCTL_IO_CMD)?  I've seen a patch bfd8947194b2 ("nvme: fixes
> for NVME_IOCTL_IO_CMD on the char device), but have no idea why it's
> been really depreacted :)

A container could read data from namespaces assigned to a different
container.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH] nvme: introduce passthrough ioctl for multipath

[Minwoo Im]

On 21-02-16 09:57:22, Keith Busch wrote:
> On Tue, Feb 16, 2021 at 06:51:47PM +0900, Minwoo Im wrote:
> > On 21-02-15 09:02:33, Keith Busch wrote:
> > > On Sun, Feb 14, 2021 at 08:01:25PM +0900, Minwoo Im wrote:
> > > > We don't allow NVME_IOCTL_IO_CMD ioctl in case that a controller has
> > > > multiple namespaces attached.  Also, I/O request to the controller
> > > > character device has not been recommended and deprecated because we have
> > > > block device to I/O with where the multipath consideration is taken.
> > > > 
> > > > Once kernel decided a path to I/O for a namespace based on the I/O
> > > > policy of a NVMe subsystem, userspace is not allowed to choose a path to
> > > > I/O.  If a path is broken(inaccessible state in ANA), then it will not
> > > > try to I/O to that path.
> > > > 
> > > > This patch introduced NVME_IOCTL_MPATH_IO command for controller
> > > > device(e.g., /dev/nvme0) to support multipath I/O passthrough for
> > > > userspace.  Regardless driver's path decision, userspace can target a
> > > > namespace to I/O.  In this case, `cmd.nsid` will be used to find out the
> > > > namespace instance target which is hidden(e.g., nvmeXcYnZ).
> > > 
> > > IO commands are not allowed through the character handle with the
> > > existing ioctls. A new ioctl doesn't make it okay. If it was okay, then
> > > we could just remove the limitation in the current ones.
> > 
> > Thanks for your feedback, Keith.  If you don't mind, may I ask why it's
> > been entirely unsafe and deprecated in the exsiting
> > ioctl(NVME_IOCTL_IO_CMD)?  I've seen a patch bfd8947194b2 ("nvme: fixes
> > for NVME_IOCTL_IO_CMD on the char device), but have no idea why it's
> > been really depreacted :)
> 
> A container could read data from namespaces assigned to a different
> container.

Now I got your point! Thanks Keith, please ignore this patch :)

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH] nvme: introduce passthrough ioctl for multipath

[Christoph Hellwig]

On Wed, Feb 17, 2021 at 11:14:55AM +0900, Minwoo Im wrote:
> > A container could read data from namespaces assigned to a different
> > container.
> 
> Now I got your point! Thanks Keith, please ignore this patch :)

Note that Javier has been working on exposing access to namespaces
with unsupported command sets for ioctls.  If that is what you are
looking for.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH] nvme: introduce passthrough ioctl for multipath

[Javier González]

On 18.02.2021 09:40, Christoph Hellwig wrote:
>On Wed, Feb 17, 2021 at 11:14:55AM +0900, Minwoo Im wrote:
>> > A container could read data from namespaces assigned to a different
>> > container.
>>
>> Now I got your point! Thanks Keith, please ignore this patch :)
>
>Note that Javier has been working on exposing access to namespaces
>with unsupported command sets for ioctls.  If that is what you are
>looking for.

Minwoo, I think this would apply good on top of the char device patches
I posted a while ago. I have a multipath version working on top, but I
am still working out a good way to expose in sysfs.

We can try to merge these and send the char device with multipath in the
following days. This has been hanging on me more than I wanted it to...

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH] nvme: introduce passthrough ioctl for multipath

[Minwoo Im]

On 21-02-18 09:40:14, Christoph Hellwig wrote:
> On Wed, Feb 17, 2021 at 11:14:55AM +0900, Minwoo Im wrote:
> > > A container could read data from namespaces assigned to a different
> > > container.
> > 
> > Now I got your point! Thanks Keith, please ignore this patch :)
> 
> Note that Javier has been working on exposing access to namespaces
> with unsupported command sets for ioctls.  If that is what you are
> looking for.

Thank you for the information!

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH] nvme: introduce passthrough ioctl for multipath

[Minwoo Im]

On 21-02-18 09:48:12, Javier González wrote:
> On 18.02.2021 09:40, Christoph Hellwig wrote:
> > On Wed, Feb 17, 2021 at 11:14:55AM +0900, Minwoo Im wrote:
> > > > A container could read data from namespaces assigned to a different
> > > > container.
> > > 
> > > Now I got your point! Thanks Keith, please ignore this patch :)
> > 
> > Note that Javier has been working on exposing access to namespaces
> > with unsupported command sets for ioctls.  If that is what you are
> > looking for.
> 
> Minwoo, I think this would apply good on top of the char device patches
> I posted a while ago. I have a multipath version working on top, but I
> am still working out a good way to expose in sysfs.
> 
> We can try to merge these and send the char device with multipath in the
> following days. This has been hanging on me more than I wanted it to...

Thanks Javier, That sounds cool.  will have a look at your patch first!

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme