| Message ID | 20120703095522.1e89cd25.weiny2@llnl.gov (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Alex Netes |
| Headers | show |
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@llnl.gov> > --- > 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 06 Jul 2012 14:34:24 -0400 Hal Rosenstock <hal@dev.mellanox.co.il> 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@llnl.gov> > > --- > > 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" >
On 7/6/2012 3:24 PM, Ira Weiny wrote: > On Fri, 06 Jul 2012 14:34:24 -0400 > Hal Rosenstock <hal@dev.mellanox.co.il> 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@llnl.gov> >>> --- >>> 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 06 Jul 2012 15:32:51 -0400 Hal Rosenstock <hal@dev.mellanox.co.il> wrote: > On 7/6/2012 3:24 PM, Ira Weiny wrote: > > On Fri, 06 Jul 2012 14:34:24 -0400 > > Hal Rosenstock <hal@dev.mellanox.co.il> 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@llnl.gov> > >>> --- > >>> 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@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
On 7/6/2012 3:38 PM, Ira Weiny wrote: > On Fri, 06 Jul 2012 15:32:51 -0400 > Hal Rosenstock <hal@dev.mellanox.co.il> wrote: > >> On 7/6/2012 3:24 PM, Ira Weiny wrote: >>> On Fri, 06 Jul 2012 14:34:24 -0400 >>> Hal Rosenstock <hal@dev.mellanox.co.il> 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@llnl.gov> >>>>> --- >>>>> 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@vger.kernel.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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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"
Document the umad_recv length parameter better. Changes since V1: add comments from Hal Signed-off-by: Ira Weiny <weiny2@llnl.gov> --- man/umad_recv.3 | 18 +++++++++++++++++- 1 files changed, 17 insertions(+), 1 deletions(-)