This patchset works towards supporting different XDP RX-ring memory
allocators. As this will be needed by the AF_XDP zero-copy mode.
The patchset uses mlx5 as the sample driver, which gets implemented
XDP_REDIRECT RX-mode, but not ndo_xdp_xmit (as this API is subject to
change thought the patchset).
A new struct xdp_frame is introduced (modeled after cpumap xdp_pkt).
And both ndo_xdp_xmit and the new xdp_return_frame end-up using this.
Support for a driver supplied allocator is implemented, and a
refurbished version of page_pool is the first return allocator type
introduced. This will be a integration point for AF_XDP zero-copy.
The mlx5 driver evolve into using the page_pool, and see a performance
increase (with ndo_xdp_xmit out ixgbe driver) from 6Mpps to 12Mpps.
The patchset stop at the 15 patches limit, but more API changes are
planned. Specifically extending ndo_xdp_xmit and xdp_return_frame
APIs to support bulking. As this will address some known limits.
---
Jesper Dangaard Brouer (15):
mlx5: basic XDP_REDIRECT forward support
xdp: introduce xdp_return_frame API and use in cpumap
ixgbe: use xdp_return_frame API
xdp: move struct xdp_buff from filter.h to xdp.h
xdp: introduce a new xdp_frame type
tun: convert to use generic xdp_frame and xdp_return_frame API
virtio_net: convert to use generic xdp_frame and xdp_return_frame API
bpf: cpumap convert to use generic xdp_frame
mlx5: register a memory model when XDP is enabled
xdp: rhashtable with allocator ID to pointer mapping
page_pool: refurbish version of page_pool code
xdp: allow page_pool as an allocator type in xdp_return_frame
mlx5: use page_pool for xdp_return_frame call
xdp: transition into using xdp_frame for return API
xdp: transition into using xdp_frame for ndo_xdp_xmit
drivers/net/ethernet/intel/ixgbe/ixgbe.h | 3
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 37 ++
drivers/net/ethernet/mellanox/mlx5/core/en.h | 4
drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 36 ++
drivers/net/ethernet/mellanox/mlx5/core/en_rx.c | 38 ++
drivers/net/tun.c | 42 +--
drivers/net/virtio_net.c | 45 ++-
drivers/vhost/net.c | 7
include/linux/filter.h | 24 --
include/linux/if_tun.h | 4
include/linux/netdevice.h | 4
include/net/page_pool.h | 123 ++++++++
include/net/xdp.h | 86 +++++
kernel/bpf/cpumap.c | 132 +++-----
net/core/Makefile | 1
net/core/filter.c | 17 +
net/core/page_pool.c | 334 +++++++++++++++++++++
net/core/xdp.c | 257 ++++++++++++++++
18 files changed, 1022 insertions(+), 172 deletions(-)
create mode 100644 include/net/page_pool.h
create mode 100644 net/core/page_pool.c
--
This implements basic XDP redirect support in mlx5 driver.
Notice that the ndo_xdp_xmit() is NOT implemented, because that API
need some changes that this patchset is working towards.
The main purpose of this patch is have different drivers doing
XDP_REDIRECT to show how different memory models behave in a cross
driver world.
Update(pre-RFCv2 Tariq): Need to DMA unmap page before xdp_do_redirect,
as the return API does not exist yet to to keep this mapped.
Update(pre-RFCv3 Saeed): Don't mix XDP_TX and XDP_REDIRECT flushing,
introduce xdpsq.db.redirect_flush boolian.
Signed-off-by: Jesper Dangaard Brouer <redacted>
---
drivers/net/ethernet/mellanox/mlx5/core/en.h | 1 +
drivers/net/ethernet/mellanox/mlx5/core/en_rx.c | 27 ++++++++++++++++++++---
2 files changed, 25 insertions(+), 3 deletions(-)
@@ -236,14 +236,20 @@ static inline int mlx5e_page_alloc_mapped(struct mlx5e_rq *rq,return0;}+staticinlinevoidmlx5e_page_dma_unmap(structmlx5e_rq*rq,+structmlx5e_dma_info*dma_info)+{+dma_unmap_page(rq->pdev,dma_info->addr,RQ_PAGE_SIZE(rq),+rq->buff.map_dir);+}+voidmlx5e_page_release(structmlx5e_rq*rq,structmlx5e_dma_info*dma_info,boolrecycle){if(likely(recycle)&&mlx5e_rx_cache_put(rq,dma_info))return;-dma_unmap_page(rq->pdev,dma_info->addr,RQ_PAGE_SIZE(rq),-rq->buff.map_dir);+mlx5e_page_dma_unmap(rq,dma_info);put_page(dma_info->page);}
@@ -822,9 +828,10 @@ static inline int mlx5e_xdp_handle(struct mlx5e_rq *rq,structmlx5e_dma_info*di,void*va,u16*rx_headroom,u32*len){-conststructbpf_prog*prog=READ_ONCE(rq->xdp_prog);+structbpf_prog*prog=READ_ONCE(rq->xdp_prog);structxdp_buffxdp;u32act;+interr;if(!prog)returnfalse;
@@ -845,6 +852,15 @@ static inline int mlx5e_xdp_handle(struct mlx5e_rq *rq,if(unlikely(!mlx5e_xmit_xdp_frame(rq,di,&xdp)))trace_xdp_exception(rq->netdev,prog,act);returntrue;+caseXDP_REDIRECT:+/* When XDP enabled then page-refcnt==1 here */+err=xdp_do_redirect(rq->netdev,&xdp,prog);+if(!err){+rq->wqe.xdp_xmit=true;/* XDP xmit owns page */+rq->xdpsq.db.redirect_flush=true;+mlx5e_page_dma_unmap(rq,di);+}+returntrue;default:bpf_warn_invalid_xdp_action(act);caseXDP_ABORTED:
@@ -1107,6 +1123,11 @@ int mlx5e_poll_rx_cq(struct mlx5e_cq *cq, int budget)xdpsq->db.doorbell=false;}+if(xdpsq->db.redirect_flush){+xdp_do_flush_map();+xdpsq->db.redirect_flush=false;+}+mlx5_cqwq_update_db_record(&cq->wq);/* ensure cq space is freed before enabling more cqes */
Introduce an xdp_return_frame API, and convert over cpumap as
the first user, given it have queued XDP frame structure to leverage.
Signed-off-by: Jesper Dangaard Brouer <redacted>
---
include/net/xdp.h | 32 +++++++++++++++++++++++++++
kernel/bpf/cpumap.c | 60 +++++++++++++++++++++++++++++++--------------------
net/core/xdp.c | 18 +++++++++++++++
3 files changed, 86 insertions(+), 24 deletions(-)
@@ -33,16 +33,48 @@*alsomandatoryduringRX-ringsetup.*/+enummem_type{+MEM_TYPE_PAGE_SHARED=0,/* Split-page refcnt based model */+MEM_TYPE_PAGE_ORDER0,/* Orig XDP full page model */+// Possible new ideas for types:+// MEM_TYPE_PAGE_POOL, /* Will be added later */+// MEM_TYPE_AF_XDP,+// MEM_TYPE_DEVICE_OWNED -- invoking an dev->ndo?+MEM_TYPE_MAX,+};++structxdp_mem_info{+u32type;/* enum mem_type, but known size type */+u32id;// Needed later (to lookup struct xdp_rxq_info)+};+structxdp_rxq_info{structnet_device*dev;u32queue_index;u32reg_state;+structxdp_mem_infomem;}____cacheline_aligned;/* perf critical, avoid false-sharing */++staticinline+voidxdp_return_frame(void*data,structxdp_mem_info*mem)+{+if(mem->type==MEM_TYPE_PAGE_SHARED)+page_frag_free(data);++if(mem->type==MEM_TYPE_PAGE_ORDER0){+structpage*page=virt_to_page(data);/* Assumes order0 page*/++put_page(page);+}+}+intxdp_rxq_info_reg(structxdp_rxq_info*xdp_rxq,structnet_device*dev,u32queue_index);voidxdp_rxq_info_unreg(structxdp_rxq_info*xdp_rxq);voidxdp_rxq_info_unused(structxdp_rxq_info*xdp_rxq);boolxdp_rxq_info_is_reg(structxdp_rxq_info*xdp_rxq);+intxdp_rxq_info_reg_mem_model(structxdp_rxq_info*xdp_rxq,+enummem_typetype,void*allocator);#endif /* __LINUX_NET_XDP_H__ */
@@ -137,27 +138,6 @@ static struct bpf_map *cpu_map_alloc(union bpf_attr *attr)returnERR_PTR(err);}-staticvoid__cpu_map_queue_destructor(void*ptr)-{-/* The tear-down procedure should have made sure that queue is-*empty.See__cpu_map_entry_replace()andwork-queue-*invokedcpu_map_kthread_stop().Catchanybrokenbehaviour-*gracefullyandwarnonce.-*/-if(WARN_ON_ONCE(ptr))-page_frag_free(ptr);-}--staticvoidput_cpu_map_entry(structbpf_cpu_map_entry*rcpu)-{-if(atomic_dec_and_test(&rcpu->refcnt)){-/* The queue should be empty at this point */-ptr_ring_cleanup(rcpu->queue,__cpu_map_queue_destructor);-kfree(rcpu->queue);-kfree(rcpu);-}-}-staticvoidget_cpu_map_entry(structbpf_cpu_map_entry*rcpu){atomic_inc(&rcpu->refcnt);
@@ -188,6 +168,10 @@ struct xdp_pkt {u16len;u16headroom;u16metasize;+/* Lifetime of xdp_rxq_info is limited to NAPI/enqueue time,+*whilememinfoisvalidonremoteCPU.+*/+structxdp_mem_infomem;structnet_device*dev_rx;};
@@ -213,6 +197,9 @@ static struct xdp_pkt *convert_to_xdp_pkt(struct xdp_buff *xdp)xdp_pkt->headroom=headroom-sizeof(*xdp_pkt);xdp_pkt->metasize=metasize;+/* rxq only valid until napi_schedule ends, convert to xdp_mem_info */+xdp_pkt->mem=xdp->rxq->mem;+returnxdp_pkt;}
@@ -265,6 +252,31 @@ static struct sk_buff *cpu_map_build_skb(struct bpf_cpu_map_entry *rcpu,returnskb;}+staticvoid__cpu_map_ring_cleanup(structptr_ring*ring)+{+/* The tear-down procedure should have made sure that queue is+*empty.See__cpu_map_entry_replace()andwork-queue+*invokedcpu_map_kthread_stop().Catchanybrokenbehaviour+*gracefullyandwarnonce.+*/+structxdp_pkt*xdp_pkt;++while((xdp_pkt=ptr_ring_consume(ring)))+if(WARN_ON_ONCE(xdp_pkt))+xdp_return_frame(xdp_pkt,&xdp_pkt->mem);+}++staticvoidput_cpu_map_entry(structbpf_cpu_map_entry*rcpu)+{+if(atomic_dec_and_test(&rcpu->refcnt)){+/* The queue should be empty at this point */+__cpu_map_ring_cleanup(rcpu->queue);+ptr_ring_cleanup(rcpu->queue,NULL);+kfree(rcpu->queue);+kfree(rcpu);+}+}+staticintcpu_map_kthread_run(void*data){structbpf_cpu_map_entry*rcpu=data;
@@ -307,7 +319,7 @@ static int cpu_map_kthread_run(void *data)skb=cpu_map_build_skb(rcpu,xdp_pkt);if(!skb){-page_frag_free(xdp_pkt);+xdp_return_frame(xdp_pkt,&xdp_pkt->mem);continue;}
@@ -71,3 +71,21 @@ bool xdp_rxq_info_is_reg(struct xdp_rxq_info *xdp_rxq)return(xdp_rxq->reg_state==REG_STATE_REGISTERED);}EXPORT_SYMBOL_GPL(xdp_rxq_info_is_reg);++intxdp_rxq_info_reg_mem_model(structxdp_rxq_info*xdp_rxq,+enummem_typetype,void*allocator)+{+if(type>=MEM_TYPE_MAX)+return-EINVAL;++xdp_rxq->mem.type=type;++if(allocator)+return-EOPNOTSUPP;++/* TODO: Allocate an ID that maps to allocator pointer+*See:https://www.kernel.org/doc/html/latest/core-api/idr.html+*/+return0;+}+EXPORT_SYMBOL_GPL(xdp_rxq_info_reg_mem_model);
@@ -5787,7 +5787,7 @@ static void ixgbe_clean_tx_ring(struct ixgbe_ring *tx_ring)/* Free all the Tx ring sk_buffs */if(ring_is_xdp(tx_ring))-page_frag_free(tx_buffer->data);+xdp_return_frame(tx_buffer->data,&tx_buffer->xdp_mem);elsedev_kfree_skb_any(tx_buffer->skb);
@@ -8351,6 +8351,8 @@ static int ixgbe_xmit_xdp_ring(struct ixgbe_adapter *adapter,dma_unmap_len_set(tx_buffer,len,len);dma_unmap_addr_set(tx_buffer,dma,dma);tx_buffer->data=xdp->data;+tx_buffer->xdp_mem=xdp->rxq->mem;+tx_desc->read.buffer_addr=cpu_to_le64(dma);/* put descriptor type bits */
This is done to prepare for the next patch, and it is also
nice to move this XDP related struct out of filter.h.
Signed-off-by: Jesper Dangaard Brouer <redacted>
---
include/linux/filter.h | 24 +-----------------------
include/net/xdp.h | 22 ++++++++++++++++++++++
2 files changed, 23 insertions(+), 23 deletions(-)
@@ -499,14 +500,6 @@ struct bpf_skb_data_end {void*data_end;};-structxdp_buff{-void*data;-void*data_end;-void*data_meta;-void*data_hard_start;-structxdp_rxq_info*rxq;-};-/* Compute the linear packet data range [data, data_end) which*willbeaccessedbyvariousprogramtypes(cls_bpf,act_bpf,*lwt,...).Subsystemsallowingdirectdataaccessmust(!)
@@ -753,21 +746,6 @@ int xdp_do_redirect(struct net_device *dev,structbpf_prog*prog);voidxdp_do_flush_map(void);-/* Drivers not supporting XDP metadata can use this helper, which-*rejectsanyroomexpansionformetadataasaresult.-*/-static__always_inlinevoid-xdp_set_data_meta_invalid(structxdp_buff*xdp)-{-xdp->data_meta=xdp->data+1;-}--static__always_inlinebool-xdp_data_meta_unsupported(conststructxdp_buff*xdp)-{-returnunlikely(xdp->data_meta>xdp->data);-}-voidbpf_warn_invalid_xdp_action(u32act);structsock*do_sk_redirect_map(structsk_buff*skb);
This is needed to convert drivers tuntap and virtio_net.
This is a generalization of what is done inside cpumap, which will be
converted later.
Signed-off-by: Jesper Dangaard Brouer <redacted>
---
include/net/xdp.h | 40 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 40 insertions(+)
@@ -63,6 +63,46 @@ struct xdp_buff {structxdp_rxq_info*rxq;};+structxdp_frame{+void*data;+u16len;+u16headroom;+u16metasize;+/* Lifetime of xdp_rxq_info is limited to NAPI/enqueue time,+*whilememinfoisvalidonremoteCPU.+*/+structxdp_mem_infomem;+};++/* Convert xdp_buff to xdp_frame */+staticinline+structxdp_frame*convert_to_xdp_frame(structxdp_buff*xdp)+{+structxdp_frame*xdp_frame;+intmetasize;+intheadroom;++/* Assure headroom is available for storing info */+headroom=xdp->data-xdp->data_hard_start;+metasize=xdp->data-xdp->data_meta;+metasize=metasize>0?metasize:0;+if(unlikely((headroom-metasize)<sizeof(*xdp_frame)))+returnNULL;++/* Store info in top of packet */+xdp_frame=xdp->data_hard_start;++xdp_frame->data=xdp->data;+xdp_frame->len=xdp->data_end-xdp->data;+xdp_frame->headroom=headroom-sizeof(*xdp_frame);+xdp_frame->metasize=metasize;++/* rxq only valid until napi_schedule ends, convert to xdp_mem_info */+xdp_frame->mem=xdp->rxq->mem;++returnxdp_frame;+}+staticinlinevoidxdp_return_frame(void*data,structxdp_mem_info*mem){
The tuntap driver invented it's own driver specific way of queuing
XDP packets, by storing the xdp_buff information in the top of
the XDP frame data.
Convert it over to use the more generic xdp_frame structure. The
main problem with the in-driver method is that the xdp_rxq_info pointer
cannot be trused/used when dequeueing the frame.
Signed-off-by: Jesper Dangaard Brouer <redacted>
---
drivers/net/tun.c | 45 +++++++++++++++++++++++----------------------
drivers/vhost/net.c | 7 ++++---
include/linux/if_tun.h | 4 ++--
3 files changed, 29 insertions(+), 27 deletions(-)
@@ -1290,17 +1290,18 @@ static const struct net_device_ops tun_netdev_ops = {staticinttun_xdp_xmit(structnet_device*dev,structxdp_buff*xdp){structtun_struct*tun=netdev_priv(dev);-structxdp_buff*buff=xdp->data_hard_start;-intheadroom=xdp->data-xdp->data_hard_start;+structxdp_frame*frame;structtun_file*tfile;u32numqueues;intret=0;-/* Assure headroom is available and buff is properly aligned */-if(unlikely(headroom<sizeof(*xdp)||tun_is_xdp_buff(xdp)))-return-ENOSPC;+/* FIXME: Explain why this check is the needed! */+if(unlikely(tun_is_xdp_frame(xdp)))+return-EBADRQC;-*buff=*xdp;+frame=convert_to_xdp_frame(xdp);+if(unlikely(!frame))+return-EOVERFLOW;rcu_read_lock();
@@ -1315,7 +1316,7 @@ static int tun_xdp_xmit(struct net_device *dev, struct xdp_buff *xdp)/* Encode the XDP flag into lowest bit for consumer to differ*XDPbufferfromsk_buff.*/-if(ptr_ring_produce(&tfile->tx_ring,tun_xdp_to_ptr(buff))){+if(ptr_ring_produce(&tfile->tx_ring,tun_xdp_to_ptr(frame))){this_cpu_inc(tun->pcpu_stats->tx_dropped);ret=-ENOSPC;}
The virtio_net driver assumes XDP frames are always released based on
page refcnt (via put_page). Thus, is only queues the XDP data pointer
address and uses virt_to_head_page() to retrieve struct page.
Use the XDP return API to get away from such assumptions. Instead
queue an xdp_frame, which allow us to use the xdp_return_frame API,
when releasing the frame.
Signed-off-by: Jesper Dangaard Brouer <redacted>
---
drivers/net/virtio_net.c | 31 +++++++++++++++++++++----------
1 file changed, 21 insertions(+), 10 deletions(-)
@@ -419,30 +419,41 @@ static bool __virtnet_xdp_xmit(struct virtnet_info *vi,structxdp_buff*xdp){structvirtio_net_hdr_mrg_rxbuf*hdr;-unsignedintlen;+structxdp_frame*xdpf,*xdpf_sent;structsend_queue*sq;+unsignedintlen;unsignedintqp;-void*xdp_sent;interr;qp=vi->curr_queue_pairs-vi->xdp_queue_pairs+smp_processor_id();sq=&vi->sq[qp];/* Free up any pending old buffers before queueing new ones. */-while((xdp_sent=virtqueue_get_buf(sq->vq,&len))!=NULL){-structpage*sent_page=virt_to_head_page(xdp_sent);+while((xdpf_sent=virtqueue_get_buf(sq->vq,&len))!=NULL)+xdp_return_frame(xdpf_sent->data,&xdpf_sent->mem);-put_page(sent_page);-}+xdpf=convert_to_xdp_frame(xdp);+if(unlikely(!xdpf))+return-EOVERFLOW;++/* virtqueue want to use data area in-front of packet */+if(unlikely(xdpf->metasize>0))+return-EOPNOTSUPP;++if(unlikely(xdpf->headroom<vi->hdr_len))+return-EOVERFLOW;-xdp->data-=vi->hdr_len;+/* Make room for virtqueue hdr (also change xdpf->headroom?) */+xdpf->data-=vi->hdr_len;/* Zero header and leave csum up to XDP layers */-hdr=xdp->data;+hdr=xdpf->data;memset(hdr,0,vi->hdr_len);+hdr->hdr.hdr_len=xdpf->len;/* Q: is this needed? */+xdpf->len+=vi->hdr_len;-sg_init_one(sq->sg,xdp->data,xdp->data_end-xdp->data);+sg_init_one(sq->sg,xdpf->data,xdpf->len);-err=virtqueue_add_outbuf(sq->vq,sq->sg,1,xdp->data,GFP_ATOMIC);+err=virtqueue_add_outbuf(sq->vq,sq->sg,1,xdpf,GFP_ATOMIC);if(unlikely(err))returnfalse;/* Caller handle free/refcnt */
The generic xdp_frame format, was inspired by the cpumap own internal
xdp_pkt format. It is now time to convert it over to the generic
xdp_frame format. The cpumap needs one extra field dev_rx.
Signed-off-by: Jesper Dangaard Brouer <redacted>
---
include/net/xdp.h | 1 +
kernel/bpf/cpumap.c | 100 ++++++++++++++-------------------------------------
2 files changed, 29 insertions(+), 72 deletions(-)
@@ -159,52 +159,8 @@ static void cpu_map_kthread_stop(struct work_struct *work)kthread_stop(rcpu->kthread);}-/* For now, xdp_pkt is a cpumap internal data structure, with info-*carriedbetweenenqueuetodequeue.Itismappedintothetop-*headroomofthepacket,toavoidallocatingseparatemem.-*/-structxdp_pkt{-void*data;-u16len;-u16headroom;-u16metasize;-/* Lifetime of xdp_rxq_info is limited to NAPI/enqueue time,-*whilememinfoisvalidonremoteCPU.-*/-structxdp_mem_infomem;-structnet_device*dev_rx;-};--/* Convert xdp_buff to xdp_pkt */-staticstructxdp_pkt*convert_to_xdp_pkt(structxdp_buff*xdp)-{-structxdp_pkt*xdp_pkt;-intmetasize;-intheadroom;--/* Assure headroom is available for storing info */-headroom=xdp->data-xdp->data_hard_start;-metasize=xdp->data-xdp->data_meta;-metasize=metasize>0?metasize:0;-if(unlikely((headroom-metasize)<sizeof(*xdp_pkt)))-returnNULL;--/* Store info in top of packet */-xdp_pkt=xdp->data_hard_start;--xdp_pkt->data=xdp->data;-xdp_pkt->len=xdp->data_end-xdp->data;-xdp_pkt->headroom=headroom-sizeof(*xdp_pkt);-xdp_pkt->metasize=metasize;--/* rxq only valid until napi_schedule ends, convert to xdp_mem_info */-xdp_pkt->mem=xdp->rxq->mem;--returnxdp_pkt;-}-staticstructsk_buff*cpu_map_build_skb(structbpf_cpu_map_entry*rcpu,-structxdp_pkt*xdp_pkt)+structxdp_frame*xdpf){unsignedintframe_size;void*pkt_data_start;
@@ -290,7 +246,7 @@ static int cpu_map_kthread_run(void *data)*/while(!kthread_should_stop()||!__ptr_ring_empty(rcpu->queue)){unsignedintprocessed=0,drops=0,sched=0;-structxdp_pkt*xdp_pkt;+structxdp_frame*xdpf;/* Release CPU reschedule checks */if(__ptr_ring_empty(rcpu->queue)){
@@ -313,13 +269,13 @@ static int cpu_map_kthread_run(void *data)*kthreadCPUpinned.Locklessaccesstoptr_ring*consumesidevalidasno-resizeallowedofqueue.*/-while((xdp_pkt=__ptr_ring_consume(rcpu->queue))){+while((xdpf=__ptr_ring_consume(rcpu->queue))){structsk_buff*skb;intret;-skb=cpu_map_build_skb(rcpu,xdp_pkt);+skb=cpu_map_build_skb(rcpu,xdpf);if(!skb){-xdp_return_frame(xdp_pkt,&xdp_pkt->mem);+xdp_return_frame(xdpf->data,&xdpf->mem);continue;}
@@ -616,13 +572,13 @@ static int bq_flush_to_queue(struct bpf_cpu_map_entry *rcpu,spin_lock(&q->producer_lock);for(i=0;i<bq->count;i++){-structxdp_pkt*xdp_pkt=bq->q[i];+structxdp_frame*xdpf=bq->q[i];interr;-err=__ptr_ring_produce(q,xdp_pkt);+err=__ptr_ring_produce(q,xdpf);if(err){drops++;-xdp_return_frame(xdp_pkt->data,&xdp_pkt->mem);+xdp_return_frame(xdpf->data,&xdpf->mem);}processed++;}
@@ -637,7 +593,7 @@ static int bq_flush_to_queue(struct bpf_cpu_map_entry *rcpu,/* Runs under RCU-read-side, plus in softirq under NAPI protection.*Thus,safepercpuvariableaccess.*/-staticintbq_enqueue(structbpf_cpu_map_entry*rcpu,structxdp_pkt*xdp_pkt)+staticintbq_enqueue(structbpf_cpu_map_entry*rcpu,structxdp_frame*xdpf){structxdp_bulk_queue*bq=this_cpu_ptr(rcpu->bulkq);
@@ -648,28 +604,28 @@ static int bq_enqueue(struct bpf_cpu_map_entry *rcpu, struct xdp_pkt *xdp_pkt)*drivertocodeinvokingustofinished,duetodriver*(e.g.ixgbe)recycletricksbasedonpage-refcnt.*-*Thus,incomingxdp_pktisalwaysqueuedhere(elsewerace+*Thus,incomingxdp_frameisalwaysqueuedhere(elsewerace*withanotherCPUonpage-refcntandremainingdrivercode).*Queuetimeisveryshort,asdriverwillinvokeflush*operation,whencompletingnapi->pollcall.*/-bq->q[bq->count++]=xdp_pkt;+bq->q[bq->count++]=xdpf;return0;}intcpu_map_enqueue(structbpf_cpu_map_entry*rcpu,structxdp_buff*xdp,structnet_device*dev_rx){-structxdp_pkt*xdp_pkt;+structxdp_frame*xdpf;-xdp_pkt=convert_to_xdp_pkt(xdp);-if(unlikely(!xdp_pkt))+xdpf=convert_to_xdp_frame(xdp);+if(unlikely(!xdpf))return-EOVERFLOW;/* Info needed when constructing SKB on remote CPU */-xdp_pkt->dev_rx=dev_rx;+xdpf->dev_rx=dev_rx;-bq_enqueue(rcpu,xdp_pkt);+bq_enqueue(rcpu,xdpf);return0;}
Now all the users of ndo_xdp_xmit have been converted to use xdp_return_frame.
This enable a different memory model, thus activating another code path
in the xdp_return_frame API.
Signed-off-by: Jesper Dangaard Brouer <redacted>
---
drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 7 +++++++
1 file changed, 7 insertions(+)
@@ -506,6 +506,13 @@ static int mlx5e_alloc_rq(struct mlx5e_channel *c,rq->mkey_be=c->mkey_be;}+/* This must only be activate for order-0 pages */+if(rq->xdp_prog)+err=xdp_rxq_info_reg_mem_model(&rq->xdp_rxq,+MEM_TYPE_PAGE_ORDER0,NULL);+if(err<0)+gotoerr_rq_wq_destroy;+for(i=0;i<wq_sz;i++){structmlx5e_rx_wqe*wqe=mlx5_wq_ll_get_wqe(&rq->wq,i);
Use the IDA infrastructure for getting a cyclic increasing ID number,
that is used for keeping track of each registered allocator per
RX-queue xdp_rxq_info. Instead of using the IDR infrastructure, which
uses a radix tree, use a dynamic rhashtable, for creating ID to
pointer lookup table, because this is faster.
The problem that is being solved here is that, the xdp_rxq_info
pointer (stored in xdp_buff) cannot be used directly, as the
guaranteed lifetime is too short. The info is needed on a
(potentially) remote CPU during DMA-TX completion time . In an
xdp_frame the xdp_mem_info is stored, when it got converted from an
xdp_buff, which is sufficient for the simple page refcnt based recycle
schemes.
For more advanced allocators there is a need to store a pointer to the
registered allocator. Thus, there is a need to guard the lifetime or
validity of the allocator pointer, which is done through this
rhashtable ID map to pointer. The removal and validity of of the
allocator and helper struct xdp_mem_allocator is guarded by RCU. The
allocator will be created by the driver, and registered with
xdp_rxq_info_reg_mem_model().
It is up-to debate who is responsible for freeing the allocator
pointer or invoking the allocator destructor function. In any case,
this must happen via RCU freeing.
Use the IDA infrastructure for getting a cyclic increasing ID number,
that is used for keeping track of each registered allocator per
RX-queue xdp_rxq_info.
Signed-off-by: Jesper Dangaard Brouer <redacted>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 9 +
drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 2
include/net/xdp.h | 15 -
net/core/xdp.c | 230 +++++++++++++++++++++
4 files changed, 236 insertions(+), 20 deletions(-)
@@ -45,7 +45,7 @@ enum mem_type {structxdp_mem_info{u32type;/* enum mem_type, but known size type */-u32id;// Needed later (to lookup struct xdp_rxq_info)+u32id;};structxdp_rxq_info{
@@ -13,6 +16,99 @@#define REG_STATE_UNREGISTERED 0x2#define REG_STATE_UNUSED 0x3+DEFINE_IDA(mem_id_pool);+staticDEFINE_MUTEX(mem_id_lock);+#define MEM_ID_MAX 0xFFFE+#define MEM_ID_MIN 1+staticintmem_id_next=MEM_ID_MIN;++staticboolmem_id_init;/* false */+staticstructrhashtable*mem_id_ht;++structxdp_mem_allocator{+structxdp_mem_infomem;+void*allocator;+structrhash_headnode;+structrcu_headrcu;+};++staticu32xdp_mem_id_hashfn(constvoid*data,u32len,u32seed)+{+constu32*k=data;+constu32key=*k;++BUILD_BUG_ON(FIELD_SIZEOF(structxdp_mem_allocator,mem.id)+!=sizeof(u32));++/* Use cyclic increasing ID as direct hash key, see rht_bucket_index */+returnkey<<RHT_HASH_RESERVED_SPACE;+}++staticintxdp_mem_id_cmp(structrhashtable_compare_arg*arg,+constvoid*ptr)+{+conststructxdp_mem_allocator*xa=ptr;+u32mem_id=*(u32*)arg->key;++returnxa->mem.id!=mem_id;+}++staticconststructrhashtable_paramsmem_id_rht_params={+.nelem_hint=64,+.head_offset=offsetof(structxdp_mem_allocator,node),+.key_offset=offsetof(structxdp_mem_allocator,mem.id),+.key_len=FIELD_SIZEOF(structxdp_mem_allocator,mem.id),+.max_size=MEM_ID_MAX,+.min_size=8,+.automatic_shrinking=true,+.hashfn=xdp_mem_id_hashfn,+.obj_cmpfn=xdp_mem_id_cmp,+};++void__xdp_mem_allocator_rcu_free(structrcu_head*rcu)+{+structxdp_mem_allocator*xa;++xa=container_of(rcu,structxdp_mem_allocator,rcu);++/* Allow this ID to be reused */+ida_simple_remove(&mem_id_pool,xa->mem.id);++/* TODO: Depending on allocator type/pointer free resources */++/* Poison memory */+xa->mem.id=0xFFFF;+xa->mem.type=0xF0F0;+xa->allocator=(void*)0xDEAD9001;++kfree(xa);+}++void__xdp_rxq_info_unreg_mem_model(structxdp_rxq_info*xdp_rxq)+{+structxdp_mem_allocator*xa;+intid=xdp_rxq->mem.id;+interr;++if(id==0)+return;++mutex_lock(&mem_id_lock);++xa=rhashtable_lookup(mem_id_ht,&id,mem_id_rht_params);+if(!xa){+mutex_unlock(&mem_id_lock);+return;+}++err=rhashtable_remove_fast(mem_id_ht,&xa->node,mem_id_rht_params);+WARN_ON(err);++call_rcu(&xa->rcu,__xdp_mem_allocator_rcu_free);++mutex_unlock(&mem_id_lock);+}+voidxdp_rxq_info_unreg(structxdp_rxq_info*xdp_rxq){/* Simplify driver cleanup code paths, allow unreg "unused" */
@@ -21,8 +117,14 @@ void xdp_rxq_info_unreg(struct xdp_rxq_info *xdp_rxq)WARN(!(xdp_rxq->reg_state==REG_STATE_REGISTERED),"Driver BUG");+__xdp_rxq_info_unreg_mem_model(xdp_rxq);+xdp_rxq->reg_state=REG_STATE_UNREGISTERED;xdp_rxq->dev=NULL;++/* Reset mem info to defaults */+xdp_rxq->mem.id=0;+xdp_rxq->mem.type=0;}EXPORT_SYMBOL_GPL(xdp_rxq_info_unreg);
@@ -72,20 +174,138 @@ bool xdp_rxq_info_is_reg(struct xdp_rxq_info *xdp_rxq)}EXPORT_SYMBOL_GPL(xdp_rxq_info_is_reg);+int__mem_id_init_hash_table(void)+{+structrhashtable*rht;+intret;++if(unlikely(mem_id_init))+return0;++rht=kzalloc(sizeof(*rht),GFP_KERNEL);+if(!rht)+return-ENOMEM;++ret=rhashtable_init(rht,&mem_id_rht_params);+if(ret<0){+kfree(rht);+returnret;+}+mem_id_ht=rht;+smp_mb();/* mutex lock should provide enough pairing */+mem_id_init=true;++return0;+}++/* Allocate a cyclic ID that maps to allocator pointer.+*See:https://www.kernel.org/doc/html/latest/core-api/idr.html+*+*Callermustlockmem_id_lock.+*/+staticint__mem_id_cyclic_get(gfp_tgfp)+{+intretries=1;+intid;++again:+id=ida_simple_get(&mem_id_pool,mem_id_next,MEM_ID_MAX,gfp);+if(id<0){+if(id==-ENOSPC){+/* Cyclic allocator, reset next id */+if(retries--){+mem_id_next=MEM_ID_MIN;+gotoagain;+}+}+returnid;/* errno */+}+mem_id_next=id+1;++returnid;+}+intxdp_rxq_info_reg_mem_model(structxdp_rxq_info*xdp_rxq,enummem_typetype,void*allocator){+structxdp_mem_allocator*xdp_alloc;+gfp_tgfp=GFP_KERNEL;+intid,errno,ret;+void*ptr;++if(xdp_rxq->reg_state!=REG_STATE_REGISTERED){+WARN(1,"Missing register, driver bug");+return-EFAULT;+}+if(type>=MEM_TYPE_MAX)return-EINVAL;xdp_rxq->mem.type=type;-if(allocator)-return-EOPNOTSUPP;+if(!allocator)+return0;++/* Delay init of rhashtable to save memory if feature isn't used */+if(!mem_id_init){+mutex_lock(&mem_id_lock);+ret=__mem_id_init_hash_table();+mutex_unlock(&mem_id_lock);+if(ret<0){+WARN_ON(1);+returnret;+}+}++xdp_alloc=kzalloc(sizeof(*xdp_alloc),gfp);+if(!xdp_alloc)+return-ENOMEM;++mutex_lock(&mem_id_lock);+id=__mem_id_cyclic_get(gfp);+if(id<0){+errno=id;+gotoerr;+}+xdp_rxq->mem.id=id;+xdp_alloc->mem=xdp_rxq->mem;+xdp_alloc->allocator=allocator;++/* Insert allocator into ID lookup table */+ptr=rhashtable_insert_slow(mem_id_ht,&id,&xdp_alloc->node);+if(IS_ERR(ptr)){+errno=PTR_ERR(ptr);+gotoerr;+}++mutex_unlock(&mem_id_lock);-/* TODO: Allocate an ID that maps to allocator pointer-*See:https://www.kernel.org/doc/html/latest/core-api/idr.html-*/return0;+err:+mutex_unlock(&mem_id_lock);+kfree(xdp_alloc);+returnerrno;}EXPORT_SYMBOL_GPL(xdp_rxq_info_reg_mem_model);++voidxdp_return_frame(void*data,structxdp_mem_info*mem)+{+structxdp_mem_allocator*xa;++rcu_read_lock();+if(mem->id)+xa=rhashtable_lookup(mem_id_ht,&mem->id,mem_id_rht_params);+rcu_read_unlock();++if(mem->type==MEM_TYPE_PAGE_SHARED){+page_frag_free(data);+return;+}++if(mem->type==MEM_TYPE_PAGE_ORDER0){+structpage*page=virt_to_page(data);/* Assumes order0 page*/++put_page(page);+}+}+EXPORT_SYMBOL_GPL(xdp_return_frame);
Need a fast page recycle mechanism for ndo_xdp_xmit API for returning
pages on DMA-TX completion time, which have good cross CPU
performance, given DMA-TX completion time can happen on a remote CPU.
Refurbish my page_pool code, that was presented[1] at MM-summit 2016.
Adapted page_pool code to not depend the page allocator and
integration into struct page. The DMA mapping feature is kept,
even-though it will not be activated/used in this patchset.
[1] http://people.netfilter.org/hawk/presentations/MM-summit2016/generic_page_pool_mm_summit2016.pdf
Signed-off-by: Jesper Dangaard Brouer <redacted>
---
include/net/page_pool.h | 123 +++++++++++++++++
net/core/Makefile | 1
net/core/page_pool.c | 334 +++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 458 insertions(+)
create mode 100644 include/net/page_pool.h
create mode 100644 net/core/page_pool.c
@@ -0,0 +1,123 @@+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note+*+*page_pool.h+*Author:JesperDangaardBrouer<netoptimizer@brouer.com>+*Copyright(C)2016RedHat,Inc.+*+*Notice:thisispage_poolvariantisno-longerhookedintothe+*put_page()code.Thus,itistheresponsabilityoftheAPIuserto+*returnpages.+*+*UPDATE:ThiscanbeusedwithoutDMAmappingpart(whichuse+*page->private).Thismeans,thatthepages-must-be-returnedto+*poolrequirementcanberelaxed,asnostateismaintainedin+*struct-page.+*+*/+#ifndef _NET_PAGE_POOL_H+#define _NET_PAGE_POOL_H++#include<linux/mm.h>/* Needed by ptr_ring */+#include<linux/ptr_ring.h>+#include<linux/dma-direction.h>++#define PP_FLAG_DMA_MAP 1 /* Should page_pool do the DMA map/unmap */+#define PP_FLAG_ALL PP_FLAG_DMA_MAP++/*+*Fastallocationsidecachearray/stack+*+*Thecachesizeandrefillwatermarkisrelatedtothenetwork+*use-case.TheNAPIbudgetis64packets.AfteraNAPIpolltheRX+*ringisusuallyrefilledandthemaxconsumedelementswillbe64,+*thusanaturalmaxsizeofobjectsneededinthecache.+*+*Keepingroomformoreobjects,isduetoXDP_DROPuse-case.As+*XDP_DROPallowstheopportunitytorecycleobjectsdirectlyinto+*thisarray,asitsharesthesamesoftirq/NAPIprotection.If+*cacheisalreadyfull(orpartlyfull)thentheXDP_DROPrecycles+*wouldhavetotakeaslowercodepath.+*/+#define PP_ALLOC_CACHE_SIZE 128+#define PP_ALLOC_CACHE_REFILL 64+structpp_alloc_cache{+u32count____cacheline_aligned_in_smp;+u32refill;/* not used atm */+void*cache[PP_ALLOC_CACHE_SIZE];+};++structpage_pool_params{+u32size;/* caller sets size of struct */+unsignedintorder;+unsignedlongflags;+structdevice*dev;/* device, for DMA pre-mapping purposes */+intnid;/* Numa node id to allocate from pages from */+enumdma_data_directiondma_dir;/* DMA mapping direction */+unsignedintpool_size;+charend_marker[0];/* must be last struct member */+};+#define PAGE_POOL_PARAMS_SIZE offsetof(struct page_pool_params, end_marker)++structpage_pool{+structpage_pool_paramsp;++/*+*Datastructureforallocationside+*+*Driversallocationsideusuallyalreadyperformsomekind+*ofresourceprotection.Piggybackonthisprotection,and+*requiredrivertoprotectallocationside.+*+*ForNICdriversthismeans,allocateapage_poolper+*RX-queue.AstheRX-queueisalreadyprotectedby+*Softirq/BHschedulingandnapi_schedule.NAPIschedule+*guaranteethatasinglenapi_structwillonlybescheduled+*onasingleCPU(seenapi_schedule).+*/+structpp_alloc_cachealloc;++/* Data structure for storing recycled pages.+*+*Returning/freeingpagesismorecomplicatedsynchronization+*wise,becausefree'scanhappenonremoteCPUs,withno+*associationwithallocationresource.+*+*Useptr_ring,asitseparatesconsumerandproducer+*effeciently,itawaythatdoesn'tbouncecache-lines.+*+*TODO:Implementbulkreturnpagesintothisstructure.+*/+structptr_ringring;++structrcu_headrcu;+};++structpage*page_pool_alloc_pages(structpage_pool*pool,gfp_tgfp);++staticinlinestructpage*page_pool_dev_alloc_pages(structpage_pool*pool)+{+gfp_tgfp=(GFP_ATOMIC|__GFP_NOWARN);++returnpage_pool_alloc_pages(pool,gfp);+}++structpage_pool*page_pool_create(conststructpage_pool_params*params);++voidpage_pool_destroy_rcu(structpage_pool*pool);++/* Never call this directly, use helpers below */+void__page_pool_put_page(structpage_pool*pool,+structpage*page,boolallow_direct);++staticinlinevoidpage_pool_put_page(structpage_pool*pool,structpage*page)+{+__page_pool_put_page(pool,page,false);+}+/* Very limited use-cases allow recycle direct */+staticinlinevoidpage_pool_recycle_direct(structpage_pool*pool,+structpage*page)+{+__page_pool_put_page(pool,page,true);+}++#endif /* _NET_PAGE_POOL_H */
@@ -0,0 +1,334 @@+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note+*+*page_pool.c+*Author:JesperDangaardBrouer<netoptimizer@brouer.com>+*Copyright(C)2016RedHat,Inc.+*/+#include<linux/types.h>+#include<linux/kernel.h>+#include<linux/slab.h>++#include<net/page_pool.h>+#include<linux/dma-direction.h>+#include<linux/dma-mapping.h>+#include<linux/page-flags.h>+#include<linux/mm.h>/* for __put_page() */++intpage_pool_init(structpage_pool*pool,+conststructpage_pool_params*params)+{+intring_qsize=1024;/* Default */+intparam_copy_sz;++if(!pool)+return-EFAULT;++/* Note, below struct compat code was primarily needed when+*page_poolcodelivedunderMM-treecontrol,givenmmotsand+*net-nexttreesprogressinverydifferentrates.+*+*Allowkerneldeveltreesanddrivertoprogressatdifferentrates+*/+param_copy_sz=PAGE_POOL_PARAMS_SIZE;+memset(&pool->p,0,param_copy_sz);+if(params->size<param_copy_sz){+/* Older module calling newer kernel, handled by only+*copyingsuppliedsize,andkeepremainingparamszero+*/+param_copy_sz=params->size;+}elseif(params->size>param_copy_sz){+/* Newer module calling older kernel. Need to validate+*nonewfeatureswererequested.+*/+unsignedchar*addr=(unsignedchar*)params+param_copy_sz;+unsignedchar*end=(unsignedchar*)params+params->size;++for(;addr<end;addr++){+if(*addr!=0)+return-E2BIG;+}+}+memcpy(&pool->p,params,param_copy_sz);++/* Validate only known flags were used */+if(pool->p.flags&~(PP_FLAG_ALL))+return-EINVAL;++if(pool->p.pool_size)+ring_qsize=pool->p.pool_size;++if(ptr_ring_init(&pool->ring,ring_qsize,GFP_KERNEL)<0)+return-ENOMEM;++/* DMA direction is either DMA_FROM_DEVICE or DMA_BIDIRECTIONAL.+*DMA_BIDIRECTIONALisforallowingpageusedforDMAsending,+*whichistheXDP_TXuse-case.+*/+if((pool->p.dma_dir!=DMA_FROM_DEVICE)&&+(pool->p.dma_dir!=DMA_BIDIRECTIONAL))+return-EINVAL;++return0;+}++structpage_pool*page_pool_create(conststructpage_pool_params*params)+{+structpage_pool*pool;+interr=0;++if(params->size<offsetof(structpage_pool_params,nid)){+WARN(1,"Fix page_pool_params->size code\n");+returnNULL;+}++pool=kzalloc_node(sizeof(*pool),GFP_KERNEL,params->nid);+err=page_pool_init(pool,params);+if(err<0){+pr_warn("%s() gave up with errno %d\n",__func__,err);+kfree(pool);+returnERR_PTR(err);+}+returnpool;+}+EXPORT_SYMBOL(page_pool_create);++/* fast path */+staticstructpage*__page_pool_get_cached(structpage_pool*pool)+{+structpage*page;++/* Test for safe-context, caller should provide this guarantee */+if(likely(in_serving_softirq())){+structptr_ring*r;++if(likely(pool->alloc.count)){+/* Fast-path */+page=pool->alloc.cache[--pool->alloc.count];+returnpage;+}+/* Slower-path: Alloc array empty, time to refill */+r=&pool->ring;+/* Open-coded bulk ptr_ring consumer.+*+*Discussion:ATMtheringconsumerlockisnot+*reallyneededduetothesoftirq/NAPIprotection,+*butlaterMM-layerneedtheabilitytoreclaim+*pagesonthering.Thus,keepingthelocks.+*/+spin_lock(&r->consumer_lock);+while((page=__ptr_ring_consume(r))){+if(pool->alloc.count==PP_ALLOC_CACHE_REFILL)+break;+pool->alloc.cache[pool->alloc.count++]=page;+}+spin_unlock(&r->consumer_lock);+returnpage;+}++/* Slow-path: Get page from locked ring queue */+page=ptr_ring_consume(&pool->ring);+returnpage;+}++/* slow path */+noinline+staticstructpage*__page_pool_alloc_pages(structpage_pool*pool,+gfp_t_gfp)+{+structpage*page;+gfp_tgfp=_gfp;+dma_addr_tdma;++/* We could always set __GFP_COMP, and avoid this branch, as+*prep_new_page()canhandleorder-0with__GFP_COMP.+*/+if(pool->p.order)+gfp|=__GFP_COMP;++/* FUTURE development:+*+*Currentslow-pathessentiallyfallsbacktosinglepage+*allocations,whichdoesn'timproveperformance.Thiscode+*needbulkallocationsupportfromthepageallocatorcode.+*+*Fornow,pagepoolrecyclecacheisnotrefilled.Hint:+*whenpagesarereturned,theywillgointotherecycle+*cache.+*/++/* Cache was empty, do real allocation */+page=alloc_pages_node(pool->p.nid,gfp,pool->p.order);+if(!page)+returnNULL;++if(!(pool->p.flags&PP_FLAG_DMA_MAP))+gotoskip_dma_map;++/* Setup DMA mapping: use page->private for DMA-addr+*Thismappingiskeptforlifetimeofpage,untilleavingpool.+*/+dma=dma_map_page(pool->p.dev,page,0,+(PAGE_SIZE<<pool->p.order),+pool->p.dma_dir);+if(dma_mapping_error(pool->p.dev,dma)){+put_page(page);+returnNULL;+}+set_page_private(page,dma);/* page->private = dma; */++skip_dma_map:+/* When page just alloc'ed is should/must have refcnt 1. */+returnpage;+}++/* For using page_pool replace: alloc_pages() API calls, but provide+*synchronizationguaranteeforallocationside.+*/+structpage*page_pool_alloc_pages(structpage_pool*pool,gfp_tgfp)+{+structpage*page;++/* Fast-path: Get a page from cache */+page=__page_pool_get_cached(pool);+if(page)+returnpage;++/* Slow-path: cache empty, do real allocation */+page=__page_pool_alloc_pages(pool,gfp);+returnpage;+}+EXPORT_SYMBOL(page_pool_alloc_pages);++/* Cleanup page_pool state from page */+staticvoid__page_pool_clean_page(structpage_pool*pool,+structpage*page)+{+if(!(pool->p.flags&PP_FLAG_DMA_MAP))+return;++/* DMA unmap */+dma_unmap_page(pool->p.dev,page_private(page),+PAGE_SIZE<<pool->p.order,pool->p.dma_dir);+set_page_private(page,0);+}++/* Return a page to the page allocator, cleaning up our state */+staticvoid__page_pool_return_page(structpage_pool*pool,structpage*page)+{+__page_pool_clean_page(pool,page);+/* Given state was cleared, the page should be freed here.+*Thus,codeinvariantassumesrefcnt==1,as__free_pages()+*callput_page_testzero().+*/+__free_pages(page,pool->p.order);+}++bool__page_pool_recycle_into_ring(structpage_pool*pool,+structpage*page)+{+intret;+/* BH protection not needed if current is serving softirq */+if(in_serving_softirq())+ret=ptr_ring_produce(&pool->ring,page);+else+ret=ptr_ring_produce_bh(&pool->ring,page);++return(ret==0)?true:false;+}++/* Only allow direct recycling in very special circumstances, into the+*alloccache.E.g.XDP_DROPuse-case.+*+*Callermustprovideappropriatesafecontext.+*/+staticbool__page_pool_recycle_direct(structpage*page,+structpage_pool*pool)+{+/* WARN_RATELIMIT(!in_serving_softirq(), "Wrong context\n"); */++if(unlikely(pool->alloc.count==PP_ALLOC_CACHE_SIZE))+returnfalse;++/* Caller MUST have verified/know (page_ref_count(page) == 1) */+pool->alloc.cache[pool->alloc.count++]=page;+returntrue;+}++void__page_pool_put_page(structpage_pool*pool,+structpage*page,boolallow_direct)+{+/* This is a fast-path optimization, that avoids an atomic+*operation,inthecasewhereasingleobjectis(refcnt)+*usingthepage.+*+*refcnt==1meanspage_poolownspage,andcanrecycleit.+*/+if(likely(page_ref_count(page)==1)){+/* Read barrier implicit paired with full MB of atomic ops */+smp_rmb();++if(allow_direct&&in_serving_softirq())+if(__page_pool_recycle_direct(page,pool))+return;++if(!__page_pool_recycle_into_ring(pool,page)){+/* Cache full, do real __free_pages() */+__page_pool_return_page(pool,page);+}+return;+}+/* Many drivers splitting up the page into fragments, and some+*wanttokeepdoingthistosavememory.Theput_page_testzero()+*functionasarefcntdecrement,andshouldnotreturntrue.+*/+if(unlikely(put_page_testzero(page))){+/* Reaching refcnt zero should not be possible,+*indicatecodeerror.Don'tcrashbutwarn,handle+*casebynot-recycling,butreturnpagetopage+*allocator.+*/+WARN(1,"%s() violating page_pool invariance refcnt:%d\n",+__func__,page_ref_count(page));+/* Cleanup state before directly returning page */+__page_pool_clean_page(pool,page);+__put_page(page);+}+}+EXPORT_SYMBOL(__page_pool_put_page);++/* Cleanup and release resources */+void__page_pool_destroy_rcu(structrcu_head*rcu)+{+structpage_pool*pool;+structpage*page;++pool=container_of(rcu,structpage_pool,rcu);++/* Empty alloc cache, assume caller made sure this is+*no-longerinuse,andpage_pool_alloc_pages()cannotbe+*callconcurrently.+*/+while(pool->alloc.count){+page=pool->alloc.cache[--pool->alloc.count];+__page_pool_return_page(pool,page);+}++/* Empty recycle ring */+while((page=ptr_ring_consume(&pool->ring))){+/* Verify the refcnt invariant of cached pages */+if(!(page_ref_count(page)==1)){+pr_crit("%s() page_pool refcnt %d violation\n",+__func__,page_ref_count(page));+WARN_ON(1);+}+__page_pool_return_page(pool,page);+}+ptr_ring_cleanup(&pool->ring,NULL);+kfree(pool);+}++voidpage_pool_destroy_rcu(structpage_pool*pool)+{+call_rcu(&pool->rcu,__page_pool_destroy_rcu);+}+EXPORT_SYMBOL(page_pool_destroy_rcu);
New allocator type MEM_TYPE_PAGE_POOL for page_pool usage.
The registered allocator page_pool pointer is not available directly
from xdp_rxq_info, but it could be (if needed). For now, the driver
should keep separate track of the page_pool pointer, which it should
use for RX-ring page allocation.
As suggested by Saeed, to maintain a symmetric API it is the drivers
responsibility to allocate/create and free/destroy the page_pool.
Thus, after the driver have called xdp_rxq_info_unreg(), it is drivers
responsibility to free the page_pool, but with a RCU free call. This
is done easily via the page_pool helper page_pool_destroy_rcu() (which
avoids touching any driver code during the RCU callback, which could
happen after the driver have been unloaded).
Signed-off-by: Jesper Dangaard Brouer <redacted>
---
include/net/xdp.h | 4 +++-
net/core/xdp.c | 23 ++++++++++++++++++++---
2 files changed, 23 insertions(+), 4 deletions(-)
@@ -36,8 +36,8 @@enummem_type{MEM_TYPE_PAGE_SHARED=0,/* Split-page refcnt based model */MEM_TYPE_PAGE_ORDER0,/* Orig XDP full page model */+MEM_TYPE_PAGE_POOL,// Possible new ideas for types:-// MEM_TYPE_PAGE_POOL, /* Will be added later */// MEM_TYPE_AF_XDP,// MEM_TYPE_DEVICE_OWNED -- invoking an dev->ndo?MEM_TYPE_MAX,
@@ -74,7 +78,9 @@ void __xdp_mem_allocator_rcu_free(struct rcu_head *rcu)/* Allow this ID to be reused */ida_simple_remove(&mem_id_pool,xa->mem.id);-/* TODO: Depending on allocator type/pointer free resources */+/* Notice, driver is expected to free the *allocator,+*e.g.page_pool,andMUSTalsouseRCUfree.+*//* Poison memory */xa->mem.id=0xFFFF;
@@ -396,6 +398,8 @@ static int mlx5e_alloc_rq(struct mlx5e_channel *c,interr;inti;+structpage_pool_paramspp_params={0};+rqp->wq.db_numa_node=cpu_to_node(c->cpu);err=mlx5_wq_ll_create(mdev,&rqp->wq,rqc_wq,&rq->wq,
@@ -506,12 +510,33 @@ static int mlx5e_alloc_rq(struct mlx5e_channel *c,rq->mkey_be=c->mkey_be;}-/* This must only be activate for order-0 pages */-if(rq->xdp_prog)-err=xdp_rxq_info_reg_mem_model(&rq->xdp_rxq,-MEM_TYPE_PAGE_ORDER0,NULL);-if(err)+/* Create a page_pool and register it with rxq */+pp_params.size=PAGE_POOL_PARAMS_SIZE;+pp_params.order=rq->buff.page_order;+pp_params.dev=c->pdev;+pp_params.nid=cpu_to_node(c->cpu);+pp_params.dma_dir=rq->buff.map_dir;+pp_params.pool_size=1<<params->log_rq_size;+pp_params.flags=0;/* No-internal DMA mapping in page_pool */++/* page_pool can be used even when there is no rq->xdp_prog,+*givenpage_pooldoesnothandleDMAmappingthereisno+*requiredstatetoclear.Andpage_poolgracefullyhandle+*elevatedrefcnt.+*/+rq->page_pool=page_pool_create(&pp_params);+if(IS_ERR_OR_NULL(rq->page_pool)){+kfree(rq->wqe.frag_info);+rq->page_pool=NULL;+err=-ENOMEM;gotoerr_rq_wq_destroy;+}+err=xdp_rxq_info_reg_mem_model(&rq->xdp_rxq,+MEM_TYPE_PAGE_POOL,rq->page_pool);+if(err){+err=-ENOMEM;+gotoerr_rq_wq_destroy;+}for(i=0;i<wq_sz;i++){structmlx5e_rx_wqe*wqe=mlx5_wq_ll_get_wqe(&rq->wq,i);
@@ -549,6 +574,8 @@ static int mlx5e_alloc_rq(struct mlx5e_channel *c,if(rq->xdp_prog)bpf_prog_put(rq->xdp_prog);xdp_rxq_info_unreg(&rq->xdp_rxq);+if(rq->page_pool)+page_pool_destroy_rcu(rq->page_pool);mlx5_wq_destroy(&rq->wq_ctrl);returnerr;
Changing API xdp_return_frame() to take struct xdp_frame as argument,
seems like a natural choice. But there are some subtle performance
details here that needs extra care, which is a deliberate choice.
When de-referencing xdp_frame on a remote CPU during DMA-TX
completion, result in the cache-line is change to "Shared"
state. Later when the page is reused for RX, then this xdp_frame
cache-line is written, which change the state to "Modified".
This situation already happens (naturally) for, virtio_net, tun and
cpumap as the xdp_frame pointer is the queued object. In tun and
cpumap, the ptr_ring is used for efficiently transferring cache-lines
(with pointers) between CPUs. Thus, the only option is to
de-referencing xdp_frame.
It is only the ixgbe driver that had an optimization, in which it can
avoid doing the de-reference of xdp_frame. The driver already have
TX-ring queue, which (in case of remote DMA-TX completion) have to be
transferred between CPUs anyhow. In this data area, we stored a
struct xdp_mem_info and a data pointer, which allowed us to avoid
de-referencing xdp_frame.
To compensate for this, a prefetchw is used for telling the cache
coherency protocol about our access pattern. My benchmarks show that
this prefetchw is enough to compensate the ixgbe driver.
Signed-off-by: Jesper Dangaard Brouer <redacted>
---
drivers/net/ethernet/intel/ixgbe/ixgbe.h | 4 +---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 17 +++++++++++------
drivers/net/ethernet/mellanox/mlx5/core/en_rx.c | 1 +
drivers/net/tun.c | 4 ++--
drivers/net/virtio_net.c | 2 +-
include/net/xdp.h | 2 +-
kernel/bpf/cpumap.c | 6 +++---
net/core/xdp.c | 4 +++-
8 files changed, 23 insertions(+), 17 deletions(-)
@@ -430,7 +430,7 @@ static bool __virtnet_xdp_xmit(struct virtnet_info *vi,/* Free up any pending old buffers before queueing new ones. */while((xdpf_sent=virtqueue_get_buf(sq->vq,&len))!=NULL)-xdp_return_frame(xdpf_sent->data,&xdpf_sent->mem);+xdp_return_frame(xdpf_sent);xdpf=convert_to_xdp_frame(xdp);if(unlikely(!xdpf))
Changing API ndo_xdp_xmit to take a struct xdp_frame instead of struct
xdp_buff. This brings xdp_return_frame and ndp_xdp_xmit in sync.
This builds towards changing the API further to become a bulk API,
because xdp_buff is not a queue-able object while xdp_frame is.
Signed-off-by: Jesper Dangaard Brouer <redacted>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 21 +++++++++++----------
drivers/net/tun.c | 11 +----------
drivers/net/virtio_net.c | 24 ++++++++++++++----------
include/linux/netdevice.h | 4 ++--
net/core/filter.c | 17 +++++++++++++++--
5 files changed, 43 insertions(+), 34 deletions(-)
@@ -432,10 +432,6 @@ static bool __virtnet_xdp_xmit(struct virtnet_info *vi,while((xdpf_sent=virtqueue_get_buf(sq->vq,&len))!=NULL)xdp_return_frame(xdpf_sent);-xdpf=convert_to_xdp_frame(xdp);-if(unlikely(!xdpf))-return-EOVERFLOW;-/* virtqueue want to use data area in-front of packet */if(unlikely(xdpf->metasize>0))return-EOPNOTSUPP;
On 06/03/2018 11:48 PM, Jesper Dangaard Brouer wrote:
quoted hunk
Now all the users of ndo_xdp_xmit have been converted to use xdp_return_frame.
This enable a different memory model, thus activating another code path
in the xdp_return_frame API.
Signed-off-by: Jesper Dangaard Brouer <redacted>
---
drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 7 +++++++
1 file changed, 7 insertions(+)
@@ -506,6 +506,13 @@ static int mlx5e_alloc_rq(struct mlx5e_channel *c,rq->mkey_be=c->mkey_be;}+/* This must only be activate for order-0 pages */+if(rq->xdp_prog)+err=xdp_rxq_info_reg_mem_model(&rq->xdp_rxq,+MEM_TYPE_PAGE_ORDER0,NULL);+if(err<0)+gotoerr_rq_wq_destroy;+
Use "if (err)" here, instead of changing this in next patch.
Also, get it into the "if (rq->xdp_prog)" block.
for (i = 0; i < wq_sz; i++) {
struct mlx5e_rx_wqe *wqe = mlx5_wq_ll_get_wqe(&rq->wq, i);
@@ -396,6 +398,8 @@ static int mlx5e_alloc_rq(struct mlx5e_channel *c,interr;inti;+structpage_pool_paramspp_params={0};+rqp->wq.db_numa_node=cpu_to_node(c->cpu);err=mlx5_wq_ll_create(mdev,&rqp->wq,rqc_wq,&rq->wq,
@@ -506,12 +510,33 @@ static int mlx5e_alloc_rq(struct mlx5e_channel *c,rq->mkey_be=c->mkey_be;}-/* This must only be activate for order-0 pages */-if(rq->xdp_prog)-err=xdp_rxq_info_reg_mem_model(&rq->xdp_rxq,-MEM_TYPE_PAGE_ORDER0,NULL);-if(err)+/* Create a page_pool and register it with rxq */+pp_params.size=PAGE_POOL_PARAMS_SIZE;+pp_params.order=rq->buff.page_order;+pp_params.dev=c->pdev;+pp_params.nid=cpu_to_node(c->cpu);+pp_params.dma_dir=rq->buff.map_dir;+pp_params.pool_size=1<<params->log_rq_size;
if rq->wq_type == MLX5_WQ_TYPE_LINKED_LIST_STRIDING_RQ, need to multiply by MLX5_MPWRQ_PAGES_PER_WQE.
quoted hunk
+ pp_params.flags = 0; /* No-internal DMA mapping in page_pool */++ /* page_pool can be used even when there is no rq->xdp_prog,+ * given page_pool does not handle DMA mapping there is no+ * required state to clear. And page_pool gracefully handle+ * elevated refcnt.+ */+ rq->page_pool = page_pool_create(&pp_params);+ if (IS_ERR_OR_NULL(rq->page_pool)) {+ kfree(rq->wqe.frag_info);+ rq->page_pool = NULL;+ err = -ENOMEM;
we can save a branch, this way:
- if (likely(recycle) && mlx5e_rx_cache_put(rq, dma_info))
+ if (likely(recycle)) {
+ if (mlx5e_rx_cache_put(rq, dma_info))
+ return;
+
+ mlx5e_page_dma_unmap(rq, dma_info);
+ page_pool_recycle_direct(rq->page_pool, dma_info->page);
return;
+ }
On Wed, 7 Mar 2018 13:50:19 +0200
Tariq Toukan [off-list ref] wrote:
On 06/03/2018 11:48 PM, Jesper Dangaard Brouer wrote:
quoted
Now all the users of ndo_xdp_xmit have been converted to use xdp_return_frame.
This enable a different memory model, thus activating another code path
in the xdp_return_frame API.
Signed-off-by: Jesper Dangaard Brouer <redacted>
---
drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 7 +++++++
1 file changed, 7 insertions(+)
@@ -506,6 +506,13 @@ static int mlx5e_alloc_rq(struct mlx5e_channel *c,rq->mkey_be=c->mkey_be;}+/* This must only be activate for order-0 pages */+if(rq->xdp_prog)+err=xdp_rxq_info_reg_mem_model(&rq->xdp_rxq,+MEM_TYPE_PAGE_ORDER0,NULL);+if(err<0)+gotoerr_rq_wq_destroy;+
Use "if (err)" here, instead of changing this in next patch.
Also, get it into the "if (rq->xdp_prog)" block.
Good point, fixed!
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
you can have the include here instead:
#include <net/page_pool.h>
and remove it from the .c files.
Hmm... I'm not too happy with this change request. I did this to avoid
having to recompile all the drivers files when I changed page_pool.h
(yes, the kernel makefile system detect the dependencies automatically).
I'll keep this, let me know if you feel strongly about this...
@@ -396,6 +398,8 @@ static int mlx5e_alloc_rq(struct mlx5e_channel *c,interr;inti;+structpage_pool_paramspp_params={0};+rqp->wq.db_numa_node=cpu_to_node(c->cpu);err=mlx5_wq_ll_create(mdev,&rqp->wq,rqc_wq,&rq->wq,
@@ -506,12 +510,33 @@ static int mlx5e_alloc_rq(struct mlx5e_channel *c,rq->mkey_be=c->mkey_be;}-/* This must only be activate for order-0 pages */-if(rq->xdp_prog)-err=xdp_rxq_info_reg_mem_model(&rq->xdp_rxq,-MEM_TYPE_PAGE_ORDER0,NULL);-if(err)+/* Create a page_pool and register it with rxq */+pp_params.size=PAGE_POOL_PARAMS_SIZE;+pp_params.order=rq->buff.page_order;+pp_params.dev=c->pdev;+pp_params.nid=cpu_to_node(c->cpu);+pp_params.dma_dir=rq->buff.map_dir;+pp_params.pool_size=1<<params->log_rq_size;
if rq->wq_type == MLX5_WQ_TYPE_LINKED_LIST_STRIDING_RQ, need to multiply
by MLX5_MPWRQ_PAGES_PER_WQE.
Okay, doing so.
quoted
+ pp_params.flags = 0; /* No-internal DMA mapping in page_pool */++ /* page_pool can be used even when there is no rq->xdp_prog,+ * given page_pool does not handle DMA mapping there is no+ * required state to clear. And page_pool gracefully handle+ * elevated refcnt.+ */+ rq->page_pool = page_pool_create(&pp_params);+ if (IS_ERR_OR_NULL(rq->page_pool)) {+ kfree(rq->wqe.frag_info);+ rq->page_pool = NULL;+ err = -ENOMEM;
better: PTR_ERR(rq->page_pool)
Ok, I changed page_pool_create() return values to be a little more sane
(not returning NULL), so we can do as you suggest (else using PTR_ERR
directly for err can return zero, which indicate success for
mlx5e_alloc_rq() even when it failed...).
we can save a branch, this way:
- if (likely(recycle) && mlx5e_rx_cache_put(rq, dma_info))
+ if (likely(recycle)) {
+ if (mlx5e_rx_cache_put(rq, dma_info))
+ return;
+
+ mlx5e_page_dma_unmap(rq, dma_info);
+ page_pool_recycle_direct(rq->page_pool,
dma_info->page); return;
+ }
Okay, doing this, but you missed that we now need to call
mlx5e_page_dma_unmap(rq, dma_info) in two cases now.
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer