From owner-freebsd-bugs@FreeBSD.ORG Fri Mar 12 13:10:15 2004 Return-Path: Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id AD58D16A4CE for ; Fri, 12 Mar 2004 13:10:15 -0800 (PST) Received: from freefall.freebsd.org (freefall.freebsd.org [216.136.204.21]) by mx1.FreeBSD.org (Postfix) with ESMTP id 9AA0843D2F for ; Fri, 12 Mar 2004 13:10:15 -0800 (PST) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) i2CLAFbv090509 for ; Fri, 12 Mar 2004 13:10:15 -0800 (PST) (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.12.10/8.12.10/Submit) id i2CLAFHB090508; Fri, 12 Mar 2004 13:10:15 -0800 (PST) (envelope-from gnats) Resent-Date: Fri, 12 Mar 2004 13:10:15 -0800 (PST) Resent-Message-Id: <200403122110.i2CLAFHB090508@freefall.freebsd.org> Resent-From: FreeBSD-gnats-submit@FreeBSD.org (GNATS Filer) Resent-To: freebsd-bugs@FreeBSD.org Resent-Reply-To: FreeBSD-gnats-submit@FreeBSD.org, Dave Chapeskie Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 3DE8716A4D1 for ; Fri, 12 Mar 2004 13:05:24 -0800 (PST) Received: from squigy.ddm.wox.org (p49ka.xDSL-1mm.sentex.ca [64.7.151.50]) by mx1.FreeBSD.org (Postfix) with ESMTP id 67F0743D2F for ; Fri, 12 Mar 2004 13:05:23 -0800 (PST) (envelope-from dchapes@ddm.wox.org) Received: by squigy.ddm.wox.org (Postfix, from userid 5000) id 86CA78B802; Fri, 12 Mar 2004 16:05:22 -0500 (EST) Message-Id: <20040312210522.86CA78B802@squigy.ddm.wox.org> Date: Fri, 12 Mar 2004 16:05:22 -0500 (EST) From: Dave Chapeskie To: FreeBSD-gnats-submit@FreeBSD.org X-Send-Pr-Version: 3.113 Subject: kern/64178: kqueue does not work with bpf when using BIOCIMMEDIATE or BIOCSRTIMEOUT [PATCH] X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list Reply-To: Dave Chapeskie List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 12 Mar 2004 21:10:15 -0000 >Number: 64178 >Category: kern >Synopsis: kqueue does not work with bpf when using BIOCIMMEDIATE or BIOCSRTIMEOUT [PATCH] >Confidential: no >Severity: non-critical >Priority: low >Responsible: freebsd-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Fri Mar 12 13:10:15 PST 2004 >Closed-Date: >Last-Modified: >Originator: Dave Chapeskie >Release: FreeBSD 4.9-STABLE i386 >Organization: Sandvine Inc. >Environment: FreeBSD 4.9R and FreeBSD 5 since Aug 2003 >Description: When bpf is used with BIOCIMMEDIATE the first packet after a read fails to active attached knotes and the returned read count does not account for most recent packet. If the received packet needs a response before additional packets will arrive this would cause the application to fail. When bpf is used with BIOCSRTIMEOUT set to a non-zero value, knotes are rarely activated at the correct time. If the attached interface should become detached knotes are not activated and the application will wait forever (even if there are some packets available in the slot buffer). >How-To-Repeat: Open a bpf node, issue a BIOCIMMEDIATE and/or a BIOCSRTIMEOUT ioctl, register a kevent on the bpf descriptor, wait for activity with kqueue. While waiting send a single packet that matches the filter. >Fix: Unlike select/poll, bpf does not know when kevent call has blocked so the exact same behaviour with respect to BIOCSRTIMEOUT can not be achieved. Instead it is assumed that any attached knote indicates the process is waiting (or will shortly) and therefore the read timeout is immediatly started when the knote is first registered and after every read or ioctl call. My tests indicate that with this change a kqueue enabled application using BIOCSRTIMEOUT sees nearly identical results to the select(2) version of the application. The following patches (against RELENG_4 and HEAD) fixes all the issues with kqueue on a bpf node that I found. - Deffer the call to bpf_wakeup() in catchpacket() since filt_bpfread needs to see the new value of slen. - Keep the read timeout callout active while any knotes are attached. - Activate knotes when the interface is not attached (bf_bif==0), a following read call will return ENXIO in this case. (RELENG_4 only): - Add a couple of SPLASSERT() calls. For RELENG_4: Index: bpf.c =================================================================== RCS file: /cvs/src/sys/net/bpf.c,v retrieving revision 1.59.2.13 diff -u -u -r1.59.2.13 bpf.c --- bpf.c 21 Aug 2003 23:50:54 -0000 1.59.2.13 +++ bpf.c 12 Mar 2004 20:46:32 -0000 @@ -163,6 +163,11 @@ static struct filterops bpfread_filtops = { 1, NULL, filt_bpfdetach, filt_bpfread }; +/* Test if we need to start the read timeout when we have knotes. */ +#define BPF_KNOTE_NEEDCALLOUT(bd) \ + (!SLIST_EMPTY(&(bd)->bd_sel.si_note) \ + && (bd)->bd_rtout > 0 && (bd)->bd_state == BPF_IDLE) + static int bpf_movein(uio, linktype, mp, sockp, datlen) register struct uio *uio; @@ -557,6 +562,12 @@ d->bd_fbuf = d->bd_hbuf; d->bd_hbuf = 0; d->bd_hlen = 0; + + /* Start the read timeout if there are any knotes attached. */ + if (BPF_KNOTE_NEEDCALLOUT(d)) { + callout_reset(&d->bd_callout, d->bd_rtout, bpf_timed_out, d); + d->bd_state = BPF_WAITING; + } splx(s); return (error); @@ -570,6 +581,7 @@ bpf_wakeup(d) register struct bpf_d *d; { + SPLASSERT(imp, "bpf_wakeup()"); if (d->bd_state == BPF_WAITING) { callout_stop(&d->bd_callout); d->bd_state = BPF_IDLE; @@ -660,6 +672,7 @@ reset_d(d) struct bpf_d *d; { + SPLASSERT(imp, "bpf reset_d()"); if (d->bd_hbuf) { /* Free the hold buffer. */ d->bd_fbuf = d->bd_hbuf; @@ -966,6 +979,13 @@ *(u_int *)addr = d->bd_sig; break; } + s = splimp(); + /* Start the read timeout if there are any knotes attached. */ + if (BPF_KNOTE_NEEDCALLOUT(d)) { + callout_reset(&d->bd_callout, d->bd_rtout, bpf_timed_out, d); + d->bd_state = BPF_WAITING; + } + splx(s); return (error); } @@ -1131,6 +1151,11 @@ kn->kn_hook = (caddr_t)d; s = splimp(); SLIST_INSERT_HEAD(&d->bd_sel.si_note, kn, kn_selnext); + /* Start the read timeout if required. */ + if (d->bd_rtout > 0 && d->bd_state == BPF_IDLE) { + callout_reset(&d->bd_callout, d->bd_rtout, bpf_timed_out, d); + d->bd_state = BPF_WAITING; + } splx(s); return (0); @@ -1157,7 +1182,7 @@ int s, ready; s = splimp(); - ready = bpf_ready(d); + ready = (d->bd_bif == NULL || bpf_ready(d)); if (ready) { kn->kn_data = d->bd_slen; if (d->bd_hbuf) @@ -1274,6 +1299,7 @@ register struct bpf_hdr *hp; register int totlen, curlen; register int hdrlen = d->bd_bif->bif_hdrlen; + int need_wakeup = 0; /* * Figure out how many bytes to move. If the packet is * greater or equal to the snapshot length, transfer that @@ -1303,7 +1329,7 @@ return; } ROTATE_BUFFERS(d); - bpf_wakeup(d); + ++need_wakeup; curlen = 0; } else if (d->bd_immediate || d->bd_state == BPF_TIMED_OUT) @@ -1312,7 +1338,7 @@ * already expired during a select call. A packet * arrived, so the reader should be woken up. */ - bpf_wakeup(d); + ++need_wakeup; /* * Append the bpf header. @@ -1332,6 +1358,8 @@ */ (*cpfn)(pkt, (u_char *)hp + hdrlen, (hp->bh_caplen = totlen - hdrlen)); d->bd_slen = curlen + totlen; + if (need_wakeup) + bpf_wakeup(d); } /* For -CURRENT: Index: bpf.c =================================================================== RCS file: /cvs/src/sys/net/bpf.c,v retrieving revision 1.124 diff -u -u -r1.124 bpf.c --- bpf.c 29 Feb 2004 15:32:33 -0000 1.124 +++ bpf.c 12 Mar 2004 20:49:54 -0000 @@ -142,6 +142,11 @@ static struct filterops bpfread_filtops = { 1, NULL, filt_bpfdetach, filt_bpfread }; +/* Test if we need to start the read timeout when we have knotes. */ +#define BPF_KNOTE_NEEDCALLOUT(bd) \ + (!SLIST_EMPTY(&(bd)->bd_sel.si_note) \ + && (bd)->bd_rtout > 0 && (bd)->bd_state == BPF_IDLE) + static int bpf_movein(uio, linktype, mp, sockp, datlen) struct uio *uio; @@ -507,6 +512,12 @@ d->bd_fbuf = d->bd_hbuf; d->bd_hbuf = 0; d->bd_hlen = 0; + + /* Start the read timeout if there are any knotes attached. */ + if (BPF_KNOTE_NEEDCALLOUT(d)) { + callout_reset(&d->bd_callout, d->bd_rtout, bpf_timed_out, d); + d->bd_state = BPF_WAITING; + } BPFD_UNLOCK(d); return (error); @@ -924,6 +935,13 @@ *(u_int *)addr = d->bd_sig; break; } + BPFD_LOCK(d); + /* Start the read timeout if there are any knotes attached. */ + if (BPF_KNOTE_NEEDCALLOUT(d)) { + callout_reset(&d->bd_callout, d->bd_rtout, bpf_timed_out, d); + d->bd_state = BPF_WAITING; + } + BPFD_UNLOCK(d); return (error); } @@ -1094,6 +1112,11 @@ kn->kn_hook = d; BPFD_LOCK(d); SLIST_INSERT_HEAD(&d->bd_sel.si_note, kn, kn_selnext); + /* Start the read timeout if required. */ + if (d->bd_rtout > 0 && d->bd_state == BPF_IDLE) { + callout_reset(&d->bd_callout, d->bd_rtout, bpf_timed_out, d); + d->bd_state = BPF_WAITING; + } BPFD_UNLOCK(d); return (0); @@ -1119,7 +1142,7 @@ int ready; BPFD_LOCK(d); - ready = bpf_ready(d); + ready = (d->bd_bif == NULL || bpf_ready(d)); if (ready) { kn->kn_data = d->bd_slen; if (d->bd_hbuf) @@ -1289,6 +1312,7 @@ struct bpf_hdr *hp; int totlen, curlen; int hdrlen = d->bd_bif->bif_hdrlen; + int need_wakeup = 0; /* * Figure out how many bytes to move. If the packet is @@ -1319,7 +1343,7 @@ return; } ROTATE_BUFFERS(d); - bpf_wakeup(d); + ++need_wakeup; curlen = 0; } else if (d->bd_immediate || d->bd_state == BPF_TIMED_OUT) @@ -1328,7 +1352,7 @@ * already expired during a select call. A packet * arrived, so the reader should be woken up. */ - bpf_wakeup(d); + ++need_wakeup; /* * Append the bpf header. @@ -1342,6 +1366,8 @@ */ (*cpfn)(pkt, (u_char *)hp + hdrlen, (hp->bh_caplen = totlen - hdrlen)); d->bd_slen = curlen + totlen; + if (need_wakeup) + bpf_wakeup(d); } /* >Release-Note: >Audit-Trail: >Unformatted: