From: David Gwynne <david@gwynne.id.au>
Subject: Re: [patch] change BPF filters without discarding buffered packets
To: Matthew Luckie <mjl@luckie.org.nz>, tech@openbsd.org
Date: Fri, 9 Aug 2024 14:45:34 +1000

This makes sense to me and reads well too. What are you using it for?

On 24/07/2024 15:11, Matthew Luckie wrote:
> Hi,
>
> BIOCSETF (change BPF read filter) and BIOCSETWF (change BPF write
> filter) call bpf_resetd, 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 read
> filter without resetting the hold buffer and without resetting the
> packet rx counts, which is handy when the application wants to
> dynamically adjust its filter program, without discarding whatever the
> system might have buffered, yet to be delivered to userspace.
>
> I've also changed BIOCSETWF to map to the new function.  I don't see
> the rationale in having BIOCSETWF (which is about changing write
> filters) resetting the receive stats and read hold buffer.
>
> Patch is against HEAD.  I've tested it on 7.5.  The patch includes
> modifications to bpf(4) manual page.  FreeBSD and MacOS have the same
> ioctl.  I initially did the work for FreeBSD.
>
> https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=118486
>
> Matthew
>
> diff --git share/man/man4/bpf.4 share/man/man4/bpf.4
> index 46429f1cde5..7a884c727bf 100644
> --- share/man/man4/bpf.4
> +++ share/man/man4/bpf.4
> @@ -317,6 +317,7 @@ the age of packets in the kernel buffer does not make the buffer
>   readable.
>   .Pp
>   .It Dv BIOCSETF Fa "struct bpf_program *"
> +.It Dv BIOCSETFNR Fa "struct bpf_program *"
>   Sets the filter program used by the kernel to discard uninteresting packets.
>   An array of instructions and its length are passed in using the following
>   structure:
> @@ -334,9 +335,11 @@ field, while its length in units of
>   is given by the
>   .Fa bf_len
>   field.
> -Also, the actions of
> +If
> +.Dv BIOCSETF
> +is used, the actions of
>   .Dv BIOCFLUSH
> -are performed.
> +are also performed.
>   .Pp
>   See section
>   .Sx FILTER MACHINE
> @@ -349,8 +352,6 @@ network.
>   See
>   .Dv BIOCSETF
>   for a description of the filter program.
> -This ioctl also acts as
> -.Dv BIOCFLUSH .
>   .Pp
>   Note that the filter operates on the packet data written to the descriptor.
>   If the
> diff --git sys/net/bpf.c sys/net/bpf.c
> index 51a1ed66ff4..8cd74c5bbec 100644
> --- sys/net/bpf.c
> +++ sys/net/bpf.c
> @@ -758,7 +758,8 @@ bpf_get_wtimeout(struct bpf_d *d, struct timeval *tv)
>   /*
>    *  FIONREAD		Check for read packet available.
>    *  BIOCGBLEN		Get buffer len [for read()].
> - *  BIOCSETF		Set ethernet read filter.
> + *  BIOCSETF		Set read filter.
> + *  BIOCSETFNR          Set read filter without resetting descriptor.
>    *  BIOCFLUSH		Flush read packet buffer.
>    *  BIOCPROMISC		Put interface into promiscuous mode.
>    *  BIOCGDLTLIST	Get supported link layer types.
> @@ -863,17 +864,12 @@ bpfioctl(dev_t dev, u_long cmd, caddr_t addr, int flag, struct proc *p)
>   		break;
>   
>   	/*
> -	 * Set link layer read filter.
> +	 * Set link layer read/write filter.
>   	 */
>   	case BIOCSETF:
> -		error = bpf_setf(d, (struct bpf_program *)addr, 0);
> -		break;
> -
> -	/*
> -	 * Set link layer write filter.
> -	 */
> +	case BIOCSETFNR:
>   	case BIOCSETWF:
> -		error = bpf_setf(d, (struct bpf_program *)addr, 1);
> +		error = bpf_setf(d, (struct bpf_program *)addr, cmd);
>   		break;
>   
>   	/*
> @@ -1118,7 +1114,7 @@ bpfioctl(dev_t dev, u_long cmd, caddr_t addr, int flag, struct proc *p)
>    * free it and replace it.  Returns EINVAL for bogus requests.
>    */
>   int
> -bpf_setf(struct bpf_d *d, struct bpf_program *fp, int wf)
> +bpf_setf(struct bpf_d *d, struct bpf_program *fp, u_long cmd)
>   {
>   	struct bpf_program_smr *bps, *old_bps;
>   	struct bpf_insn *fcode;
> @@ -1153,7 +1149,7 @@ bpf_setf(struct bpf_d *d, struct bpf_program *fp, int wf)
>   		bps->bps_bf.bf_insns = fcode;
>   	}
>   
> -	if (wf == 0) {
> +	if (cmd != BIOCSETWF) {
>   		old_bps = SMR_PTR_GET_LOCKED(&d->bd_rfilter);
>   		SMR_PTR_SET_LOCKED(&d->bd_rfilter, bps);
>   	} else {
> @@ -1161,9 +1157,12 @@ bpf_setf(struct bpf_d *d, struct bpf_program *fp, int wf)
>   		SMR_PTR_SET_LOCKED(&d->bd_wfilter, bps);
>   	}
>   
> -	mtx_enter(&d->bd_mtx);
> -	bpf_resetd(d);
> -	mtx_leave(&d->bd_mtx);
> +	if (cmd == BIOCSETF) {
> +		mtx_enter(&d->bd_mtx);
> +		bpf_resetd(d);
> +		mtx_leave(&d->bd_mtx);
> +	}
> +
>   	if (old_bps != NULL)
>   		smr_call(&old_bps->bps_smr, bpf_prog_smr, old_bps);
>   
> diff --git sys/net/bpf.h sys/net/bpf.h
> index ab64061fb97..f35ab0d628b 100644
> --- sys/net/bpf.h
> +++ sys/net/bpf.h
> @@ -122,6 +122,7 @@ struct bpf_version {
>   #define BIOCSWTIMEOUT	_IOW('B',126, struct timeval)
>   #define BIOCGWTIMEOUT	_IOR('B',126, struct timeval)
>   #define BIOCDWTIMEOUT	_IO('B',126)
> +#define BIOCSETFNR	_IOW('B',127, struct bpf_program)
>   
>   /*
>    * Direction filters for BIOCSDIRFILT/BIOCGDIRFILT
> diff --git sys/net/bpfdesc.h sys/net/bpfdesc.h
> index 76b75ff5a20..2f18d734e89 100644
> --- sys/net/bpfdesc.h
> +++ sys/net/bpfdesc.h
> @@ -123,6 +123,6 @@ struct bpf_if {
>   	struct ifnet *bif_ifp;		/* corresponding interface */
>   };
>   
> -int	 bpf_setf(struct bpf_d *, struct bpf_program *, int);
> +int	 bpf_setf(struct bpf_d *, struct bpf_program *, u_long);
>   #endif /* _KERNEL */
>   #endif /* _NET_BPFDESC_H_ */
>