From owner-freebsd-firewire@FreeBSD.ORG Wed May 26 14:00:12 2004 Return-Path: Delivered-To: freebsd-firewire@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 8748216A4D3 for ; Wed, 26 May 2004 14:00:12 -0700 (PDT) Received: from gaia.arc.nasa.gov (gaia.arc.nasa.gov [143.232.155.74]) by mx1.FreeBSD.org (Postfix) with ESMTP id 674F543D1F for ; Wed, 26 May 2004 14:00:12 -0700 (PDT) (envelope-from buzz@gaia.arc.nasa.gov) Received: from mono.arc.nasa.gov (mono.arc.nasa.gov [143.232.155.67]) by gaia.arc.nasa.gov (8.11.7/8.11.6) with ESMTP id i4QKxJZ03477 for ; Wed, 26 May 2004 13:59:19 -0700 (PDT) Date: Wed, 26 May 2004 13:59:16 -0700 (PDT) From: Buzz Slye X-X-Sender: buzz@mono.arc.nasa.gov To: freebsd-firewire@freebsd.org Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Subject: Quadlet read/write bug X-BeenThere: freebsd-firewire@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Firewire support in FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 26 May 2004 21:00:12 -0000 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 From owner-freebsd-firewire@FreeBSD.ORG Thu May 27 07:39:01 2004 Return-Path: Delivered-To: freebsd-firewire@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id A5C8116A4CE for ; Thu, 27 May 2004 07:39:01 -0700 (PDT) Received: from tora.nunu.org (YahooBB219003182029.bbtec.net [219.3.182.29]) by mx1.FreeBSD.org (Postfix) with ESMTP id AF75043D2D for ; Thu, 27 May 2004 07:38:59 -0700 (PDT) (envelope-from simokawa@sat.t.u-tokyo.ac.jp) Received: from tora.nunu.org (unknown [192.168.1.2]) by tora.nunu.org (Postfix) with ESMTP id 36ACF4B105; Thu, 27 May 2004 23:37:55 +0900 (JST) Date: Thu, 27 May 2004 23:37:55 +0900 Message-ID: <87r7t6ey0s.wl@tora.nunu.org> From: Hidetoshi Shimokawa To: Buzz Slye In-Reply-To: References: User-Agent: Wanderlust/2.10.1 (Watching The Wheels) SEMI/1.14.5 (Awara-Onsen) FLIM/1.14.5 (Demachiyanagi) APEL/10.6 MULE XEmacs/21.4 (patch 14) (Reasonable Discussion) (i386--freebsd) MIME-Version: 1.0 (generated by SEMI 1.14.5 - "Awara-Onsen") Content-Type: text/plain; charset=US-ASCII cc: freebsd-firewire@freebsd.org Subject: Re: Quadlet read/write bug X-BeenThere: freebsd-firewire@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Firewire support in FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 27 May 2004 14:39:01 -0000 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" >