DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH v1] net: expand the data size of L3 length to 16 bits
@ 2020-09-04  5:40 Haiyue Wang
  2020-09-04 18:55 ` Stephen Hemminger
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Haiyue Wang @ 2020-09-04  5:40 UTC (permalink / raw)
  To: dev; +Cc: Haiyue Wang, Olivier Matz

The IPv6 can have large extension header, like the "Segment Routing" may
have (32 + 1) * 8 = 264 bytes, the 8 bits size of L3 length can't handle
this case correctly.

Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
---
 lib/librte_net/rte_net.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/librte_net/rte_net.h b/lib/librte_net/rte_net.h
index 94b06d9ee..992fb088a 100644
--- a/lib/librte_net/rte_net.h
+++ b/lib/librte_net/rte_net.h
@@ -20,11 +20,11 @@ extern "C" {
  */
 struct rte_net_hdr_lens {
 	uint8_t l2_len;
-	uint8_t l3_len;
+	uint16_t l3_len;
 	uint8_t l4_len;
 	uint8_t tunnel_len;
 	uint8_t inner_l2_len;
-	uint8_t inner_l3_len;
+	uint16_t inner_l3_len;
 	uint8_t inner_l4_len;
 };
 
-- 
2.28.0


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

* Re: [dpdk-dev] [PATCH v1] net: expand the data size of L3 length to 16 bits
  2020-09-04  5:40 [dpdk-dev] [PATCH v1] net: expand the data size of L3 length to 16 bits Haiyue Wang
@ 2020-09-04 18:55 ` Stephen Hemminger
  2020-09-05  3:17   ` Wang, Haiyue
  2020-09-05  3:06 ` [dpdk-dev] [PATCH v2] net: adjust the header length parse size Haiyue Wang
  2020-09-07  1:56 ` [dpdk-dev] [PATCH v3] " Haiyue Wang
  2 siblings, 1 reply; 9+ messages in thread
From: Stephen Hemminger @ 2020-09-04 18:55 UTC (permalink / raw)
  To: Haiyue Wang; +Cc: dev, Olivier Matz

On Fri,  4 Sep 2020 13:40:20 +0800
Haiyue Wang <haiyue.wang@intel.com> wrote:

> diff --git a/lib/librte_net/rte_net.h b/lib/librte_net/rte_net.h
> index 94b06d9ee..992fb088a 100644
> --- a/lib/librte_net/rte_net.h
> +++ b/lib/librte_net/rte_net.h
> @@ -20,11 +20,11 @@ extern "C" {
>   */
>  struct rte_net_hdr_lens {
>  	uint8_t l2_len;
> -	uint8_t l3_len;
> +	uint16_t l3_len;
>  	uint8_t l4_len;

Please reorder these to avoid creating a structure hole.

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

* [dpdk-dev] [PATCH v2] net: adjust the header length parse size
  2020-09-04  5:40 [dpdk-dev] [PATCH v1] net: expand the data size of L3 length to 16 bits Haiyue Wang
  2020-09-04 18:55 ` Stephen Hemminger
