[PATCH] multipath-tools: replace round-robin path selector with the default for NFINIDAT/InfiniBox

[PATCH] multipath-tools: replace round-robin path selector with the default for NFINIDAT/InfiniBox

[Xose Vazquez Perez]

round-robin is a basic selector, and only works well under ideal conditions,

A nvme benchmark, round-robin vs queue-depth, shows how bad it is:
https://marc.info/?l=linux-kernel&m=171931850925572
https://marc.info/?l=linux-kernel&m=171931852025575
https://github.com/johnmeneghini/iopolicy/?tab=readme-ov-file#sample-data
https://people.redhat.com/jmeneghi/ALPSS_2023/NVMe_QD_Multipathing.pdf
The same happens for scsi.

Cc: Martin Wilck <mwilck@suse.com>
Cc: Benjamin Marzinski <bmarzins@redhat.com>
Cc: Christophe Varoqui <christophe.varoqui@opensvc.com>
Cc: DM-DEVEL ML <dm-devel@lists.linux.dev>
Signed-off-by: Xose Vazquez Perez <xose.vazquez@gmail.com>
---
Should global keywords be banned in the devices section, or deleted from hwtable ???
dev_loss
fast_io_fail
max_sectors_kb
...

They interfere when two, or more, different arrays are connected to the same host.
---
 libmultipath/hwtable.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/libmultipath/hwtable.c b/libmultipath/hwtable.c
