Re: [PATCH V2] libibumad: update umad_recv man page.

Hal Rosenstock
2012-07-06T19:32:51Z

On 7/6/2012 3:24 PM, Ira Weiny wrote:
> On Fri, 06 Jul 2012 14:34:24 -0400
> Hal Rosenstock <hal-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org> wrote:
>
>> On 7/3/2012 12:55 PM, Ira Weiny wrote:
>>>
>>> Document the umad_recv length parameter better.
>>>
>>> Changes since V1:
>>> add comments from Hal
>>>
>>> Signed-off-by: Ira Weiny <weiny2-i2BcT+NCU+M@public.gmane.org>
>>> ---
>>> man/umad_recv.3 | 18 +++++++++++++++++-
>>> 1 files changed, 17 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/man/umad_recv.3 b/man/umad_recv.3
>>> index e1b2985..310d3d2 100644
>>> --- a/man/umad_recv.3
>>> +++ b/man/umad_recv.3
>>> @@ -27,10 +27,26 @@ A negative
>>> makes the function block until a packet is received. A
>>> .I timeout_ms\fR
>>> parameter of zero indicates a non blocking read.
>>> +
>>> +.B Note
>>> +.I length
>>> +is a pointer to the length of the
>>> +.B data
>>> +portion of the umad buffer. This means that
>>> +.I umad
>>> +should point to a buffer at least umad_size() +
>>> +.I *length
>>> +bytes long.
>>> +
>>> +.B Note also
>>> +that
>>> +.I *length\fR
>>> +must be >= 256 bytes.
>>
>> This seems somewhat redundant to me as just above it says "should point
>> to a bugger that's at least umad_size() + *length bytes long.
>>
>> Also, if this remains, should "must be" be "should be" ?
>
> I _guess_ it _could_ be less than 256.

My only point here is that if it's less than 256 + header, then the
error is returned (and nothing useful can be done). To me, that makes it
a should rather than a must but just make the two lines consistent.
Roland does have this as a must in user_mad.txt kernel doc:
"The buffer passed to read() must be at least one struct ib_user_mad +
256 bytes."

-- Hal

> However the specification states:
>
> "C13-3: The data payload (as used in Chapter 9: Transport Layer on page 233) for all MADs shall be exactly 256 bytes."
>
> Furthermore, the kernel checks to ensure that *length is > the first (or only) packet in the MAD transaction. While some Class/Attributes may allow for less "valid" data I'm not sure the kernel distinguishes that. Therefore, I think it is safer to just specify it. Don't you think?

> Ira
>
>>
>>> +
>>> .SH "RETURN VALUE"
>>> .B umad_recv()
>>> returns non negative receiving agentid on success, and a negative value on error as follows:
>>> - -EINVAL invalid port handle or agentid
>>> + -EINVAL invalid port handle or agentid or *length is less than the minimum
>>
>> Rather than just minimum, minimum support length might be clearer.
>>
>> -- Hal
>>
>>> -EIO receive operation failed
>>> -EWOULDBLOCK non blocking read can't be fulfilled
>>> .SH "SEE ALSO"
>>