Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 19 Nov 2009 08:26:27 +0000 (GMT)
From:      Robert Watson <rwatson@FreeBSD.org>
To:        Jung-uk Kim <jkim@FreeBSD.org>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org
Subject:   Re: svn commit: r199498 - in head/sys: amd64/amd64 i386/i386 net
Message-ID:  <alpine.BSF.2.00.0911190819550.12162@fledge.watson.org>
In-Reply-To: <200911182340.nAINeJ3W087652@svn.freebsd.org>
References:  <200911182340.nAINeJ3W087652@svn.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help

On Wed, 18 Nov 2009, Jung-uk Kim wrote:

>  - Change internal function bpf_jit_compile() to return allocated size of
>  the generated binary and remove page size limitation for userland.
>  - Use contigmalloc(9)/contigfree(9) instead of malloc(9)/free(9) to make
>  sure the generated binary aligns properly and make it physically contiguous.

Is physical contiguity actually required here -- I would have thought virtual 
contiguity and alignment would be sufficient, in which case the normal trick 
is to allocate using malloc the size + min-align + 1 and then fudge the 
pointer forward until it's properly aligned.

Also, in 9.x I'm going to be looking at parallel execution within BPF 
descriptors, and I notice that the JIT compiles the register array pointer 
into the generated code, rather than allowing a pointer to be passed in to 
each instance.  I guess it's non-trivial to change that, suggesting that we 
have a pool of compiled instances, but it would be preferable to be able to do 
what the normal BPF code does: allocate a per-thread register block when 
needed.

Robert N M Watson
Computer Laboratory
University of Cambridge

