On Thu, 2020-12-10 at 18:59 +0100, Lorenzo Bianconi wrote: > On Dec 10, Maciej Fijalkowski wrote: > > On Thu, Dec 10, 2020 at 05:32:41PM +0100, Lorenzo Bianconi wrote: > > > > On Thu, Dec 10, 2020 at 04:50:42PM +0100, Lorenzo Bianconi > > > > wrote: > > > > > Introduce xdp_init_buff utility routine to initialize > > > > > xdp_buff data > > > > > structure. Rely on xdp_init_buff in all XDP capable drivers. > > > > > > > > Hm, Jesper was suggesting two helpers, one that you implemented > > > > for things > > > > that are set once per NAPI and the other that is set per each > > > > buffer. > > > > > > > > Not sure about the naming for a second one - xdp_prepare_buff ? > > > > xdp_init_buff that you have feels ok. > > > > > > ack, so we can have xdp_init_buff() for initialization done once > > > per NAPI run and > > > xdp_prepare_buff() for per-NAPI iteration initialization, e.g. > > > > > > static inline void > > > xdp_prepare_buff(struct xdp_buff *xdp, unsigned char *hard_start, > > > int headroom, int data_len) > > > { > > > xdp->data_hard_start = hard_start; > > > xdp->data = hard_start + headroom; > > > xdp->data_end = xdp->data + data_len; > > > xdp_set_data_meta_invalid(xdp); > > > } > > > > I think we should allow for setting the data_meta as well. > > x64 calling convention states that first four args are placed onto > > registers, so to keep it fast maybe have a third helper: > > > > static inline void > > xdp_prepare_buff_meta(struct xdp_buff *xdp, unsigned char > > *hard_start, > > int headroom, int data_len) > > { > > xdp->data_hard_start = hard_start; > > xdp->data = hard_start + headroom; > > xdp->data_end = xdp->data + data_len; > > xdp->data_meta = xdp->data; > > } > > > > Thoughts? > > ack, I am fine with it. Let's wait for some feedback. > > Do you prefer to have xdp_prepare_buff/xdp_prepare_buff_meta in the > same series > of xdp_buff_init() or is it ok to address it in a separate patch? > you only need 2 why do you need xpd_prepare_buff_meta? that's exactly what xdp_set_data_meta_invalid(xdp) is all about.