On 2020/3/31 7:35, Toshiaki Makita wrote: > Hi Mao & Jesper > (Resending with plain text...) > > On 2020/03/30 20:34, Jesper Dangaard Brouer wrote: >> On Mon, 30 Mar 2020 18:26:31 +0800 >> Mao Wenan <maowenan@xxxxxxxxxx> wrote: >> >>> xdp.data_hard_start is mapped to the first >>> address of xdp_frame, but the pointer hard_start >>> is the offset(sizeof(struct xdp_frame)) of xdp_frame, >>> it should use head instead of hard_start to >>> set xdp.data_hard_start. Otherwise, if BPF program >>> calls helper_function such as bpf_xdp_adjust_head, it >>> will be confused for xdp_frame_end. >> >> I have noticed this[1] and have a patch in my current patchset for >> fixing this. IMHO is is not so important fix right now, as the effect >> is that you currently only lose 32 bytes of headroom. >> I consider that it is needed because bpf_xdp_adjust_head() just a common helper function, veth as one driver application should keep the same as 32 bytes of headroom as other driver. And convert_to_xdp_frame set() also store info in top of packet, and set: xdp_frame = xdp->data_hard_start; >> [1] https://lore.kernel.org/netdev/158446621887.702578.17234304084556809684.stgit@firesoul/ > > You are right, the subtraction is not necessary here. I guess you mean that previous subtraction is not necessary ? this line : void *head = hard_start - sizeof(struct xdp_frame); ? But in the veth_xdp_rcv_one,below line will use head pointer, case XDP_TX: orig_frame = *frame; xdp.data_hard_start = head; > Thank you for working on this. > > Toshiaki Makita > > .
