Re: [PATCH] libibumad: update umad_recv man page.
On 6/29/2012 12:23 PM, Ira Weiny wrote: > On Fri, 29 Jun 2012 10:10:42 -0400 > Hal Rosenstock wrote: > >> On 6/28/2012 9:13 PM, Ira Weiny wrote: >>> Document the umad and length parameters better. >>> >>> Signed-off-by: Ira Weiny >>> --- >>> 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..3c93c4a 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 the length of the >> >> is a pointer to the length of the > > good point. > >> >>> +.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. >> >> I think that this should say "should" rather than "must". > > The RHEL 6.2 kernel (and Rolands master) will return -EINVAL if the length is > not large enough to handle the first packet or RMPP segment. > > /* We need enough room to copy the first (or only) MAD segment. */ > recv_buf = &packet->recv_wc->recv_buf; > if ((packet->length <= sizeof (*recv_buf->mad) && >count < hdr_size(file) + packet->length) || > (packet->length > sizeof (*recv_buf->mad) && >count < hdr_size(file) + sizeof (*recv_buf->mad))) > return -EINVAL; > > The -EINVAL from the kernel is not processed by the library and simply fails > the call rather than returning the length required. Right; if the user buffer is not at least struct ib_user_mad + 256 bytes, it fails with -EINVAL. > Would you like to change the above to return -ENOSPC? Do we need to discern this case from the others ? Is this worth changing ? >> >>> + >>> .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 too short >> >> Shouldn't this just be length (pointer) NULL/not supplied (and not >> length too short) ? > > I was simply trying to document the case above. I see. There are two length too short conditions. One is this where the user supplied buffer less than this minimum size (for one MAD + header) and the other is for large response handling. Better than "length too short", maybe "length is less than minimum allowed buffer size". -- Hal >> >> Length handling is already described in the man page: >> "The packet is copied to the umad buffer if there is sufficient room >> and the received length is indicated. If the buffer is not large >> enough, the size of the umad buffer needed is returned in length." > > Yes, I thought that too but looking at the kernel I figured there was a > requirement for the length to be at least be a single packet long and the > length being returned was a mechanism to return information about an RMPP > session packet which is longer than a single MAD. > > If this is not the intended behaviour then the kernel is broken and should be > fixed. > > Ira > >> >> -- 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] libibumad: update umad_recv man page.
On Fri, 29 Jun 2012 10:10:42 -0400 Hal Rosenstock wrote: > On 6/28/2012 9:13 PM, Ira Weiny wrote: > > Document the umad and length parameters better. > > > > Signed-off-by: Ira Weiny > > --- > > 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..3c93c4a 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 the length of the > > is a pointer to the length of the good point. > > > +.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. > > I think that this should say "should" rather than "must". The RHEL 6.2 kernel (and Rolands master) will return -EINVAL if the length is not large enough to handle the first packet or RMPP segment. /* We need enough room to copy the first (or only) MAD segment. */ recv_buf = &packet->recv_wc->recv_buf; if ((packet->length <= sizeof (*recv_buf->mad) && count < hdr_size(file) + packet->length) || (packet->length > sizeof (*recv_buf->mad) && count < hdr_size(file) + sizeof (*recv_buf->mad))) return -EINVAL; The -EINVAL from the kernel is not processed by the library and simply fails the call rather than returning the length required. Would you like to change the above to return -ENOSPC? > > > + > > .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 too short > > Shouldn't this just be length (pointer) NULL/not supplied (and not > length too short) ? I was simply trying to document the case above. > > Length handling is already described in the man page: > "The packet is copied to the umad buffer if there is sufficient room > and the received length is indicated. If the buffer is not large > enough, the size of the umad buffer needed is returned in length." Yes, I thought that too but looking at the kernel I figured there was a requirement for the length to be at least be a single packet long and the length being returned was a mechanism to return information about an RMPP session packet which is longer than a single MAD. If this is not the intended behaviour then the kernel is broken and should be fixed. Ira > > -- 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 wei...@llnl.gov -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] libibumad: update umad_recv man page.
On 6/28/2012 9:13 PM, Ira Weiny wrote: > Document the umad and length parameters better. > > Signed-off-by: Ira Weiny > --- > 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..3c93c4a 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 the length of the 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. I think that this should say "should" rather than "must". > + > .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 too short Shouldn't this just be length (pointer) NULL/not supplied (and not length too short) ? Length handling is already described in the man page: "The packet is copied to the umad buffer if there is sufficient room and the received length is indicated. If the buffer is not large enough, the size of the umad buffer needed is returned in length." -- 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] libibumad: update umad_recv man page.
Document the umad and length parameters better. Signed-off-by: Ira Weiny --- 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..3c93c4a 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 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 too short -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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
