Date: Sun, 18 Nov 2007 19:51:56 -0800 From: "Kip Macy" <kip.macy@gmail.com> To: "Matthew Luckie" <mluckie@cs.waikato.ac.nz> Cc: freebsd-net@freebsd.org, "Christian S.J. Peron" <csjp@freebsd.org> Subject: Re: BPF BIOCSETF Message-ID: <b1fa29170711181951rae189a8i7ef03c956d1ee7a4@mail.gmail.com> In-Reply-To: <20071119031823.GA8141@sorcerer.cs.waikato.ac.nz> References: <20071119031823.GA8141@sorcerer.cs.waikato.ac.nz>
next in thread | previous in thread | raw e-mail | index | archive | help
On Nov 18, 2007 7:18 PM, Matthew Luckie <mluckie@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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?b1fa29170711181951rae189a8i7ef03c956d1ee7a4>