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

[Leah Rumancik <leah.rumancik@gmail.com>]

On Thu, Aug 13, 2020 at 04:00:51PM -0700, Martin KaFai Lau wrote:
> 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.
I'll update this, thanks for the example.

> > diff --git a/include/uapi/linux/bpf.h Vb/include/uapi/linux/bpf.h
> > +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.
Fields can be added to the bpf_io_request later if needed. I'll add some
more tests and clarification in comments in the next version.

> 
> > 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?
Does not look like this is needed. I will remove it.

Thanks for the review,
Leah