On Tue, Oct 13, 2015 at 01:46:54PM -0400, Mike Marshall wrote:
> > *shrug*
>
> Al, I guess what I wrote sounded pi**y, but I promise my attitude
> is 180 degrees away from that... I'm just trying to focus on what is
> most important... you've identified numerous issues, all of which
> need to be addressed, but the state of the API used by userspace
> is a show stopper: it needs documented, and then probably improved...
>
> When we have something coherent started, we'll send it your way
> so you can help make sure we're on the right track...
No problem... BTW, one note regarding the iov_iter - unlike an array of
iovec, things like "copy <that many> bytes" are easy; you don't need the
thing to start on the iovec boundary or anything like that. Simple
copy_from_iter(dest, size, iter) will take care of everything.
Something like
struct {
__u32 version;
__u32 magic;
__u64 tag;
} head;
total = iov_iter_count(iter);
if (total < 24)
short packet
n = copy_from_iter(&head, 16, iter);
if (n != 16)
failed copy
<check head.magic, find the matching upcall by head.tag>
if (not found)
stray response
/* get the beginning of response proper */
memset(op->downcall, 0, 16);
n = copy_from_iter(&op->downcall, 16, iter);
if (n < 16) {
/* it might be really short... */
if (iov_iter_count())
failed copy
/* yes, and that's too short to have a trailer */
/* just zero-pad and that's it */
memset((char *)&op->downcall + n, 0,
sizeof(op->downcall) - n);
goto done;
}
/* will there be a trailer? */
trailer_count = 0;
if (op->downcall.status == 0 && op->downcall.trailer_count) {
trailer_count = op->downcall.trailer_count;
if (total - 32 < trailer_count)
packet too short
buf = vmalloc(trailer_count)
if (!buf)
op->downcall.status = -ENOMEM;
}
n = total - 32 - trailer_count;
if (sizeof(op->downcall) - 16 < n)
packet too long
if (copy_from_iter((char *)op->downcall + 16, 0, n) != n)
failed copy
memset((char *)op->downcall + 16 + n, 0,
sizeof(op->downcall) - n - 16);
if (trailer_count) {
/* read or skip the trailer */
if (!buf) {
iov_iter_advance(trailer_count, iter);
} else {
n = copy_from_iter(buf, trailer_count, iter);
if (n < trailer_count)
failed copy
}
}
done:
...
return total;
would do marshalling and accept all cases where your current code doesn't
produce an obvious breakage. It's a lot more flexible than your current
*userland* code needs and probably more flexible than it would make sense
to have; e.g. if packets are not allowed to be just 24 bytes long, we
could turn that if (n < 16) {...} into if (n != 16) failed copy,
if we can always assume that packet is at least 16 + sizeof(op->downcall),
if could be simplified even more, etc.
There's no real benefit in using the boundary of the magic 5th segment to
tell where the trailer starts. The above is just a demo, but hopefully it
does demonstrate how to use those primitives. Basically, use copy_from_iter()
as you would use read() on stdin when writing a userland filter, with
iov_iter_advance() used to skip a given amount and iov_iter_count() telling
how much input is left.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
