On 8 Apr 2021, at 23:04, Vladimir Oltean wrote:
On Thu, Apr 08, 2021 at 02:51:01PM +0200, Lorenzo Bianconi wrote:
From: Eelco Chaudron <echaudro@xxxxxxxxxx>
This patch adds support for multi-buffer for the following helpers:
- bpf_xdp_output()
- bpf_perf_event_output()
Signed-off-by: Eelco Chaudron <echaudro@xxxxxxxxxx>
Signed-off-by: Lorenzo Bianconi <lorenzo@xxxxxxxxxx>
---
Also there is a typo in the commit message: bpd -> bpf.
ACK, will fix in next version.
net/core/filter.c | 63 ++++++++-
.../selftests/bpf/prog_tests/xdp_bpf2bpf.c | 127
++++++++++++------
.../selftests/bpf/progs/test_xdp_bpf2bpf.c | 3 +-
3 files changed, 149 insertions(+), 44 deletions(-)
diff --git a/net/core/filter.c b/net/core/filter.c
index c4eb1392f88e..c00f52ab2532 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -4549,10 +4549,56 @@ static const struct bpf_func_proto
bpf_sk_ancestor_cgroup_id_proto = {
};
#endif
-static unsigned long bpf_xdp_copy(void *dst_buff, const void
*src_buff,
+static unsigned long bpf_xdp_copy(void *dst_buff, const void *ctx,
unsigned long off, unsigned long len)
{
- memcpy(dst_buff, src_buff + off, len);
+ struct xdp_buff *xdp = (struct xdp_buff *)ctx;
There is no need to cast a void pointer in C.
I added this as the void pointer is a const. However, looking at it
again, we should probably change xdp_get_shared_info_from_buff() to also
take a const pointer, i.e.:
static inline struct xdp_shared_info *
-xdp_get_shared_info_from_buff(struct xdp_buff *xdp)
+xdp_get_shared_info_from_buff(const struct xdp_buff *xdp)
{
BUILD_BUG_ON(sizeof(struct xdp_shared_info) >
sizeof(struct skb_shared_info));
What do you think Lorenzo?
+ struct xdp_shared_info *xdp_sinfo;
+ unsigned long base_len;
+
+ if (likely(!xdp->mb)) {
+ memcpy(dst_buff, xdp->data + off, len);
+ return 0;
+ }
+
+ base_len = xdp->data_end - xdp->data;
Would a static inline int xdp_buff_head_len() be useful?
Guess everybody is using the xdp->data_end - xdp->data, in there code.
But I guess we can add a static inline and change all code, but I
don’t think we should do it as part of this patchset. I would also
call it something like xdp_buff_data_len()?
+ xdp_sinfo = xdp_get_shared_info_from_buff(xdp);
+ do {
+ const void *src_buff = NULL;
+ unsigned long copy_len = 0;
+
+ if (off < base_len) {
+ src_buff = xdp->data + off;
+ copy_len = min(len, base_len - off);
+ } else {
+ unsigned long frag_off_total = base_len;
+ int i;
+
+ for (i = 0; i < xdp_sinfo->nr_frags; i++) {
+ skb_frag_t *frag = &xdp_sinfo->frags[i];
+ unsigned long frag_len, frag_off;
+
+ frag_len = xdp_get_frag_size(frag);
+ frag_off = off - frag_off_total;
+ if (frag_off < frag_len) {
+ src_buff = xdp_get_frag_address(frag) +
+ frag_off;
+ copy_len = min(len,
+ frag_len - frag_off);
+ break;
+ }
+ frag_off_total += frag_len;
+ }
+ }
+ if (!src_buff)
+ break;
+
+ memcpy(dst_buff, src_buff, copy_len);
+ off += copy_len;
+ len -= copy_len;
+ dst_buff += copy_len;
+ } while (len);
+
return 0;
}
