* [PATCH v1 0/2] Fix failover to non integrity NVMe path
@ 2023-04-23 14:13 Max Gurtovoy
2023-04-23 14:13 ` [PATCH v1 1/2] block: bio-integrity: export bio_integrity_free func Max Gurtovoy
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Max Gurtovoy @ 2023-04-23 14:13 UTC (permalink / raw
To: martin.petersen, hch, sagi, linux-nvme
Cc: kbusch, axboe, linux-block, oren, oevron, israelr, Max Gurtovoy
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="y", Size: 2685 bytes --]
Hi Christoph/Sagi/Martin,
We're encountered a crash while testing failover between NVMeF/RDMA
paths to a target that expose a namespace with metadata. The scenario is
the following:
Configure one initiator/host path on PI offload capable port (e.g ConnectX-5
device) and configure second initiator/host path on non PI offload capable
port (e.g ConnectX-3).
In case of failover, the original rq/bio is protected with integrity
context but the failover port is not integrity capable and it didn't allocate
the metadata resources for request. Thus we get NULL deref in case
blk_integrity_rq(rq) return true while req->metadata_sgl is NULL.
Bellow snip on the trace:
[Tue Feb 14 18:48:25 2023] mlx5_core 0000:02:00.0 ens2f0np0: Link down
[Tue Feb 14 18:48:32 2023] nvme nvme0: starting error recovery
[Tue Feb 14 18:48:32 2023] BUG: kernel NULL pointer dereference, address: 0000000000000010
[Tue Feb 14 18:48:32 2023] #PF: supervisor read access in kernel mode
[Tue Feb 14 18:48:32 2023] #PF: error_code(0x0000) - not-present page
[Tue Feb 14 18:48:32 2023] PGD 0 P4D 0
[Tue Feb 14 18:48:32 2023] Oops: 0000 [#1] PREEMPT SMP PTI
[Tue Feb 14 18:48:32 2023] CPU: 17 PID: 518 Comm: kworker/17:1H Tainted: G S E 6.2.0-rc4+ #224
[Tue Feb 14 18:48:32 2023] Hardware name: Supermicro SYS-6018R-WTR/X10DRW-i, BIOS 2.0 12/17/2015
[Tue Feb 14 18:48:32 2023] Workqueue: kblockd nvme_requeue_work [nvme_core]
...
...
[Tue Feb 14 18:48:32 2023] Call Trace:
[Tue Feb 14 18:48:32 2023] <TASK>
[Tue Feb 14 18:48:32 2023] nvme_rdma_queue_rq+0x194/0xa20 [nvme_rdma]
[Tue Feb 14 18:48:32 2023] ? __blk_mq_try_issue_directly+0x13f/0x1a0
[Tue Feb 14 18:48:32 2023] __blk_mq_try_issue_directly+0x13f/0x1a0
[Tue Feb 14 18:48:32 2023] blk_mq_try_issue_directly+0x15/0x50
[Tue Feb 14 18:48:32 2023] blk_mq_submit_bio+0x539/0x580
[Tue Feb 14 18:48:32 2023] __submit_bio+0xfa/0x170
[Tue Feb 14 18:48:32 2023] submit_bio_noacct_nocheck+0xe1/0x2a0
[Tue Feb 14 18:48:32 2023] nvme_requeue_work+0x4e/0x60 [nvme_core]
To solve that we'll expose API to release the integrity context from a
bio and call it in case of failover for each bio.
Another way to solve this is to free the context during
bio_integrity_prep.
I choose the first option because I thought it is better to avoid this
check in the fast path but the price is exporting new API from the block
layer.
Max Gurtovoy (2):
block: bio-integrity: export bio_integrity_free func
nvme-multipath: fix path failover for integrity ns
block/bio-integrity.c | 1 +
drivers/nvme/host/multipath.c | 9 +++++++++
include/linux/bio.h | 5 +++++
3 files changed, 15 insertions(+)
--
2.18.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v1 1/2] block: bio-integrity: export bio_integrity_free func
2023-04-23 14:13 [PATCH v1 0/2] Fix failover to non integrity NVMe path Max Gurtovoy
@ 2023-04-23 14:13 ` Max Gurtovoy
2023-04-23 16:34 ` kernel test robot
2023-04-23 16:34 ` kernel test robot
2023-04-23 14:13 ` [PATCH v1 2/2] nvme-multipath: fix path failover for integrity ns Max Gurtovoy
2023-04-24 5:11 ` [PATCH v1 0/2] Fix failover to non integrity NVMe path Christoph Hellwig
2 siblings, 2 replies; 11+ messages in thread
From: Max Gurtovoy @ 2023-04-23 14:13 UTC (permalink / raw
To: martin.petersen, hch, sagi, linux-nvme
Cc: kbusch, axboe, linux-block, oren, oevron, israelr, Max Gurtovoy
This function is the complementary function to bio_integrity_alloc.
Export it for users that would like to free the integrity context
explicitly.
Signed-off-by: Max Gurtovoy <mgurtovoy@nvidia.com>
---
block/bio-integrity.c | 1 +
include/linux/bio.h | 5 +++++
2 files changed, 6 insertions(+)
diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index 4533eb491661..276ca86fc1d3 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -110,6 +110,7 @@ void bio_integrity_free(struct bio *bio)
bio->bi_integrity = NULL;
bio->bi_opf &= ~REQ_INTEGRITY;
}
+EXPORT_SYMBOL(bio_integrity_free);
/**
* bio_integrity_add_page - Attach integrity metadata
diff --git a/include/linux/bio.h b/include/linux/bio.h
index d766be7152e1..3888a609b4bb 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -700,6 +700,7 @@ static inline bool bioset_initialized(struct bio_set *bs)
bip_for_each_vec(_bvl, _bio->bi_integrity, _iter)
extern struct bio_integrity_payload *bio_integrity_alloc(struct bio *, gfp_t, unsigned int);
+extern void bio_integrity_free(struct bio *);
extern int bio_integrity_add_page(struct bio *, struct page *, unsigned int, unsigned int);
extern bool bio_integrity_prep(struct bio *);
extern void bio_integrity_advance(struct bio *, unsigned int);
@@ -764,6 +765,10 @@ static inline void *bio_integrity_alloc(struct bio * bio, gfp_t gfp,
return ERR_PTR(-EINVAL);
}
+void bio_integrity_free(struct bio *bio)
+{
+}
+
static inline int bio_integrity_add_page(struct bio *bio, struct page *page,
unsigned int len, unsigned int offset)
{
--
2.18.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v1 2/2] nvme-multipath: fix path failover for integrity ns
2023-04-23 14:13 [PATCH v1 0/2] Fix failover to non integrity NVMe path Max Gurtovoy
2023-04-23 14:13 ` [PATCH v1 1/2] block: bio-integrity: export bio_integrity_free func Max Gurtovoy
@ 2023-04-23 14:13 ` Max Gurtovoy
2023-04-23 14:22 ` Sagi Grimberg
2023-04-24 5:11 ` [PATCH v1 0/2] Fix failover to non integrity NVMe path Christoph Hellwig
2 siblings, 1 reply; 11+ messages in thread
From: Max Gurtovoy @ 2023-04-23 14:13 UTC (permalink / raw
To: martin.petersen, hch, sagi, linux-nvme
Cc: kbusch, axboe, linux-block, oren, oevron, israelr, Max Gurtovoy
In case the integrity capabilities of the failed path and the failover
path don't match, we may run into NULL dereference. Free the integrity
context during the path failover and let the block layer prepare it
again if needed during bio_submit.
Reviewed-by: Israel Rukshin <israelr@nvidia.com>
Tested-by: Ori Evron <oevron@nvidia.com>
Signed-off-by: Ori Evron <oevron@nvidia.com>
Signed-off-by: Max Gurtovoy <mgurtovoy@nvidia.com>
---
drivers/nvme/host/multipath.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index 9171452e2f6d..f439916f4447 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -6,6 +6,7 @@
#include <linux/backing-dev.h>
#include <linux/moduleparam.h>
#include <linux/vmalloc.h>
+#include <linux/blk-integrity.h>
#include <trace/events/block.h>
#include "nvme.h"
@@ -106,6 +107,14 @@ void nvme_failover_req(struct request *req)
bio->bi_opf &= ~REQ_POLLED;
bio->bi_cookie = BLK_QC_T_NONE;
}
+ /*
+ * If the failover path will not be integrity capable the bio
+ * should not have integrity context.
+ * If the failover path will be integrity capable the bio will
+ * be prepared for integrity again.
+ */
+ if (bio_integrity(bio))
+ bio_integrity_free(bio);
}
blk_steal_bios(&ns->head->requeue_list, req);
spin_unlock_irqrestore(&ns->head->requeue_lock, flags);
--
2.18.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v1 2/2] nvme-multipath: fix path failover for integrity ns
2023-04-23 14:13 ` [PATCH v1 2/2] nvme-multipath: fix path failover for integrity ns Max Gurtovoy
@ 2023-04-23 14:22 ` Sagi Grimberg
0 siblings, 0 replies; 11+ messages in thread
From: Sagi Grimberg @ 2023-04-23 14:22 UTC (permalink / raw
To: Max Gurtovoy, martin.petersen, hch, linux-nvme
Cc: kbusch, axboe, linux-block, oren, oevron, israelr
> In case the integrity capabilities of the failed path and the failover
> path don't match, we may run into NULL dereference. Free the integrity
> context during the path failover and let the block layer prepare it
> again if needed during bio_submit.
>
> Reviewed-by: Israel Rukshin <israelr@nvidia.com>
> Tested-by: Ori Evron <oevron@nvidia.com>
> Signed-off-by: Ori Evron <oevron@nvidia.com>
> Signed-off-by: Max Gurtovoy <mgurtovoy@nvidia.com>
> ---
> drivers/nvme/host/multipath.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
> index 9171452e2f6d..f439916f4447 100644
> --- a/drivers/nvme/host/multipath.c
> +++ b/drivers/nvme/host/multipath.c
> @@ -6,6 +6,7 @@
> #include <linux/backing-dev.h>
> #include <linux/moduleparam.h>
> #include <linux/vmalloc.h>
> +#include <linux/blk-integrity.h>
> #include <trace/events/block.h>
> #include "nvme.h"
>
> @@ -106,6 +107,14 @@ void nvme_failover_req(struct request *req)
> bio->bi_opf &= ~REQ_POLLED;
> bio->bi_cookie = BLK_QC_T_NONE;
> }
> + /*
> + * If the failover path will not be integrity capable the bio
> + * should not have integrity context.
> + * If the failover path will be integrity capable the bio will
> + * be prepared for integrity again.
> + */
> + if (bio_integrity(bio))
> + bio_integrity_free(bio);
> }
> blk_steal_bios(&ns->head->requeue_list, req);
> spin_unlock_irqrestore(&ns->head->requeue_lock, flags);
This looks good to me,
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 1/2] block: bio-integrity: export bio_integrity_free func
2023-04-23 14:13 ` [PATCH v1 1/2] block: bio-integrity: export bio_integrity_free func Max Gurtovoy
@ 2023-04-23 16:34 ` kernel test robot
2023-04-23 16:34 ` kernel test robot
1 sibling, 0 replies; 11+ messages in thread
From: kernel test robot @ 2023-04-23 16:34 UTC (permalink / raw
To: Max Gurtovoy, martin.petersen, hch, sagi, linux-nvme
Cc: oe-kbuild-all, kbusch, axboe, linux-block, oren, oevron, israelr,
Max Gurtovoy
Hi Max,
kernel test robot noticed the following build errors:
[auto build test ERROR on axboe-block/for-next]
[also build test ERROR on hch-configfs/for-next linus/master v6.3-rc7 next-20230421]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Max-Gurtovoy/block-bio-integrity-export-bio_integrity_free-func/20230423-222054
base: https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
patch link: https://lore.kernel.org/r/20230423141330.40437-2-mgurtovoy%40nvidia.com
patch subject: [PATCH v1 1/2] block: bio-integrity: export bio_integrity_free func
config: alpha-allnoconfig (https://download.01.org/0day-ci/archive/20230424/202304240022.AvNPfDhp-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/1459ddb310ef47b008de7669cc2c57e02edf4edb
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Max-Gurtovoy/block-bio-integrity-export-bio_integrity_free-func/20230423-222054
git checkout 1459ddb310ef47b008de7669cc2c57e02edf4edb
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=alpha olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=alpha SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202304240022.AvNPfDhp-lkp@intel.com/
All error/warnings (new ones prefixed by >>):
In file included from include/linux/blkdev.h:17,
from init/main.c:86:
>> include/linux/bio.h:768:6: warning: no previous prototype for 'bio_integrity_free' [-Wmissing-prototypes]
768 | void bio_integrity_free(struct bio *bio)
| ^~~~~~~~~~~~~~~~~~
init/main.c:779:20: warning: no previous prototype for 'arch_post_acpi_subsys_init' [-Wmissing-prototypes]
779 | void __init __weak arch_post_acpi_subsys_init(void) { }
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
init/main.c:791:20: warning: no previous prototype for 'mem_encrypt_init' [-Wmissing-prototypes]
791 | void __init __weak mem_encrypt_init(void) { }
| ^~~~~~~~~~~~~~~~
init/main.c:793:20: warning: no previous prototype for 'poking_init' [-Wmissing-prototypes]
793 | void __init __weak poking_init(void) { }
| ^~~~~~~~~~~
--
In file included from include/linux/blkdev.h:17,
from init/do_mounts.h:3,
from init/do_mounts.c:28:
>> include/linux/bio.h:768:6: warning: no previous prototype for 'bio_integrity_free' [-Wmissing-prototypes]
768 | void bio_integrity_free(struct bio *bio)
| ^~~~~~~~~~~~~~~~~~
--
In file included from include/linux/blkdev.h:17,
from kernel/exit.c:52:
>> include/linux/bio.h:768:6: warning: no previous prototype for 'bio_integrity_free' [-Wmissing-prototypes]
768 | void bio_integrity_free(struct bio *bio)
| ^~~~~~~~~~~~~~~~~~
kernel/exit.c:1915:32: warning: no previous prototype for 'abort' [-Wmissing-prototypes]
1915 | __weak __function_aligned void abort(void)
| ^~~~~
--
In file included from include/linux/blkdev.h:17,
from block/bdev.c:14:
>> include/linux/bio.h:768:6: warning: no previous prototype for 'bio_integrity_free' [-Wmissing-prototypes]
768 | void bio_integrity_free(struct bio *bio)
| ^~~~~~~~~~~~~~~~~~
In file included from block/bdev.c:31:
>> block/blk.h:248:20: error: redefinition of 'bio_integrity_free'
248 | static inline void bio_integrity_free(struct bio *bio)
| ^~~~~~~~~~~~~~~~~~
include/linux/bio.h:768:6: note: previous definition of 'bio_integrity_free' with type 'void(struct bio *)'
768 | void bio_integrity_free(struct bio *bio)
| ^~~~~~~~~~~~~~~~~~
--
In file included from include/linux/blkdev.h:17,
from lib/vsprintf.c:46:
>> include/linux/bio.h:768:6: warning: no previous prototype for 'bio_integrity_free' [-Wmissing-prototypes]
768 | void bio_integrity_free(struct bio *bio)
| ^~~~~~~~~~~~~~~~~~
lib/vsprintf.c: In function 'va_format':
lib/vsprintf.c:1681:9: warning: function 'va_format' might be a candidate for 'gnu_printf' format attribute [-Wsuggest-attribute=format]
1681 | buf += vsnprintf(buf, end > buf ? end - buf : 0, va_fmt->fmt, va);
| ^~~
--
In file included from include/linux/blkdev.h:17,
from block/partitions/check.h:3,
from block/partitions/core.c:13:
>> include/linux/bio.h:768:6: warning: no previous prototype for 'bio_integrity_free' [-Wmissing-prototypes]
768 | void bio_integrity_free(struct bio *bio)
| ^~~~~~~~~~~~~~~~~~
In file included from block/partitions/check.h:4:
>> block/partitions/../blk.h:248:20: error: redefinition of 'bio_integrity_free'
248 | static inline void bio_integrity_free(struct bio *bio)
| ^~~~~~~~~~~~~~~~~~
include/linux/bio.h:768:6: note: previous definition of 'bio_integrity_free' with type 'void(struct bio *)'
768 | void bio_integrity_free(struct bio *bio)
| ^~~~~~~~~~~~~~~~~~
vim +/bio_integrity_free +248 block/blk.h
43b729bfe9cf30 Christoph Hellwig 2018-09-24 240
5a48fc147d7f27 Dan Williams 2015-10-21 241 static inline void blk_flush_integrity(void)
5a48fc147d7f27 Dan Williams 2015-10-21 242 {
5a48fc147d7f27 Dan Williams 2015-10-21 243 }
7c20f11680a441 Christoph Hellwig 2017-07-03 244 static inline bool bio_integrity_endio(struct bio *bio)
7c20f11680a441 Christoph Hellwig 2017-07-03 245 {
7c20f11680a441 Christoph Hellwig 2017-07-03 246 return true;
7c20f11680a441 Christoph Hellwig 2017-07-03 247 }
ece841abbed2da Justin Tee 2019-12-05 @248 static inline void bio_integrity_free(struct bio *bio)
ece841abbed2da Justin Tee 2019-12-05 249 {
ece841abbed2da Justin Tee 2019-12-05 250 }
614310c9c8ca15 Luis Chamberlain 2021-08-18 251 static inline int blk_integrity_add(struct gendisk *disk)
581e26004a09c5 Christoph Hellwig 2020-03-25 252 {
614310c9c8ca15 Luis Chamberlain 2021-08-18 253 return 0;
581e26004a09c5 Christoph Hellwig 2020-03-25 254 }
581e26004a09c5 Christoph Hellwig 2020-03-25 255 static inline void blk_integrity_del(struct gendisk *disk)
581e26004a09c5 Christoph Hellwig 2020-03-25 256 {
581e26004a09c5 Christoph Hellwig 2020-03-25 257 }
43b729bfe9cf30 Christoph Hellwig 2018-09-24 258 #endif /* CONFIG_BLK_DEV_INTEGRITY */
8324aa91d1e11a Jens Axboe 2008-01-29 259
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 1/2] block: bio-integrity: export bio_integrity_free func
2023-04-23 14:13 ` [PATCH v1 1/2] block: bio-integrity: export bio_integrity_free func Max Gurtovoy
2023-04-23 16:34 ` kernel test robot
@ 2023-04-23 16:34 ` kernel test robot
1 sibling, 0 replies; 11+ messages in thread
From: kernel test robot @ 2023-04-23 16:34 UTC (permalink / raw
To: Max Gurtovoy, martin.petersen, hch, sagi, linux-nvme
Cc: oe-kbuild-all, kbusch, axboe, linux-block, oren, oevron, israelr,
Max Gurtovoy
Hi Max,
kernel test robot noticed the following build errors:
[auto build test ERROR on axboe-block/for-next]
[also build test ERROR on hch-configfs/for-next linus/master v6.3-rc7 next-20230421]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Max-Gurtovoy/block-bio-integrity-export-bio_integrity_free-func/20230423-222054
base: https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
patch link: https://lore.kernel.org/r/20230423141330.40437-2-mgurtovoy%40nvidia.com
patch subject: [PATCH v1 1/2] block: bio-integrity: export bio_integrity_free func
config: powerpc-allnoconfig (https://download.01.org/0day-ci/archive/20230424/202304240037.LakzPMU9-lkp@intel.com/config)
compiler: powerpc-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/1459ddb310ef47b008de7669cc2c57e02edf4edb
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Max-Gurtovoy/block-bio-integrity-export-bio_integrity_free-func/20230423-222054
git checkout 1459ddb310ef47b008de7669cc2c57e02edf4edb
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=powerpc olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=powerpc SHELL=/bin/bash arch/powerpc/lib/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202304240037.LakzPMU9-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from include/linux/libnvdimm.h:14,
from arch/powerpc/lib/pmem.c:9:
>> include/linux/bio.h:768:6: error: no previous prototype for 'bio_integrity_free' [-Werror=missing-prototypes]
768 | void bio_integrity_free(struct bio *bio)
| ^~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
vim +/bio_integrity_free +768 include/linux/bio.h
767
> 768 void bio_integrity_free(struct bio *bio)
769 {
770 }
771
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 0/2] Fix failover to non integrity NVMe path
2023-04-23 14:13 [PATCH v1 0/2] Fix failover to non integrity NVMe path Max Gurtovoy
2023-04-23 14:13 ` [PATCH v1 1/2] block: bio-integrity: export bio_integrity_free func Max Gurtovoy
2023-04-23 14:13 ` [PATCH v1 2/2] nvme-multipath: fix path failover for integrity ns Max Gurtovoy
@ 2023-04-24 5:11 ` Christoph Hellwig
2023-04-24 6:10 ` Hannes Reinecke
2 siblings, 1 reply; 11+ messages in thread
From: Christoph Hellwig @ 2023-04-24 5:11 UTC (permalink / raw
To: Max Gurtovoy
Cc: martin.petersen, hch, sagi, linux-nvme, kbusch, axboe,
linux-block, oren, oevron, israelr
On Sun, Apr 23, 2023 at 05:13:28PM +0300, Max Gurtovoy wrote:
> Hi Christoph/Sagi/Martin,
>
> We're encountered a crash while testing failover between NVMeF/RDMA
> paths to a target that expose a namespace with metadata. The scenario is
> the following:
> Configure one initiator/host path on PI offload capable port (e.g ConnectX-5
> device) and configure second initiator/host path on non PI offload capable
> port (e.g ConnectX-3).
Hmm. I suspect the right thing to do here is to just fail the second
connect.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 0/2] Fix failover to non integrity NVMe path
2023-04-24 5:11 ` [PATCH v1 0/2] Fix failover to non integrity NVMe path Christoph Hellwig
@ 2023-04-24 6:10 ` Hannes Reinecke
2023-04-24 6:20 ` Christoph Hellwig
0 siblings, 1 reply; 11+ messages in thread
From: Hannes Reinecke @ 2023-04-24 6:10 UTC (permalink / raw
To: Christoph Hellwig, Max Gurtovoy
Cc: martin.petersen, sagi, linux-nvme, kbusch, axboe, linux-block,
oren, oevron, israelr
On 4/24/23 07:11, Christoph Hellwig wrote:
> On Sun, Apr 23, 2023 at 05:13:28PM +0300, Max Gurtovoy wrote:
>> Hi Christoph/Sagi/Martin,
>>
>> We're encountered a crash while testing failover between NVMeF/RDMA
>> paths to a target that expose a namespace with metadata. The scenario is
>> the following:
>> Configure one initiator/host path on PI offload capable port (e.g ConnectX-5
>> device) and configure second initiator/host path on non PI offload capable
>> port (e.g ConnectX-3).
>
> Hmm. I suspect the right thing to do here is to just fail the second
> connect.
Yeah, I'm slightly unhappy with this whole setup.
If we were just doing DIF I guess the setup could work, but then we have
to disable DIX (as we cannot support integrity data on the non-PI path).
But we would need an additional patch to disable DIX functionality in
those cases.
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 0/2] Fix failover to non integrity NVMe path
2023-04-24 6:10 ` Hannes Reinecke
@ 2023-04-24 6:20 ` Christoph Hellwig
2023-04-24 8:53 ` Sagi Grimberg
0 siblings, 1 reply; 11+ messages in thread
From: Christoph Hellwig @ 2023-04-24 6:20 UTC (permalink / raw
To: Hannes Reinecke
Cc: Christoph Hellwig, Max Gurtovoy, martin.petersen, sagi,
linux-nvme, kbusch, axboe, linux-block, oren, oevron, israelr
On Mon, Apr 24, 2023 at 08:10:59AM +0200, Hannes Reinecke wrote:
> Yeah, I'm slightly unhappy with this whole setup.
> If we were just doing DIF I guess the setup could work, but then we have to
> disable DIX (as we cannot support integrity data on the non-PI path).
> But we would need an additional patch to disable DIX functionality in those
> cases.
NVMeoF only supports inline integrity data, the remapping from out of
line integrity data is always done by the HCA for NVMe over RDMA,
and integrity data is not supported without that.
Because of that I can't see how we could sensibly support one path with
integrity offload and one without. And yes, it might make sense to
offer a way to explicitly disable integrity support to allow forming such
a multipath setup.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 0/2] Fix failover to non integrity NVMe path
2023-04-24 6:20 ` Christoph Hellwig
@ 2023-04-24 8:53 ` Sagi Grimberg
2023-04-24 9:17 ` Max Gurtovoy
0 siblings, 1 reply; 11+ messages in thread
From: Sagi Grimberg @ 2023-04-24 8:53 UTC (permalink / raw
To: Christoph Hellwig, Hannes Reinecke
Cc: Max Gurtovoy, martin.petersen, linux-nvme, kbusch, axboe,
linux-block, oren, oevron, israelr
>> Yeah, I'm slightly unhappy with this whole setup.
>> If we were just doing DIF I guess the setup could work, but then we have to
>> disable DIX (as we cannot support integrity data on the non-PI path).
>> But we would need an additional patch to disable DIX functionality in those
>> cases.
>
> NVMeoF only supports inline integrity data, the remapping from out of
> line integrity data is always done by the HCA for NVMe over RDMA,
> and integrity data is not supported without that.
>
> Because of that I can't see how we could sensibly support one path with
> integrity offload and one without. And yes, it might make sense to
> offer a way to explicitly disable integrity support to allow forming such
> a multipath setup.
I agree. I didn't read through the change log well enough, I thought
that one path is DIF and the other is DIX.
I agree that we should not permit such a configuration.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 0/2] Fix failover to non integrity NVMe path
2023-04-24 8:53 ` Sagi Grimberg
@ 2023-04-24 9:17 ` Max Gurtovoy
0 siblings, 0 replies; 11+ messages in thread
From: Max Gurtovoy @ 2023-04-24 9:17 UTC (permalink / raw
To: Sagi Grimberg, Christoph Hellwig, Hannes Reinecke
Cc: martin.petersen, linux-nvme, kbusch, axboe, linux-block, oren,
oevron, israelr
On 24/04/2023 11:53, Sagi Grimberg wrote:
>
>>> Yeah, I'm slightly unhappy with this whole setup.
>>> If we were just doing DIF I guess the setup could work, but then we
>>> have to
>>> disable DIX (as we cannot support integrity data on the non-PI path).
>>> But we would need an additional patch to disable DIX functionality in
>>> those
>>> cases.
>>
>> NVMeoF only supports inline integrity data, the remapping from out of
>> line integrity data is always done by the HCA for NVMe over RDMA,
>> and integrity data is not supported without that.
>>
>> Because of that I can't see how we could sensibly support one path with
>> integrity offload and one without. And yes, it might make sense to
>> offer a way to explicitly disable integrity support to allow forming such
>> a multipath setup.
>
> I agree. I didn't read through the change log well enough, I thought
> that one path is DIF and the other is DIX.
>
> I agree that we should not permit such a configuration.
I'm not yet convinced why not to permit it.
The spec allows this to happen and I can think about scenarios that
users will want this kind of configuration.
We also support it today (with the exception on this bug).
There is a special PRACT bit in the spec that asks the controller to
take action upon each R/W IO request.
The Multipath is not related to md IMO. One path can generate/verify md
and other can raise PRACT bit.
You can also create 2 paths from different hosts to same target and one
will have ConnectX-5 and other ConnectX-3. The fact that these path are
from the same host is not so important IMO.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-04-24 9:17 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-04-23 14:13 [PATCH v1 0/2] Fix failover to non integrity NVMe path Max Gurtovoy
2023-04-23 14:13 ` [PATCH v1 1/2] block: bio-integrity: export bio_integrity_free func Max Gurtovoy
2023-04-23 16:34 ` kernel test robot
2023-04-23 16:34 ` kernel test robot
2023-04-23 14:13 ` [PATCH v1 2/2] nvme-multipath: fix path failover for integrity ns Max Gurtovoy
2023-04-23 14:22 ` Sagi Grimberg
2023-04-24 5:11 ` [PATCH v1 0/2] Fix failover to non integrity NVMe path Christoph Hellwig
2023-04-24 6:10 ` Hannes Reinecke
2023-04-24 6:20 ` Christoph Hellwig
2023-04-24 8:53 ` Sagi Grimberg
2023-04-24 9:17 ` Max Gurtovoy
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).