@ 2020-09-05  3:06 ` Haiyue Wang
  2020-09-05 16:56   ` Stephen Hemminger
  2020-09-07  1:56 ` [dpdk-dev] [PATCH v3] " Haiyue Wang
  2 siblings, 1 reply; 9+ messages in thread
From: Haiyue Wang @ 2020-09-05  3:06 UTC (permalink / raw)
  To: dev; +Cc: stephen, ferruh.yigit, Haiyue Wang, Olivier Matz

Align to the rte_mbuf's design about Tx header length data size for the
header length parse result.
	struct {
		uint64_t   l2_len:7;             /*    88: 0  8 */
		uint64_t   l3_len:9;             /*    88: 7  8 */
		uint64_t   l4_len:8;             /*    88:16  8 */
		uint64_t   tso_segsz:16;         /*    88:24  8 */
		uint64_t   outer_l3_len:9;       /*    88:40  8 */
		uint64_t   outer_l2_len:7;       /*    88:49  8 */
	};

Now the IPv6 can support bigger extension header.

The below is the structure hole analysis result:

Before:
struct rte_net_hdr_lens {
        uint8_t                    l2_len;               /*     0     1 */
        uint8_t                    l3_len;               /*     1     1 */
        uint8_t                    l4_len;               /*     2     1 */
        uint8_t                    tunnel_len;           /*     3     1 */
        uint8_t                    inner_l2_len;         /*     4     1 */
        uint8_t                    inner_l3_len;         /*     5     1 */
        uint8_t                    inner_l4_len;         /*     6     1 */

        /* size: 7, cachelines: 1, members: 7 */
        /* last cacheline: 7 bytes */
};

Now:
struct rte_net_hdr_lens {
        uint64_t                   l2_len:7;             /*     0: 0  8 */
        uint64_t                   l3_len:9;             /*     0: 7  8 */
        uint64_t                   l4_len:8;             /*     0:16  8 */
        uint64_t                   tunnel_len:8;         /*     0:24  8 */
        uint64_t                   inner_l2_len:7;       /*     0:32  8 */
        uint64_t                   inner_l3_len:9;       /*     0:39  8 */
        uint64_t                   inner_l4_len:8;       /*     0:48  8 */

        /* size: 8, cachelines: 1, members: 7 */
        /* bit_padding: 8 bits */
        /* last cacheline: 8 bytes */
};

Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
---
v2: use bit field to avoid creating a structure hole.
---
 lib/librte_net/rte_net.h | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/lib/librte_net/rte_net.h b/lib/librte_net/rte_net.h
index 94b06d9ee..a14e3d814 100644
--- a/lib/librte_net/rte_net.h
+++ b/lib/librte_net/rte_net.h
@@ -18,14 +18,15 @@ extern "C" {
  * Structure containing header lengths associated to a packet, filled
  * by rte_net_get_ptype().
  */
+__extension__
 struct rte_net_hdr_lens {
-	uint8_t l2_len;
-	uint8_t l3_len;
-	uint8_t l4_len;
-	uint8_t tunnel_len;
-	uint8_t inner_l2_len;
-	uint8_t inner_l3_len;
-	uint8_t inner_l4_len;
+	uint64_t l2_len:7;
+	uint64_t l3_len:9;
+	uint64_t l4_len:8;
+	uint64_t tunnel_len:8;
+	uint64_t inner_l2_len:7;
+	uint64_t inner_l3_len:9;
+	uint64_t inner_l4_len:8;
 };
 
 /**
-- 
2.28.0


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

* Re: [dpdk-dev] [PATCH v1] net: expand the data size of L3 length to 16 bits
  2020-09-04 18:55 ` Stephen Hemminger
@ 2020-09-05  3:17   ` Wang, Haiyue
  0 siblings, 0 replies; 9+ messages in thread
From: Wang, Haiyue @ 2020-09-05  3:17 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev, Olivier Matz

> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Saturday, September 5, 2020 02:56
> To: Wang, Haiyue <haiyue.wang@intel.com>
> Cc: dev@dpdk.org; Olivier Matz <olivier.matz@6wind.com>
> Subject: Re: [dpdk-dev] [PATCH v1] net: expand the data size of L3 length to 16 bits
> 
> On Fri,  4 Sep 2020 13:40:20 +0800
> Haiyue Wang <haiyue.wang@intel.com> wrote:
> 
> > diff --git a/lib/librte_net/rte_net.h b/lib/librte_net/rte_net.h
> > index 94b06d9ee..992fb088a 100644
> > --- a/lib/librte_net/rte_net.h
> > +++ b/lib/librte_net/rte_net.h
> > @@ -20,11 +20,11 @@ extern "C" {
> >   */
> >  struct rte_net_hdr_lens {
> >  	uint8_t l2_len;
> > -	uint8_t l3_len;
> > +	uint16_t l3_len;
> >  	uint8_t l4_len;
> 
> Please reorder these to avoid creating a structure hole.

Done in v2, I use the bit field like rte_mbuf, and keep the member's original order.

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

* Re: [dpdk-dev] [PATCH v2] net: adjust the header length parse size
  2020-09-05  3:06 ` [dpdk-dev] [PATCH v2] net: adjust the header length parse size Haiyue Wang
@ 2020-09-05 16:56   ` Stephen Hemminger
  2020-09-07  2:14     ` Wang, Haiyue
  0 siblings, 1 reply; 9+ messages in thread
From: Stephen Hemminger @ 2020-09-05 16:56 UTC (permalink / raw)
  To: Haiyue Wang; +Cc: dev, ferruh.yigit, Olivier Matz

