| Message ID | 80a0128d78f6c77210a8cccf7c5a78f53c45e7d3.1590571528.git.lorenzo@kernel.org |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | [v2,bpf-next] xdp: introduce convert_to_xdp_buff utility routine | expand |
On Wed, 27 May 2020 11:28:03 +0200 Lorenzo Bianconi <lorenzo@kernel.org> wrote: > Introduce convert_to_xdp_buff utility routine to initialize xdp_buff > fields from xdp_frames ones. Rely on convert_to_xdp_buff in veth xdp > code > > Suggested-by: Jesper Dangaard Brouer <brouer@redhat.com> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> > --- > Changes since v1: > - rely on frame->data pointer to compute xdp->data_hard_start one LGTM Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
On Wed, May 27, 2020 at 11:28:03AM +0200, Lorenzo Bianconi wrote: > Introduce convert_to_xdp_buff utility routine to initialize xdp_buff > fields from xdp_frames ones. Rely on convert_to_xdp_buff in veth xdp > code > > Suggested-by: Jesper Dangaard Brouer <brouer@redhat.com> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> > --- > Changes since v1: > - rely on frame->data pointer to compute xdp->data_hard_start one > --- > drivers/net/veth.c | 6 +----- > include/net/xdp.h | 10 ++++++++++ > 2 files changed, 11 insertions(+), 5 deletions(-) > > diff --git a/drivers/net/veth.c b/drivers/net/veth.c > index b586d2fa5551..9f91e79b7823 100644 > --- a/drivers/net/veth.c > +++ b/drivers/net/veth.c > @@ -575,11 +575,7 @@ static struct sk_buff *veth_xdp_rcv_one(struct veth_rq *rq, > struct xdp_buff xdp; > u32 act; > > - xdp.data_hard_start = hard_start; > - xdp.data = frame->data; > - xdp.data_end = frame->data + frame->len; > - xdp.data_meta = frame->data - frame->metasize; > - xdp.frame_sz = frame->frame_sz; > + convert_to_xdp_buff(frame, &xdp); > xdp.rxq = &rq->xdp_rxq; > > act = bpf_prog_run_xdp(xdp_prog, &xdp); > diff --git a/include/net/xdp.h b/include/net/xdp.h > index 90f11760bd12..df99d5d267b2 100644 > --- a/include/net/xdp.h > +++ b/include/net/xdp.h > @@ -106,6 +106,16 @@ void xdp_warn(const char *msg, const char *func, const int line); > > struct xdp_frame *xdp_convert_zc_to_xdp_frame(struct xdp_buff *xdp); > > +static inline > +void convert_to_xdp_buff(struct xdp_frame *frame, struct xdp_buff *xdp) > +{ > + xdp->data_hard_start = frame->data - frame->headroom - sizeof(*frame); > + xdp->data = frame->data; > + xdp->data_end = frame->data + frame->len; > + xdp->data_meta = frame->data - frame->metasize; > + xdp->frame_sz = frame->frame_sz; > +} From an API PoV, could we please prefix these with xdp_*(). Looks like there is also convert_to_xdp_frame() as an outlier in there, but lets clean these up once in the tree to avoid getting this more messy and harder to fix later on. How about: - xdp_convert_frame_to_buff() - xdp_convert_buff_to_frame() This will have both more self-documented and makes it obvious from where to where we convert something. Thanks, Daniel
> On Wed, May 27, 2020 at 11:28:03AM +0200, Lorenzo Bianconi wrote: > > Introduce convert_to_xdp_buff utility routine to initialize xdp_buff > > fields from xdp_frames ones. Rely on convert_to_xdp_buff in veth xdp > > code > > > > Suggested-by: Jesper Dangaard Brouer <brouer@redhat.com> > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> > > --- > > Changes since v1: > > - rely on frame->data pointer to compute xdp->data_hard_start one > > --- > > drivers/net/veth.c | 6 +----- > > include/net/xdp.h | 10 ++++++++++ > > 2 files changed, 11 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/net/veth.c b/drivers/net/veth.c > > index b586d2fa5551..9f91e79b7823 100644 > > --- a/drivers/net/veth.c > > +++ b/drivers/net/veth.c > > @@ -575,11 +575,7 @@ static struct sk_buff *veth_xdp_rcv_one(struct veth_rq *rq, > > struct xdp_buff xdp; > > u32 act; > > > > - xdp.data_hard_start = hard_start; > > - xdp.data = frame->data; > > - xdp.data_end = frame->data + frame->len; > > - xdp.data_meta = frame->data - frame->metasize; > > - xdp.frame_sz = frame->frame_sz; > > + convert_to_xdp_buff(frame, &xdp); > > xdp.rxq = &rq->xdp_rxq; > > > > act = bpf_prog_run_xdp(xdp_prog, &xdp); > > diff --git a/include/net/xdp.h b/include/net/xdp.h > > index 90f11760bd12..df99d5d267b2 100644 > > --- a/include/net/xdp.h > > +++ b/include/net/xdp.h > > @@ -106,6 +106,16 @@ void xdp_warn(const char *msg, const char *func, const int line); > > > > struct xdp_frame *xdp_convert_zc_to_xdp_frame(struct xdp_buff *xdp); > > > > +static inline > > +void convert_to_xdp_buff(struct xdp_frame *frame, struct xdp_buff *xdp) > > +{ > > + xdp->data_hard_start = frame->data - frame->headroom - sizeof(*frame); > > + xdp->data = frame->data; > > + xdp->data_end = frame->data + frame->len; > > + xdp->data_meta = frame->data - frame->metasize; > > + xdp->frame_sz = frame->frame_sz; > > +} > > From an API PoV, could we please prefix these with xdp_*(). Looks like there > is also convert_to_xdp_frame() as an outlier in there, but lets clean these > up once in the tree to avoid getting this more messy and harder to fix later > on. How about: > > - xdp_convert_frame_to_buff() > - xdp_convert_buff_to_frame() > > This will have both more self-documented and makes it obvious from where to > where we convert something. ack fine to me, I will fix it in v3. Regards, Lorenzo > > Thanks, > Daniel >
diff --git a/drivers/net/veth.c b/drivers/net/veth.c index b586d2fa5551..9f91e79b7823 100644 --- a/drivers/net/veth.c +++ b/drivers/net/veth.c @@ -575,11 +575,7 @@ static struct sk_buff *veth_xdp_rcv_one(struct veth_rq *rq, struct xdp_buff xdp; u32 act; - xdp.data_hard_start = hard_start; - xdp.data = frame->data; - xdp.data_end = frame->data + frame->len; - xdp.data_meta = frame->data - frame->metasize; - xdp.frame_sz = frame->frame_sz; + convert_to_xdp_buff(frame, &xdp); xdp.rxq = &rq->xdp_rxq; act = bpf_prog_run_xdp(xdp_prog, &xdp); diff --git a/include/net/xdp.h b/include/net/xdp.h index 90f11760bd12..df99d5d267b2 100644 --- a/include/net/xdp.h +++ b/include/net/xdp.h @@ -106,6 +106,16 @@ void xdp_warn(const char *msg, const char *func, const int line); struct xdp_frame *xdp_convert_zc_to_xdp_frame(struct xdp_buff *xdp); +static inline +void convert_to_xdp_buff(struct xdp_frame *frame, struct xdp_buff *xdp) +{ + xdp->data_hard_start = frame->data - frame->headroom - sizeof(*frame); + xdp->data = frame->data; + xdp->data_end = frame->data + frame->len; + xdp->data_meta = frame->data - frame->metasize; + xdp->frame_sz = frame->frame_sz; +} + /* Convert xdp_buff to xdp_frame */ static inline struct xdp_frame *convert_to_xdp_frame(struct xdp_buff *xdp)
Introduce convert_to_xdp_buff utility routine to initialize xdp_buff fields from xdp_frames ones. Rely on convert_to_xdp_buff in veth xdp code Suggested-by: Jesper Dangaard Brouer <brouer@redhat.com> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> --- Changes since v1: - rely on frame->data pointer to compute xdp->data_hard_start one --- drivers/net/veth.c | 6 +----- include/net/xdp.h | 10 ++++++++++ 2 files changed, 11 insertions(+), 5 deletions(-)