On Mon, Apr 23, 2018 at 9:56 AM, Björn Töpel <[email protected]> wrote:
> From: Björn Töpel <[email protected]>
>
> In this commit the base structure of the AF_XDP address family is set
> up. Further, we introduce the abilty register a window of user memory
> to the kernel via the XDP_UMEM_REG setsockopt syscall. The memory
> window is viewed by an AF_XDP socket as a set of equally large
> frames. After a user memory registration all frames are "owned" by the
> user application, and not the kernel.
>
> Co-authored-by: Magnus Karlsson <[email protected]>
> Signed-off-by: Magnus Karlsson <[email protected]>
> Signed-off-by: Björn Töpel <[email protected]>
> +static void xdp_umem_release(struct xdp_umem *umem)
> +{
> + struct task_struct *task;
> + struct mm_struct *mm;
> + unsigned long diff;
> +
> + if (umem->pgs) {
> + xdp_umem_unpin_pages(umem);
> +
> + task = get_pid_task(umem->pid, PIDTYPE_PID);
> + put_pid(umem->pid);
> + if (!task)
> + goto out;
> + mm = get_task_mm(task);
> + put_task_struct(task);
> + if (!mm)
> + goto out;
> +
> + diff = umem->size >> PAGE_SHIFT;
Need to round up or size must always be a multiple of PAGE_SIZE.
> +
> + down_write(&mm->mmap_sem);
> + mm->pinned_vm -= diff;
> + up_write(&mm->mmap_sem);
When using user->locked_vm for resource limit checks, no need
to also update mm->pinned_vm?
> +static int __xdp_umem_reg(struct xdp_umem *umem, struct xdp_umem_reg *mr)
> +{
> + u32 frame_size = mr->frame_size, frame_headroom = mr->frame_headroom;
> + u64 addr = mr->addr, size = mr->len;
> + unsigned int nframes;
> + int size_chk, err;
> +
> + if (frame_size < XDP_UMEM_MIN_FRAME_SIZE || frame_size > PAGE_SIZE) {
> + /* Strictly speaking we could support this, if:
> + * - huge pages, or*
> + * - using an IOMMU, or
> + * - making sure the memory area is consecutive
> + * but for now, we simply say "computer says no".
> + */
> + return -EINVAL;
> + }
Ideally, AF_XDP subsumes all packet socket use cases. It does not
have packet v3's small packet optimizations of variable sized frames
and block signaling.
I don't suggest adding that now. But for the non-zerocopy case, it may
make sense to ensure that nothing is blocking a later addition of these
features. Especially for header-only (snaplen) workloads. So far, I don't
see any issues.
> + if (!is_power_of_2(frame_size))
> + return -EINVAL;
> +
> + if (!PAGE_ALIGNED(addr)) {
> + /* Memory area has to be page size aligned. For
> + * simplicity, this might change.
> + */
> + return -EINVAL;
> + }
> +
> + if ((addr + size) < addr)
> + return -EINVAL;
> +
> + nframes = size / frame_size;
> + if (nframes == 0 || nframes > UINT_MAX)
> + return -EINVAL;
You may also want a check here that nframes * frame_size is at least
PAGE_SIZE and probably a multiple of that.
> + frame_headroom = ALIGN(frame_headroom, 64);
> +
> + size_chk = frame_size - frame_headroom - XDP_PACKET_HEADROOM;
> + if (size_chk < 0)
> + return -EINVAL;
> +
> + umem->pid = get_task_pid(current, PIDTYPE_PID);
> + umem->size = (size_t)size;
> + umem->address = (unsigned long)addr;
> + umem->props.frame_size = frame_size;
> + umem->props.nframes = nframes;
> + umem->frame_headroom = frame_headroom;
> + umem->npgs = size / PAGE_SIZE;
> + umem->pgs = NULL;
> + umem->user = NULL;
> +
> + umem->frame_size_log2 = ilog2(frame_size);
> + umem->nfpp_mask = (PAGE_SIZE / frame_size) - 1;
> + umem->nfpplog2 = ilog2(PAGE_SIZE / frame_size);
> + atomic_set(&umem->users, 1);
> +
> + err = xdp_umem_account_pages(umem);
> + if (err)
> + goto out;
> +
> + err = xdp_umem_pin_pages(umem);
> + if (err)
need to call xdp_umem_unaccount_pages on error
> + goto out;
> + return 0;
> +
> +out:
> + put_pid(umem->pid);
> + return err;
> +}