On Sat,  5 Sep 2020 11:06:46 +0800
Haiyue Wang <haiyue.wang@intel.com> wrote:

> Align to the rte_mbuf's design about Tx header length data size for the
> header length parse result.
> 	struct {
> 		uint64_t   l2_len:7;             /*    88: 0  8 */
> 		uint64_t   l3_len:9;             /*    88: 7  8 */
> 		uint64_t   l4_len:8;             /*    88:16  8 */
> 		uint64_t   tso_segsz:16;         /*    88:24  8 */
> 		uint64_t   outer_l3_len:9;       /*    88:40  8 */
> 		uint64_t   outer_l2_len:7;       /*    88:49  8 */
> 	};
> 
> Now the IPv6 can support bigger extension header.
> 
> The below is the structure hole analysis result:
> 
> Before:
> struct rte_net_hdr_lens {
>         uint8_t                    l2_len;               /*     0     1 */
>         uint8_t                    l3_len;               /*     1     1 */
>         uint8_t                    l4_len;               /*     2     1 */
>         uint8_t                    tunnel_len;           /*     3     1 */
>         uint8_t                    inner_l2_len;         /*     4     1 */
>         uint8_t                    inner_l3_len;         /*     5     1 */
>         uint8_t                    inner_l4_len;         /*     6     1 */
> 
>         /* size: 7, cachelines: 1, members: 7 */
>         /* last cacheline: 7 bytes */
> };
> 
> Now:
> struct rte_net_hdr_lens {
>         uint64_t                   l2_len:7;             /*     0: 0  8 */
>         uint64_t                   l3_len:9;             /*     0: 7  8 */
>         uint64_t                   l4_len:8;             /*     0:16  8 */
>         uint64_t                   tunnel_len:8;         /*     0:24  8 */
>         uint64_t                   inner_l2_len:7;       /*     0:32  8 */
>         uint64_t                   inner_l3_len:9;       /*     0:39  8 */
>         uint64_t                   inner_l4_len:8;       /*     0:48  8 */
> 
>         /* size: 8, cachelines: 1, members: 7 */
>         /* bit_padding: 8 bits */
>         /* last cacheline: 8 bytes */
> };
> 
> Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>

Bitfields are slow to access, compiler has to do mask/shift operations.
And there is no requirement that structure be the same size.

There is no requirement that fields be ordered the same as
the protocol header. Also tunnel length might get big.
Why not:

struct rte_net_hdr_lens {
	uint8_t l2_len;
	uint8_t inner_l2_len;
	uint16_t l3_len;
	uint16_t inner_l3_len;
	uint16_t tunnel_len;
	uint8_t l4_len;
	uint8_t inner_l4_len;
};



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

* [dpdk-dev] [PATCH v3] net: adjust the header length parse size
  2020-09-04  5:40 [dpdk-dev] [PATCH v1] net: expand the data size of L3 length to 16 bits Haiyue Wang
  2020-09-04 18:55 ` Stephen Hemminger
  2020-09-05  3:06 ` [dpdk-dev] [PATCH v2] net: adjust the header length parse size Haiyue Wang
@ 2020-09-07  1:56 ` Haiyue Wang
  2020-09-08 14:53   ` Stephen Hemminger
  2 siblings, 1 reply; 9+ messages in thread
From: Haiyue Wang @ 2020-09-07  1:56 UTC (permalink / raw)
  To: dev; +Cc: stephen, ferruh.yigit, Haiyue Wang, Olivier Matz

Enlarge the L3 and tunnel header length from 8-bit to 16-bit to handle
the bigger headers. And reorder the fields to avoid creating a structure
hole.

Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
---
v2: use bit field to avoid creating a structure hole.
v3: use basic type and reorder to avoid structure hole.
---
 lib/librte_net/rte_net.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/librte_net/rte_net.h b/lib/librte_net/rte_net.h
index 94b06d9ee..434435ffa 100644
--- a/lib/librte_net/rte_net.h
+++ b/lib/librte_net/rte_net.h
@@ -20,11 +20,11 @@ extern "C" {
  */
 struct rte_net_hdr_lens {
 	uint8_t l2_len;
-	uint8_t l3_len;
-	uint8_t l4_len;
-	uint8_t tunnel_len;
 	uint8_t inner_l2_len;
-	uint8_t inner_l3_len;
+	uint16_t l3_len;
+	uint16_t inner_l3_len;
+	uint16_t tunnel_len;
+	uint8_t l4_len;
 	uint8_t inner_l4_len;
 };
 
-- 
2.28.0


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

* Re: [dpdk-dev] [PATCH v2] net: adjust the header length parse size
  2020-09-05 16:56   ` Stephen Hemminger
@ 2020-09-07  2:14     ` Wang, Haiyue
  0 siblings, 0 replies; 9+ messages in thread
From: Wang, Haiyue @ 2020-09-07  2:14 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev, Yigit, Ferruh, Olivier Matz

Hi Stephen,

> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Sunday, September 6, 2020 00:56
> To: Wang, Haiyue <haiyue.wang@intel.com>
> Cc: dev@dpdk.org; Yigit, Ferruh <ferruh.yigit@intel.com>; Olivier Matz <olivier.matz@6wind.com>
> Subject: Re: [PATCH v2] net: adjust the header length parse size
> 
> On Sat,  5 Sep 2020 11:06:46 +0800
> Haiyue Wang <haiyue.wang@intel.com> wrote:
> 
> > Align to the rte_mbuf's design about Tx header length data size for the
> > header length parse result.
> > 	struct {
> > 		uint64_t   l2_len:7;             /*    88: 0  8 */
> > 		uint64_t   l3_len:9;             /*    88: 7  8 */
> > 		uint64_t   l4_len:8;             /*    88:16  8 */
> > 		uint64_t   tso_segsz:16;         /*    88:24  8 */
> > 		uint64_t   outer_l3_len:9;       /*    88:40  8 */
> > 		uint64_t   outer_l2_len:7;       /*    88:49  8 */
> > 	};
> >
> > Now the IPv6 can support bigger extension header.
> >
> > The below is the structure hole analysis result:
> >
> > Before:
> > struct rte_net_hdr_lens {
> >         uint8_t                    l2_len;               /*     0     1 */
> >         uint8_t                    l3_len;               /*     1     1 */
> >         uint8_t                    l4_len;               /*     2     1 */
> >         uint8_t                    tunnel_len;           /*     3     1 */
> >         uint8_t                    inner_l2_len;         /*     4     1 */
> >         uint8_t                    inner_l3_len;         /*     5     1 */
> >         uint8_t                    inner_l4_len;         /*     6     1 */
> >
> >         /* size: 7, cachelines: 1, members: 7 */
> >         /* last cacheline: 7 bytes */
> > };
> >
> > Now:
> > struct rte_net_hdr_lens {
> >         uint64_t                   l2_len:7;             /*     0: 0  8 */
> >         uint64_t                   l3_len:9;             /*     0: 7  8 */
> >         uint64_t                   l4_len:8;             /*     0:16  8 */
> >         uint64_t                   tunnel_len:8;         /*     0:24  8 */
> >         uint64_t                   inner_l2_len:7;       /*     0:32  8 */
> >         uint64_t                   inner_l3_len:9;       /*     0:39  8 */
> >         uint64_t                   inner_l4_len:8;       /*     0:48  8 */
> >
> >         /* size: 8, cachelines: 1, members: 7 */
> >         /* bit_padding: 8 bits */
> >         /* last cacheline: 8 bytes */
> > };
> >
> > Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> 
> Bitfields are slow to access, compiler has to do mask/shift operations.

Yes, you are right, I use rdtsc to trace the rte_net_get_ptype run clock
about 100000 times, bitfields is near 40, but original is about 30.


> And there is no requirement that structure be the same size.
> 
> There is no requirement that fields be ordered the same as
> the protocol header. Also tunnel length might get big.
> Why not:
> 
> struct rte_net_hdr_lens {
> 	uint8_t l2_len;
> 	uint8_t inner_l2_len;
> 	uint16_t l3_len;
> 	uint16_t inner_l3_len;
> 	uint16_t tunnel_len;
> 	uint8_t l4_len;
> 	uint8_t inner_l4_len;
> };
> 

Thanks for your comment, this is better, and in v3. ;-) The run clock is nearly
the same as original type.

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

* Re: [dpdk-dev] [PATCH v3] net: adjust the header length parse size
  2020-09-07  1:56 ` [dpdk-dev] [PATCH v3] " Haiyue Wang
@ 2020-09-08 14:53   ` Stephen Hemminger
  2020-09-18  8:47     ` Ferruh Yigit
  0 siblings, 1 reply; 9+ messages in thread
From: Stephen Hemminger @ 2020-09-08 14:53 UTC (permalink / raw)
  To: Haiyue Wang; +Cc: dev, ferruh.yigit, Olivier Matz

On Mon,  7 Sep 2020 09:56:50 +0800
Haiyue Wang <haiyue.wang@intel.com> wrote:

> Enlarge the L3 and tunnel header length from 8-bit to 16-bit to handle
> the bigger headers. And reorder the fields to avoid creating a structure
> hole.
> 
> Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> ---
> v2: use bit field to avoid creating a structure hole.
> v3: use basic type and reorder to avoid structure hole.
> ---
>  lib/librte_net/rte_net.h | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/librte_net/rte_net.h b/lib/librte_net/rte_net.h
> index 94b06d9ee..434435ffa 100644
> --- a/lib/librte_net/rte_net.h
> +++ b/lib/librte_net/rte_net.h
> @@ -20,11 +20,11 @@ extern "C" {
>   */
>  struct rte_net_hdr_lens {
>  	uint8_t l2_len;
> -	uint8_t l3_len;
> -	uint8_t l4_len;
> -	uint8_t tunnel_len;
>  	uint8_t inner_l2_len;
> -	uint8_t inner_l3_len;
> +	uint16_t l3_len;
> +	uint16_t inner_l3_len;
> +	uint16_t tunnel_len;
> +	uint8_t l4_len;
>  	uint8_t inner_l4_len;
>  };
>  

Acked-by: Stephhen Hemminger <stephen@networkplumber.org>

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

* Re: [dpdk-dev] [PATCH v3] net: adjust the header length parse size
  2020-09-08 14:53   ` Stephen Hemminger
@ 2020-09-18  8:47     ` Ferruh Yigit
  0 siblings, 0 replies; 9+ messages in thread
From: Ferruh Yigit @ 2020-09-18  8:47 UTC (permalink / raw)
  To: Stephen Hemminger, Haiyue Wang; +Cc: dev, Olivier Matz

On 9/8/2020 3:53 PM, Stephen Hemminger wrote:
> On Mon,  7 Sep 2020 09:56:50 +0800
> Haiyue Wang <haiyue.wang@intel.com> wrote:
> 
>> Enlarge the L3 and tunnel header length from 8-bit to 16-bit to handle
>> the bigger headers. And reorder the fields to avoid creating a structure
>> hole.
>>
>> Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
>> ---
>> v2: use bit field to avoid creating a structure hole.
>> v3: use basic type and reorder to avoid structure hole.
>> ---
>>   lib/librte_net/rte_net.h | 8 ++++----
>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/lib/librte_net/rte_net.h b/lib/librte_net/rte_net.h
>> index 94b06d9ee..434435ffa 100644
>> --- a/lib/librte_net/rte_net.h
>> +++ b/lib/librte_net/rte_net.h
>> @@ -20,11 +20,11 @@ extern "C" {
>>    */
>>   struct rte_net_hdr_lens {
>>   	uint8_t l2_len;
>> -	uint8_t l3_len;
>> -	uint8_t l4_len;
>> -	uint8_t tunnel_len;
>>   	uint8_t inner_l2_len;
>> -	uint8_t inner_l3_len;
>> +	uint16_t l3_len;
>> +	uint16_t inner_l3_len;
>> +	uint16_t tunnel_len;
>> +	uint8_t l4_len;
>>   	uint8_t inner_l4_len;
>>   };
>>   
> 
> Acked-by: Stephhen Hemminger <stephen@networkplumber.org>
> 

Applied to dpdk-next-net/main, thanks.

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

end of thread, other threads:[~2020-09-18  8:47 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-04  5:40 [dpdk-dev] [PATCH v1] net: expand the data size of L3 length to 16 bits Haiyue Wang
2020-09-04 18:55 ` Stephen Hemminger
2020-09-05  3:17   ` Wang, Haiyue
2020-09-05  3:06 ` [dpdk-dev] [PATCH v2] net: adjust the header length parse size Haiyue Wang
2020-09-05 16:56   ` Stephen Hemminger
2020-09-07  2:14     ` Wang, Haiyue
2020-09-07  1:56 ` [dpdk-dev] [PATCH v3] " Haiyue Wang
2020-09-08 14:53   ` Stephen Hemminger
2020-09-18  8:47     ` Ferruh Yigit

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).