Re: [RFC bpf-next 1/4] bpf: Introduce new structs and struct fields

[Eduard Zingerman <eddyz87@gmail.com>]

Raj Sahu <rjsu26@gmail.com> writes:

Hi Raj,

Sorry for delayed response, finally got to read through this series.
Please find a few comments below and in patch #3.
I understand that things are in an incomplete state atm.

[...]

> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> index ba6b6118cf50..27dcf59f4445 100644
> --- a/kernel/bpf/core.c
> +++ b/kernel/bpf/core.c

[...]

> @@ -135,6 +160,16 @@ struct bpf_prog *bpf_prog_alloc_no_stats(unsigned int size, gfp_t gfp_extra_flag
>  	mutex_init(&fp->aux->dst_mutex);
>  
>  	return fp;
> +
> +free_per_cpu_state:
> +	kfree(termination_states->per_cpu_state);
> +free_bpf_termination_states:
> +	kfree(termination_states);
> +free_bpf_struct_ptr_alloc:

Nit: In verifier code base such exit labels are usually collapsed as one,
     as free() functions can handle NULL arguments.

> +	free_percpu(fp->active);
> +	vfree(fp);
> +	kfree(aux);
> +	return NULL;
>  }
>  
>  struct bpf_prog *bpf_prog_alloc(unsigned int size, gfp_t gfp_extra_flags)
> @@ -282,6 +317,13 @@ void __bpf_prog_free(struct bpf_prog *fp)
>  		kfree(fp->aux->poke_tab);
>  		kfree(fp->aux);
>  	}
> +
> +	if (fp->termination_states) {
> +		kfree(fp->termination_states->pre_execution_state);
> +		kfree(fp->termination_states->per_cpu_state);
> +		kfree(fp->termination_states);
> +	}
> +

Does this need special handling in core.c:bpf_prog_realloc ?
Also, is it possible to use alloc_percpu_gfp()/free_percpu() functions
for these fields?

>  	free_percpu(fp->stats);
>  	free_percpu(fp->active);
>  	vfree(fp);