Re: [RFC PATCH 1/4] bpf: add new prog_type BPF_PROG_TYPE_IO_FILTER

From: Martin KaFai Lau <kafai@fb.com>
To: Leah Rumancik <leah.rumancik@gmail.com>
Cc: <bpf@vger.kernel.org>, <linux-block@vger.kernel.org>,
	<orbekk@google.com>, <harshads@google.com>, <jasiu@google.com>,
	<saranyamohan@google.com>, <tytso@google.com>,
	<bvanassche@google.com>
Subject: Re: [RFC PATCH 1/4] bpf: add new prog_type BPF_PROG_TYPE_IO_FILTER
Date: Thu, 13 Aug 2020 16:00:51 -0700	[thread overview]
Message-ID: <20200813230021.llbkj5ihyadcbuia@kafai-mbp.dhcp.thefacebook.com> (raw)
In-Reply-To: <20200812163305.545447-2-leah.rumancik@gmail.com>

On Wed, Aug 12, 2020 at 04:33:02PM +0000, Leah Rumancik wrote:
> Introducing a new program type BPF_PROG_TYPE_IO_FILTER and a new
> attach type BPF_BIO_SUBMIT.
> 
> This program type is intended to help filter and monitor IO requests.

[ ... ]

> +#define BPF_MAX_PROGS 64
> +
> +int io_filter_prog_attach(const union bpf_attr *attr, struct bpf_prog *prog)
> +{
> +	struct gendisk *disk;
> +	struct fd f;
> +	struct bpf_prog_array *old_array;
> +	struct bpf_prog_array *new_array;
> +	int ret;
> +
> +	if (attr->attach_flags)
> +		return -EINVAL;
> +
> +	f = fdget(attr->target_fd);
> +	if (!f.file)
> +		return -EBADF;
> +
> +	disk = I_BDEV(f.file->f_mapping->host)->bd_disk;
> +	if (disk == NULL)
> +		return -ENXIO;
> +
> +	ret = mutex_lock_interruptible(&disk->io_filter_lock);
> +	if (ret)
> +		return ret;
> +
> +	old_array = io_filter_rcu_dereference_progs(disk);
> +	if (old_array && bpf_prog_array_length(old_array) >= BPF_MAX_PROGS) {
> +		ret = -E2BIG;
> +		goto unlock;
> +	}
> +
> +	ret = bpf_prog_array_copy(old_array, NULL, prog, &new_array);
> +	if (ret < 0)
> +		goto unlock;
> +
> +	rcu_assign_pointer(disk->progs, new_array);
> +	bpf_prog_array_free(old_array);
> +
> +unlock:
> +	mutex_unlock(&disk->io_filter_lock);
> +	return ret;
> +}
bpf link should be used.
netns_bpf_link_create() can be used as an example.

[ ... ]

> +int io_filter_bpf_run(struct bio *bio)
> +{
> +	struct bpf_io_request io_req = {
> +		.sector_start = bio->bi_iter.bi_sector,
> +		.sector_cnt = bio_sectors(bio),
> +		.opf = bio->bi_opf,
> +	};
> +
> +	return BPF_PROG_RUN_ARRAY_CHECK(bio->bi_disk->progs, &io_req, BPF_PROG_RUN);
> +}

[ ... ]

> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 8bd33050b7bb..4f84ab93d82c 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -189,6 +189,7 @@ enum bpf_prog_type {
>  	BPF_PROG_TYPE_STRUCT_OPS,
>  	BPF_PROG_TYPE_EXT,
>  	BPF_PROG_TYPE_LSM,
> +	BPF_PROG_TYPE_IO_FILTER,
>  };
>  
>  enum bpf_attach_type {
> @@ -226,6 +227,7 @@ enum bpf_attach_type {
>  	BPF_CGROUP_INET4_GETSOCKNAME,
>  	BPF_CGROUP_INET6_GETSOCKNAME,
>  	BPF_XDP_DEVMAP,
> +	BPF_BIO_SUBMIT,
>  	__MAX_BPF_ATTACH_TYPE
>  };
>  
> @@ -4261,4 +4263,13 @@ struct bpf_pidns_info {
>  	__u32 pid;
>  	__u32 tgid;
>  };
> +
> +#define IO_ALLOW 1
> +#define IO_BLOCK 0
> +
> +struct bpf_io_request {
> +	__u64 sector_start;	/* first sector */
> +	__u32 sector_cnt;	/* number of sectors */
> +	__u32 opf;		/* bio->bi_opf */
> +};
Is it all that are needed from "struct bio" to do the filtering and monitoring?
Please elaborate a few more specific filtering usecases in the comment
or even better is to add those usecases to the tests.

[ ... ]

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 94cead5a43e5..71372e99a722 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -2613,6 +2613,7 @@ static bool may_access_direct_pkt_data(struct bpf_verifier_env *env,
>  	case BPF_PROG_TYPE_LWT_SEG6LOCAL:
>  	case BPF_PROG_TYPE_SK_REUSEPORT:
>  	case BPF_PROG_TYPE_FLOW_DISSECTOR:
> +	case BPF_PROG_TYPE_IO_FILTER:
Why it is needed?

>  	case BPF_PROG_TYPE_CGROUP_SKB:
>  		if (t == BPF_WRITE)
>  			return false;