Re: [PATCH v2 bpf-next] net: veth: alloc skb in bulk for ndo_xdp_xmit

On 2021/01/29 4:41, Lorenzo Bianconi wrote:

quoted

Split ndo_xdp_xmit and ndo_start_xmit use cases in veth_xdp_rcv routine
in order to alloc skbs in bulk for XDP_PASS verdict.
Introduce xdp_alloc_skb_bulk utility routine to alloc skb bulk list.
The proposed approach has been tested in the following scenario:

eth (ixgbe) --> XDP_REDIRECT --> veth0 --> (remote-ns) veth1 --> XDP_PASS

XDP_REDIRECT: xdp_redirect_map bpf sample
XDP_PASS: xdp_rxq_info bpf sample

traffic generator: pkt_gen sending udp traffic on a remote device

bpf-next master: ~3.64Mpps
bpf-next + skb bulking allocation: ~3.75Mpps

Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---

...

quoted

+/* frames array contains VETH_XDP_BATCH at most */
+static void veth_xdp_rcv_batch(struct veth_rq *rq, void **frames,
+			       int n_xdpf, struct veth_xdp_tx_bq *bq,
+			       struct veth_stats *stats)
+{
+	void *skbs[VETH_XDP_BATCH];
+	int i, n_skb = 0;
+
+	for (i = 0; i < n_xdpf; i++) {
+		struct xdp_frame *frame = frames[i];
+
+		stats->xdp_bytes += frame->len;
+		frame = veth_xdp_rcv_one(rq, frame, bq, stats);
+		if (frame)
+			frames[n_skb++] = frame;
+	}

Maybe we can move this block to veth_xdp_rcv() and make the logic even more simple?

Something like this:

static int veth_xdp_rcv(struct veth_rq *rq, int budget,
	...
		if (veth_is_xdp_frame(ptr)) {
			struct xdp_frame *frame = veth_ptr_to_xdp(ptr);

			stats->xdp_bytes += frame->len;
			frame = veth_xdp_rcv_one(rq, frame, bq, stats);
			if (frame) {
				xdpf[n_xdpf++] = frame;
				if (n_xdpf == VETH_XDP_BATCH) {
					veth_xdp_rcv_batch(rq, xdpf, n_xdpf, bq,
							   stats);
					n_xdpf = 0;
				}
			}

Then we can fully make use of skb bulk allocation as xdpf[] does not include
frames which may be XDP_TX'ed or XDP_REDIRECT'ed.

WDYT?

I gues the code is more readable, I will fix in v3. Thanks.

Regards,
Lorenzo

Toshiaki Makita