Hi,
this patch series implements several performance improvements on the
mvneta driver.
- The first 3 patches are essentially cleanups, code deduplication
and minor optimizations for not re-fetching a value we already have
(status).
- patch 4 changes the prefetch of Rx descriptor from current one to
next one. In benchmarks, it results in about 1% general performance
increase on HTTP traffic, probably because prefetching the current
descriptor does not leave enough time between the start of prefetch
and its usage.
- patch 5 implements support for build_skb() on Rx path. The driver
now preallocates frags instead of skbs and builds an skb just before
delivering it. This results in a 2% performance increase on HTTP
traffic, and up to 5% on small packet Rx rate.
- patch 6 implements rx_copybreak for small packets (256 bytes). It
avoids a dma_map_single()/dma_unmap_single() and increases the Rx
rate by 16.4%, from 486kpps to 573kpps. Further improvements up to
711kpps are possible depending how the DMA is used.
This patch series depends on the previous series of fixes and is only
for the net-next tree.
Thanks!
Willy
---
Willy Tarreau (6):
net: mvneta: remove tests for impossible cases in the tx_done path
net: mvneta: factor rx refilling code
net: mvneta: simplify access to the rx descriptor status
net: mvneta: prefetch next rx descriptor instead of current one
net: mvneta: convert to build_skb()
net: mvneta: implement rx_copybreak
drivers/net/ethernet/marvell/mvneta.c | 152 +++++++++++++++++++++-------------
1 file changed, 95 insertions(+), 57 deletions(-)
--
1.7.12.2.21.g234cd45.dirty
Currently, mvneta_txq_bufs_free() calls mvneta_tx_done_policy() with
a non-null cause to retrieve the pointer to the next queue to process.
There are useless tests on the return queue number and on the pointer,
all of which are well defined within a known limited set. This code
path is fast, although not critical. Removing 3 tests here that the
compiler could not optimize (verified) is always desirable.
Cc: Thomas Petazzoni <redacted>
Cc: Gregory CLEMENT <redacted>
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
drivers/net/ethernet/marvell/mvneta.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
@@ -1276,13 +1276,16 @@ static void mvneta_rx_csum(struct mvneta_port *pp,skb->ip_summed=CHECKSUM_NONE;}-/* Return tx queue pointer (find last set bit) according to causeTxDone reg */+/* Return tx queue pointer (find last set bit) according to <cause> returned+*formtx_donereg.<cause>mustnotbenull.Thereturnvalueisalwaysa+*validqueueformatchingthefirstonefoundin<cause>.+*/staticstructmvneta_tx_queue*mvneta_tx_done_policy(structmvneta_port*pp,u32cause){intqueue=fls(cause)-1;-return(queue<0||queue>=txq_number)?NULL:&pp->txqs[queue];+return&pp->txqs[queue];}/* Free tx queue skbuffs */
@@ -1651,7 +1654,9 @@ static void mvneta_txq_done_force(struct mvneta_port *pp,txq->txq_get_index=0;}-/* handle tx done - called from tx done timer callback */+/* Handle tx done - called in softirq context. The <cause_tx_done> argument+*mustbeavalidcauseaccordingtoMVNETA_TXQ_INTR_MASK_ALL.+*/staticu32mvneta_tx_done_gbe(structmvneta_port*pp,u32cause_tx_done,int*tx_todo){
At several places, we already know the value of the rx status but
we call functions which dereference the pointer again to get it
and don't need the descriptor for anything else. Simplify this
task by replacing the rx desc pointer by the status word itself.
Cc: Thomas Petazzoni <redacted>
Cc: Gregory CLEMENT <redacted>
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
drivers/net/ethernet/marvell/mvneta.c | 25 ++++++++++++-------------
1 file changed, 12 insertions(+), 13 deletions(-)
@@ -528,14 +528,14 @@ struct rtnl_link_stats64 *mvneta_get_stats64(struct net_device *dev,/* Rx descriptors helper methods */-/* Checks whether the given RX descriptor is both the first and the-*lastdescriptorfortheRXpacket.EachRXpacketiscurrently+/* Checks whether the RX descriptor having this status is both the first+*andthelastdescriptorfortheRXpacket.EachRXpacketiscurrently*receivedthroughasingleRXdescriptor,sonothavingeachRX*descriptorwithitsfirstandlastbitssetisanerror*/-staticintmvneta_rxq_desc_is_first_last(structmvneta_rx_desc*desc)+staticintmvneta_rxq_desc_is_first_last(u32status){-return(desc->status&MVNETA_RXD_FIRST_LAST_DESC)==+return(status&MVNETA_RXD_FIRST_LAST_DESC)==MVNETA_RXD_FIRST_LAST_DESC;}
Make use of build_skb() to allocate frags on the RX path. When frag size
is lower than a page size, we can use netdev_alloc_frag(), and we fall back
to kmalloc() for larger sizes. The frag size is stored into the mvneta_port
struct. The alloc/free functions check the frag size to decide what alloc/
free method to use. MTU changes are safe because the MTU change function
stops the device and clears the queues before applying the change.
With this patch, I observed a reproducible 2% performance improvement on
HTTP-based benchmarks, and 5% on small packet RX rate.
Cc: Thomas Petazzoni <redacted>
Cc: Gregory CLEMENT <redacted>
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
drivers/net/ethernet/marvell/mvneta.c | 49 +++++++++++++++++++++++++----------
1 file changed, 35 insertions(+), 14 deletions(-)
@@ -1440,20 +1456,21 @@ static int mvneta_rx(struct mvneta_port *pp, int rx_todo,while(rx_done<rx_todo){structmvneta_rx_desc*rx_desc=mvneta_rxq_next_desc_get(rxq);structsk_buff*skb;+unsignedchar*data;u32rx_status;intrx_bytes,err;rx_done++;rx_filled++;rx_status=rx_desc->status;-skb=(structsk_buff*)rx_desc->buf_cookie;+data=(unsignedchar*)rx_desc->buf_cookie;if(!mvneta_rxq_desc_is_first_last(rx_status)||-(rx_status&MVNETA_RXD_ERR_SUMMARY)){+(rx_status&MVNETA_RXD_ERR_SUMMARY)||+!(skb=build_skb(data,pp->frag_size>PAGE_SIZE?0:pp->frag_size))){dev->stats.rx_errors++;mvneta_rx_error(pp,rx_desc);-mvneta_rx_desc_fill(rx_desc,rx_desc->buf_phys_addr,-(u32)skb);+/* leave the descriptor untouched */continue;}
@@ -1466,7 +1483,7 @@ static int mvneta_rx(struct mvneta_port *pp, int rx_todo,rcvd_bytes+=rx_bytes;/* Linux processing */-skb_reserve(skb,MVNETA_MH_SIZE);+skb_reserve(skb,MVNETA_MH_SIZE+NET_SKB_PAD);skb_put(skb,rx_bytes);skb->protocol=eth_type_trans(skb,dev);
@@ -2276,6 +2293,8 @@ static int mvneta_change_mtu(struct net_device *dev, int mtu)mvneta_cleanup_rxqs(pp);pp->pkt_size=MVNETA_RX_PKT_SIZE(pp->dev->mtu);+pp->frag_size=SKB_DATA_ALIGN(MVNETA_RX_BUF_SIZE(pp->pkt_size))++SKB_DATA_ALIGN(sizeof(structskb_shared_info));ret=mvneta_setup_rxqs(pp);if(ret){
@@ -2423,6 +2442,8 @@ static int mvneta_open(struct net_device *dev)mvneta_mac_addr_set(pp,dev->dev_addr,rxq_def);pp->pkt_size=MVNETA_RX_PKT_SIZE(pp->dev->mtu);+pp->frag_size=SKB_DATA_ALIGN(MVNETA_RX_BUF_SIZE(pp->pkt_size))++SKB_DATA_ALIGN(sizeof(structskb_shared_info));ret=mvneta_setup_rxqs(pp);if(ret)
@@ -1969,32 +1969,15 @@ static int mvneta_poll(struct napi_struct *napi, int budget)staticintmvneta_rxq_fill(structmvneta_port*pp,structmvneta_rx_queue*rxq,intnum){-structnet_device*dev=pp->dev;inti;for(i=0;i<num;i++){-structsk_buff*skb;-structmvneta_rx_desc*rx_desc;-unsignedlongphys_addr;--skb=dev_alloc_skb(pp->pkt_size);-if(!skb){-netdev_err(dev,"%s:rxq %d, %d of %d buffs filled\n",+memset(rxq->descs+i,0,sizeof(structmvneta_rx_desc));+if(mvneta_rx_refill(pp,rxq->descs+i)!=0){+netdev_err(pp->dev,"%s:rxq %d, %d of %d buffs filled\n",__func__,rxq->id,i,num);break;}--rx_desc=rxq->descs+i;-memset(rx_desc,0,sizeof(structmvneta_rx_desc));-phys_addr=dma_map_single(dev->dev.parent,skb->head,-MVNETA_RX_BUF_SIZE(pp->pkt_size),-DMA_FROM_DEVICE);-if(unlikely(dma_mapping_error(dev->dev.parent,phys_addr))){-dev_kfree_skb(skb);-break;-}--mvneta_rx_desc_fill(rx_desc,phys_addr,(u32)skb);}/* Add this number of RX descriptors as non occupied (ready to
Currently, the mvneta driver tries to prefetch the current Rx
descriptor during read. Tests have shown that prefetching the
next one instead increases general performance by about 1% on
HTTP traffic.
Cc: Thomas Petazzoni <redacted>
Cc: Gregory CLEMENT <redacted>
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
drivers/net/ethernet/marvell/mvneta.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
calling dma_map_single()/dma_unmap_single() is quite expensive compared
to copying a small packet. So let's copy short frames and keep the buffers
mapped. We set the limit to 256 bytes which seems to give good results both
on the XP-GP board and on the AX3/4.
The Rx small packet rate increased by 16.4% doing this, from 486kpps to
573kpps. It is worth noting that even the call to the function
dma_sync_single_range_for_cpu() is expensive (300 ns) although less
than dma_unmap_single(). Without it, the packet rate raises to 711kpps
(+24% more). Thus on systems where coherency from device to CPU is
guaranteed by a snoop control unit, this patch should provide even more
gains, and probably rx_copybreak could be increased.
Cc: Thomas Petazzoni <redacted>
Cc: Gregory CLEMENT <redacted>
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
drivers/net/ethernet/marvell/mvneta.c | 44 ++++++++++++++++++++++++++++++-----
1 file changed, 38 insertions(+), 6 deletions(-)
@@ -1463,22 +1465,51 @@ static int mvneta_rx(struct mvneta_port *pp, int rx_todo,rx_done++;rx_filled++;rx_status=rx_desc->status;+rx_bytes=rx_desc->data_size-(ETH_FCS_LEN+MVNETA_MH_SIZE);data=(unsignedchar*)rx_desc->buf_cookie;if(!mvneta_rxq_desc_is_first_last(rx_status)||-(rx_status&MVNETA_RXD_ERR_SUMMARY)||-!(skb=build_skb(data,pp->frag_size>PAGE_SIZE?0:pp->frag_size))){+(rx_status&MVNETA_RXD_ERR_SUMMARY)){+err_drop_frame:dev->stats.rx_errors++;mvneta_rx_error(pp,rx_desc);/* leave the descriptor untouched */continue;}-dma_unmap_single(pp->dev->dev.parent,rx_desc->buf_phys_addr,+if(rx_bytes<=rx_copybreak){+/* better copy a small frame and not unmap the DMA region */+skb=netdev_alloc_skb_ip_align(dev,rx_bytes);+if(unlikely(!skb))+gotoerr_drop_frame;++dma_sync_single_range_for_cpu(dev->dev.parent,+rx_desc->buf_phys_addr,+MVNETA_MH_SIZE+NET_SKB_PAD,+rx_bytes,+DMA_FROM_DEVICE);+memcpy(skb_put(skb,rx_bytes),+data+MVNETA_MH_SIZE+NET_SKB_PAD,+rx_bytes);++skb->protocol=eth_type_trans(skb,dev);+mvneta_rx_csum(pp,rx_status,skb);+napi_gro_receive(&pp->napi,skb);++rcvd_pkts++;+rcvd_bytes+=rx_bytes;++/* leave the descriptor and buffer untouched */+continue;+}++skb=build_skb(data,pp->frag_size>PAGE_SIZE?0:pp->frag_size);+if(!skb)+gotoerr_drop_frame;++dma_unmap_single(dev->dev.parent,rx_desc->buf_phys_addr,MVNETA_RX_BUF_SIZE(pp->pkt_size),DMA_FROM_DEVICE);-rx_bytes=rx_desc->data_size--(ETH_FCS_LEN+MVNETA_MH_SIZE);rcvd_pkts++;rcvd_bytes+=rx_bytes;
From: David Laight <hidden> Date: 2014-01-13 10:15:17
From: Willy Tarreau
calling dma_map_single()/dma_unmap_single() is quite expensive compared
to copying a small packet. So let's copy short frames and keep the buffers
mapped. We set the limit to 256 bytes which seems to give good results both
on the XP-GP board and on the AX3/4.
Which architecture is this?
I presume it is one that needs iommu setup and/or cache flushing.
The Rx small packet rate increased by 16.4% doing this, from 486kpps to
573kpps. It is worth noting that even the call to the function
dma_sync_single_range_for_cpu() is expensive (300 ns) although less
than dma_unmap_single(). Without it, the packet rate raises to 711kpps
(+24% more). Thus on systems where coherency from device to CPU is
guaranteed by a snoop control unit, this patch should provide even more
gains, and probably rx_copybreak could be increased.
Is that the right way around?
If cache coherency is guaranteed then I'd have thought that the dma sync
would be a nop.
...
You can probably arrange for the copy to be fully aligned since
the partial words at both ends can be safely read and written.
That might speed things up further.
David
Hi David,
On Mon, Jan 13, 2014 at 10:13:16AM +0000, David Laight wrote:
From: Willy Tarreau
quoted
calling dma_map_single()/dma_unmap_single() is quite expensive compared
to copying a small packet. So let's copy short frames and keep the buffers
mapped. We set the limit to 256 bytes which seems to give good results both
on the XP-GP board and on the AX3/4.
Which architecture is this?
It's an ARMv7.
I presume it is one that needs iommu setup and/or cache flushing.
Just wait for cache snoop completion.
quoted
The Rx small packet rate increased by 16.4% doing this, from 486kpps to
573kpps. It is worth noting that even the call to the function
dma_sync_single_range_for_cpu() is expensive (300 ns) although less
than dma_unmap_single(). Without it, the packet rate raises to 711kpps
(+24% more). Thus on systems where coherency from device to CPU is
guaranteed by a snoop control unit, this patch should provide even more
gains, and probably rx_copybreak could be increased.
Is that the right way around?
If cache coherency is guaranteed then I'd have thought that the dma sync
would be a nop.
It's a bit more tricky. I found that the DMA API is not optimal for such
an architecture, because we need to wait *once* for the cache snooping to
complete at the beginning of the Rx loop, and then all other access may be
done with a NOP. However, since we don't currently have the ability to do
a first call and replace the other ones with a NOP, we still have to do
a dma_sync_single_for_cpu() for each packet, resulting in waiting for cache
snoop completion for each packet.
I've hacked the DMA API to add support for ops->iobarrier and test for it
at the beginning of the loop, call it, then avoid doing dma_sync_* afterwards
if it's defined. That way I reach the higher performance mentionned above.
But in my opinion, this is only material for future discussions.
You can probably arrange for the copy to be fully aligned since
the partial words at both ends can be safely read and written.
That might speed things up further.
In fact it does not, this is what the very first patch did but I did not
see any difference.
Thanks,
Willy
this patch series implements several performance improvements on the
mvneta driver.
Ignore that last email, I meant to say that this series did not apply
cleanly to net-next, sorry for the confusion.
David, this is because the first series is not applied first. I can
understand it was not very clear (just mentionned in the e-mail's body).
Do you prefer me to send you the two series for net-next or is it better
to wait for the first series to be merged first before applying the
second one ?
Just tell me what you prefer and I'll adapt.
Thanks,
Willy
this patch series implements several performance improvements on the
mvneta driver.
Ignore that last email, I meant to say that this series did not apply
cleanly to net-next, sorry for the confusion.
David, this is because the first series is not applied first. I can
understand it was not very clear (just mentionned in the e-mail's body).
Do you prefer me to send you the two series for net-next or is it better
to wait for the first series to be merged first before applying the
second one ?
Just tell me what you prefer and I'll adapt.
this patch series implements several performance improvements on the
mvneta driver.
Ignore that last email, I meant to say that this series did not apply
cleanly to net-next, sorry for the confusion.
David, this is because the first series is not applied first. I can
understand it was not very clear (just mentionned in the e-mail's body).
Do you prefer me to send you the two series for net-next or is it better
to wait for the first series to be merged first before applying the
second one ?
Just tell me what you prefer and I'll adapt.