On Thu, 9 Apr 2020 00:48:30 +0000 Saeed Mahameed wrote: > > > + * This macro reserves tailroom in the XDP buffer by limiting the > > > + * XDP/BPF data access to data_hard_end. Notice same area (and > > > size) > > > + * is used for XDP_PASS, when constructing the SKB via > > > build_skb(). > > > + */ > > > +#define xdp_data_hard_end(xdp) \ > > > + ((xdp)->data_hard_start + (xdp)->frame_sz - \ > > > + SKB_DATA_ALIGN(sizeof(struct skb_shared_info))) > > > > I think it should be said somewhere that the drivers are expected to > > DMA map memory up to xdp_data_hard_end(xdp). > > > > but this works on a specific xdp buff, drivers work with mtu > > and what if the driver want to have this as an option per packet .. > i.e.: if there is enough tail room, then build_skb, otherwise > alloc new skb, copy headers, setup data frags.. etc > > having such limitations on driver can be very strict, i think the > decision must remain dynamic per frame.. > > of-course drivers should optimize to preserve enough tail room for all > rx packets.. My concern is that driver may allocate a full page for each frame but only DMA map the amount that can reasonably contain data given the MTU. To save on DMA syncs. Today that wouldn't be a problem, because XDP_REDIRECT will re-map the page, and XDP_TX has the same MTU. In this set xdp_data_hard_end is used both to find the end of memory buffer, and end of DMA buffer. Implementation of bpf_xdp_adjust_tail() assumes anything < SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) from the end is fair game. So I was trying to say that we should warn driver authors that the DMA buffer can now grow / move beyond what the driver may expect in XDP_TX. Drivers can either DMA map enough memory, or handle the corner case in a special way. IDK if that makes sense, we may be talking past each other :)