> 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.
IIUC what Maciej means is to avoid to overwrite xdp->data_meta with
xdp_set_data_meta_invalid() after setting it to xdp->data in
xdp_prepare_buff_meta().
I guess setting xdp->data_meta to xdp->data is valid, it means an empty meta
area.
Anyway I guess we can set xdp->data_meta to xdp->data wherever we need and just
keep xdp_prepare_buff(). Agree?
Regards,
Lorenzo
>
>
Attachment:
signature.asc
Description: PGP signature
