Re: [PATCH v2 net-next 1/4] bpf: xdp: Allow head adjustment in XDP prog

[John Fastabend <john.fastabend@gmail.com>]

On 16-12-03 07:17 PM, Martin KaFai Lau wrote:
> This patch allows XDP prog to extend/remove the packet
> data at the head (like adding or removing header).  It is
> done by adding a new XDP helper bpf_xdp_adjust_head().
> 
> It also renames bpf_helper_changes_skb_data() to
> bpf_helper_changes_pkt_data() to better reflect
> that XDP prog does not work on skb.
> 
> Acked-by: Alexei Starovoitov <ast@kernel.org>
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> ---


[...]

>  
> -bool bpf_helper_changes_skb_data(void *func)
> +BPF_CALL_2(bpf_xdp_adjust_head, struct xdp_buff *, xdp, int, offset)
> +{
> +	/* Both mlx4 and mlx5 driver align each packet to PAGE_SIZE when
> +	 * XDP prog is set.
> +	 * If the above is not true for the other drivers to support
> +	 * bpf_xdp_adjust_head, struct xdp_buff can be extended.
> +	 */
> +	unsigned long addr = (unsigned long)xdp->data & PAGE_MASK;
> +	void *data_hard_start = (void *)addr;
> +	void *data = xdp->data + offset;
> +
> +	if (unlikely(data < data_hard_start || data > xdp->data_end - ETH_HLEN))
> +		return -EINVAL;
> +
> +	xdp->data = data;
> +
> +	return 0;
> +}
> +

Sorry for the delay but I don't like the assumptions being made here
with regards to page alignment and free space.

Instead of taking the offset from PAGE_MASK how about adding a pointer
to xdp_buff so that we get,

 struct xdp_buff {
 	void *data;
	void *data_start;
	void *data_end;
 };

This gives the headroom explicitly in the data structure. This way we
can handle non-paged aligned usages and also some of the drivers are
putting metadata (descriptors) at the front of the page. With this
patch we could stomp on that metadata with the above we avoid that
problem altogether.

Thanks,
John