diff mbox series

net/memif: replace memcpy() with rte_memcpy() to improve perf

Message ID 20210623164935.59796-1-drc@linux.vnet.ibm.com (mailing list archive)
State Rejected, archived
Delegated to: Andrew Rybchenko
Headers
Series net/memif: replace memcpy() with rte_memcpy() to improve perf |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-testing warning apply patch failure
ci/Intel-compilation warning apply issues

Commit Message

David Christensen June 23, 2021, 4:49 p.m. UTC 
  Replacing memcpy() with rte_memcpy() improved 64 byte packet
performance by 33% on a POWER9 system and by 10% on an x86_64
system.

Signed-off-by: David Christensen <drc@linux.vnet.ibm.com>
---
 drivers/net/memif/rte_eth_memif.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comments

Stephen Hemminger June 23, 2021, 5:03 p.m. UTC | #1 
On Wed, 23 Jun 2021 09:49:35 -0700
David Christensen <drc@linux.vnet.ibm.com> wrote:

> Replacing memcpy() with rte_memcpy() improved 64 byte packet
> performance by 33% on a POWER9 system and by 10% on an x86_64
> system.

I see rte_memcpy was already used in the patch diff

> Signed-off-by: David Christensen <drc@linux.vnet.ibm.com>
> ---
>  drivers/net/memif/rte_eth_memif.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c
> index d17222c612..330c9c2fd6 100644
> --- a/drivers/net/memif/rte_eth_memif.c
> +++ b/drivers/net/memif/rte_eth_memif.c
> @@ -384,8 +384,8 @@ eth_memif_rx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
>  				rte_pktmbuf_pkt_len(mbuf_head) += cp_len;
>  
>  			rte_memcpy(rte_pktmbuf_mtod_offset(mbuf, void *, dst_off),
> -			       (uint8_t *)memif_get_buffer(proc_private, d0) + src_off,
> -			       cp_len);
> +			       (uint8_t *)memif_get_buffer(proc_private, d0)
> +			       + src_off, cp_len);

This just changes line break for no good reason.
>  
>  			src_off += cp_len;
>  			dst_off += cp_len;
> @@ -644,7 +644,8 @@ eth_memif_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
>  			}
>  			cp_len = RTE_MIN(dst_len, src_len);
>  
> -			rte_memcpy((uint8_t *)memif_get_buffer(proc_private, d0) + dst_off,
> +			rte_memcpy((uint8_t *)
> +			       memif_get_buffer(proc_private, d0) + dst_off,
>  			       rte_pktmbuf_mtod_offset(mbuf, void *, src_off),
>  			       cp_len);
>  

ditto.

Look like this patch is confused, the description does not match the code.
Andrew Rybchenko July 1, 2021, 4:22 p.m. UTC | #2 
On 6/23/21 8:03 PM, Stephen Hemminger wrote:
> On Wed, 23 Jun 2021 09:49:35 -0700
> David Christensen <drc@linux.vnet.ibm.com> wrote:
> 
>> Replacing memcpy() with rte_memcpy() improved 64 byte packet
>> performance by 33% on a POWER9 system and by 10% on an x86_64
>> system.
> 
> I see rte_memcpy was already used in the patch diff
> 
>> Signed-off-by: David Christensen <drc@linux.vnet.ibm.com>
>> ---
>>  drivers/net/memif/rte_eth_memif.c | 7 ++++---
>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c
>> index d17222c612..330c9c2fd6 100644
>> --- a/drivers/net/memif/rte_eth_memif.c
>> +++ b/drivers/net/memif/rte_eth_memif.c
>> @@ -384,8 +384,8 @@ eth_memif_rx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
>>  				rte_pktmbuf_pkt_len(mbuf_head) += cp_len;
>>  
>>  			rte_memcpy(rte_pktmbuf_mtod_offset(mbuf, void *, dst_off),
>> -			       (uint8_t *)memif_get_buffer(proc_private, d0) + src_off,
>> -			       cp_len);
>> +			       (uint8_t *)memif_get_buffer(proc_private, d0)
>> +			       + src_off, cp_len);
> 
> This just changes line break for no good reason.
>>  
>>  			src_off += cp_len;
>>  			dst_off += cp_len;
>> @@ -644,7 +644,8 @@ eth_memif_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
>>  			}
>>  			cp_len = RTE_MIN(dst_len, src_len);
>>  
>> -			rte_memcpy((uint8_t *)memif_get_buffer(proc_private, d0) + dst_off,
>> +			rte_memcpy((uint8_t *)
>> +			       memif_get_buffer(proc_private, d0) + dst_off,
>>  			       rte_pktmbuf_mtod_offset(mbuf, void *, src_off),
>>  			       cp_len);
>>  
> 
> ditto.
> 
> Look like this patch is confused, the description does not match the code.
> 


I guess it was a patch on top of [1]. Marking this one as rejected since
I've applied style fixes before applying it.

[1]
https://patches.dpdk.org/project/dpdk/patch/20210623203154.72409-1-drc@linux.vnet.ibm.com/
diff mbox series

Patch

diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c
index d17222c612..330c9c2fd6 100644
--- a/drivers/net/memif/rte_eth_memif.c
+++ b/drivers/net/memif/rte_eth_memif.c
@@ -384,8 +384,8 @@  eth_memif_rx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 				rte_pktmbuf_pkt_len(mbuf_head) += cp_len;
 
 			rte_memcpy(rte_pktmbuf_mtod_offset(mbuf, void *, dst_off),
-			       (uint8_t *)memif_get_buffer(proc_private, d0) + src_off,
-			       cp_len);
+			       (uint8_t *)memif_get_buffer(proc_private, d0)
+			       + src_off, cp_len);
 
 			src_off += cp_len;
 			dst_off += cp_len;
@@ -644,7 +644,8 @@  eth_memif_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 			}
 			cp_len = RTE_MIN(dst_len, src_len);
 
-			rte_memcpy((uint8_t *)memif_get_buffer(proc_private, d0) + dst_off,
+			rte_memcpy((uint8_t *)
+			       memif_get_buffer(proc_private, d0) + dst_off,
 			       rte_pktmbuf_mtod_offset(mbuf, void *, src_off),
 			       cp_len);