bpf(4): separate nonblocking status from read timeout

classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

bpf(4): separate nonblocking status from read timeout

Scott Cheloha
Hi,

I want to remove the use of ticks from bpf(4) and replace it
with the system uptime clock.

Before we can do that, though, we need to separate the bpf(4)
descriptor's nonblocking status and its read timeout.  The
two states share the same struct member, bpf_d.bd_rtout.

Currently, if bpf_d.bd_rtout == -1 you have a nonblocking read.  If it
is equal to 0, there is no read timeout.  If it is greater than or
equal to 1, it is the number of ticks the read will wait for data
before giving up.

This is a potentially confusing mixture of states.

I propose moving nonblocking status to a seprate boolean,
bpf_d.bd_rnonblock.

This has one side-effect: the bpf(4) descriptor will no
longer "forget" its read timeout if the program toggles
nonblocking status on/off on the descriptor.

I would argue that this behavior is more intuitive.

Thoughts?  ok?

Index: bpfdesc.h
===================================================================
RCS file: /cvs/src/sys/net/bpfdesc.h,v
retrieving revision 1.40
diff -u -p -r1.40 bpfdesc.h
--- bpfdesc.h 2 Jan 2020 16:23:01 -0000 1.40
+++ bpfdesc.h 25 Mar 2020 00:32:10 -0000
@@ -74,6 +74,7 @@ struct bpf_d {
  struct bpf_if  *bd_bif; /* interface descriptor */
  u_long bd_rtout; /* Read timeout in 'ticks' */
  u_long bd_rdStart; /* when the read started */
+ int bd_rnonblock; /* true if nonblocking reads are set */
  struct bpf_program_smr
        *bd_rfilter; /* read filter code */
  struct bpf_program_smr
Index: bpf.c
===================================================================
RCS file: /cvs/src/sys/net/bpf.c,v
retrieving revision 1.188
diff -u -p -r1.188 bpf.c
--- bpf.c 20 Feb 2020 16:56:52 -0000 1.188
+++ bpf.c 25 Mar 2020 00:32:10 -0000
@@ -379,8 +379,8 @@ bpfopen(dev_t dev, int flag, int mode, s
  smr_init(&bd->bd_smr);
  sigio_init(&bd->bd_sigio);
 
- if (flag & FNONBLOCK)
- bd->bd_rtout = -1;
+ bd->bd_rtout = 0; /* no timeout by default */
+ bd->bd_rnonblock = ISSET(flag, FNONBLOCK);
 
  bpf_get(bd);
  LIST_INSERT_HEAD(&bpf_d_list, bd, bd_list);
@@ -453,7 +453,7 @@ bpfread(dev_t dev, struct uio *uio, int
  * If there's a timeout, bd_rdStart is tagged when we start the read.
  * we can then figure out when we're done reading.
  */
- if (d->bd_rtout != -1 && d->bd_rdStart == 0)
+ if (d->bd_rnonblock == 0 && d->bd_rdStart == 0)
  d->bd_rdStart = ticks;
  else
  d->bd_rdStart = 0;
@@ -482,7 +482,7 @@ bpfread(dev_t dev, struct uio *uio, int
  ROTATE_BUFFERS(d);
  break;
  }
- if (d->bd_rtout == -1) {
+ if (d->bd_rnonblock) {
  /* User requested non-blocking I/O */
  error = EWOULDBLOCK;
  } else {
@@ -960,9 +960,9 @@ bpfioctl(dev_t dev, u_long cmd, caddr_t
 
  case FIONBIO: /* Non-blocking I/O */
  if (*(int *)addr)
- d->bd_rtout = -1;
+ d->bd_rnonblock = 1;
  else
- d->bd_rtout = 0;
+ d->bd_rnonblock = 0;
  break;
 
  case FIOASYNC: /* Send signal on receive packets */
@@ -1153,7 +1153,7 @@ bpfpoll(dev_t dev, int events, struct pr
  * if there's a timeout, mark the time we
  * started waiting.
  */
- if (d->bd_rtout != -1 && d->bd_rdStart == 0)
+ if (d->bd_rnonblock == 0 && d->bd_rdStart == 0)
  d->bd_rdStart = ticks;
  selrecord(p, &d->bd_sel);
  }
@@ -1193,7 +1193,7 @@ bpfkqfilter(dev_t dev, struct knote *kn)
  SLIST_INSERT_HEAD(klist, kn, kn_selnext);
 
  mtx_enter(&d->bd_mtx);
- if (d->bd_rtout != -1 && d->bd_rdStart == 0)
+ if (d->bd_rnonblock == 0 && d->bd_rdStart == 0)
  d->bd_rdStart = ticks;
  mtx_leave(&d->bd_mtx);