[PATCH v1 0/2] Fix failover to non integrity NVMe path

[PATCH v1 0/2] Fix failover to non integrity NVMe path

[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

[PATCH v1 1/2] block: bio-integrity: export bio_integrity_free func

[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

Re: [PATCH v1 1/2] block: bio-integrity: export bio_integrity_free func

[kernel test robot]

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

Re: [PATCH v1 1/2] block: bio-integrity: export bio_integrity_free func

[kernel test robot]

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

[PATCH v1 2/2] nvme-multipath: fix path failover for integrity ns

[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

Re: [PATCH v1 2/2] nvme-multipath: fix path failover for integrity ns

[Sagi Grimberg]

> 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>

Re: [PATCH v1 0/2] Fix failover to non integrity NVMe path

[Christoph Hellwig]

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.

Re: [PATCH v1 0/2] Fix failover to non integrity NVMe path

[Hannes Reinecke]

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

Re: [PATCH v1 0/2] Fix failover to non integrity NVMe path

[Christoph Hellwig]

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.

Re: [PATCH v1 0/2] Fix failover to non integrity NVMe path

[Sagi Grimberg]

>> 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.

Re: [PATCH v1 0/2] Fix failover to non integrity NVMe path

[Max Gurtovoy]

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.