> Modified:
>  head/sys/amd64/amd64/bpf_jit_machdep.c
>  head/sys/i386/i386/bpf_jit_machdep.c
>  head/sys/net/bpf_jitter.c
>  head/sys/net/bpf_jitter.h
>
> Modified: head/sys/amd64/amd64/bpf_jit_machdep.c
> ==============================================================================
> --- head/sys/amd64/amd64/bpf_jit_machdep.c	Wed Nov 18 22:53:05 2009	(r199497)
> +++ head/sys/amd64/amd64/bpf_jit_machdep.c	Wed Nov 18 23:40:19 2009	(r199498)
> @@ -53,7 +53,7 @@ __FBSDID("$FreeBSD$");
>
> #include <amd64/amd64/bpf_jit_machdep.h>
>
> -bpf_filter_func	bpf_jit_compile(struct bpf_insn *, u_int, int *);
> +bpf_filter_func	bpf_jit_compile(struct bpf_insn *, u_int, size_t *, int *);
>
> /*
>  * emit routine to update the jump table
> @@ -97,7 +97,7 @@ emit_code(bpf_bin_stream *stream, u_int
>  * Function that does the real stuff
>  */
> bpf_filter_func
> -bpf_jit_compile(struct bpf_insn *prog, u_int nins, int *mem)
> +bpf_jit_compile(struct bpf_insn *prog, u_int nins, size_t *size, int *mem)
> {
> 	bpf_bin_stream stream;
> 	struct bpf_insn *ins;
> @@ -481,23 +481,21 @@ bpf_jit_compile(struct bpf_insn *prog, u
> #ifndef _KERNEL
> 			if (mprotect(stream.ibuf, stream.cur_ip,
> 			    PROT_READ | PROT_EXEC) != 0) {
> -				munmap(stream.ibuf, BPF_JIT_MAXSIZE);
> +				munmap(stream.ibuf, stream.cur_ip);
> 				stream.ibuf = NULL;
> 			}
> #endif
> +			*size = stream.cur_ip;
> 			break;
> 		}
>
> #ifdef _KERNEL
> -		stream.ibuf = (char *)malloc(stream.cur_ip, M_BPFJIT, M_NOWAIT);
> +		stream.ibuf = (char *)contigmalloc(stream.cur_ip, M_BPFJIT,
> +		    M_NOWAIT, 0, ~0ULL, 16, 0);
> 		if (stream.ibuf == NULL)
> 			break;
> #else
> -		if (stream.cur_ip > BPF_JIT_MAXSIZE) {
> -			stream.ibuf = NULL;
> -			break;
> -		}
> -		stream.ibuf = (char *)mmap(NULL, BPF_JIT_MAXSIZE,
> +		stream.ibuf = (char *)mmap(NULL, stream.cur_ip,
> 		    PROT_READ | PROT_WRITE, MAP_ANON, -1, 0);
> 		if (stream.ibuf == MAP_FAILED) {
> 			stream.ibuf = NULL;
>
> Modified: head/sys/i386/i386/bpf_jit_machdep.c
> ==============================================================================
> --- head/sys/i386/i386/bpf_jit_machdep.c	Wed Nov 18 22:53:05 2009	(r199497)
> +++ head/sys/i386/i386/bpf_jit_machdep.c	Wed Nov 18 23:40:19 2009	(r199498)
> @@ -53,7 +53,7 @@ __FBSDID("$FreeBSD$");
>
> #include <i386/i386/bpf_jit_machdep.h>
>
> -bpf_filter_func	bpf_jit_compile(struct bpf_insn *, u_int, int *);
> +bpf_filter_func	bpf_jit_compile(struct bpf_insn *, u_int, size_t *, int *);
>
> /*
>  * emit routine to update the jump table
> @@ -97,7 +97,7 @@ emit_code(bpf_bin_stream *stream, u_int
>  * Function that does the real stuff
>  */
> bpf_filter_func
> -bpf_jit_compile(struct bpf_insn *prog, u_int nins, int *mem)
> +bpf_jit_compile(struct bpf_insn *prog, u_int nins, size_t *size, int *mem)
> {
> 	bpf_bin_stream stream;
> 	struct bpf_insn *ins;
> @@ -504,23 +504,21 @@ bpf_jit_compile(struct bpf_insn *prog, u
> #ifndef _KERNEL
> 			if (mprotect(stream.ibuf, stream.cur_ip,
> 			    PROT_READ | PROT_EXEC) != 0) {
> -				munmap(stream.ibuf, BPF_JIT_MAXSIZE);
> +				munmap(stream.ibuf, stream.cur_ip);
> 				stream.ibuf = NULL;
> 			}
> #endif
> +			*size = stream.cur_ip;
> 			break;
> 		}
>
> #ifdef _KERNEL
> -		stream.ibuf = (char *)malloc(stream.cur_ip, M_BPFJIT, M_NOWAIT);
> +		stream.ibuf = (char *)contigmalloc(stream.cur_ip, M_BPFJIT,
> +		    M_NOWAIT, 0, ~0ULL, 16, 0);
> 		if (stream.ibuf == NULL)
> 			break;
> #else
> -		if (stream.cur_ip > BPF_JIT_MAXSIZE) {
> -			stream.ibuf = NULL;
> -			break;
> -		}
> -		stream.ibuf = (char *)mmap(NULL, BPF_JIT_MAXSIZE,
> +		stream.ibuf = (char *)mmap(NULL, stream.cur_ip,
> 		    PROT_READ | PROT_WRITE, MAP_ANON, -1, 0);
> 		if (stream.ibuf == MAP_FAILED) {
> 			stream.ibuf = NULL;
>
> Modified: head/sys/net/bpf_jitter.c
> ==============================================================================
> --- head/sys/net/bpf_jitter.c	Wed Nov 18 22:53:05 2009	(r199497)
> +++ head/sys/net/bpf_jitter.c	Wed Nov 18 23:40:19 2009	(r199498)
> @@ -51,7 +51,7 @@ __FBSDID("$FreeBSD$");
> #include <net/bpf.h>
> #include <net/bpf_jitter.h>
>
> -bpf_filter_func	bpf_jit_compile(struct bpf_insn *, u_int, int *);
> +bpf_filter_func	bpf_jit_compile(struct bpf_insn *, u_int, size_t *, int *);
>
> static u_int	bpf_jit_accept_all(u_char *, u_int, u_int);
>
> @@ -81,7 +81,8 @@ bpf_jitter(struct bpf_insn *fp, int nins
> 	}
>
> 	/* Create the binary */
> -	if ((filter->func = bpf_jit_compile(fp, nins, filter->mem)) == NULL) {
> +	if ((filter->func = bpf_jit_compile(fp, nins, &filter->size,
> +	    filter->mem)) == NULL) {
> 		free(filter, M_BPFJIT);
> 		return (NULL);
> 	}
> @@ -94,7 +95,7 @@ bpf_destroy_jit_filter(bpf_jit_filter *f
> {
>
> 	if (filter->func != bpf_jit_accept_all)
> -		free(filter->func, M_BPFJIT);
> +		contigfree(filter->func, filter->size, M_BPFJIT);
> 	free(filter, M_BPFJIT);
> }
> #else
> @@ -116,7 +117,8 @@ bpf_jitter(struct bpf_insn *fp, int nins
> 	}
>
> 	/* Create the binary */
> -	if ((filter->func = bpf_jit_compile(fp, nins, filter->mem)) == NULL) {
> +	if ((filter->func = bpf_jit_compile(fp, nins, &filter->size,
> +	    filter->mem)) == NULL) {
> 		free(filter);
> 		return (NULL);
> 	}
> @@ -129,7 +131,7 @@ bpf_destroy_jit_filter(bpf_jit_filter *f
> {
>
> 	if (filter->func != bpf_jit_accept_all)
> -		munmap(filter->func, BPF_JIT_MAXSIZE);
> +		munmap(filter->func, filter->size);
> 	free(filter);
> }
> #endif
>
> Modified: head/sys/net/bpf_jitter.h
> ==============================================================================
> --- head/sys/net/bpf_jitter.h	Wed Nov 18 22:53:05 2009	(r199497)
> +++ head/sys/net/bpf_jitter.h	Wed Nov 18 23:40:19 2009	(r199498)
> @@ -36,8 +36,6 @@
>
> #ifdef _KERNEL
> MALLOC_DECLARE(M_BPFJIT);
> -#else
> -#define	BPF_JIT_MAXSIZE		PAGE_SIZE
> #endif
>
> extern int bpf_jitter_enable;
> @@ -55,7 +53,7 @@ typedef u_int (*bpf_filter_func)(u_char
> typedef struct bpf_jit_filter {
> 	/* The native filtering binary, in the form of a bpf_filter_func. */
> 	bpf_filter_func	func;
> -
> +	size_t		size;
> 	int		mem[BPF_MEMWORDS];	/* Scratch memory */
> } bpf_jit_filter;
>
>



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?alpine.BSF.2.00.0911190819550.12162>