From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hal Rosenstock Subject: Re: [PATCH] libibumad: update umad_recv man page. Date: Fri, 29 Jun 2012 18:22:49 -0400 Message-ID: <4FEE2AB9.6060204@dev.mellanox.co.il> References: <20120628181343.7f931c33.weiny2@llnl.gov> <4FEDB762.8090603@dev.mellanox.co.il> <20120629092343.6119ad47.weiny2@llnl.gov> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20120629092343.6119ad47.weiny2-i2BcT+NCU+M@public.gmane.org> Sender: linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Ira Weiny Cc: Alex Netes , "linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" List-Id: linux-rdma@vger.kernel.org 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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html