Re: [PATCH rdma-next V1 2/4] IB/core: Support rate limit for packet pacing

[Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]

[-- Attachment #1.1: Type: text/plain, Size: 2188 bytes --]

On 12/4/2016 6:53 AM, Liran Liss wrote:
>> From: linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org [mailto:linux-rdma-
>> owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org] On Behalf Of Leon Romanovsky
> 
>>
>> On Thu, Dec 01, 2016 at 07:40:44PM +0000, Hefty, Sean wrote:
>>>>  enum ib_qp_state {
>>>> @@ -1151,6 +1152,7 @@ struct ib_qp_attr {
>>>>  	u8			rnr_retry;
>>>>  	u8			alt_port_num;
>>>>  	u8			alt_timeout;
>>>> +	u32			rate_limit;
>>>>  };
>>>
>>> And I still disagree with this approach, as there is already an existing field in the
>> API that limits rate.
>>
>> Hi Sean,
>>
>> We would like to elaborate more on the subject, and we need your help here.
>> Our cover letter [1] has description why the existing field is not enough. Can you
>> write a little bit more why you didn't like the proposed approach to an existing
>> and real problem?
>>
> 
> Clearly, we need a new field that is greater than 8 bits that uses different encoding (actual rate rather than a predefined enumeration of values).
> 
> However, this is not only a new field - this rate is also conceptually different than AH static rate:
> 1) It is not related to the fabric path.
> 2) It is determined by the application rather than the fabric.
> 3) It needs to apply to QPs that don't use AH (raw Ethernet QPs)
> 
> Of course, we might require raw Ethernet QPs to use an AH just for the sake of rate-limiting.
> But 1) + 2) indicate that this should be a concept orthogonal to network paths. In fact, it could apply to RC and UD QPs as well.
> In addition, if we implement full connection establishment in the kernel (i.e., the static_rate field is filled in the kernel), we would still like to allow applications to further restrict their rate using a different component mask.
> --Liran
> 

I applied this series.  I agree with Liran that the need for something
other than an enum to denote rates is needed.  We will have to work out
exactly how to present this to user space (keep both, replace
static_rate and bumb the verbs API number, other options?)

-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
    GPG Key ID: 0E572FDD


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]