Date: Thu, 27 May 2004 23:37:55 +0900 From: Hidetoshi Shimokawa <simokawa@sat.t.u-tokyo.ac.jp> To: Buzz Slye <buzz@gaia.arc.nasa.gov> Cc: freebsd-firewire@freebsd.org Subject: Re: Quadlet read/write bug Message-ID: <87r7t6ey0s.wl@tora.nunu.org> In-Reply-To: <Pine.GSO.4.58.0405261357400.2183@mono.arc.nasa.gov> References: <Pine.GSO.4.58.0405261357400.2183@mono.arc.nasa.gov>
next in thread | previous in thread | raw e-mail | index | archive | help
As you know, userland interface, fwdev.c is broken and annoying in
many respects. I suppose it was designed just for debugging and it's
inefficient for real use. Some of those interfaces were broken
originally and I broke some more interfaces when I changed packet
format in struct fw_xfer :-(.
You are right that there is a problem in handling of payload length.
Your change and Doug's change committed to -current seem to be a
enough workaround. Though I have also an incomplete/untested patch, I
should explain why the payload for read request response is 4. As you
mentioned, actual IEEE1394 packet length is 16 bytes which includes 4
bytes read data and payload length should be 0. But I intentionally
copy the 4 bytes data to xfer->recv.payload in the following code in
fw_rcv_copy().
/* special handling for RRESQ */
if (pkt->mode.hdr.tcode == FWTCODE_RRESQ &&
p != NULL && res >= sizeof(u_int32_t)) {
*(u_int32_t *)p = pkt->mode.rresq.data;
rb->xfer->recv.pay_len = sizeof(u_int32_t);
return;
}
This is mainly for the fwmem_read_quad() interface. In my opinion, it
is useful that we have such uniform interface for quad and block read
that the caller pass a pointer of the buffer and the data is copied to
the buffer by callee.
Gotanda-san reported me that FW_SBINDADDR ioctl is also broken. The
lower kernel interface fw_bindadd() is well tested by
sbp(4)/sbp_targ(4). So it just a problem of fwdev.c.
We need someone to rewrite userland interface. Compatibility for Linux
may be useful but I'm sure how hard it is.
I hope this helps you.
/\ Hidetoshi Shimokawa
\/ simokawa@sat.t.u-tokyo.ac.jp
PGP public key: http://www.sat.t.u-tokyo.ac.jp/~simokawa/pgp.html
At Wed, 26 May 2004 13:59:16 -0700 (PDT),
Buzz Slye wrote:
>
> A temporary fix to the asyncronous read and write cases of fw_ioctl
> for a req.len = 16 is (fwdev.c line 595):
> int tc;
> .....
> /* copy response */
> tc = xfer->recv.hdr.mode.hdr.tcode;
> tinfo = &sc->fc->tcode[tc];
> if (tc == FWTCODE_RRESQ || tc == FWTCODE_WRES)
> asyreq->req.len = xfer->recv.pay_len;
> else if (asyreq->req.len >= xfer->recv.pay_len + tinfo->hdr_len)
> asyreq->req.len = xfer->recv.pay_len;
> else
> err = EINVAL;
>
> The above will work for rreqq and wreqq, but I didn't look at the other cases.
> Note that for the read request response, the payload length is 4, but the
> header length is 16. This adds up to 20 which doesn't work for req.len=16.
> The response header should be 12 maybe, if the payload is 4 ?
> For the write request response, the payload length is 4096, but there really
> isn't any payload returned. Returning req.len=4096 isn't good, but if the
> application doesn't check it, it certainly beats returning EINVAL.
>
> R. E. Slye
> NASA/Ames
>
> _______________________________________________
> freebsd-firewire@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-firewire
> To unsubscribe, send any mail to "freebsd-firewire-unsubscribe@freebsd.org"
>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?87r7t6ey0s.wl>