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>