[PATCH V2] libibumad: update umad_recv man page.

[PATCH V2] libibumad: update umad_recv man page.

[Ira Weiny]

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.
+
 .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
  -EIO         receive operation failed
  -EWOULDBLOCK non blocking read can't be fulfilled
 .SH "SEE ALSO"
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Hal Rosenstock]

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" ?

> +
>  .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"

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Ira Weiny]

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.  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"
> 


-- 
Ira Weiny
Member of Technical Staff
Lawrence Livermore National Lab
925-423-8008
weiny2-i2BcT+NCU+M@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Hal Rosenstock]

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"
>>
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Ira Weiny]

On Fri, 06 Jul 2012 15:32:51 -0400
Hal Rosenstock <hal-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org> wrote:

> 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() +
       ^^^^^^
       must

Oh, You mean must here?

Ira


> >>> +.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"
> >>
> > 
> > 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


-- 
Ira Weiny
Member of Technical Staff
Lawrence Livermore National Lab
925-423-8008
weiny2-i2BcT+NCU+M@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Hal Rosenstock]

On 7/6/2012 3:38 PM, Ira Weiny wrote:
> On Fri, 06 Jul 2012 15:32:51 -0400
> Hal Rosenstock <hal-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org> wrote:
> 
>> 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() +
>        ^^^^^^
>        must
> 
> Oh, You mean must here?

Yes, if that's the direction you choose (must rather than should).

-- Hal

> Ira
> 
> 
>>>>> +.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"
>>>>
>>>
>>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html