Re: [PATCH v2 net-next 7/9] bpf: helpers: add multibuffer support

On Thu, Sep 03, 2020 at 10:58:51PM +0200, Lorenzo Bianconi wrote:

quoted

From: Sameeh Jubran <redacted>

The implementation is based on this [0] draft by Jesper D. Brouer.

Provided two new helpers:

* bpf_xdp_get_frag_count()
* bpf_xdp_get_frags_total_size()

[0] xdp mb design - https://github.com/xdp-project/xdp-project/blob/master/areas/core/xdp-multi-buffer01-design.org
Signed-off-by: Sameeh Jubran <redacted>
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 include/uapi/linux/bpf.h       | 14 ++++++++++++
 net/core/filter.c              | 39 ++++++++++++++++++++++++++++++++++
 tools/include/uapi/linux/bpf.h | 14 ++++++++++++
 3 files changed, 67 insertions(+)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index c4a6d245619c..53db75095306 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h

@@ -3590,6 +3590,18 @@ union bpf_attr {
  *
  *	Return
  *		0 on success, or a negative error in case of failure.

[...]

quoted

+

I only quickly jumped through series and IMHO this helper should be
rewritten/optimized in a way that we bail out as early as possible if
!xdp->mb as the rest of the code on that condition will do nothing and i'm
not sure if compiler would optimize it.


	struct skb_shared_info *sinfo;
	int nfrags, i;
	int size = 0;

	if (!xdp->mb)
		return 0;

	sinfo = xdp_get_shared_info_from_buff(xdp);

	nfrags = min(sinfo->nr_frags, MAX_SKB_FRAGS);

	for (i = 0; i < nfrags; i++)
		size += skb_frag_size(&sinfo->frags[i]);

	return size;

Thoughts?

I agree.

Regards,
Lorenzo

quoted

+	return size;
+}
+
+const struct bpf_func_proto bpf_xdp_get_frags_total_size_proto = {
+	.func		= bpf_xdp_get_frags_total_size,
+	.gpl_only	= false,
+	.ret_type	= RET_INTEGER,
+	.arg1_type	= ARG_PTR_TO_CTX,
+};
+
 BPF_CALL_2(bpf_xdp_adjust_tail, struct xdp_buff *, xdp, int, offset)
 {
 	void *data_hard_end = xdp_data_hard_end(xdp); /* use xdp->frame_sz */

@@ -6889,6 +6924,10 @@ xdp_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return &bpf_xdp_adjust_tail_proto;
 	case BPF_FUNC_xdp_adjust_mb_header:
 		return &bpf_xdp_adjust_mb_header_proto;
+	case BPF_FUNC_xdp_get_frag_count:
+		return &bpf_xdp_get_frag_count_proto;
+	case BPF_FUNC_xdp_get_frags_total_size:
+		return &bpf_xdp_get_frags_total_size_proto;
 	case BPF_FUNC_fib_lookup:
 		return &bpf_xdp_fib_lookup_proto;
 #ifdef CONFIG_INET

diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 392d52a2ecef..dd4669096cbb 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h

@@ -3591,6 +3591,18 @@ union bpf_attr {
  *
  *	Return
  *		0 on success, or a negative error in case of failure.
+ *
+ * int bpf_xdp_get_frag_count(struct xdp_buff *xdp_md)
+ *	Description
+ *		Get the total number of frags for a given packet.
+ *	Return
+ *		The number of frags
+ *
+ * int bpf_xdp_get_frags_total_size(struct xdp_buff *xdp_md)
+ *	Description
+ *		Get the total size of frags for a given packet.
+ *	Return
+ *		The total size of frags for a given packet.
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\

@@ -3743,6 +3755,8 @@ union bpf_attr {
 	FN(d_path),			\
 	FN(copy_from_user),		\
 	FN(xdp_adjust_mb_header),	\
+	FN(xdp_get_frag_count),		\
+	FN(xdp_get_frags_total_size),	\
 	/* */
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper

-- 
2.26.2