Re: [openib-general] [PATCH] user_mad: Add receive side RMPP support
On Thu, 2005-06-30 at 17:40, Hal Rosenstock wrote: Given that, I think that it makes more sense to add a length field to the ib_user_mad_hdr. That's my conclusion too. ABI change :-( Is there a need to any backward compatibility ? Specifically should the current ABI version (4) be supported as well as the one this will be bumped to (5) or just the new one ? -- Hal ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH] user_mad: Add receive side RMPP support
Hal Rosenstock wrote: - if (count sizeof (struct ib_user_mad) + 256) /* until RMPP supported */ + if (count sizeof (struct ib_user_mad) + 256) return -EINVAL; Does it make more sense to use sizeof(struct ib_mad) rather than 256? - Sean ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH] user_mad: Add receive side RMPP support
On Thu, 2005-06-30 at 12:34, Sean Hefty wrote: Hal Rosenstock wrote: - if (count sizeof (struct ib_user_mad) + 256) /* until RMPP supported */ + if (count sizeof (struct ib_user_mad) + 256) return -EINVAL; Does it make more sense to use sizeof(struct ib_mad) rather than 256? But of course. I'll update and reissue the patch. Thanks. -- Hal ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH] user_mad: Add receive side RMPP support
Looking at the code, I have to say that I don't like the strategy of returning the actual length of the MAD but not copying anything if the MAD is too big. It seems error prone to return a length bigger than the user passed in to read(), and it doesn't feel right either. I understand and agree with the sentiment of not wanting to add another ioctl() to get the length. Instead, how about returning a ib_user_mad_hdr with a status of ENOSPC and putting the actual length somewhere. I'm not sure if it's better to change the ABI and add a length field to ib_user_mad_hdr, or if we want to return the first 36 bytes of an RMPP MAD so the user can figure out the correct length. Also, since we check if the MAD at the head of the receive queue won't fit in the user's buffer when we're about to pop it off the queue, instead of dequeuing and then requeuing it, we can just leave it at the head of the queue. (I'm not sure if this buys us anything but it seems there might be a race MADs get out of order) Sorry for not bringing this up when you asked about how to handle too-big MADs a few days ago -- I didn't understand your question really. - R. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH] user_mad: Add receive side RMPP support
Roland Dreier wrote: I understand and agree with the sentiment of not wanting to add another ioctl() to get the length. Instead, how about returning a ib_user_mad_hdr with a status of ENOSPC and putting the actual length somewhere. I'm not sure if it's better to change the ABI and add a length field to ib_user_mad_hdr, or if we want to return the first 36 bytes of an RMPP MAD so the user can figure out the correct length. Unless the MAD layer modifies the received MAD, the length may not be set in the header. Setting this doesn't seem like a big deal. If it is set, then I'm guessing that we'd want to set the PayloadLength to the value indicated by the spec, but that's not the easiest value to use in order to determine the size of the read. Given that, I think that it makes more sense to add a length field to the ib_user_mad_hdr. - Sean ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH] user_mad: Add receive side RMPP support
On Thu, 2005-06-30 at 14:45, Roland Dreier wrote: Looking at the code, I have to say that I don't like the strategy of returning the actual length of the MAD but not copying anything if the MAD is too big. It seems error prone to return a length bigger than the user passed in to read(), and it doesn't feel right either. I understand and agree with the sentiment of not wanting to add another ioctl() to get the length. Instead, how about returning a ib_user_mad_hdr with a status of ENOSPC and putting the actual length somewhere. I'm not sure if it's better to change the ABI and add a length field to ib_user_mad_hdr, or if we want to return the first 36 bytes of an RMPP MAD so the user can figure out the correct length. Also, since we check if the MAD at the head of the receive queue won't fit in the user's buffer when we're about to pop it off the queue, instead of dequeuing and then requeuing it, we can just leave it at the head of the queue. (I'm not sure if this buys us anything but it seems there might be a race MADs get out of order) Sorry for not bringing this up when you asked about how to handle too-big MADs a few days ago -- I didn't understand your question really. You understand it now and answered it well :-) I'm not wedded to the read approach and you have offered 2 alternatives. Unfortunately, I don't think the length in the first segment is reliable (it works for OpenIB but may not for other RMPP implementations) so I would rather not rely on that. So I guess this means an ABI change :-( Do you concur ? If so, I will work on that approach. -- Hal ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
RE: [openib-general] [PATCH] user_mad: Add receive side RMPP support
From: Sean Hefty [mailto:[EMAIL PROTECTED] Sent: Thursday, June 30, 2005 2:32 PM Roland Dreier wrote: I understand and agree with the sentiment of not wanting to add another ioctl() to get the length. Instead, how about returning a ib_user_mad_hdr with a status of ENOSPC and putting the actual length somewhere. I'm not sure if it's better to change the ABI and add a length field to ib_user_mad_hdr, or if we want to return the first 36 bytes of an RMPP MAD so the user can figure out the correct length. Unless the MAD layer modifies the received MAD, the length may not be set in the header. Setting this doesn't seem like a big deal. If it is set, then I'm guessing that we'd want to set the PayloadLength to the value indicated by the spec, but that's not the easiest value to use in order to determine the size of the read. Given that, I think that it makes more sense to add a length field to the ib_user_mad_hdr. Why not just expect the user to read a MAD until the read returns zero? I'm thinking something like this: read( offset = 0, len = 256 ) read( offset 256, len = value determined by first read? ) So a read at offset 0 would block waiting for the next MAD, but a read with a non-zero offset would return EOF if the full MAD was read. Thoughts? - Fab ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH] user_mad: Add receive side RMPP support
On Thu, 2005-06-30 at 17:32, Sean Hefty wrote: Roland Dreier wrote: I understand and agree with the sentiment of not wanting to add another ioctl() to get the length. Instead, how about returning a ib_user_mad_hdr with a status of ENOSPC and putting the actual length somewhere. I'm not sure if it's better to change the ABI and add a length field to ib_user_mad_hdr, or if we want to return the first 36 bytes of an RMPP MAD so the user can figure out the correct length. Unless the MAD layer modifies the received MAD, the length may not be set in the header. Setting this doesn't seem like a big deal. If it is set, then I'm guessing that we'd want to set the PayloadLength to the value indicated by the spec, but that's not the easiest value to use in order to determine the size of the read. Given that, I think that it makes more sense to add a length field to the ib_user_mad_hdr. That's my conclusion too. ABI change :-( -- Hal ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH] user_mad: Add receive side RMPP support
Fab Tillier wrote: Why not just expect the user to read a MAD until the read returns zero? I'm thinking something like this: read( offset = 0, len = 256 ) read( offset 256, len = value determined by first read? ) So a read at offset 0 would block waiting for the next MAD, but a read with a non-zero offset would return EOF if the full MAD was read. Thoughts? The user would need to know how large of a buffer to allocate for the read. If the user needs to allocate two buffers, then they either need to be able to process data the spans multiple buffers, or a second data copy is required. There's also an issue if the user is using multiple threads... - Sean ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
RE: [openib-general] [PATCH] user_mad: Add receive side RMPP support
From: Sean Hefty [mailto:[EMAIL PROTECTED] Sent: Thursday, June 30, 2005 2:46 PM Fab Tillier wrote: Why not just expect the user to read a MAD until the read returns zero? I'm thinking something like this: read( offset = 0, len = 256 ) read( offset 256, len = value determined by first read? ) So a read at offset 0 would block waiting for the next MAD, but a read with a non-zero offset would return EOF if the full MAD was read. Thoughts? The user would need to know how large of a buffer to allocate for the read. If the user needs to allocate two buffers, then they either need to be able to process data the spans multiple buffers, or a second data copy is required. There's also an issue if the user is using multiple threads... Ok, so my idea sucked. What about not coalescing in the kernel, and just handing up MADs to be coalesced in user-mode? It would require a buffer allocation in UM, as well as copies, but those currently happen in the kernel and could be eliminated, no? - Fab ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH] user_mad: Add receive side RMPP support
Fab Tillier wrote: The user would need to know how large of a buffer to allocate for the read. If the user needs to allocate two buffers, then they either need to be able to process data the spans multiple buffers, or a second data copy is required. There's also an issue if the user is using multiple threads... Ok, so my idea sucked. What about not coalescing in the kernel, and just handing up MADs to be coalesced in user-mode? It would require a buffer allocation in UM, as well as copies, but those currently happen in the kernel and could be eliminated, no? Nah... your idea was fine as something to throw out. :) Currently, only a single data copy is performed to transfer received RMPP MADs from the kernel to a usermode app. The coalescing occurs as the data is copied from a chain of MADs in the kernel to the app's buffer. - Sean ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
