On Thu, 4 Nov 2021 18:35:33 +0100 Lorenzo Bianconi wrote:
> -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);
> + unsigned long base_len, copy_len, frag_off_total;
> + struct xdp_buff *xdp = (struct xdp_buff *)ctx;
> + struct skb_shared_info *sinfo;
> + int i;
> +
> + if (likely(!xdp_buff_is_mb(xdp))) {
Would it be better to do
if (xdp->data_end - xdp->data >= off + len)
?
> + memcpy(dst_buff, xdp->data + off, len);
> + return 0;
> + }
> +
> + base_len = xdp->data_end - xdp->data;
> + frag_off_total = base_len;
> + sinfo = xdp_get_shared_info_from_buff(xdp);
> +
> + /* If we need to copy data from the base buffer do it */
> + if (off < base_len) {
> + copy_len = min(len, base_len - off);
> + memcpy(dst_buff, xdp->data + off, copy_len);
> +
> + off += copy_len;
> + len -= copy_len;
> + dst_buff += copy_len;
> + }
> +
> + /* Copy any remaining data from the fragments */
> + for (i = 0; len && i < sinfo->nr_frags; i++) {
> + skb_frag_t *frag = &sinfo->frags[i];
> + unsigned long frag_len, frag_off;
> +
> + frag_len = skb_frag_size(frag);
> + frag_off = off - frag_off_total;
> + if (frag_off < frag_len) {
> + copy_len = min(len, frag_len - frag_off);
> + memcpy(dst_buff,
> + skb_frag_address(frag) + frag_off, copy_len);
> +
> + off += copy_len;
> + len -= copy_len;
> + dst_buff += copy_len;
> + }
> + frag_off_total += frag_len;
> + }
> +
nit: can't help but feel that you can merge base copy and frag copy:
sinfo = xdp_get_shared_info_from_buff(xdp);
next_frag = &sinfo->frags[0];
end_frag = &sinfo->frags[sinfo->nr_frags];
ptr_off = 0;
ptr_buf = xdp->data;
ptr_len = xdp->data_end - xdp->data;
while (true) {
if (off < ptr_off + ptr_len) {
copy_off = ptr_off - off;
copy_len = min(len, ptr_len - copy_off);
memcpy(dst_buff, ptr_buf + copy_off, copy_len);
off += copy_len;
len -= copy_len;
dst_buff += copy_len;
}
if (!len || next_frag == end_frag)
break;
ptr_off += ptr_len;
ptr_buf = skb_frag_address(next_frag);
ptr_len = skb_frag_size(next_frag);
next_frag++;
}
Up to you.
