Re: [PATCH bpf-next v2 2/8] bpf: XDP metadata RX kfuncs

From: Martin KaFai Lau <martin.lau@linux.dev>
To: Stanislav Fomichev <sdf@google.com>
Cc: ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org,
	song@kernel.org, yhs@fb.com, john.fastabend@gmail.com,
	kpsingh@kernel.org, haoluo@google.com, jolsa@kernel.org,
	David Ahern <dsahern@gmail.com>, Jakub Kicinski <kuba@kernel.org>,
	Willem de Bruijn <willemb@google.com>,
	Jesper Dangaard Brouer <brouer@redhat.com>,
	Anatoly Burakov <anatoly.burakov@intel.com>,
	Alexander Lobakin <alexandr.lobakin@intel.com>,
	Magnus Karlsson <magnus.karlsson@gmail.com>,
	Maryam Tahhan <mtahhan@redhat.com>,
	xdp-hints@xdp-project.net, netdev@vger.kernel.org,
	bpf@vger.kernel.org
Subject: Re: [PATCH bpf-next v2 2/8] bpf: XDP metadata RX kfuncs
Date: Tue, 22 Nov 2022 22:34:02 -0800	[thread overview]
Message-ID: <ac5a7f9d-1bbf-db28-e494-44ac47a48fe6@linux.dev> (raw)
In-Reply-To: <20221121182552.2152891-3-sdf@google.com>

On 11/21/22 10:25 AM, Stanislav Fomichev wrote:
> --- a/kernel/bpf/core.c
> +++ b/kernel/bpf/core.c
> @@ -2576,6 +2576,7 @@ static void bpf_prog_free_deferred(struct work_struct *work)
>   	} else {
>   		bpf_jit_free(aux->prog);
>   	}
> +	dev_put(aux->xdp_netdev);

I think dev_put needs to be done during unregister_netdevice event also. 
Otherwise, a loaded bpf prog may hold the dev for a long time.  May be there is 
ideas in offload.c.

[ ... ]

> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 35972afb6850..ece7f9234b2d 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -2491,7 +2491,8 @@ static int bpf_prog_load(union bpf_attr *attr, bpfptr_t uattr)
>   				 BPF_F_TEST_STATE_FREQ |
>   				 BPF_F_SLEEPABLE |
>   				 BPF_F_TEST_RND_HI32 |
> -				 BPF_F_XDP_HAS_FRAGS))
> +				 BPF_F_XDP_HAS_FRAGS |
> +				 BPF_F_XDP_HAS_METADATA))
>   		return -EINVAL;
>   
>   	if (!IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) &&
> @@ -2579,6 +2580,20 @@ static int bpf_prog_load(union bpf_attr *attr, bpfptr_t uattr)
>   	prog->aux->sleepable = attr->prog_flags & BPF_F_SLEEPABLE;
>   	prog->aux->xdp_has_frags = attr->prog_flags & BPF_F_XDP_HAS_FRAGS;
>   
> +	if (attr->prog_flags & BPF_F_XDP_HAS_METADATA) {
> +		/* Reuse prog_ifindex to bind to the device
> +		 * for XDP metadata kfuncs.
> +		 */
> +		prog->aux->offload_requested = false;
> +
> +		prog->aux->xdp_netdev = dev_get_by_index(current->nsproxy->net_ns,
> +							 attr->prog_ifindex);
> +		if (!prog->aux->xdp_netdev) {
> +			err = -EINVAL;
> +			goto free_prog;
> +		}
> +	}