* [PATCH bpf-next 1/2] net: xdp: introduce __xdp_build_skb_from_frame utility routine
2020-12-16 18:38 [PATCH bpf-next 0/2] add xdp_build_skb_from_frame utility routine Lorenzo Bianconi
@ 2020-12-16 18:38 ` Lorenzo Bianconi
2021-01-12 16:46 ` Jesper Dangaard Brouer
2020-12-16 18:38 ` [PATCH bpf-next 2/2] net: xdp: introduce xdp_build_skb_from_frame " Lorenzo Bianconi
2021-01-12 16:08 ` [PATCH bpf-next 0/2] add " Lorenzo Bianconi
2 siblings, 1 reply; 9+ messages in thread
From: Lorenzo Bianconi @ 2020-12-16 18:38 UTC (permalink / raw)
To: bpf, netdev
Cc: davem, kuba, ast, daniel, brouer, lorenzo.bianconi, toshiaki.makita1
Introduce __xdp_build_skb_from_frame utility routine to build
the skb from xdp_frame. Rely on __xdp_build_skb_from_frame in
cpumap code.
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
include/net/xdp.h | 3 +++
kernel/bpf/cpumap.c | 45 +--------------------------------------------
net/core/xdp.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 48 insertions(+), 44 deletions(-)
diff --git a/include/net/xdp.h b/include/net/xdp.h
index 600acb307db6..da62fa20fa8a 100644
--- a/include/net/xdp.h
+++ b/include/net/xdp.h
@@ -145,6 +145,9 @@ void xdp_warn(const char *msg, const char *func, const int line);
#define XDP_WARN(msg) xdp_warn(msg, __func__, __LINE__)
struct xdp_frame *xdp_convert_zc_to_xdp_frame(struct xdp_buff *xdp);
+struct sk_buff *__xdp_build_skb_from_frame(struct xdp_frame *xdpf,
+ struct sk_buff *skb,
+ struct net_device *dev);
static inline
void xdp_convert_frame_to_buff(struct xdp_frame *frame, struct xdp_buff *xdp)
diff --git a/kernel/bpf/cpumap.c b/kernel/bpf/cpumap.c
index 747313698178..49113880b82a 100644
--- a/kernel/bpf/cpumap.c
+++ b/kernel/bpf/cpumap.c
@@ -141,49 +141,6 @@ static void cpu_map_kthread_stop(struct work_struct *work)
kthread_stop(rcpu->kthread);
}
-static struct sk_buff *cpu_map_build_skb(struct xdp_frame *xdpf,
- struct sk_buff *skb)
-{
- unsigned int hard_start_headroom;
- unsigned int frame_size;
- void *pkt_data_start;
-
- /* Part of headroom was reserved to xdpf */
- hard_start_headroom = sizeof(struct xdp_frame) + xdpf->headroom;
-
- /* Memory size backing xdp_frame data already have reserved
- * room for build_skb to place skb_shared_info in tailroom.
- */
- frame_size = xdpf->frame_sz;
-
- pkt_data_start = xdpf->data - hard_start_headroom;
- skb = build_skb_around(skb, pkt_data_start, frame_size);
- if (unlikely(!skb))
- return NULL;
-
- skb_reserve(skb, hard_start_headroom);
- __skb_put(skb, xdpf->len);
- if (xdpf->metasize)
- skb_metadata_set(skb, xdpf->metasize);
-
- /* Essential SKB info: protocol and skb->dev */
- skb->protocol = eth_type_trans(skb, xdpf->dev_rx);
-
- /* Optional SKB info, currently missing:
- * - HW checksum info (skb->ip_summed)
- * - HW RX hash (skb_set_hash)
- * - RX ring dev queue index (skb_record_rx_queue)
- */
-
- /* Until page_pool get SKB return path, release DMA here */
- xdp_release_frame(xdpf);
-
- /* Allow SKB to reuse area used by xdp_frame */
- xdp_scrub_frame(xdpf);
-
- return skb;
-}
-
static void __cpu_map_ring_cleanup(struct ptr_ring *ring)
{
/* The tear-down procedure should have made sure that queue is
@@ -350,7 +307,7 @@ static int cpu_map_kthread_run(void *data)
struct sk_buff *skb = skbs[i];
int ret;
- skb = cpu_map_build_skb(xdpf, skb);
+ skb = __xdp_build_skb_from_frame(xdpf, skb, xdpf->dev_rx);
if (!skb) {
xdp_return_frame(xdpf);
continue;
diff --git a/net/core/xdp.c b/net/core/xdp.c
index 3a8c9ab4ecbe..aeb09ed0704c 100644
--- a/net/core/xdp.c
+++ b/net/core/xdp.c
@@ -513,3 +513,47 @@ void xdp_warn(const char *msg, const char *func, const int line)
WARN(1, "XDP_WARN: %s(line:%d): %s\n", func, line, msg);
};
EXPORT_SYMBOL_GPL(xdp_warn);
+
+struct sk_buff *__xdp_build_skb_from_frame(struct xdp_frame *xdpf,
+ struct sk_buff *skb,
+ struct net_device *dev)
+{
+ unsigned int headroom, frame_size;
+ void *hard_start;
+
+ /* Part of headroom was reserved to xdpf */
+ headroom = sizeof(*xdpf) + xdpf->headroom;
+
+ /* Memory size backing xdp_frame data already have reserved
+ * room for build_skb to place skb_shared_info in tailroom.
+ */
+ frame_size = xdpf->frame_sz;
+
+ hard_start = xdpf->data - headroom;
+ skb = build_skb_around(skb, hard_start, frame_size);
+ if (unlikely(!skb))
+ return NULL;
+
+ skb_reserve(skb, headroom);
+ __skb_put(skb, xdpf->len);
+ if (xdpf->metasize)
+ skb_metadata_set(skb, xdpf->metasize);
+
+ /* Essential SKB info: protocol and skb->dev */
+ skb->protocol = eth_type_trans(skb, dev);
+
+ /* Optional SKB info, currently missing:
+ * - HW checksum info (skb->ip_summed)
+ * - HW RX hash (skb_set_hash)
+ * - RX ring dev queue index (skb_record_rx_queue)
+ */
+
+ /* Until page_pool get SKB return path, release DMA here */
+ xdp_release_frame(xdpf);
+
+ /* Allow SKB to reuse area used by xdp_frame */
+ xdp_scrub_frame(xdpf);
+
+ return skb;
+}
+EXPORT_SYMBOL_GPL(__xdp_build_skb_from_frame);
--
2.29.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next 1/2] net: xdp: introduce __xdp_build_skb_from_frame utility routine
2020-12-16 18:38 ` [PATCH bpf-next 1/2] net: xdp: introduce __xdp_build_skb_from_frame " Lorenzo Bianconi
@ 2021-01-12 16:46 ` Jesper Dangaard Brouer
0 siblings, 0 replies; 9+ messages in thread
From: Jesper Dangaard Brouer @ 2021-01-12 16:46 UTC (permalink / raw)
To: Lorenzo Bianconi
Cc: bpf, netdev, davem, kuba, ast, daniel, lorenzo.bianconi,
toshiaki.makita1, brouer
On Wed, 16 Dec 2020 19:38:33 +0100
Lorenzo Bianconi <lorenzo@kernel.org> wrote:
> Introduce __xdp_build_skb_from_frame utility routine to build
> the skb from xdp_frame. Rely on __xdp_build_skb_from_frame in
> cpumap code.
>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
> include/net/xdp.h | 3 +++
> kernel/bpf/cpumap.c | 45 +--------------------------------------------
> net/core/xdp.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 48 insertions(+), 44 deletions(-)
Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH bpf-next 2/2] net: xdp: introduce xdp_build_skb_from_frame utility routine
2020-12-16 18:38 [PATCH bpf-next 0/2] add xdp_build_skb_from_frame utility routine Lorenzo Bianconi
2020-12-16 18:38 ` [PATCH bpf-next 1/2] net: xdp: introduce __xdp_build_skb_from_frame " Lorenzo Bianconi
@ 2020-12-16 18:38 ` Lorenzo Bianconi
2020-12-17 13:12 ` Toshiaki Makita
2021-01-12 16:50 ` Jesper Dangaard Brouer
2021-01-12 16:08 ` [PATCH bpf-next 0/2] add " Lorenzo Bianconi
2 siblings, 2 replies; 9+ messages in thread
From: Lorenzo Bianconi @ 2020-12-16 18:38 UTC (permalink / raw)
To: bpf, netdev
Cc: davem, kuba, ast, daniel, brouer, lorenzo.bianconi, toshiaki.makita1
Introduce xdp_build_skb_from_frame utility routine to build the skb
from xdp_frame. Respect to __xdp_build_skb_from_frame,
xdp_build_skb_from_frame will allocate the skb object. Rely on
xdp_build_skb_from_frame in veth driver.
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
drivers/net/veth.c | 18 +++---------------
include/net/xdp.h | 2 ++
net/core/xdp.c | 15 +++++++++++++++
3 files changed, 20 insertions(+), 15 deletions(-)
diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 02bfcdf50a7a..dbacf90df2b5 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -567,16 +567,10 @@ static struct sk_buff *veth_xdp_rcv_one(struct veth_rq *rq,
struct veth_xdp_tx_bq *bq,
struct veth_stats *stats)
{
- void *hard_start = frame->data - frame->headroom;
- int len = frame->len, delta = 0;
struct xdp_frame orig_frame;
struct bpf_prog *xdp_prog;
- unsigned int headroom;
struct sk_buff *skb;
- /* bpf_xdp_adjust_head() assures BPF cannot access xdp_frame area */
- hard_start -= sizeof(struct xdp_frame);
-
rcu_read_lock();
xdp_prog = rcu_dereference(rq->xdp_prog);
if (likely(xdp_prog)) {
@@ -590,8 +584,8 @@ static struct sk_buff *veth_xdp_rcv_one(struct veth_rq *rq,
switch (act) {
case XDP_PASS:
- delta = frame->data - xdp.data;
- len = xdp.data_end - xdp.data;
+ if (xdp_update_frame_from_buff(&xdp, frame))
+ goto err_xdp;
break;
case XDP_TX:
orig_frame = *frame;
@@ -629,18 +623,12 @@ static struct sk_buff *veth_xdp_rcv_one(struct veth_rq *rq,
}
rcu_read_unlock();
- headroom = sizeof(struct xdp_frame) + frame->headroom - delta;
- skb = veth_build_skb(hard_start, headroom, len, frame->frame_sz);
+ skb = xdp_build_skb_from_frame(frame, rq->dev);
if (!skb) {
xdp_return_frame(frame);
stats->rx_drops++;
- goto err;
}
- xdp_release_frame(frame);
- xdp_scrub_frame(frame);
- skb->protocol = eth_type_trans(skb, rq->dev);
-err:
return skb;
err_xdp:
rcu_read_unlock();
diff --git a/include/net/xdp.h b/include/net/xdp.h
index da62fa20fa8a..11ec93f827c0 100644
--- a/include/net/xdp.h
+++ b/include/net/xdp.h
@@ -148,6 +148,8 @@ struct xdp_frame *xdp_convert_zc_to_xdp_frame(struct xdp_buff *xdp);
struct sk_buff *__xdp_build_skb_from_frame(struct xdp_frame *xdpf,
struct sk_buff *skb,
struct net_device *dev);
+struct sk_buff *xdp_build_skb_from_frame(struct xdp_frame *xdpf,
+ struct net_device *dev);
static inline
void xdp_convert_frame_to_buff(struct xdp_frame *frame, struct xdp_buff *xdp)
diff --git a/net/core/xdp.c b/net/core/xdp.c
index aeb09ed0704c..0d2630a35c3e 100644
--- a/net/core/xdp.c
+++ b/net/core/xdp.c
@@ -557,3 +557,18 @@ struct sk_buff *__xdp_build_skb_from_frame(struct xdp_frame *xdpf,
return skb;
}
EXPORT_SYMBOL_GPL(__xdp_build_skb_from_frame);
+
+struct sk_buff *xdp_build_skb_from_frame(struct xdp_frame *xdpf,
+ struct net_device *dev)
+{
+ struct sk_buff *skb;
+
+ skb = kmem_cache_alloc(skbuff_head_cache, GFP_ATOMIC);
+ if (unlikely(!skb))
+ return NULL;
+
+ memset(skb, 0, offsetof(struct sk_buff, tail));
+
+ return __xdp_build_skb_from_frame(xdpf, skb, dev);
+}
+EXPORT_SYMBOL_GPL(xdp_build_skb_from_frame);
--
2.29.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next 2/2] net: xdp: introduce xdp_build_skb_from_frame utility routine
2020-12-16 18:38 ` [PATCH bpf-next 2/2] net: xdp: introduce xdp_build_skb_from_frame " Lorenzo Bianconi
@ 2020-12-17 13:12 ` Toshiaki Makita
2021-01-12 16:50 ` Jesper Dangaard Brouer
1 sibling, 0 replies; 9+ messages in thread
From: Toshiaki Makita @ 2020-12-17 13:12 UTC (permalink / raw)
To: Lorenzo Bianconi, bpf, netdev
Cc: davem, kuba, ast, daniel, brouer, lorenzo.bianconi
On 2020/12/17 3:38, Lorenzo Bianconi wrote:
> Introduce xdp_build_skb_from_frame utility routine to build the skb
> from xdp_frame. Respect to __xdp_build_skb_from_frame,
> xdp_build_skb_from_frame will allocate the skb object. Rely on
> xdp_build_skb_from_frame in veth driver.
>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
Thanks.
It seems you added missing metadata support in veth_xdp_rcv_one()?
It might be better to note that in the commitlog.
The code looks fine to me.
Reviewed-by: Toshiaki Makita <toshiaki.makita1@gmail.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next 2/2] net: xdp: introduce xdp_build_skb_from_frame utility routine
2020-12-16 18:38 ` [PATCH bpf-next 2/2] net: xdp: introduce xdp_build_skb_from_frame " Lorenzo Bianconi
2020-12-17 13:12 ` Toshiaki Makita
@ 2021-01-12 16:50 ` Jesper Dangaard Brouer
1 sibling, 0 replies; 9+ messages in thread
From: Jesper Dangaard Brouer @ 2021-01-12 16:50 UTC (permalink / raw)
To: Lorenzo Bianconi
Cc: bpf, netdev, davem, kuba, ast, daniel, lorenzo.bianconi,
toshiaki.makita1, brouer
On Wed, 16 Dec 2020 19:38:34 +0100
Lorenzo Bianconi <lorenzo@kernel.org> wrote:
> Introduce xdp_build_skb_from_frame utility routine to build the skb
> from xdp_frame. Respect to __xdp_build_skb_from_frame,
> xdp_build_skb_from_frame will allocate the skb object. Rely on
> xdp_build_skb_from_frame in veth driver.
>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
> drivers/net/veth.c | 18 +++---------------
> include/net/xdp.h | 2 ++
> net/core/xdp.c | 15 +++++++++++++++
> 3 files changed, 20 insertions(+), 15 deletions(-)
Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next 0/2] add xdp_build_skb_from_frame utility routine
2020-12-16 18:38 [PATCH bpf-next 0/2] add xdp_build_skb_from_frame utility routine Lorenzo Bianconi
2020-12-16 18:38 ` [PATCH bpf-next 1/2] net: xdp: introduce __xdp_build_skb_from_frame " Lorenzo Bianconi
2020-12-16 18:38 ` [PATCH bpf-next 2/2] net: xdp: introduce xdp_build_skb_from_frame " Lorenzo Bianconi
@ 2021-01-12 16:08 ` Lorenzo Bianconi
2021-01-12 16:50 ` Daniel Borkmann
2 siblings, 1 reply; 9+ messages in thread
From: Lorenzo Bianconi @ 2021-01-12 16:08 UTC (permalink / raw)
To: bpf; +Cc: netdev, davem, kuba, ast, daniel, brouer, toshiaki.makita1
[-- Attachment #1: Type: text/plain, Size: 897 bytes --]
> Introduce __xdp_build_skb_from_frame and xdp_build_skb_from_frame routines to
> build the skb from a xdp_frame. Respect to __xdp_build_skb_from_frame,
> xdp_build_skb_from_frame will allocate the skb object.
> Rely on __xdp_build_skb_from_frame/xdp_build_skb_from_frame in cpumap and veth
> code.
Hi Daniel/Alexei,
since this series is marked as "archived" in patchwork, do I need to resubmit it?
Regards,
Lorenzo
>
> Lorenzo Bianconi (2):
> net: xdp: introduce __xdp_build_skb_from_frame utility routine
> net: xdp: introduce xdp_build_skb_from_frame utility routine
>
> drivers/net/veth.c | 18 +++-----------
> include/net/xdp.h | 5 ++++
> kernel/bpf/cpumap.c | 45 +---------------------------------
> net/core/xdp.c | 59 +++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 68 insertions(+), 59 deletions(-)
>
> --
> 2.29.2
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next 0/2] add xdp_build_skb_from_frame utility routine
2021-01-12 16:08 ` [PATCH bpf-next 0/2] add " Lorenzo Bianconi
@ 2021-01-12 16:50 ` Daniel Borkmann
2021-01-12 16:53 ` Lorenzo Bianconi
0 siblings, 1 reply; 9+ messages in thread
From: Daniel Borkmann @ 2021-01-12 16:50 UTC (permalink / raw)
To: Lorenzo Bianconi, bpf; +Cc: netdev, davem, kuba, ast, brouer, toshiaki.makita1
On 1/12/21 5:08 PM, Lorenzo Bianconi wrote:
>> Introduce __xdp_build_skb_from_frame and xdp_build_skb_from_frame routines to
>> build the skb from a xdp_frame. Respect to __xdp_build_skb_from_frame,
>> xdp_build_skb_from_frame will allocate the skb object.
>> Rely on __xdp_build_skb_from_frame/xdp_build_skb_from_frame in cpumap and veth
>> code.
>
> since this series is marked as "archived" in patchwork, do I need to resubmit it?
Yes please do, afaik there was some minor feedback from Toshiaki which was not yet
addressed from your side.
Thanks,
Daniel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next 0/2] add xdp_build_skb_from_frame utility routine
2021-01-12 16:50 ` Daniel Borkmann
@ 2021-01-12 16:53 ` Lorenzo Bianconi
0 siblings, 0 replies; 9+ messages in thread
From: Lorenzo Bianconi @ 2021-01-12 16:53 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: bpf, netdev, davem, kuba, ast, brouer, toshiaki.makita1
[-- Attachment #1: Type: text/plain, Size: 652 bytes --]
> On 1/12/21 5:08 PM, Lorenzo Bianconi wrote:
> > > Introduce __xdp_build_skb_from_frame and xdp_build_skb_from_frame routines to
> > > build the skb from a xdp_frame. Respect to __xdp_build_skb_from_frame,
> > > xdp_build_skb_from_frame will allocate the skb object.
> > > Rely on __xdp_build_skb_from_frame/xdp_build_skb_from_frame in cpumap and veth
> > > code.
> >
> > since this series is marked as "archived" in patchwork, do I need to resubmit it?
>
> Yes please do, afaik there was some minor feedback from Toshiaki which was not yet
> addressed from your side.
ack, will do.
Regards,
Lorenzo
>
> Thanks,
> Daniel
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread