DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH] ip_frag: replace the rte memcpy with memcpy
@ 2022-06-18 14:09 Huichao Cai
  2022-06-21 15:11 ` David Marchand
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Huichao Cai @ 2022-06-18 14:09 UTC (permalink / raw)
  To: dev; +Cc: konstantin.ananyev

To resolve the compilation warning,replace the rte_memcpy with memcpy.
Modify in file test_ipfrag.c and rte_ipv4_fragmentation.c.

Signed-off-by: Huichao Cai <chcchc88@163.com>
---
 app/test/test_ipfrag.c               | 13 ++++++-------
 lib/ip_frag/rte_ipv4_fragmentation.c |  7 +++----
 2 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/app/test/test_ipfrag.c b/app/test/test_ipfrag.c
index dc62b0e..ba0ffd0 100644
--- a/app/test/test_ipfrag.c
+++ b/app/test/test_ipfrag.c
@@ -23,7 +23,6 @@
 
 #include <rte_ip_frag.h>
 #include <rte_mbuf.h>
-#include <rte_memcpy.h>
 #include <rte_random.h>
 
 #define NUM_MBUFS 128
@@ -147,13 +146,13 @@ static void ut_teardown(void)
 		if (opt_copied) {
 			expected_opt->len =
 				sizeof(expected_first_frag_ipv4_opts_copied);
-			rte_memcpy(expected_opt->data,
+			memcpy(expected_opt->data,
 				expected_first_frag_ipv4_opts_copied,
 				sizeof(expected_first_frag_ipv4_opts_copied));
 		} else {
 			expected_opt->len =
 				sizeof(expected_first_frag_ipv4_opts_nocopied);
-			rte_memcpy(expected_opt->data,
+			memcpy(expected_opt->data,
 				expected_first_frag_ipv4_opts_nocopied,
 				sizeof(expected_first_frag_ipv4_opts_nocopied));
 		}
@@ -161,13 +160,13 @@ static void ut_teardown(void)
 		if (opt_copied) {
 			expected_opt->len =
 				sizeof(expected_sub_frag_ipv4_opts_copied);
-			rte_memcpy(expected_opt->data,
+			memcpy(expected_opt->data,
 				expected_sub_frag_ipv4_opts_copied,
 				sizeof(expected_sub_frag_ipv4_opts_copied));
 		} else {
 			expected_opt->len =
 				sizeof(expected_sub_frag_ipv4_opts_nocopied);
-			rte_memcpy(expected_opt->data,
+			memcpy(expected_opt->data,
 				expected_sub_frag_ipv4_opts_nocopied,
 				sizeof(expected_sub_frag_ipv4_opts_nocopied));
 		}
@@ -227,7 +226,7 @@ static void ut_teardown(void)
 	hdr->src_addr = rte_cpu_to_be_32(0x8080808);
 	hdr->dst_addr = rte_cpu_to_be_32(0x8080404);
 
-	rte_memcpy(hdr + 1, opt.data, opt.len);
+	memcpy(hdr + 1, opt.data, opt.len);
 }
 
 static void
@@ -312,7 +311,7 @@ static void ut_teardown(void)
 				char *iph_opt = rte_pktmbuf_mtod_offset(mb[i],
 				    char *, sizeof(struct rte_ipv4_hdr));
 				opt->len = opt_len;
-				rte_memcpy(opt->data, iph_opt, opt_len);
+				memcpy(opt->data, iph_opt, opt_len);
 			} else {
 				opt->len = RTE_IPV4_HDR_OPT_MAX_LEN;
 				memset(opt->data, RTE_IPV4_HDR_OPT_EOL,
diff --git a/lib/ip_frag/rte_ipv4_fragmentation.c b/lib/ip_frag/rte_ipv4_fragmentation.c
index a19f6fd..27a8ad2 100644
--- a/lib/ip_frag/rte_ipv4_fragmentation.c
+++ b/lib/ip_frag/rte_ipv4_fragmentation.c
@@ -5,7 +5,6 @@
 #include <stddef.h>
 #include <errno.h>
 
-#include <rte_memcpy.h>
 #include <rte_ether.h>
 
 #include "ip_frag_common.h"
@@ -26,7 +25,7 @@ static inline void __fill_ipv4hdr_frag(struct rte_ipv4_hdr *dst,
 		const struct rte_ipv4_hdr *src, uint16_t header_len,
 		uint16_t len, uint16_t fofs, uint16_t dofs, uint32_t mf)
 {
-	rte_memcpy(dst, src, header_len);
+	memcpy(dst, src, header_len);
 	fofs = (uint16_t)(fofs + (dofs >> RTE_IPV4_HDR_FO_SHIFT));
 	fofs = (uint16_t)(fofs | mf << RTE_IPV4_HDR_MF_SHIFT);
 	dst->fragment_offset = rte_cpu_to_be_16(fofs);
@@ -48,7 +47,7 @@ static inline uint16_t __create_ipopt_frag_hdr(uint8_t *iph,
 	struct rte_ipv4_hdr *iph_opt = (struct rte_ipv4_hdr *)ipopt_frag_hdr;
 
 	ipopt_len = 0;
-	rte_memcpy(ipopt_frag_hdr, iph, sizeof(struct rte_ipv4_hdr));
+	memcpy(ipopt_frag_hdr, iph, sizeof(struct rte_ipv4_hdr));
 	ipopt_frag_hdr += sizeof(struct rte_ipv4_hdr);
 
 	uint8_t *p_opt = iph + sizeof(struct rte_ipv4_hdr);
@@ -65,7 +64,7 @@ static inline uint16_t __create_ipopt_frag_hdr(uint8_t *iph,
 			break;
 
 		if (RTE_IPV4_HDR_OPT_COPIED(*p_opt)) {
-			rte_memcpy(ipopt_frag_hdr + ipopt_len,
+			memcpy(ipopt_frag_hdr + ipopt_len,
 				p_opt, p_opt[1]);
 			ipopt_len += p_opt[1];
 		}
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] ip_frag: replace the rte memcpy with memcpy
  2022-06-18 14:09 [PATCH] ip_frag: replace the rte memcpy with memcpy Huichao Cai
@ 2022-06-21 15:11 ` David Marchand
  2022-06-22 22:49 ` Konstantin Ananyev
  2022-06-23 14:26 ` David Marchand
  2 siblings, 0 replies; 8+ messages in thread
From: David Marchand @ 2022-06-21 15:11 UTC (permalink / raw)
  To: Huichao Cai, Konstantin Ananyev; +Cc: dev, Stephen Hemminger, Thomas Monjalon

On Sat, Jun 18, 2022 at 4:10 PM Huichao Cai <chcchc88@163.com> wrote:
>
> To resolve the compilation warning,replace the rte_memcpy with memcpy.
> Modify in file test_ipfrag.c and rte_ipv4_fragmentation.c.
>
> Signed-off-by: Huichao Cai <chcchc88@163.com>

Fixed Konstantin mail address.


> ---
>  app/test/test_ipfrag.c               | 13 ++++++-------
>  lib/ip_frag/rte_ipv4_fragmentation.c |  7 +++----
>  2 files changed, 9 insertions(+), 11 deletions(-)
>
> diff --git a/app/test/test_ipfrag.c b/app/test/test_ipfrag.c
> index dc62b0e..ba0ffd0 100644
> --- a/app/test/test_ipfrag.c
> +++ b/app/test/test_ipfrag.c
> @@ -23,7 +23,6 @@
>
>  #include <rte_ip_frag.h>
>  #include <rte_mbuf.h>
> -#include <rte_memcpy.h>
>  #include <rte_random.h>
>
>  #define NUM_MBUFS 128
> @@ -147,13 +146,13 @@ static void ut_teardown(void)
>                 if (opt_copied) {
>                         expected_opt->len =
>                                 sizeof(expected_first_frag_ipv4_opts_copied);
> -                       rte_memcpy(expected_opt->data,
> +                       memcpy(expected_opt->data,
>                                 expected_first_frag_ipv4_opts_copied,
>                                 sizeof(expected_first_frag_ipv4_opts_copied));
>                 } else {
>                         expected_opt->len =
>                                 sizeof(expected_first_frag_ipv4_opts_nocopied);
> -                       rte_memcpy(expected_opt->data,
> +                       memcpy(expected_opt->data,
>                                 expected_first_frag_ipv4_opts_nocopied,
>                                 sizeof(expected_first_frag_ipv4_opts_nocopied));
>                 }
> @@ -161,13 +160,13 @@ static void ut_teardown(void)
>                 if (opt_copied) {
>                         expected_opt->len =
>                                 sizeof(expected_sub_frag_ipv4_opts_copied);
> -                       rte_memcpy(expected_opt->data,
> +                       memcpy(expected_opt->data,
>                                 expected_sub_frag_ipv4_opts_copied,
>                                 sizeof(expected_sub_frag_ipv4_opts_copied));
>                 } else {
>                         expected_opt->len =
>                                 sizeof(expected_sub_frag_ipv4_opts_nocopied);
> -                       rte_memcpy(expected_opt->data,
> +                       memcpy(expected_opt->data,
>                                 expected_sub_frag_ipv4_opts_nocopied,
>                                 sizeof(expected_sub_frag_ipv4_opts_nocopied));
>                 }
> @@ -227,7 +226,7 @@ static void ut_teardown(void)
>         hdr->src_addr = rte_cpu_to_be_32(0x8080808);
>         hdr->dst_addr = rte_cpu_to_be_32(0x8080404);
>
> -       rte_memcpy(hdr + 1, opt.data, opt.len);
> +       memcpy(hdr + 1, opt.data, opt.len);
>  }
>
>  static void
> @@ -312,7 +311,7 @@ static void ut_teardown(void)
>                                 char *iph_opt = rte_pktmbuf_mtod_offset(mb[i],
>                                     char *, sizeof(struct rte_ipv4_hdr));
>                                 opt->len = opt_len;
> -                               rte_memcpy(opt->data, iph_opt, opt_len);
> +                               memcpy(opt->data, iph_opt, opt_len);
>                         } else {
>                                 opt->len = RTE_IPV4_HDR_OPT_MAX_LEN;
>                                 memset(opt->data, RTE_IPV4_HDR_OPT_EOL,
> diff --git a/lib/ip_frag/rte_ipv4_fragmentation.c b/lib/ip_frag/rte_ipv4_fragmentation.c
> index a19f6fd..27a8ad2 100644
> --- a/lib/ip_frag/rte_ipv4_fragmentation.c
> +++ b/lib/ip_frag/rte_ipv4_fragmentation.c
> @@ -5,7 +5,6 @@
>  #include <stddef.h>
>  #include <errno.h>
>
> -#include <rte_memcpy.h>
>  #include <rte_ether.h>
>
>  #include "ip_frag_common.h"
> @@ -26,7 +25,7 @@ static inline void __fill_ipv4hdr_frag(struct rte_ipv4_hdr *dst,
>                 const struct rte_ipv4_hdr *src, uint16_t header_len,
>                 uint16_t len, uint16_t fofs, uint16_t dofs, uint32_t mf)
>  {
> -       rte_memcpy(dst, src, header_len);
> +       memcpy(dst, src, header_len);
>         fofs = (uint16_t)(fofs + (dofs >> RTE_IPV4_HDR_FO_SHIFT));
>         fofs = (uint16_t)(fofs | mf << RTE_IPV4_HDR_MF_SHIFT);
>         dst->fragment_offset = rte_cpu_to_be_16(fofs);
> @@ -48,7 +47,7 @@ static inline uint16_t __create_ipopt_frag_hdr(uint8_t *iph,
>         struct rte_ipv4_hdr *iph_opt = (struct rte_ipv4_hdr *)ipopt_frag_hdr;
>
>         ipopt_len = 0;
> -       rte_memcpy(ipopt_frag_hdr, iph, sizeof(struct rte_ipv4_hdr));
> +       memcpy(ipopt_frag_hdr, iph, sizeof(struct rte_ipv4_hdr));
>         ipopt_frag_hdr += sizeof(struct rte_ipv4_hdr);
>
>         uint8_t *p_opt = iph + sizeof(struct rte_ipv4_hdr);
> @@ -65,7 +64,7 @@ static inline uint16_t __create_ipopt_frag_hdr(uint8_t *iph,
>                         break;
>
>                 if (RTE_IPV4_HDR_OPT_COPIED(*p_opt)) {
> -                       rte_memcpy(ipopt_frag_hdr + ipopt_len,
> +                       memcpy(ipopt_frag_hdr + ipopt_len,
>                                 p_opt, p_opt[1]);
>                         ipopt_len += p_opt[1];
>                 }
> --
> 1.8.3.1
>

-- 
David Marchand


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] ip_frag: replace the rte memcpy with memcpy
  2022-06-18 14:09 [PATCH] ip_frag: replace the rte memcpy with memcpy Huichao Cai
  2022-06-21 15:11 ` David Marchand
@ 2022-06-22 22:49 ` Konstantin Ananyev
  2022-06-23  2:35   ` Stephen Hemminger
  2022-06-23 14:26 ` David Marchand
  2 siblings, 1 reply; 8+ messages in thread
From: Konstantin Ananyev @ 2022-06-22 22:49 UTC (permalink / raw)
  To: Huichao Cai, dev; +Cc: konstantin.ananyev

18/06/2022 15:09, Huichao Cai пишет:
> To resolve the compilation warning,replace the rte_memcpy with memcpy.
> Modify in file test_ipfrag.c and rte_ipv4_fragmentation.c.
> 
> Signed-off-by: Huichao Cai <chcchc88@163.com>
> ---
>   app/test/test_ipfrag.c               | 13 ++++++-------
>   lib/ip_frag/rte_ipv4_fragmentation.c |  7 +++----
>   2 files changed, 9 insertions(+), 11 deletions(-)
> 
> diff --git a/app/test/test_ipfrag.c b/app/test/test_ipfrag.c
> index dc62b0e..ba0ffd0 100644
> --- a/app/test/test_ipfrag.c
> +++ b/app/test/test_ipfrag.c
> @@ -23,7 +23,6 @@
>   
>   #include <rte_ip_frag.h>
>   #include <rte_mbuf.h>
> -#include <rte_memcpy.h>
>   #include <rte_random.h>
>   
>   #define NUM_MBUFS 128
> @@ -147,13 +146,13 @@ static void ut_teardown(void)
>   		if (opt_copied) {
>   			expected_opt->len =
>   				sizeof(expected_first_frag_ipv4_opts_copied);
> -			rte_memcpy(expected_opt->data,
> +			memcpy(expected_opt->data,
>   				expected_first_frag_ipv4_opts_copied,
>   				sizeof(expected_first_frag_ipv4_opts_copied));
>   		} else {
>   			expected_opt->len =
>   				sizeof(expected_first_frag_ipv4_opts_nocopied);
> -			rte_memcpy(expected_opt->data,
> +			memcpy(expected_opt->data,
>   				expected_first_frag_ipv4_opts_nocopied,
>   				sizeof(expected_first_frag_ipv4_opts_nocopied));
>   		}
> @@ -161,13 +160,13 @@ static void ut_teardown(void)
>   		if (opt_copied) {
>   			expected_opt->len =
>   				sizeof(expected_sub_frag_ipv4_opts_copied);
> -			rte_memcpy(expected_opt->data,
> +			memcpy(expected_opt->data,
>   				expected_sub_frag_ipv4_opts_copied,
>   				sizeof(expected_sub_frag_ipv4_opts_copied));
>   		} else {
>   			expected_opt->len =
>   				sizeof(expected_sub_frag_ipv4_opts_nocopied);
> -			rte_memcpy(expected_opt->data,
> +			memcpy(expected_opt->data,
>   				expected_sub_frag_ipv4_opts_nocopied,
>   				sizeof(expected_sub_frag_ipv4_opts_nocopied));
>   		}
> @@ -227,7 +226,7 @@ static void ut_teardown(void)
>   	hdr->src_addr = rte_cpu_to_be_32(0x8080808);
>   	hdr->dst_addr = rte_cpu_to_be_32(0x8080404);
>   
> -	rte_memcpy(hdr + 1, opt.data, opt.len);
> +	memcpy(hdr + 1, opt.data, opt.len);
>   }
>   
>   static void
> @@ -312,7 +311,7 @@ static void ut_teardown(void)
>   				char *iph_opt = rte_pktmbuf_mtod_offset(mb[i],
>   				    char *, sizeof(struct rte_ipv4_hdr));
>   				opt->len = opt_len;
> -				rte_memcpy(opt->data, iph_opt, opt_len);
> +				memcpy(opt->data, iph_opt, opt_len);
>   			} else {
>   				opt->len = RTE_IPV4_HDR_OPT_MAX_LEN;
>   				memset(opt->data, RTE_IPV4_HDR_OPT_EOL,
> diff --git a/lib/ip_frag/rte_ipv4_fragmentation.c b/lib/ip_frag/rte_ipv4_fragmentation.c
> index a19f6fd..27a8ad2 100644
> --- a/lib/ip_frag/rte_ipv4_fragmentation.c
> +++ b/lib/ip_frag/rte_ipv4_fragmentation.c
> @@ -5,7 +5,6 @@
>   #include <stddef.h>
>   #include <errno.h>
>   
> -#include <rte_memcpy.h>
>   #include <rte_ether.h>
>   
>   #include "ip_frag_common.h"
> @@ -26,7 +25,7 @@ static inline void __fill_ipv4hdr_frag(struct rte_ipv4_hdr *dst,
>   		const struct rte_ipv4_hdr *src, uint16_t header_len,
>   		uint16_t len, uint16_t fofs, uint16_t dofs, uint32_t mf)
>   {
> -	rte_memcpy(dst, src, header_len);
> +	memcpy(dst, src, header_len);


I am fine with replacements in test and inside the lib, for cases
where 'len' parameter is constant value.
Though as I said before, here 'header_len' is not a constant value.
Are you sure it will not introduce any performance regression?


>   	fofs = (uint16_t)(fofs + (dofs >> RTE_IPV4_HDR_FO_SHIFT));
>   	fofs = (uint16_t)(fofs | mf << RTE_IPV4_HDR_MF_SHIFT);
>   	dst->fragment_offset = rte_cpu_to_be_16(fofs);
> @@ -48,7 +47,7 @@ static inline uint16_t __create_ipopt_frag_hdr(uint8_t *iph,
>   	struct rte_ipv4_hdr *iph_opt = (struct rte_ipv4_hdr *)ipopt_frag_hdr;
>   
>   	ipopt_len = 0;
> -	rte_memcpy(ipopt_frag_hdr, iph, sizeof(struct rte_ipv4_hdr));
> +	memcpy(ipopt_frag_hdr, iph, sizeof(struct rte_ipv4_hdr));
>   	ipopt_frag_hdr += sizeof(struct rte_ipv4_hdr);
>   
>   	uint8_t *p_opt = iph + sizeof(struct rte_ipv4_hdr);
> @@ -65,7 +64,7 @@ static inline uint16_t __create_ipopt_frag_hdr(uint8_t *iph,
>   			break;
>   
>   		if (RTE_IPV4_HDR_OPT_COPIED(*p_opt)) {
> -			rte_memcpy(ipopt_frag_hdr + ipopt_len,
> +			memcpy(ipopt_frag_hdr + ipopt_len,
>   				p_opt, p_opt[1]);
>   			ipopt_len += p_opt[1];
>   		}


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] ip_frag: replace the rte memcpy with memcpy
  2022-06-22 22:49 ` Konstantin Ananyev
@ 2022-06-23  2:35   ` Stephen Hemminger
  2022-06-23 14:24     ` David Marchand
  2022-06-24 17:25     ` Konstantin Ananyev
  0 siblings, 2 replies; 8+ messages in thread
From: Stephen Hemminger @ 2022-06-23  2:35 UTC (permalink / raw)
  To: Konstantin Ananyev; +Cc: Huichao Cai, dev, konstantin.ananyev

On Wed, 22 Jun 2022 23:49:39 +0100
Konstantin Ananyev <konstantin.v.ananyev@yandex.ru> wrote:

> > @@ -26,7 +25,7 @@ static inline void __fill_ipv4hdr_frag(struct rte_ipv4_hdr *dst,
> >   		const struct rte_ipv4_hdr *src, uint16_t header_len,
> >   		uint16_t len, uint16_t fofs, uint16_t dofs, uint32_t mf)
> >   {
> > -	rte_memcpy(dst, src, header_len);
> > +	memcpy(dst, src, header_len);  
> 
> 
> I am fine with replacements in test and inside the lib, for cases
> where 'len' parameter is constant value.
> Though as I said before, here 'header_len' is not a constant value.
> Are you sure it will not introduce any performance regression?

Do you have any performance tests. The ip header options are very small.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] ip_frag: replace the rte memcpy with memcpy
  2022-06-23  2:35   ` Stephen Hemminger
@ 2022-06-23 14:24     ` David Marchand
  2022-06-24 17:25     ` Konstantin Ananyev
  1 sibling, 0 replies; 8+ messages in thread
From: David Marchand @ 2022-06-23 14:24 UTC (permalink / raw)
  To: Stephen Hemminger, Konstantin Ananyev
  Cc: Huichao Cai, dev, Ananyev, Konstantin

On Thu, Jun 23, 2022 at 4:35 AM Stephen Hemminger
<stephen@networkplumber.org> wrote:
> On Wed, 22 Jun 2022 23:49:39 +0100
> Konstantin Ananyev <konstantin.v.ananyev@yandex.ru> wrote:
>
> > > @@ -26,7 +25,7 @@ static inline void __fill_ipv4hdr_frag(struct rte_ipv4_hdr *dst,
> > >             const struct rte_ipv4_hdr *src, uint16_t header_len,
> > >             uint16_t len, uint16_t fofs, uint16_t dofs, uint32_t mf)
> > >   {
> > > -   rte_memcpy(dst, src, header_len);
> > > +   memcpy(dst, src, header_len);
> >
> >
> > I am fine with replacements in test and inside the lib, for cases
> > where 'len' parameter is constant value.
> > Though as I said before, here 'header_len' is not a constant value.
> > Are you sure it will not introduce any performance regression?
>
> Do you have any performance tests. The ip header options are very small.

We have no alternative to this patch for fixing build with gcc 12
(which we want for rc2).
As I mentionned during the maintainers call, I will be merging this
patch for rc2 and wait for non regression tests.

We can still revert this patch if the performance is impacted and go
with an alternative approach.

-- 
David Marchand


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] ip_frag: replace the rte memcpy with memcpy
  2022-06-18 14:09 [PATCH] ip_frag: replace the rte memcpy with memcpy Huichao Cai
  2022-06-21 15:11 ` David Marchand
  2022-06-22 22:49 ` Konstantin Ananyev
@ 2022-06-23 14:26 ` David Marchand
  2 siblings, 0 replies; 8+ messages in thread
From: David Marchand @ 2022-06-23 14:26 UTC (permalink / raw)
  To: Huichao Cai; +Cc: dev, Ananyev, Konstantin, Stephen Hemminger

On Sat, Jun 18, 2022 at 4:10 PM Huichao Cai <chcchc88@163.com> wrote:
>
> To resolve the compilation warning,replace the rte_memcpy with memcpy.
> Modify in file test_ipfrag.c and rte_ipv4_fragmentation.c.
>

These warnings appeared with:
Fixes: b50a14a853aa ("ip_frag: add IPv4 options fragment")

> Signed-off-by: Huichao Cai <chcchc88@163.com>

I updated the commitlog to give some detail.
Applied to fix compilation with GCC 12 for -rc2.

Thanks.

-- 
David Marchand


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] ip_frag: replace the rte memcpy with memcpy
  2022-06-23  2:35   ` Stephen Hemminger
  2022-06-23 14:24     ` David Marchand
@ 2022-06-24 17:25     ` Konstantin Ananyev
  2022-06-27 11:12       ` Liang Ma
  1 sibling, 1 reply; 8+ messages in thread
From: Konstantin Ananyev @ 2022-06-24 17:25 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Huichao Cai, dev, konstantin.ananyev, David Marchand

23/06/2022 03:35, Stephen Hemminger пишет:
> On Wed, 22 Jun 2022 23:49:39 +0100
> Konstantin Ananyev <konstantin.v.ananyev@yandex.ru> wrote:
> 
>>> @@ -26,7 +25,7 @@ static inline void __fill_ipv4hdr_frag(struct rte_ipv4_hdr *dst,
>>>    		const struct rte_ipv4_hdr *src, uint16_t header_len,
>>>    		uint16_t len, uint16_t fofs, uint16_t dofs, uint32_t mf)
>>>    {
>>> -	rte_memcpy(dst, src, header_len);
>>> +	memcpy(dst, src, header_len);
>>
>>
>> I am fine with replacements in test and inside the lib, for cases
>> where 'len' parameter is constant value.
>> Though as I said before, here 'header_len' is not a constant value.
>> Are you sure it will not introduce any performance regression?
> 
> Do you have any performance tests. The ip header options are very small.


 From my experience - usually it is not about how big or small amount
we need to copy. It is about can compiler evaluate 'size' parameter
for memcpy() at compilation time or not.
If it can, great - it will most likely replace memcpy()
with some really well optimized code.
If not it has to generate a proper call to actual
memcpy() function. Which again, can be well optimized, but the
overhead of the function call itself can still be noticeable,
specially for small copies.
Anyway, as I can see, David already integrated these changes anyway.
So now, we'll have to wait and see would anyone complain or not.
About performance testing, the only one I am aware about:
examples/ip_fragmentation

Konstantin



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] ip_frag: replace the rte memcpy with memcpy
  2022-06-24 17:25     ` Konstantin Ananyev
@ 2022-06-27 11:12       ` Liang Ma
  0 siblings, 0 replies; 8+ messages in thread
From: Liang Ma @ 2022-06-27 11:12 UTC (permalink / raw)
  To: Konstantin Ananyev
  Cc: Stephen Hemminger, Huichao Cai, dev, konstantin.ananyev, David Marchand

On Fri, Jun 24, 2022 at 06:25:10PM +0100, Konstantin Ananyev wrote:
> 23/06/2022 03:35, Stephen Hemminger пишет:
> > On Wed, 22 Jun 2022 23:49:39 +0100
> > Konstantin Ananyev <konstantin.v.ananyev@yandex.ru> wrote:
> > 
> > > > @@ -26,7 +25,7 @@ static inline void __fill_ipv4hdr_frag(struct rte_ipv4_hdr *dst,
> > > >    		const struct rte_ipv4_hdr *src, uint16_t header_len,
> > > >    		uint16_t len, uint16_t fofs, uint16_t dofs, uint32_t mf)
> > > >    {
> > > > -	rte_memcpy(dst, src, header_len);
> > > > +	memcpy(dst, src, header_len);
> > > 
> > > 
> > > I am fine with replacements in test and inside the lib, for cases
> > > where 'len' parameter is constant value.
> > > Though as I said before, here 'header_len' is not a constant value.
> > > Are you sure it will not introduce any performance regression?
> > 
> > Do you have any performance tests. The ip header options are very small.
> 
> 
> From my experience - usually it is not about how big or small amount
> we need to copy. It is about can compiler evaluate 'size' parameter
> for memcpy() at compilation time or not.
> If it can, great - it will most likely replace memcpy()
> with some really well optimized code.
> If not it has to generate a proper call to actual
> memcpy() function. Which again, can be well optimized, but the
> overhead of the function call itself can still be noticeable,
> specially for small copies.
> Anyway, as I can see, David already integrated these changes anyway.
> So now, we'll have to wait and see would anyone complain or not.
> About performance testing, the only one I am aware about:
> examples/ip_fragmentation
> 
> Konstantin
> 
> 
For some small(<1k) size, " rep movsb" is very fast. much faster than I
expected. I just noticed this in another application. 


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2022-06-27 11:12 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-18 14:09 [PATCH] ip_frag: replace the rte memcpy with memcpy Huichao Cai
2022-06-21 15:11 ` David Marchand
2022-06-22 22:49 ` Konstantin Ananyev
2022-06-23  2:35   ` Stephen Hemminger
2022-06-23 14:24     ` David Marchand
2022-06-24 17:25     ` Konstantin Ananyev
2022-06-27 11:12       ` Liang Ma
2022-06-23 14:26 ` David Marchand

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).