Re: [PATCH 1/1] IB/isert: align target max I/O size to initiator size

From: Max Gurtovoy <mgurtovoy@nvidia.com>
To: Kamal Heib <kheib@redhat.com>, Sagi Grimberg <sagi@grimberg.me>
Cc: <israelr@nvidia.com>, <alaa@nvidia.com>,
	<linux-rdma@vger.kernel.org>, <jgg@nvidia.com>
Subject: Re: [PATCH 1/1] IB/isert: align target max I/O size to initiator size
Date: Sun, 20 Jun 2021 14:29:14 +0300	[thread overview]
Message-ID: <90153fcc-8ed5-6ad5-0539-bcf97d8e0ce3@nvidia.com> (raw)
In-Reply-To: <82ef613c-2697-261e-317e-b40d09cf0764@redhat.com>

On 6/20/2021 11:11 AM, Kamal Heib wrote:
>
> On 6/9/21 11:45 AM, Max Gurtovoy wrote:
>> On 6/9/2021 2:04 AM, Sagi Grimberg wrote:
>>>>> On 5/25/21 7:22 PM, Max Gurtovoy wrote:
>>>>>> On 5/25/2021 6:54 PM, Sagi Grimberg wrote:
>>>>>>>> Since the Linux iser initiator default max I/O size set to 512KB and
>>>>>>>> since there is no handshake procedure for this size in iser
>>>>>>>> protocol,
>>>>>>>> set the default max IO size of the target to 512KB as well.
>>>>>>>>
>>>>>>>> For changing the default values, there is a module parameter for
>>>>>>>> both
>>>>>>>> drivers.
>>>>>>> Is this solving a bug?
>>>>>> No. Only OOB for some old connect-IB devices.
>>>>>>
>>>>>> I think it's reasonable to align initiator and target defaults anyway.
>>>>>>
>>>>>>
>>>>> Actually, this patch is solving a bug when trying iser over
>>>>> Connect-IB, We see
>>>>> the following failure when trying to do discovery:
>>>> You can work around this using the ib_isert sg_tablesize module param
>>>> and set it to 128.
>>>>
>>>> So it's more OOB behavior than a bug.
>>>>
>>>> Anyway, This is good practice to be able to establish connections
>>>> also for old devices without WAs and we also aligning to the sg_table
>>>> size in the initiator side.
>>>>
>>>> Jason/Sagi,
>>>>
>>>> can you comment on this patch for 5.14 ?
>>> Actually, if this is the case, why not have a fallback when creating the
>>> QP? Seems more reasonable to have the exception for the old devices
>>> rather than having those mandate the common denominator no?
>> We first wanted to support 16MiB for isert but then we get a report from
>> Chelsio that it will dramatically reduce the total amount of connections
>> the can support.
>>
>> So we created a module param and reduced the default to 1MiB. Now we
>> have similar issue with Connect-IB so reducing it to 512KiB (same as the
>> default for Linux iser initiator) seems reasonable.
>>
>> Users that would like larger sg_table will use the module param.
>>
>> I would avoid doing fallbacks for that and maintain a code that might be
>> dead in a year or two.
>>
>>
> Well, from the distro's point of view this code is not going to be dead any time
> soon..., And the current user experience is very bad, Could you guys please
> decide on a way to fix this issue?

As mention above, I prefer the simple solution for this issue.

I guess the most of iSER users are using pretty old HW so defaults 
should be accordingly.

For NVMe/RDMA this is a different story and we can use higher defaults

Adding fallbacks will complicate the code without a real justification 
for doing it.

>
> Thanks,
> Kamal
>