index 081d119c..3ade3669 100644
--- a/libmultipath/hwtable.c
+++ b/libmultipath/hwtable.c
@@ -1125,11 +1125,7 @@ static struct hwentry default_hw[] = {
 		.pgpolicy      = GROUP_BY_PRIO,
 		.pgfailback    = 30,
 		.prio_name     = PRIO_ALUA,
-		.selector      = "round-robin 0",
-		.rr_weight     = RR_WEIGHT_PRIO,
 		.no_path_retry = NO_PATH_RETRY_FAIL,
-		.minio         = 1,
-		.minio_rq      = 1,
 		.fast_io_fail  = 15,
 	},
 	/*
-- 
2.49.0

Re: [PATCH] multipath-tools: replace round-robin path selector with the default for NFINIDAT/InfiniBox

[Martin Wilck]

On Fri, 2025-05-09 at 15:44 +0200, Xose Vazquez Perez wrote:
> round-robin is a basic selector, and only works well under ideal
> conditions,
> 
> A nvme benchmark, round-robin vs queue-depth, shows how bad it is:
> https://marc.info/?l=linux-kernel&m=171931850925572
> https://marc.info/?l=linux-kernel&m=171931852025575
> https://github.com/johnmeneghini/iopolicy/?tab=readme-ov-file#sample-data
> https://people.redhat.com/jmeneghi/ALPSS_2023/NVMe_QD_Multipathing.pdf

All these are about native nvme multipath. None is about dm-multipath.

> The same happens for scsi.

Please support this claim with actual numbers. I'm not saying that it's
wrong (it makes sense that John's argument "the round-robin path
selector is inefficient in cases where there is a difference in latency
between paths" applies to all implementations), but you can't present
numbers for one technology and simply apply them to a different
technology without providing any evidence for the latter.

I'd accept this statement in a weaker form, like "It is reasonable to
assume that the same effects would be measured with dm-multipath over
SCSI devices, too".

> 
> Cc: Martin Wilck <mwilck@suse.com>
> Cc: Benjamin Marzinski <bmarzins@redhat.com>
> Cc: Christophe Varoqui <christophe.varoqui@opensvc.com>
> Cc: DM-DEVEL ML <dm-devel@lists.linux.dev>
> Signed-off-by: Xose Vazquez Perez <xose.vazquez@gmail.com>
> ---
> Should global keywords be banned in the devices section, or deleted
> from hwtable ???
> dev_loss
> fast_io_fail
> max_sectors_kb
> ...
> 
> They interfere when two, or more, different arrays are connected to
> the same host.
> ---
>  libmultipath/hwtable.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/libmultipath/hwtable.c b/libmultipath/hwtable.c
> index 081d119c..3ade3669 100644
> --- a/libmultipath/hwtable.c
> +++ b/libmultipath/hwtable.c
> @@ -1125,11 +1125,7 @@ static struct hwentry default_hw[] = {
>  		.pgpolicy      = GROUP_BY_PRIO,
>  		.pgfailback    = 30,
>  		.prio_name     = PRIO_ALUA,
> -		.selector      = "round-robin 0",
> -		.rr_weight     = RR_WEIGHT_PRIO,
>  		.no_path_retry = NO_PATH_RETRY_FAIL,
> -		.minio         = 1,
> -		.minio_rq      = 1,
>  		.fast_io_fail  = 15,
>  	},
>  	/*

This code is from 55da608 ("libmultipath: update INFINIDAT builtin
config"), signed-off by arnony@infinidat.com. We usually don't change
defaults from explicit vendor recommendations without consent from the
vendor. I've added Arnon to cc, maybe he wants to comment on your
suggestion.

Regards,
Martin

Re: [PATCH] multipath-tools: replace round-robin path selector with the default for NFINIDAT/InfiniBox

[Xose Vazquez Perez]

On 5/9/25 4:08 PM, Martin Wilck wrote:

> Please support this claim with actual numbers. I'm not saying that it's
> wrong (it makes sense that John's argument "the round-robin path
> selector is inefficient in cases where there is a difference in latency
> between paths" applies to all implementations), but you can't present
> numbers for one technology and simply apply them to a different
> technology without providing any evidence for the latter.
> 
> I'd accept this statement in a weaker form, like "It is reasonable to
> assume that the same effects would be measured with dm-multipath over
> SCSI devices, too".

Marzinski did some tests time ago:

git show c015b1281
commit c015b128103e7a6426d124a38cd679a181573b88
Author: Benjamin Marzinski <bmarzins@redhat.com>
Date:   Sat Jan 12 00:04:40 2013 -0600

     multipath: change default path_selector to

     My testing has showed service-time to be as good and occassionally
     noticeably better than round-robin.  So the patch switches the
     default selector to give better performance out of the box.

     Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>

diff --git a/libmultipath/defaults.h b/libmultipath/defaults.h
index f647ee08..097c66f7 100644
--- a/libmultipath/defaults.h
+++ b/libmultipath/defaults.h
@@ -1,7 +1,7 @@
  #define DEFAULT_UID_ATTRIBUTE  "ID_SERIAL"
  #define DEFAULT_UDEVDIR                "/dev"
  #define DEFAULT_MULTIPATHDIR   "/" LIB_STRING "/multipath"
-#define DEFAULT_SELECTOR       "round-robin 0"
+#define DEFAULT_SELECTOR       "service-time 0"
  #define DEFAULT_ALIAS_PREFIX   "mpath"
  #define DEFAULT_FEATURES       "0"
  #define DEFAULT_HWHANDLER      "0"

I didn't do any test, sorry.

Re: [PATCH] multipath-tools: replace round-robin path selector with the default for NFINIDAT/InfiniBox

[Martin Wilck]

On Fri, 2025-05-09 at 16:08 +0200, Martin Wilck wrote:
> 
> This code is from 55da608 ("libmultipath: update INFINIDAT builtin
> config"), signed-off by arnony@infinidat.com. We usually don't change
> defaults from explicit vendor recommendations without consent from
> the
> vendor. I've added Arnon to cc, maybe he wants to comment on your
> suggestion.

Arnon's email address bounces. Unless we find someone else from
Infinidat to comment, I think we should keep the current default. Users
are free to override the defaults if the aren't happy with the
performance.

Thanks
Martin

Re: [PATCH] multipath-tools: replace round-robin path selector with the default for NFINIDAT/InfiniBox

[Benjamin Marzinski]

On Fri, May 09, 2025 at 04:22:40PM +0200, Martin Wilck wrote:
> On Fri, 2025-05-09 at 16:08 +0200, Martin Wilck wrote:
> > 
> > This code is from 55da608 ("libmultipath: update INFINIDAT builtin
> > config"), signed-off by arnony@infinidat.com. We usually don't change
> > defaults from explicit vendor recommendations without consent from
> > the
> > vendor. I've added Arnon to cc, maybe he wants to comment on your
> > suggestion.
> 
> Arnon's email address bounces. Unless we find someone else from
> Infinidat to comment, I think we should keep the current default. Users
> are free to override the defaults if the aren't happy with the
> performance.

I agree. This device config was set to round-robin after we switched the
default to service-time, so I think it's reasonable to assume that this
was intentional. Barring an email or updated documentation from
INFINIDAT that shows it's no longer the recommended config, I feel like
we should leave it as is. 

-Ben

> 
> Thanks
> Martin

Re: [PATCH] multipath-tools: replace round-robin path selector with the default for NFINIDAT/InfiniBox

[Xose Vazquez Perez]

On 5/12/25 7:11 PM, Benjamin Marzinski wrote:

> On Fri, May 09, 2025 at 04:22:40PM +0200, Martin Wilck wrote:
>> On Fri, 2025-05-09 at 16:08 +0200, Martin Wilck wrote:
>>>
>>> This code is from 55da608 ("libmultipath: update INFINIDAT builtin
>>> config"), signed-off by arnony@infinidat.com. We usually don't change
>>> defaults from explicit vendor recommendations without consent from
>>> the
>>> vendor. I've added Arnon to cc, maybe he wants to comment on your
>>> suggestion.
>>
>> Arnon's email address bounces. Unless we find someone else from
>> Infinidat to comment, I think we should keep the current default. Users
>> are free to override the defaults if the aren't happy with the
>> performance.
> 
> I agree. This device config was set to round-robin after we switched the
> default to service-time, so I think it's reasonable to assume that this
> was intentional. Barring an email or updated documentation from
> INFINIDAT that shows it's no longer the recommended config, I feel like
> we should leave it as is.

At Infinidat support site(user required), there are new configs for SLES 15 and RHEL 8, or above:
https://lh3.googleusercontent.com/pw/AP1GczPhlOIUXYWsuCYQJ42ORS_77xnwTmlN02ZkDU4__6jAvQ_6bBLvvcFsQ9luhSMcB887pXZOaVCrCNZUITTl8eEztD7l568BTSNHL2cB9Dh2SfAAXye7K9Tu8nkjCKMkhQM4H7LRiY4Rp0ygQqCrMmM=w0-h0
https://lh3.googleusercontent.com/pw/AP1GczMchJ6bcMIohp_g3Ik5DS6BZv_IW0iwaEXf968sJsR8fr_p3mR4ThRpmHpZE_VMnlcV8j0uuuI2kz-eoqekRCUBxyTBKS7n-4WFMsUiecq8i7nIjEuhfZFrV54DBQLDzGt6ofakAAF1L-ZcAuCWW18=w0-h0

device {
    vendor "NFINIDAT"
    product "InfiniBox"
    path_grouping_policy "group_by_prio"
    path_checker "tur"
    features 0
    hardware_handler "1 alua"
    prio "alua"
    rr_weight "priorities"
    no_path_retry "queue"
    rr_min_io 1
    rr_min_io_rq 1
    flush_on_last_del "yes"
    fast_io_fail_tmo 15
    dev_loss_tmo "infinity"
    path_selector "service-time 0"
    failback "immediate"
    detect_prio "no"
    user_friendly_names "no"
   }

I will send a new patch.