bpf.c select fix for review
While working on the new argus version I discovered that there is a
bug in the /sys/net/bpf.c code if you are doing select with a timeout (which
tcpdump isn't). Basically if you set up tcpreplay on another machine and send
a less than full buffer (9 packets 2300 bytes in my case) to a FreeBSD 4.1
RELEASE machine, argus won't see the packets. It turns out that because it
is using select (and not immediate mode) the partially full buffer won't get
passed up to libpcap. The OpenBSD folks have a partial fix (which only gets
the first packet up and leaves the other 8 dangling). I ported that to FreeBSD
and then made a fix which appears to work based on what their code looked to
be trying to do (as opposed to what it does which is incorrect).
Basically at receive packet time the tick counter is stored in a
variable in the capture structure. When the select/poll timeout occurs it
checks if the current tick time is then the last received packet count +
the timeout value. If so it rotates the (partially full) packet buffer and
signals to wake up the process. While this appears to work (and match the
Solaris behavior on the same file) I figured I'd see if anyone sees a glaring
hole in the fix before submitting a bug report on it.
Peter Van Epp / Operations and Technical Support
Simon Fraser University, Burnaby, B.C. Canada
*** /sys/net/bpf.c.orig Sat Oct 14 19:00:59 2000
--- /sys/net/bpf.c Mon Oct 16 09:30:24 2000
***
*** 1054,1061
if (events (POLLIN | POLLRDNORM)) {
if (d-bd_hlen != 0 || (d-bd_immediate d-bd_slen != 0))
revents |= events (POLLIN | POLLRDNORM);
! else
! selrecord(p, d-bd_sel);
}
splx(s);
return (revents);
--- 1054,1076
if (events (POLLIN | POLLRDNORM)) {
if (d-bd_hlen != 0 || (d-bd_immediate d-bd_slen != 0))
revents |= events (POLLIN | POLLRDNORM);
! else {
! /*
!* If there is a timeout and no data in the hold buffer
!* see if there has been data in the capture buffer
!* for more than a timeout interval. If so rotate the
!* buffer to push the packets to the user.
!*/
! if ((d-bd_slen != 0) (d-bd_hlen == 0)) {
! if ((d-bd_rtout != -1)
! (d-bd_rdStart + d-bd_rtout) ticks) {
! ROTATE_BUFFERS(d);
! revents |= events (POLLIN | POLLRDNORM);
! }
! } else
! selrecord(p, d-bd_sel);
!
! }
}
splx(s);
return (revents);
***
*** 1219,1224
--- 1234,1245
*/
(*cpfn)(pkt, (u_char *)hp + hdrlen, (hp-bh_caplen = totlen - hdrlen));
d-bd_slen = curlen + totlen;
+
+ /*
+* Mark the time the last packet was seen for poll timeout processing.
+*/
+
+ d-bd_rdStart = ticks;
}
/*
*** /sys/net/bpfdesc.h.orig Sat Oct 14 19:16:07 2000
--- /sys/net/bpfdesc.h Sat Oct 14 19:21:54 2000
***
*** 69,74
--- 69,75
struct bpf_if * bd_bif; /* interface descriptor */
u_long bd_rtout; /* Read timeout in 'ticks' */
+ u_long bd_rdStart; /* when the read started */
struct bpf_insn *bd_filter; /* filter code */
u_long bd_rcount; /* number of packets received */
u_long bd_dcount; /* number of packets dropped */
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-hackers" in the body of the message
Re: bpf.c select fix for review
Please don't use a java/c++-ism like bdStart... atleast call it bdstart.
Thanks,
Andrew
On Mon, 16 Oct 2000, Peter Van Epp wrote:
While working on the new argus version I discovered that there is a
bug in the /sys/net/bpf.c code if you are doing select with a timeout (which
tcpdump isn't). Basically if you set up tcpreplay on another machine and send
a less than full buffer (9 packets 2300 bytes in my case) to a FreeBSD 4.1
RELEASE machine, argus won't see the packets. It turns out that because it
is using select (and not immediate mode) the partially full buffer won't get
passed up to libpcap. The OpenBSD folks have a partial fix (which only gets
the first packet up and leaves the other 8 dangling). I ported that to FreeBSD
and then made a fix which appears to work based on what their code looked to
be trying to do (as opposed to what it does which is incorrect).
Basically at receive packet time the tick counter is stored in a
variable in the capture structure. When the select/poll timeout occurs it
checks if the current tick time is then the last received packet count +
the timeout value. If so it rotates the (partially full) packet buffer and
signals to wake up the process. While this appears to work (and match the
Solaris behavior on the same file) I figured I'd see if anyone sees a glaring
hole in the fix before submitting a bug report on it.
Peter Van Epp / Operations and Technical Support
Simon Fraser University, Burnaby, B.C. Canada
*** /sys/net/bpf.c.orig Sat Oct 14 19:00:59 2000
--- /sys/net/bpf.cMon Oct 16 09:30:24 2000
***
*** 1054,1061
if (events (POLLIN | POLLRDNORM)) {
if (d-bd_hlen != 0 || (d-bd_immediate d-bd_slen != 0))
revents |= events (POLLIN | POLLRDNORM);
! else
! selrecord(p, d-bd_sel);
}
splx(s);
return (revents);
--- 1054,1076
if (events (POLLIN | POLLRDNORM)) {
if (d-bd_hlen != 0 || (d-bd_immediate d-bd_slen != 0))
revents |= events (POLLIN | POLLRDNORM);
! else {
! /*
! * If there is a timeout and no data in the hold buffer
! * see if there has been data in the capture buffer
! * for more than a timeout interval. If so rotate the
! * buffer to push the packets to the user.
! */
! if ((d-bd_slen != 0) (d-bd_hlen == 0)) {
! if ((d-bd_rtout != -1)
! (d-bd_rdStart + d-bd_rtout) ticks) {
! ROTATE_BUFFERS(d);
! revents |= events (POLLIN | POLLRDNORM);
! }
! } else
! selrecord(p, d-bd_sel);
!
! }
}
splx(s);
return (revents);
***
*** 1219,1224
--- 1234,1245
*/
(*cpfn)(pkt, (u_char *)hp + hdrlen, (hp-bh_caplen = totlen - hdrlen));
d-bd_slen = curlen + totlen;
+
+ /*
+ * Mark the time the last packet was seen for poll timeout processing.
+ */
+
+ d-bd_rdStart = ticks;
}
/*
*** /sys/net/bpfdesc.h.orig Sat Oct 14 19:16:07 2000
--- /sys/net/bpfdesc.hSat Oct 14 19:21:54 2000
***
*** 69,74
--- 69,75
struct bpf_if * bd_bif; /* interface descriptor */
u_long bd_rtout; /* Read timeout in 'ticks' */
+ u_long bd_rdStart; /* when the read started */
struct bpf_insn *bd_filter; /* filter code */
u_long bd_rcount; /* number of packets received */
u_long bd_dcount; /* number of packets dropped */
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-hackers" in the body of the message
*-.
| Andrew R. Reiter
| [EMAIL PROTECTED]
| "It requires a very unusual mind
| to undertake the analysis of the obvious" -- A.N. Whitehead
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-hackers" in the body of the message
OOPS Re: bpf.c select fix for review
woops.. I meant bd_rdstart.
On Mon, 16 Oct 2000, Peter Van Epp wrote:
While working on the new argus version I discovered that there is a
bug in the /sys/net/bpf.c code if you are doing select with a timeout (which
tcpdump isn't). Basically if you set up tcpreplay on another machine and send
a less than full buffer (9 packets 2300 bytes in my case) to a FreeBSD 4.1
RELEASE machine, argus won't see the packets. It turns out that because it
is using select (and not immediate mode) the partially full buffer won't get
passed up to libpcap. The OpenBSD folks have a partial fix (which only gets
the first packet up and leaves the other 8 dangling). I ported that to FreeBSD
and then made a fix which appears to work based on what their code looked to
be trying to do (as opposed to what it does which is incorrect).
Basically at receive packet time the tick counter is stored in a
variable in the capture structure. When the select/poll timeout occurs it
checks if the current tick time is then the last received packet count +
the timeout value. If so it rotates the (partially full) packet buffer and
signals to wake up the process. While this appears to work (and match the
Solaris behavior on the same file) I figured I'd see if anyone sees a glaring
hole in the fix before submitting a bug report on it.
Peter Van Epp / Operations and Technical Support
Simon Fraser University, Burnaby, B.C. Canada
*** /sys/net/bpf.c.orig Sat Oct 14 19:00:59 2000
--- /sys/net/bpf.cMon Oct 16 09:30:24 2000
***
*** 1054,1061
if (events (POLLIN | POLLRDNORM)) {
if (d-bd_hlen != 0 || (d-bd_immediate d-bd_slen != 0))
revents |= events (POLLIN | POLLRDNORM);
! else
! selrecord(p, d-bd_sel);
}
splx(s);
return (revents);
--- 1054,1076
if (events (POLLIN | POLLRDNORM)) {
if (d-bd_hlen != 0 || (d-bd_immediate d-bd_slen != 0))
revents |= events (POLLIN | POLLRDNORM);
! else {
! /*
! * If there is a timeout and no data in the hold buffer
! * see if there has been data in the capture buffer
! * for more than a timeout interval. If so rotate the
! * buffer to push the packets to the user.
! */
! if ((d-bd_slen != 0) (d-bd_hlen == 0)) {
! if ((d-bd_rtout != -1)
! (d-bd_rdStart + d-bd_rtout) ticks) {
! ROTATE_BUFFERS(d);
! revents |= events (POLLIN | POLLRDNORM);
! }
! } else
! selrecord(p, d-bd_sel);
!
! }
}
splx(s);
return (revents);
***
*** 1219,1224
--- 1234,1245
*/
(*cpfn)(pkt, (u_char *)hp + hdrlen, (hp-bh_caplen = totlen - hdrlen));
d-bd_slen = curlen + totlen;
+
+ /*
+ * Mark the time the last packet was seen for poll timeout processing.
+ */
+
+ d-bd_rdStart = ticks;
}
/*
*** /sys/net/bpfdesc.h.orig Sat Oct 14 19:16:07 2000
--- /sys/net/bpfdesc.hSat Oct 14 19:21:54 2000
***
*** 69,74
--- 69,75
struct bpf_if * bd_bif; /* interface descriptor */
u_long bd_rtout; /* Read timeout in 'ticks' */
+ u_long bd_rdStart; /* when the read started */
struct bpf_insn *bd_filter; /* filter code */
u_long bd_rcount; /* number of packets received */
u_long bd_dcount; /* number of packets dropped */
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-hackers" in the body of the message
*-.
| Andrew R. Reiter
| [EMAIL PROTECTED]
| "It requires a very unusual mind
| to undertake the analysis of the obvious" -- A.N. Whitehead
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-hackers" in the body of the message
