lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20220308013517.b7xyhqainqmc255o@kafai-mbp.dhcp.thefacebook.com>
Date:   Mon, 7 Mar 2022 17:35:17 -0800
From:   Martin KaFai Lau <kafai@...com>
To:     Toke Høiland-Jørgensen <toke@...hat.com>
Cc:     Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        John Fastabend <john.fastabend@...il.com>,
        KP Singh <kpsingh@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Jesper Dangaard Brouer <hawk@...nel.org>,
        netdev@...r.kernel.org, bpf@...r.kernel.org
Subject: Re: [PATCH bpf-next v9 1/5] bpf: Add "live packet" mode for XDP in
 BPF_PROG_RUN

On Sun, Mar 06, 2022 at 11:34:00PM +0100, Toke Høiland-Jørgensen wrote:
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 4eebea830613..a36065872882 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -1232,6 +1232,10 @@ enum {
>  
>  /* If set, run the test on the cpu specified by bpf_attr.test.cpu */
>  #define BPF_F_TEST_RUN_ON_CPU	(1U << 0)
> +/* Guaranteed to be rejected in XDP tests (for probing) */
> +#define BPF_F_TEST_XDP_RESERVED	(1U << 1)
> +/* If set, XDP frames will be transmitted after processing */
> +#define BPF_F_TEST_XDP_LIVE_FRAMES	(1U << 2)
>  
>  /* type for BPF_ENABLE_STATS */
>  enum bpf_stats_type {
> @@ -1393,6 +1397,7 @@ union bpf_attr {
>  		__aligned_u64	ctx_out;
>  		__u32		flags;
>  		__u32		cpu;
> +		__u32		batch_size;
>  	} test;
>  
>  	struct { /* anonymous struct used by BPF_*_GET_*_ID */

[ ... ]

> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index db402ebc5570..9beb585be5a6 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -3336,7 +3336,7 @@ static int bpf_prog_query(const union bpf_attr *attr,
>  	}
>  }
>  
> -#define BPF_PROG_TEST_RUN_LAST_FIELD test.cpu
> +#define BPF_PROG_TEST_RUN_LAST_FIELD test.batch_size
Instead of adding BPF_F_TEST_XDP_RESERVED,
probing by non-zero batch_size (== 1) should be as good?

[ ... ]

> +static int xdp_test_run_batch(struct xdp_test_data *xdp, struct bpf_prog *prog,
> +			      u32 repeat)
> +{
> +	struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
> +	int err = 0, act, ret, i, nframes = 0, batch_sz;
> +	struct xdp_frame **frames = xdp->frames;
> +	struct xdp_page_head *head;
> +	struct xdp_frame *frm;
> +	bool redirect = false;
> +	struct xdp_buff *ctx;
> +	struct page *page;
> +
> +	batch_sz = min_t(u32, repeat, xdp->batch_size);
> +
> +	local_bh_disable();
> +	xdp_set_return_frame_no_direct();
> +
> +	for (i = 0; i < batch_sz; i++) {
> +		page = page_pool_dev_alloc_pages(xdp->pp);
> +		if (!page) {
> +			err = -ENOMEM;
> +			goto out;
> +		}
> +
> +		head = phys_to_virt(page_to_phys(page));
> +		reset_ctx(head);
> +		ctx = &head->ctx;
> +		frm = &head->frm;
> +		xdp->frame_cnt++;
> +
> +		act = bpf_prog_run_xdp(prog, ctx);
> +
> +		/* if program changed pkt bounds we need to update the xdp_frame */
> +		if (unlikely(ctx_was_changed(head))) {
> +			err = xdp_update_frame_from_buff(ctx, frm);
> +			if (err) {
> +				xdp_return_buff(ctx);
> +				continue;
> +			}
> +		}
> +
> +		switch (act) {
> +		case XDP_TX:
> +			/* we can't do a real XDP_TX since we're not in the
> +			 * driver, so turn it into a REDIRECT back to the same
> +			 * index
> +			 */
> +			ri->tgt_index = xdp->dev->ifindex;
> +			ri->map_id = INT_MAX;
> +			ri->map_type = BPF_MAP_TYPE_UNSPEC;
> +			fallthrough;
> +		case XDP_REDIRECT:
> +			redirect = true;
> +			err = xdp_do_redirect_frame(xdp->dev, ctx, frm, prog);
err of the previous iteration is over written.

> +			if (err)
> +				xdp_return_buff(ctx);
> +			break;
> +		case XDP_PASS:
> +			frames[nframes++] = frm;
> +			break;
> +		default:
> +			bpf_warn_invalid_xdp_action(NULL, prog, act);
> +			fallthrough;
> +		case XDP_DROP:
> +			xdp_return_buff(ctx);
> +			break;
> +		}
> +	}
> +
> +out:
> +	if (redirect)
> +		xdp_do_flush();
> +	if (nframes) {
> +		ret = xdp_recv_frames(frames, nframes, xdp->skbs, xdp->dev);
> +		if (ret)
> +			err = ret;
but here is trying to avoid over writing the err if possible.

> +	}
> +
> +	xdp_clear_return_frame_no_direct();
> +	local_bh_enable();
> +	return err;
so only err happens in the last iteration will break the loop in
bpf_test_run_xdp_live()?

> +}
> +
> +static int bpf_test_run_xdp_live(struct bpf_prog *prog, struct xdp_buff *ctx,
> +				 u32 repeat, u32 batch_size, u32 *time)
> +
> +{
> +	struct xdp_test_data xdp = { .batch_size = batch_size };
> +	struct bpf_test_timer t = { .mode = NO_MIGRATE };
> +	int ret;
> +
> +	if (!repeat)
> +		repeat = 1;
> +
> +	ret = xdp_test_run_setup(&xdp, ctx);
> +	if (ret)
> +		return ret;
> +
> +	bpf_test_timer_enter(&t);
> +	do {
> +		xdp.frame_cnt = 0;
> +		ret = xdp_test_run_batch(&xdp, prog, repeat - t.i);
> +		if (unlikely(ret < 0))
> +			break;
> +	} while (bpf_test_timer_continue(&t, xdp.frame_cnt, repeat, &ret, time));
> +	bpf_test_timer_leave(&t);
> +
> +	xdp_test_run_teardown(&xdp);
> +	return ret;
> +}
> +

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