Re: [PATCH] RDMA/cma: Make CM response timeout and # CM retries configurable
On Fri, Apr 12, 2019 at 04:30:24PM -0300, Jason Gunthorpe wrote: > On Mon, Mar 04, 2019 at 06:27:26AM +, Parav Pandit wrote: > > > I think we should use rdma_nl_register(RDMA_NL_RDMA_CM, cma_cb_table) which > > was removed as part of ID stats removal. > > Because of below reasons. > > 1. rdma netlink command auto loads the module > > This is probably the best argument to stay away from sysctl for module > parameters.. It is tricky to make sure the rdma module is loaded > before sysctl runs in boot, and I don't think sysctl autoloads missing > modules, or somehow copes with dynamic module loading, does it? As far as I remember, sysctl won't be available till module is loaded, so autoload won't work because path doesn't exist. > > To that end we should probably have the entire sysctl configurable > available in netlink > > Jason signature.asc Description: PGP signature
Re: [PATCH] RDMA/cma: Make CM response timeout and # CM retries configurable
On Mon, Mar 04, 2019 at 06:27:26AM +, Parav Pandit wrote: > I think we should use rdma_nl_register(RDMA_NL_RDMA_CM, cma_cb_table) which > was removed as part of ID stats removal. > Because of below reasons. > 1. rdma netlink command auto loads the module This is probably the best argument to stay away from sysctl for module parameters.. It is tricky to make sure the rdma module is loaded before sysctl runs in boot, and I don't think sysctl autoloads missing modules, or somehow copes with dynamic module loading, does it? To that end we should probably have the entire sysctl configurable available in netlink Jason
RE: [PATCH] RDMA/cma: Make CM response timeout and # CM retries configurable
> -Original Message-
> From: Leon Romanovsky
> Sent: Saturday, February 23, 2019 2:50 AM
> To: Doug Ledford
> Cc: Steve Wise ; Jason Gunthorpe
> ; Håkon Bugge ; Parav Pandit
> ; linux-r...@vger.kernel.org; linux-
> ker...@vger.kernel.org
> Subject: Re: [PATCH] RDMA/cma: Make CM response timeout and # CM
> retries configurable
>
> On Fri, Feb 22, 2019 at 12:51:55PM -0500, Doug Ledford wrote:
> >
> >
> > > On Feb 22, 2019, at 12:14 PM, Steve Wise
> wrote:
> > >
> > >
> > > On 2/22/2019 10:36 AM, Jason Gunthorpe wrote:
> > >> On Sun, Feb 17, 2019 at 06:09:09PM +0100, Håkon Bugge wrote:
> > >>> During certain workloads, the default CM response timeout is too
> > >>> short, leading to excessive retries. Hence, make it configurable
> > >>> through sysctl. While at it, also make number of CM retries
> > >>> configurable.
> > >>>
> > >>> The defaults are not changed.
> > >>>
> > >>> Signed-off-by: Håkon Bugge
> > >>> drivers/infiniband/core/cma.c | 51
> > >>> ++-
> > >>> 1 file changed, 44 insertions(+), 7 deletions(-)
> > >>>
> > >>> diff --git a/drivers/infiniband/core/cma.c
> > >>> b/drivers/infiniband/core/cma.c index c43512752b8a..ce99e1cd1029
> > >>> 100644
> > >>> +++ b/drivers/infiniband/core/cma.c
> > >>> @@ -43,6 +43,7 @@
> > >>> #include
> > >>> #include
> > >>> #include
> > >>> +#include
> > >>> #include
> > >>>
> > >>> #include
> > >>> @@ -68,13 +69,46 @@ MODULE_AUTHOR("Sean Hefty");
> > >>> MODULE_DESCRIPTION("Generic RDMA CM Agent");
> MODULE_LICENSE("Dual
> > >>> BSD/GPL");
> > >>>
> > >>> -#define CMA_CM_RESPONSE_TIMEOUT 20 #define
> > >>> CMA_QUERY_CLASSPORT_INFO_TIMEOUT 3000 -#define
> CMA_MAX_CM_RETRIES
> > >>> 15 #define CMA_CM_MRA_SETTING (IB_CM_MRA_FLAG_DELAY | 24)
> #define
> > >>> CMA_IBOE_PACKET_LIFETIME 18 #define
> CMA_PREFERRED_ROCE_GID_TYPE
> > >>> IB_GID_TYPE_ROCE_UDP_ENCAP
> > >>>
> > >>> +#define CMA_DFLT_CM_RESPONSE_TIMEOUT 20 static int
> > >>> +cma_cm_response_timeout = CMA_DFLT_CM_RESPONSE_TIMEOUT;
> static
> > >>> +int cma_cm_response_timeout_min = 8; static int
> > >>> +cma_cm_response_timeout_max = 31; #undef
> > >>> +CMA_DFLT_CM_RESPONSE_TIMEOUT
> > >>> +
> > >>> +#define CMA_DFLT_MAX_CM_RETRIES 15 static int
> cma_max_cm_retries
> > >>> += CMA_DFLT_MAX_CM_RETRIES; static int cma_max_cm_retries_min
> = 1;
> > >>> +static int cma_max_cm_retries_max = 100; #undef
> > >>> +CMA_DFLT_MAX_CM_RETRIES
> > >>> +
> > >>> +static struct ctl_table_header *cma_ctl_table_hdr; static struct
> > >>> +ctl_table cma_ctl_table[] = {
> > >>> + {
> > >>> + .procname = "cma_cm_response_timeout",
> > >>> + .data = _cm_response_timeout,
> > >>> + .maxlen = sizeof(cma_cm_response_timeout),
> > >>> + .mode = 0644,
> > >>> + .proc_handler = proc_dointvec_minmax,
> > >>> + .extra1 = _cm_response_timeout_min,
> > >>> + .extra2 = _cm_response_timeout_max,
> > >>> + },
> > >>> + {
> > >>> + .procname = "cma_max_cm_retries",
> > >>> + .data = _max_cm_retries,
> > >>> + .maxlen = sizeof(cma_max_cm_retries),
> > >>> + .mode = 0644,
> > >>> + .proc_handler = proc_dointvec_minmax,
> > >>> + .extra1 = _max_cm_retries_min,
> > >>> + .extra2 = _max_cm_retries_max,
> > >>> + },
> > >>> + { }
> > >>> +};
> > >> Is sysctl the right approach here? Should it be rdma tool instead?
> > >>
> > >> Jason
> > >
> > > There are other rdma sysctls currently: net.rdma_ucm.max_backlog
> > > and net.iw_cm.default_backlog. The core net
Re: [PATCH] RDMA/cma: Make CM response timeout and # CM retries configurable
> On 23 Feb 2019, at 09:49, Leon Romanovsky wrote:
>
> On Fri, Feb 22, 2019 at 12:51:55PM -0500, Doug Ledford wrote:
>>
>>
>>> On Feb 22, 2019, at 12:14 PM, Steve Wise
>>> wrote:
>>>
>>>
>>> On 2/22/2019 10:36 AM, Jason Gunthorpe wrote:
On Sun, Feb 17, 2019 at 06:09:09PM +0100, Håkon Bugge wrote:
> During certain workloads, the default CM response timeout is too
> short, leading to excessive retries. Hence, make it configurable
> through sysctl. While at it, also make number of CM retries
> configurable.
>
> The defaults are not changed.
>
> Signed-off-by: Håkon Bugge
> drivers/infiniband/core/cma.c | 51 ++-
> 1 file changed, 44 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
> index c43512752b8a..ce99e1cd1029 100644
> +++ b/drivers/infiniband/core/cma.c
> @@ -43,6 +43,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> #include
> @@ -68,13 +69,46 @@ MODULE_AUTHOR("Sean Hefty");
> MODULE_DESCRIPTION("Generic RDMA CM Agent");
> MODULE_LICENSE("Dual BSD/GPL");
>
> -#define CMA_CM_RESPONSE_TIMEOUT 20
> #define CMA_QUERY_CLASSPORT_INFO_TIMEOUT 3000
> -#define CMA_MAX_CM_RETRIES 15
> #define CMA_CM_MRA_SETTING (IB_CM_MRA_FLAG_DELAY | 24)
> #define CMA_IBOE_PACKET_LIFETIME 18
> #define CMA_PREFERRED_ROCE_GID_TYPE IB_GID_TYPE_ROCE_UDP_ENCAP
>
> +#define CMA_DFLT_CM_RESPONSE_TIMEOUT 20
> +static int cma_cm_response_timeout = CMA_DFLT_CM_RESPONSE_TIMEOUT;
> +static int cma_cm_response_timeout_min = 8;
> +static int cma_cm_response_timeout_max = 31;
> +#undef CMA_DFLT_CM_RESPONSE_TIMEOUT
> +
> +#define CMA_DFLT_MAX_CM_RETRIES 15
> +static int cma_max_cm_retries = CMA_DFLT_MAX_CM_RETRIES;
> +static int cma_max_cm_retries_min = 1;
> +static int cma_max_cm_retries_max = 100;
> +#undef CMA_DFLT_MAX_CM_RETRIES
> +
> +static struct ctl_table_header *cma_ctl_table_hdr;
> +static struct ctl_table cma_ctl_table[] = {
> + {
> + .procname = "cma_cm_response_timeout",
> + .data = _cm_response_timeout,
> + .maxlen = sizeof(cma_cm_response_timeout),
> + .mode = 0644,
> + .proc_handler = proc_dointvec_minmax,
> + .extra1 = _cm_response_timeout_min,
> + .extra2 = _cm_response_timeout_max,
> + },
> + {
> + .procname = "cma_max_cm_retries",
> + .data = _max_cm_retries,
> + .maxlen = sizeof(cma_max_cm_retries),
> + .mode = 0644,
> + .proc_handler = proc_dointvec_minmax,
> + .extra1 = _max_cm_retries_min,
> + .extra2 = _max_cm_retries_max,
> + },
> + { }
> +};
Is sysctl the right approach here? Should it be rdma tool instead?
Jason
>>>
>>> There are other rdma sysctls currently: net.rdma_ucm.max_backlog and
>>> net.iw_cm.default_backlog. The core network stack seems to use sysctl
>>> and not ip tool to set basically globals.
>>>
>>> To use rdma tool, we'd have to have some concept of a "module" object, I
>>> guess. IE there's dev, link, and resource rdma tool objects currently.
>>> But these cma timeout settings are really not per dev, link, nor a
>>> resource. Maybe we have just a "core" object: rdma core set
>>> cma_max_cm_retries min 8 max 30.
>>
>> I don’t know, I think you make a fairly good argument for leaving it as a
>> sysctl. We have infrastructure in place for admins to set persistent sysctl
>> settings. The per device/link settings need something different because
>> link names and such can change. Since these are globals, I’d leave them
>> where they are.
>>
>
> I have patches from Parav which extend rdmatool to set global to whole
> stack parameters, something like "rdma system ...", so the option to
> set through rdmatool global parameters for modules e.g. "rdma system cma set
> ..."
> exists. But I'm not sure if it is right thing to do.
If decided to for the sysctl flavour, I must just say: ahem, ahem, I forgot to
put in a unregister_net_sysctl_table() in cma_cleanup(). Will send a v2.
Thxs, Håkon
>
>> --
>> Doug Ledford
>>GPG KeyID: B826A3330E572FDD
>>Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD
Re: [PATCH] RDMA/cma: Make CM response timeout and # CM retries configurable
On Fri, Feb 22, 2019 at 12:51:55PM -0500, Doug Ledford wrote:
>
>
> > On Feb 22, 2019, at 12:14 PM, Steve Wise
> > wrote:
> >
> >
> > On 2/22/2019 10:36 AM, Jason Gunthorpe wrote:
> >> On Sun, Feb 17, 2019 at 06:09:09PM +0100, Håkon Bugge wrote:
> >>> During certain workloads, the default CM response timeout is too
> >>> short, leading to excessive retries. Hence, make it configurable
> >>> through sysctl. While at it, also make number of CM retries
> >>> configurable.
> >>>
> >>> The defaults are not changed.
> >>>
> >>> Signed-off-by: Håkon Bugge
> >>> drivers/infiniband/core/cma.c | 51 ++-
> >>> 1 file changed, 44 insertions(+), 7 deletions(-)
> >>>
> >>> diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
> >>> index c43512752b8a..ce99e1cd1029 100644
> >>> +++ b/drivers/infiniband/core/cma.c
> >>> @@ -43,6 +43,7 @@
> >>> #include
> >>> #include
> >>> #include
> >>> +#include
> >>> #include
> >>>
> >>> #include
> >>> @@ -68,13 +69,46 @@ MODULE_AUTHOR("Sean Hefty");
> >>> MODULE_DESCRIPTION("Generic RDMA CM Agent");
> >>> MODULE_LICENSE("Dual BSD/GPL");
> >>>
> >>> -#define CMA_CM_RESPONSE_TIMEOUT 20
> >>> #define CMA_QUERY_CLASSPORT_INFO_TIMEOUT 3000
> >>> -#define CMA_MAX_CM_RETRIES 15
> >>> #define CMA_CM_MRA_SETTING (IB_CM_MRA_FLAG_DELAY | 24)
> >>> #define CMA_IBOE_PACKET_LIFETIME 18
> >>> #define CMA_PREFERRED_ROCE_GID_TYPE IB_GID_TYPE_ROCE_UDP_ENCAP
> >>>
> >>> +#define CMA_DFLT_CM_RESPONSE_TIMEOUT 20
> >>> +static int cma_cm_response_timeout = CMA_DFLT_CM_RESPONSE_TIMEOUT;
> >>> +static int cma_cm_response_timeout_min = 8;
> >>> +static int cma_cm_response_timeout_max = 31;
> >>> +#undef CMA_DFLT_CM_RESPONSE_TIMEOUT
> >>> +
> >>> +#define CMA_DFLT_MAX_CM_RETRIES 15
> >>> +static int cma_max_cm_retries = CMA_DFLT_MAX_CM_RETRIES;
> >>> +static int cma_max_cm_retries_min = 1;
> >>> +static int cma_max_cm_retries_max = 100;
> >>> +#undef CMA_DFLT_MAX_CM_RETRIES
> >>> +
> >>> +static struct ctl_table_header *cma_ctl_table_hdr;
> >>> +static struct ctl_table cma_ctl_table[] = {
> >>> + {
> >>> + .procname = "cma_cm_response_timeout",
> >>> + .data = _cm_response_timeout,
> >>> + .maxlen = sizeof(cma_cm_response_timeout),
> >>> + .mode = 0644,
> >>> + .proc_handler = proc_dointvec_minmax,
> >>> + .extra1 = _cm_response_timeout_min,
> >>> + .extra2 = _cm_response_timeout_max,
> >>> + },
> >>> + {
> >>> + .procname = "cma_max_cm_retries",
> >>> + .data = _max_cm_retries,
> >>> + .maxlen = sizeof(cma_max_cm_retries),
> >>> + .mode = 0644,
> >>> + .proc_handler = proc_dointvec_minmax,
> >>> + .extra1 = _max_cm_retries_min,
> >>> + .extra2 = _max_cm_retries_max,
> >>> + },
> >>> + { }
> >>> +};
> >> Is sysctl the right approach here? Should it be rdma tool instead?
> >>
> >> Jason
> >
> > There are other rdma sysctls currently: net.rdma_ucm.max_backlog and
> > net.iw_cm.default_backlog. The core network stack seems to use sysctl
> > and not ip tool to set basically globals.
> >
> > To use rdma tool, we'd have to have some concept of a "module" object, I
> > guess. IE there's dev, link, and resource rdma tool objects currently.
> > But these cma timeout settings are really not per dev, link, nor a
> > resource. Maybe we have just a "core" object: rdma core set
> > cma_max_cm_retries min 8 max 30.
>
> I don’t know, I think you make a fairly good argument for leaving it as a
> sysctl. We have infrastructure in place for admins to set persistent sysctl
> settings. The per device/link settings need something different because link
> names and such can change. Since these are globals, I’d leave them where
> they are.
>
I have patches from Parav which extend rdmatool to set global to whole
stack parameters, something like "rdma system ...", so the option to
set through rdmatool global parameters for modules e.g. "rdma system cma set
..."
exists. But I'm not sure if it is right thing to do.
> --
> Doug Ledford
> GPG KeyID: B826A3330E572FDD
> Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD
>
>
>
>
>
>
>
signature.asc
Description: PGP signature
Re: [PATCH] RDMA/cma: Make CM response timeout and # CM retries configurable
> On Feb 22, 2019, at 12:14 PM, Steve Wise wrote:
>
>
> On 2/22/2019 10:36 AM, Jason Gunthorpe wrote:
>> On Sun, Feb 17, 2019 at 06:09:09PM +0100, Håkon Bugge wrote:
>>> During certain workloads, the default CM response timeout is too
>>> short, leading to excessive retries. Hence, make it configurable
>>> through sysctl. While at it, also make number of CM retries
>>> configurable.
>>>
>>> The defaults are not changed.
>>>
>>> Signed-off-by: Håkon Bugge
>>> drivers/infiniband/core/cma.c | 51 ++-
>>> 1 file changed, 44 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
>>> index c43512752b8a..ce99e1cd1029 100644
>>> +++ b/drivers/infiniband/core/cma.c
>>> @@ -43,6 +43,7 @@
>>> #include
>>> #include
>>> #include
>>> +#include
>>> #include
>>>
>>> #include
>>> @@ -68,13 +69,46 @@ MODULE_AUTHOR("Sean Hefty");
>>> MODULE_DESCRIPTION("Generic RDMA CM Agent");
>>> MODULE_LICENSE("Dual BSD/GPL");
>>>
>>> -#define CMA_CM_RESPONSE_TIMEOUT 20
>>> #define CMA_QUERY_CLASSPORT_INFO_TIMEOUT 3000
>>> -#define CMA_MAX_CM_RETRIES 15
>>> #define CMA_CM_MRA_SETTING (IB_CM_MRA_FLAG_DELAY | 24)
>>> #define CMA_IBOE_PACKET_LIFETIME 18
>>> #define CMA_PREFERRED_ROCE_GID_TYPE IB_GID_TYPE_ROCE_UDP_ENCAP
>>>
>>> +#define CMA_DFLT_CM_RESPONSE_TIMEOUT 20
>>> +static int cma_cm_response_timeout = CMA_DFLT_CM_RESPONSE_TIMEOUT;
>>> +static int cma_cm_response_timeout_min = 8;
>>> +static int cma_cm_response_timeout_max = 31;
>>> +#undef CMA_DFLT_CM_RESPONSE_TIMEOUT
>>> +
>>> +#define CMA_DFLT_MAX_CM_RETRIES 15
>>> +static int cma_max_cm_retries = CMA_DFLT_MAX_CM_RETRIES;
>>> +static int cma_max_cm_retries_min = 1;
>>> +static int cma_max_cm_retries_max = 100;
>>> +#undef CMA_DFLT_MAX_CM_RETRIES
>>> +
>>> +static struct ctl_table_header *cma_ctl_table_hdr;
>>> +static struct ctl_table cma_ctl_table[] = {
>>> + {
>>> + .procname = "cma_cm_response_timeout",
>>> + .data = _cm_response_timeout,
>>> + .maxlen = sizeof(cma_cm_response_timeout),
>>> + .mode = 0644,
>>> + .proc_handler = proc_dointvec_minmax,
>>> + .extra1 = _cm_response_timeout_min,
>>> + .extra2 = _cm_response_timeout_max,
>>> + },
>>> + {
>>> + .procname = "cma_max_cm_retries",
>>> + .data = _max_cm_retries,
>>> + .maxlen = sizeof(cma_max_cm_retries),
>>> + .mode = 0644,
>>> + .proc_handler = proc_dointvec_minmax,
>>> + .extra1 = _max_cm_retries_min,
>>> + .extra2 = _max_cm_retries_max,
>>> + },
>>> + { }
>>> +};
>> Is sysctl the right approach here? Should it be rdma tool instead?
>>
>> Jason
>
> There are other rdma sysctls currently: net.rdma_ucm.max_backlog and
> net.iw_cm.default_backlog. The core network stack seems to use sysctl
> and not ip tool to set basically globals.
>
> To use rdma tool, we'd have to have some concept of a "module" object, I
> guess. IE there's dev, link, and resource rdma tool objects currently.
> But these cma timeout settings are really not per dev, link, nor a
> resource. Maybe we have just a "core" object: rdma core set
> cma_max_cm_retries min 8 max 30.
I don’t know, I think you make a fairly good argument for leaving it as a
sysctl. We have infrastructure in place for admins to set persistent sysctl
settings. The per device/link settings need something different because link
names and such can change. Since these are globals, I’d leave them where they
are.
--
Doug Ledford
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD
signature.asc
Description: Message signed with OpenPGP
Re: [PATCH] RDMA/cma: Make CM response timeout and # CM retries configurable
On 2/22/2019 10:36 AM, Jason Gunthorpe wrote:
> On Sun, Feb 17, 2019 at 06:09:09PM +0100, Håkon Bugge wrote:
>> During certain workloads, the default CM response timeout is too
>> short, leading to excessive retries. Hence, make it configurable
>> through sysctl. While at it, also make number of CM retries
>> configurable.
>>
>> The defaults are not changed.
>>
>> Signed-off-by: Håkon Bugge
>> drivers/infiniband/core/cma.c | 51 ++-
>> 1 file changed, 44 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
>> index c43512752b8a..ce99e1cd1029 100644
>> +++ b/drivers/infiniband/core/cma.c
>> @@ -43,6 +43,7 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>>
>> #include
>> @@ -68,13 +69,46 @@ MODULE_AUTHOR("Sean Hefty");
>> MODULE_DESCRIPTION("Generic RDMA CM Agent");
>> MODULE_LICENSE("Dual BSD/GPL");
>>
>> -#define CMA_CM_RESPONSE_TIMEOUT 20
>> #define CMA_QUERY_CLASSPORT_INFO_TIMEOUT 3000
>> -#define CMA_MAX_CM_RETRIES 15
>> #define CMA_CM_MRA_SETTING (IB_CM_MRA_FLAG_DELAY | 24)
>> #define CMA_IBOE_PACKET_LIFETIME 18
>> #define CMA_PREFERRED_ROCE_GID_TYPE IB_GID_TYPE_ROCE_UDP_ENCAP
>>
>> +#define CMA_DFLT_CM_RESPONSE_TIMEOUT 20
>> +static int cma_cm_response_timeout = CMA_DFLT_CM_RESPONSE_TIMEOUT;
>> +static int cma_cm_response_timeout_min = 8;
>> +static int cma_cm_response_timeout_max = 31;
>> +#undef CMA_DFLT_CM_RESPONSE_TIMEOUT
>> +
>> +#define CMA_DFLT_MAX_CM_RETRIES 15
>> +static int cma_max_cm_retries = CMA_DFLT_MAX_CM_RETRIES;
>> +static int cma_max_cm_retries_min = 1;
>> +static int cma_max_cm_retries_max = 100;
>> +#undef CMA_DFLT_MAX_CM_RETRIES
>> +
>> +static struct ctl_table_header *cma_ctl_table_hdr;
>> +static struct ctl_table cma_ctl_table[] = {
>> +{
>> +.procname = "cma_cm_response_timeout",
>> +.data = _cm_response_timeout,
>> +.maxlen = sizeof(cma_cm_response_timeout),
>> +.mode = 0644,
>> +.proc_handler = proc_dointvec_minmax,
>> +.extra1 = _cm_response_timeout_min,
>> +.extra2 = _cm_response_timeout_max,
>> +},
>> +{
>> +.procname = "cma_max_cm_retries",
>> +.data = _max_cm_retries,
>> +.maxlen = sizeof(cma_max_cm_retries),
>> +.mode = 0644,
>> +.proc_handler = proc_dointvec_minmax,
>> +.extra1 = _max_cm_retries_min,
>> +.extra2 = _max_cm_retries_max,
>> +},
>> +{ }
>> +};
> Is sysctl the right approach here? Should it be rdma tool instead?
>
> Jason
There are other rdma sysctls currently: net.rdma_ucm.max_backlog and
net.iw_cm.default_backlog. The core network stack seems to use sysctl
and not ip tool to set basically globals.
To use rdma tool, we'd have to have some concept of a "module" object, I
guess. IE there's dev, link, and resource rdma tool objects currently.
But these cma timeout settings are really not per dev, link, nor a
resource. Maybe we have just a "core" object: rdma core set
cma_max_cm_retries min 8 max 30.
Re: [PATCH] RDMA/cma: Make CM response timeout and # CM retries configurable
On Sun, Feb 17, 2019 at 06:09:09PM +0100, Håkon Bugge wrote:
> During certain workloads, the default CM response timeout is too
> short, leading to excessive retries. Hence, make it configurable
> through sysctl. While at it, also make number of CM retries
> configurable.
>
> The defaults are not changed.
>
> Signed-off-by: Håkon Bugge
> drivers/infiniband/core/cma.c | 51 ++-
> 1 file changed, 44 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
> index c43512752b8a..ce99e1cd1029 100644
> +++ b/drivers/infiniband/core/cma.c
> @@ -43,6 +43,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> #include
> @@ -68,13 +69,46 @@ MODULE_AUTHOR("Sean Hefty");
> MODULE_DESCRIPTION("Generic RDMA CM Agent");
> MODULE_LICENSE("Dual BSD/GPL");
>
> -#define CMA_CM_RESPONSE_TIMEOUT 20
> #define CMA_QUERY_CLASSPORT_INFO_TIMEOUT 3000
> -#define CMA_MAX_CM_RETRIES 15
> #define CMA_CM_MRA_SETTING (IB_CM_MRA_FLAG_DELAY | 24)
> #define CMA_IBOE_PACKET_LIFETIME 18
> #define CMA_PREFERRED_ROCE_GID_TYPE IB_GID_TYPE_ROCE_UDP_ENCAP
>
> +#define CMA_DFLT_CM_RESPONSE_TIMEOUT 20
> +static int cma_cm_response_timeout = CMA_DFLT_CM_RESPONSE_TIMEOUT;
> +static int cma_cm_response_timeout_min = 8;
> +static int cma_cm_response_timeout_max = 31;
> +#undef CMA_DFLT_CM_RESPONSE_TIMEOUT
> +
> +#define CMA_DFLT_MAX_CM_RETRIES 15
> +static int cma_max_cm_retries = CMA_DFLT_MAX_CM_RETRIES;
> +static int cma_max_cm_retries_min = 1;
> +static int cma_max_cm_retries_max = 100;
> +#undef CMA_DFLT_MAX_CM_RETRIES
> +
> +static struct ctl_table_header *cma_ctl_table_hdr;
> +static struct ctl_table cma_ctl_table[] = {
> + {
> + .procname = "cma_cm_response_timeout",
> + .data = _cm_response_timeout,
> + .maxlen = sizeof(cma_cm_response_timeout),
> + .mode = 0644,
> + .proc_handler = proc_dointvec_minmax,
> + .extra1 = _cm_response_timeout_min,
> + .extra2 = _cm_response_timeout_max,
> + },
> + {
> + .procname = "cma_max_cm_retries",
> + .data = _max_cm_retries,
> + .maxlen = sizeof(cma_max_cm_retries),
> + .mode = 0644,
> + .proc_handler = proc_dointvec_minmax,
> + .extra1 = _max_cm_retries_min,
> + .extra2 = _max_cm_retries_max,
> + },
> + { }
> +};
Is sysctl the right approach here? Should it be rdma tool instead?
Jason
[PATCH] RDMA/cma: Make CM response timeout and # CM retries configurable
During certain workloads, the default CM response timeout is too
short, leading to excessive retries. Hence, make it configurable
through sysctl. While at it, also make number of CM retries
configurable.
The defaults are not changed.
Signed-off-by: Håkon Bugge
---
drivers/infiniband/core/cma.c | 51 ++-
1 file changed, 44 insertions(+), 7 deletions(-)
diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index c43512752b8a..ce99e1cd1029 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -43,6 +43,7 @@
#include
#include
#include
+#include
#include
#include
@@ -68,13 +69,46 @@ MODULE_AUTHOR("Sean Hefty");
MODULE_DESCRIPTION("Generic RDMA CM Agent");
MODULE_LICENSE("Dual BSD/GPL");
-#define CMA_CM_RESPONSE_TIMEOUT 20
#define CMA_QUERY_CLASSPORT_INFO_TIMEOUT 3000
-#define CMA_MAX_CM_RETRIES 15
#define CMA_CM_MRA_SETTING (IB_CM_MRA_FLAG_DELAY | 24)
#define CMA_IBOE_PACKET_LIFETIME 18
#define CMA_PREFERRED_ROCE_GID_TYPE IB_GID_TYPE_ROCE_UDP_ENCAP
+#define CMA_DFLT_CM_RESPONSE_TIMEOUT 20
+static int cma_cm_response_timeout = CMA_DFLT_CM_RESPONSE_TIMEOUT;
+static int cma_cm_response_timeout_min = 8;
+static int cma_cm_response_timeout_max = 31;
+#undef CMA_DFLT_CM_RESPONSE_TIMEOUT
+
+#define CMA_DFLT_MAX_CM_RETRIES 15
+static int cma_max_cm_retries = CMA_DFLT_MAX_CM_RETRIES;
+static int cma_max_cm_retries_min = 1;
+static int cma_max_cm_retries_max = 100;
+#undef CMA_DFLT_MAX_CM_RETRIES
+
+static struct ctl_table_header *cma_ctl_table_hdr;
+static struct ctl_table cma_ctl_table[] = {
+ {
+ .procname = "cma_cm_response_timeout",
+ .data = _cm_response_timeout,
+ .maxlen = sizeof(cma_cm_response_timeout),
+ .mode = 0644,
+ .proc_handler = proc_dointvec_minmax,
+ .extra1 = _cm_response_timeout_min,
+ .extra2 = _cm_response_timeout_max,
+ },
+ {
+ .procname = "cma_max_cm_retries",
+ .data = _max_cm_retries,
+ .maxlen = sizeof(cma_max_cm_retries),
+ .mode = 0644,
+ .proc_handler = proc_dointvec_minmax,
+ .extra1 = _max_cm_retries_min,
+ .extra2 = _max_cm_retries_max,
+ },
+ { }
+};
+
static const char * const cma_events[] = {
[RDMA_CM_EVENT_ADDR_RESOLVED]= "address resolved",
[RDMA_CM_EVENT_ADDR_ERROR] = "address error",
@@ -3745,8 +3779,8 @@ static int cma_resolve_ib_udp(struct rdma_id_private
*id_priv,
req.path = id_priv->id.route.path_rec;
req.sgid_attr = id_priv->id.route.addr.dev_addr.sgid_attr;
req.service_id = rdma_get_service_id(_priv->id,
cma_dst_addr(id_priv));
- req.timeout_ms = 1 << (CMA_CM_RESPONSE_TIMEOUT - 8);
- req.max_cm_retries = CMA_MAX_CM_RETRIES;
+ req.timeout_ms = 1 << (cma_cm_response_timeout - 8);
+ req.max_cm_retries = cma_max_cm_retries;
ret = ib_send_cm_sidr_req(id_priv->cm_id.ib, );
if (ret) {
@@ -3816,9 +3850,9 @@ static int cma_connect_ib(struct rdma_id_private *id_priv,
req.flow_control = conn_param->flow_control;
req.retry_count = min_t(u8, 7, conn_param->retry_count);
req.rnr_retry_count = min_t(u8, 7, conn_param->rnr_retry_count);
- req.remote_cm_response_timeout = CMA_CM_RESPONSE_TIMEOUT;
- req.local_cm_response_timeout = CMA_CM_RESPONSE_TIMEOUT;
- req.max_cm_retries = CMA_MAX_CM_RETRIES;
+ req.remote_cm_response_timeout = cma_cm_response_timeout;
+ req.local_cm_response_timeout = cma_cm_response_timeout;
+ req.max_cm_retries = cma_max_cm_retries;
req.srq = id_priv->srq ? 1 : 0;
ret = ib_send_cm_req(id_priv->cm_id.ib, );
@@ -4701,6 +4735,9 @@ static int __init cma_init(void)
goto err;
cma_configfs_init();
+ cma_ctl_table_hdr = register_net_sysctl(_net, "net/rdma_cm",
cma_ctl_table);
+ if (!cma_ctl_table_hdr)
+ pr_warn("rdma_cm: couldn't register sysctl path, using default
values\n");
return 0;
--
2.20.1
