BPF BIOCSETF

[Kip Macy]

On Nov 18, 2007 7:18 PM, Matthew Luckie <mluckie at cs.waikato.ac.nz> wrote:
> BIOCSETF and BIOCSETWF call reset_d which discards any packets
> currently in the hold buffer and resets the packet rx count.
>
> The patch below adds BIOCSETFNR, an ioctl that swaps the BPF filter
> without resetting the hold buffer and without resetting the packet rx
> counts, which is handy when the application wants to adjust its filter
> program but without discarding whatever the system might have
> buffered.
>
> I've also changed BIOCSETWF to map to the new function.  I don't see
> the rationale in having BIOCSETWF mucking with the receive stats and
> hold buffer.
>
> Patch below is against RELENG_6.  I'll send a PR tomorrow against HEAD
> if there aren't any comments or complaints; the main difference will
> be the extra cruft to make bpf_jitter work.
>
> Thoughts?
>
> Matthew
>
> --- bpf.c.orig  Mon Nov 19 14:23:52 2007
> +++ bpf.c       Mon Nov 19 15:55:05 2007
> @@ -103,7 +103,8 @@
>                     u_int, void (*)(const void *, void *, size_t),
>                     struct timeval *);
>  static void    reset_d(struct bpf_d *);
> -static int      bpf_setf(struct bpf_d *, struct bpf_program *, u_long cmd);
> +static int     bpf_setf(struct bpf_d *, struct bpf_program *);
> +static int     bpf_setfnr(struct bpf_d *, struct bpf_program *, u_long);
>  static int     bpf_getdltlist(struct bpf_d *, struct bpf_dltlist *);
>  static int     bpf_setdlt(struct bpf_d *, u_int);
>  static void    filt_bpfdetach(struct knote *);
> @@ -755,8 +756,12 @@
>          * Set link layer read filter.
>          */
>         case BIOCSETF:
> +               error = bpf_setf(d, (struct bpf_program *)addr);
> +               break;
> +
> +       case BIOCSETFNR:
>         case BIOCSETWF:
> -               error = bpf_setf(d, (struct bpf_program *)addr, cmd);
> +               error = bpf_setfnr(d, (struct bpf_program *)addr, cmd);
>                 break;
>
>         /*
> @@ -976,26 +981,17 @@
>   * free it and replace it.  Returns EINVAL for bogus requests.
>   */
>  static int
> -bpf_setf(struct bpf_d *d, struct bpf_program *fp, u_long cmd)
> +bpf_setf(struct bpf_d *d, struct bpf_program *fp)
>  {
>         struct bpf_insn *fcode, *old;
> -       u_int wfilter, flen, size;
> +       u_int flen, size;
>
> -       if (cmd == BIOCSETWF) {
> -               old = d->bd_wfilter;
> -               wfilter = 1;
> -       } else {
> -               wfilter = 0;
> -               old = d->bd_rfilter;
> -       }
>         if (fp->bf_insns == NULL) {
>                 if (fp->bf_len != 0)
>                         return (EINVAL);
>                 BPFD_LOCK(d);
> -               if (wfilter)
> -                       d->bd_wfilter = NULL;
> -               else
> -                       d->bd_rfilter = NULL;
> +               old = d->bd_rfilter;
> +               d->bd_rfilter = NULL;
>                 reset_d(d);
>                 BPFD_UNLOCK(d);
>                 if (old != NULL)
> @@ -1011,10 +1007,8 @@
>         if (copyin((caddr_t)fp->bf_insns, (caddr_t)fcode, size) == 0 &&
>             bpf_validate(fcode, (int)flen)) {
>                 BPFD_LOCK(d);
> -               if (wfilter)
> -                       d->bd_wfilter = fcode;
> -               else
> -                       d->bd_rfilter = fcode;
> +               old = d->bd_rfilter;
> +               d->bd_rfilter = fcode;
>                 reset_d(d);
>                 BPFD_UNLOCK(d);
>                 if (old != NULL)
> @@ -1024,6 +1018,50 @@
>         }
>         free((caddr_t)fcode, M_BPF);
>         return (EINVAL);
> +}
> +
> +/*
> + * Set d's packet filter program to fp.  If this file already has a filter,
> + * replace it but keep any existing buffered packets.
> + */
> +static int
> +bpf_setfnr(struct bpf_d *d, struct bpf_program *fp, u_long cmd)
> +{
> +       struct bpf_insn *fcode, *old;
> +       u_int flen, size;
> +
> +       if (fp->bf_insns != NULL) {
> +               flen = fp->bf_len;
> +               if (flen > bpf_maxinsns)
> +                       return (EINVAL);
> +
> +               size = flen * sizeof(*fp->bf_insns);
> +               fcode = (struct bpf_insn *)malloc(size, M_BPF, M_WAITOK);
> +               if (copyin((caddr_t)fp->bf_insns, (caddr_t)fcode, size) != 0 ||
> +                   bpf_validate(fcode, (int)flen) == 0) {
> +                       free((caddr_t)fcode, M_BPF);
> +                       return (EINVAL);
> +               }
> +       } else {
> +               if (fp->bf_len != 0)
> +                       return (EINVAL);
> +               fcode = NULL;
> +       }
> +
> +       BPFD_LOCK(d);
> +       if (cmd == BIOCSETFNR) {
> +               old = d->bd_rfilter;
> +               d->bd_rfilter = fcode;
> +       } else {
> +               old = d->bd_wfilter;
> +               d->bd_wfilter = fcode;
> +       }
> +       BPFD_UNLOCK(d);
> +
> +       if(old != NULL)
> +               free((caddr_t)old, M_BPF);
> +
> +       return (0);
>  }
>
>  /*
> --- bpf.h.orig  Mon Nov 19 14:24:11 2007
> +++ bpf.h       Mon Nov 19 14:25:16 2007
> @@ -115,6 +115,7 @@
>  #define        BIOCGDLTLIST    _IOWR('B',121, struct bpf_dltlist)
>  #define        BIOCLOCK        _IO('B', 122)
>  #define        BIOCSETWF       _IOW('B',123, struct bpf_program)
> +#define BIOCSETFNR     _IOW('B',124, struct bpf_program)
>
>  /*
>   * Structure prepended to each packet.

Sounds reasonable. Christian has been active in maintaining BPF
recently so he is really the one you need to follow up with.

 -Kip