DPDK patches and discussions
 help / color / mirror / Atom feed
* RE: [PATCH] app/testpmd: fix protocol headers display for Rx buffer split
  2022-10-17 17:40 [PATCH] app/testpmd: fix protocol headers display for Rx buffer split Yuan Wang
@ 2022-10-17 10:03 ` Tang, Yaqi
  2022-10-18 14:50 ` [PATCH v2] " Yuan Wang
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 15+ messages in thread
From: Tang, Yaqi @ 2022-10-17 10:03 UTC (permalink / raw)
  To: Wang, YuanX, Singh, Aman Deep, Zhang, Yuying
  Cc: andrew.rybchenko, dev, Ding, Xuan


> -----Original Message-----
> From: Wang, YuanX <yuanx.wang@intel.com>
> Sent: Tuesday, October 18, 2022 1:41 AM
> To: Singh, Aman Deep <aman.deep.singh@intel.com>; Zhang, Yuying
> <yuying.zhang@intel.com>
> Cc: andrew.rybchenko@oktetlabs.ru; dev@dpdk.org; Ding, Xuan
> <xuan.ding@intel.com>; Tang, Yaqi <yaqi.tang@intel.com>; Wang, YuanX
> <yuanx.wang@intel.com>
> Subject: [PATCH] app/testpmd: fix protocol headers display for Rx buffer split
> 
> The "show config rxhdrs" cmd displays the configured protocol headers that
> are used for protocol-based buffer split.
> However, it shows "inner-ipv6" as "inner-ipv4".
> 
> This patch fixes that by adjusting the order of condition judgments.
> 
> Signed-off-by: Yuan Wang <yuanx.wang@intel.com>
> ---

Tested-by: Yaqi Tang <yaqi.tang@intel.com>

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

* [PATCH] app/testpmd: fix protocol headers display for Rx buffer split
@ 2022-10-17 17:40 Yuan Wang
  2022-10-17 10:03 ` Tang, Yaqi
                   ` (4 more replies)
  0 siblings, 5 replies; 15+ messages in thread
From: Yuan Wang @ 2022-10-17 17:40 UTC (permalink / raw)
  To: Aman Singh, Yuying Zhang
  Cc: andrew.rybchenko, dev, xuan.ding, yaqi.tang, Yuan Wang

The "show config rxhdrs" cmd displays the configured protocol headers
that are used for protocol-based buffer split.
However, it shows "inner-ipv6" as "inner-ipv4".

This patch fixes that by adjusting the order of condition judgments.

Signed-off-by: Yuan Wang <yuanx.wang@intel.com>
---
 app/test-pmd/config.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index dec16a9049..e5fbc3a491 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -4922,15 +4922,6 @@ static const char *get_ptype_str(uint32_t ptype)
 	else if ((ptype & RTE_PTYPE_L2_ETHER) == RTE_PTYPE_L2_ETHER)
 		return "eth";
 
-	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_TCP)) ==
-		(RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_TCP))
-		return "inner-ipv4-tcp";
-	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_UDP)) ==
-		(RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_UDP))
-		return "inner-ipv4-udp";
-	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_SCTP)) ==
-		(RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_SCTP))
-		return "inner-ipv4-sctp";
 	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_TCP)) ==
 		(RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_TCP))
 		return "inner-ipv6-tcp";
@@ -4940,18 +4931,27 @@ static const char *get_ptype_str(uint32_t ptype)
 	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_SCTP)) ==
 		(RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_SCTP))
 		return "inner-ipv6-sctp";
+	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_TCP)) ==
+		(RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_TCP))
+		return "inner-ipv4-tcp";
+	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_UDP)) ==
+		(RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_UDP))
+		return "inner-ipv4-udp";
+	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_SCTP)) ==
+		(RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_SCTP))
+		return "inner-ipv4-sctp";
 	else if ((ptype & RTE_PTYPE_INNER_L4_TCP) == RTE_PTYPE_INNER_L4_TCP)
 		return "inner-tcp";
 	else if ((ptype & RTE_PTYPE_INNER_L4_UDP) == RTE_PTYPE_INNER_L4_UDP)
 		return "inner-udp";
 	else if ((ptype & RTE_PTYPE_INNER_L4_SCTP) == RTE_PTYPE_INNER_L4_SCTP)
 		return "inner-sctp";
+	else if ((ptype & RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN) ==
+		RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN)
+		return "inner-ipv6";
 	else if ((ptype & RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN) ==
 		RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN)
 		return "inner-ipv4";
-	else if ((ptype & RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN) ==
-		RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN)
-		return "inner-ipv6";
 	else if ((ptype & RTE_PTYPE_INNER_L2_ETHER) == RTE_PTYPE_INNER_L2_ETHER)
 		return "inner-eth";
 	else if ((ptype & RTE_PTYPE_TUNNEL_GRENAT) == RTE_PTYPE_TUNNEL_GRENAT)
-- 
2.25.1


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

* [PATCH v2] app/testpmd: fix protocol headers display for Rx buffer split
  2022-10-17 17:40 [PATCH] app/testpmd: fix protocol headers display for Rx buffer split Yuan Wang
  2022-10-17 10:03 ` Tang, Yaqi
@ 2022-10-18 14:50 ` Yuan Wang
  2022-10-28  2:04   ` Wang, YuanX
  2022-11-06  9:58   ` Andrew Rybchenko
  2022-11-07  8:45 ` [PATCH v3] app/testpmd: fix protocol header " Yuan Wang
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 15+ messages in thread
From: Yuan Wang @ 2022-10-18 14:50 UTC (permalink / raw)
  To: andrew.rybchenko, Aman Singh, Yuying Zhang
  Cc: dev, xuan.ding, yaqi.tang, Yuan Wang

The "show config rxhdrs" cmd displays the configured protocol headers
that are used for protocol-based buffer split.
However, it shows "inner-ipv6" as "inner-ipv4".

This patch fixes that by adjusting the order of condition judgments.

Fixes: 52e2e7edcf48 ("app/testpmd: add protocol-based buffer split")

Signed-off-by: Yuan Wang <yuanx.wang@intel.com>
Tested-by: Yaqi Tang <yaqi.tang@intel.com>

---
v2: add fixline.

---
 app/test-pmd/config.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 0f7dbd698f..82fbbc9944 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -4937,15 +4937,6 @@ static const char *get_ptype_str(uint32_t ptype)
 	else if ((ptype & RTE_PTYPE_L2_ETHER) == RTE_PTYPE_L2_ETHER)
 		return "eth";
 
-	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_TCP)) ==
-		(RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_TCP))
-		return "inner-ipv4-tcp";
-	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_UDP)) ==
-		(RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_UDP))
-		return "inner-ipv4-udp";
-	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_SCTP)) ==
-		(RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_SCTP))
-		return "inner-ipv4-sctp";
 	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_TCP)) ==
 		(RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_TCP))
 		return "inner-ipv6-tcp";
@@ -4955,18 +4946,27 @@ static const char *get_ptype_str(uint32_t ptype)
 	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_SCTP)) ==
 		(RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_SCTP))
 		return "inner-ipv6-sctp";
+	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_TCP)) ==
+		(RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_TCP))
+		return "inner-ipv4-tcp";
+	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_UDP)) ==
+		(RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_UDP))
+		return "inner-ipv4-udp";
+	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_SCTP)) ==
+		(RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_SCTP))
+		return "inner-ipv4-sctp";
 	else if ((ptype & RTE_PTYPE_INNER_L4_TCP) == RTE_PTYPE_INNER_L4_TCP)
 		return "inner-tcp";
 	else if ((ptype & RTE_PTYPE_INNER_L4_UDP) == RTE_PTYPE_INNER_L4_UDP)
 		return "inner-udp";
 	else if ((ptype & RTE_PTYPE_INNER_L4_SCTP) == RTE_PTYPE_INNER_L4_SCTP)
 		return "inner-sctp";
+	else if ((ptype & RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN) ==
+		RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN)
+		return "inner-ipv6";
 	else if ((ptype & RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN) ==
 		RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN)
 		return "inner-ipv4";
-	else if ((ptype & RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN) ==
-		RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN)
-		return "inner-ipv6";
 	else if ((ptype & RTE_PTYPE_INNER_L2_ETHER) == RTE_PTYPE_INNER_L2_ETHER)
 		return "inner-eth";
 	else if ((ptype & RTE_PTYPE_TUNNEL_GRENAT) == RTE_PTYPE_TUNNEL_GRENAT)
-- 
2.25.1


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

* RE: [PATCH v2] app/testpmd: fix protocol headers display for Rx buffer split
  2022-10-18 14:50 ` [PATCH v2] " Yuan Wang
@ 2022-10-28  2:04   ` Wang, YuanX
  2022-11-06  9:58   ` Andrew Rybchenko
  1 sibling, 0 replies; 15+ messages in thread
From: Wang, YuanX @ 2022-10-28  2:04 UTC (permalink / raw)
  To: Singh, Aman Deep, Zhang, Yuying
  Cc: dev, Ding, Xuan, Tang,  Yaqi, andrew.rybchenko

Hi All,

Could you please review and provide suggestions if any.

Thanks,
Yuan

> -----Original Message-----
> From: Wang, YuanX <yuanx.wang@intel.com>
> Sent: Tuesday, October 18, 2022 10:50 PM
> To: andrew.rybchenko@oktetlabs.ru; Singh, Aman Deep
> <aman.deep.singh@intel.com>; Zhang, Yuying <yuying.zhang@intel.com>
> Cc: dev@dpdk.org; Ding, Xuan <xuan.ding@intel.com>; Tang, Yaqi
> <yaqi.tang@intel.com>; Wang, YuanX <yuanx.wang@intel.com>
> Subject: [PATCH v2] app/testpmd: fix protocol headers display for Rx buffer
> split
> 
> The "show config rxhdrs" cmd displays the configured protocol headers that
> are used for protocol-based buffer split.
> However, it shows "inner-ipv6" as "inner-ipv4".
> 
> This patch fixes that by adjusting the order of condition judgments.
> 
> Fixes: 52e2e7edcf48 ("app/testpmd: add protocol-based buffer split")
> 
> Signed-off-by: Yuan Wang <yuanx.wang@intel.com>
> Tested-by: Yaqi Tang <yaqi.tang@intel.com>
> 
> ---
> v2: add fixline.
> 
> ---
>  app/test-pmd/config.c | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index
> 0f7dbd698f..82fbbc9944 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -4937,15 +4937,6 @@ static const char *get_ptype_str(uint32_t ptype)
>  	else if ((ptype & RTE_PTYPE_L2_ETHER) == RTE_PTYPE_L2_ETHER)
>  		return "eth";
> 
> -	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN |
> RTE_PTYPE_INNER_L4_TCP)) ==
> -		(RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN |
> RTE_PTYPE_INNER_L4_TCP))
> -		return "inner-ipv4-tcp";
> -	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN |
> RTE_PTYPE_INNER_L4_UDP)) ==
> -		(RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN |
> RTE_PTYPE_INNER_L4_UDP))
> -		return "inner-ipv4-udp";
> -	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN |
> RTE_PTYPE_INNER_L4_SCTP)) ==
> -		(RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN |
> RTE_PTYPE_INNER_L4_SCTP))
> -		return "inner-ipv4-sctp";
>  	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN |
> RTE_PTYPE_INNER_L4_TCP)) ==
>  		(RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN |
> RTE_PTYPE_INNER_L4_TCP))
>  		return "inner-ipv6-tcp";
> @@ -4955,18 +4946,27 @@ static const char *get_ptype_str(uint32_t ptype)
>  	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN |
> RTE_PTYPE_INNER_L4_SCTP)) ==
>  		(RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN |
> RTE_PTYPE_INNER_L4_SCTP))
>  		return "inner-ipv6-sctp";
> +	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN |
> RTE_PTYPE_INNER_L4_TCP)) ==
> +		(RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN |
> RTE_PTYPE_INNER_L4_TCP))
> +		return "inner-ipv4-tcp";
> +	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN |
> RTE_PTYPE_INNER_L4_UDP)) ==
> +		(RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN |
> RTE_PTYPE_INNER_L4_UDP))
> +		return "inner-ipv4-udp";
> +	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN |
> RTE_PTYPE_INNER_L4_SCTP)) ==
> +		(RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN |
> RTE_PTYPE_INNER_L4_SCTP))
> +		return "inner-ipv4-sctp";
>  	else if ((ptype & RTE_PTYPE_INNER_L4_TCP) ==
> RTE_PTYPE_INNER_L4_TCP)
>  		return "inner-tcp";
>  	else if ((ptype & RTE_PTYPE_INNER_L4_UDP) ==
> RTE_PTYPE_INNER_L4_UDP)
>  		return "inner-udp";
>  	else if ((ptype & RTE_PTYPE_INNER_L4_SCTP) ==
> RTE_PTYPE_INNER_L4_SCTP)
>  		return "inner-sctp";
> +	else if ((ptype & RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN) ==
> +		RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN)
> +		return "inner-ipv6";
>  	else if ((ptype & RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN) ==
>  		RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN)
>  		return "inner-ipv4";
> -	else if ((ptype & RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN) ==
> -		RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN)
> -		return "inner-ipv6";
>  	else if ((ptype & RTE_PTYPE_INNER_L2_ETHER) ==
> RTE_PTYPE_INNER_L2_ETHER)
>  		return "inner-eth";
>  	else if ((ptype & RTE_PTYPE_TUNNEL_GRENAT) ==
> RTE_PTYPE_TUNNEL_GRENAT)
> --
> 2.25.1


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

* Re: [PATCH v2] app/testpmd: fix protocol headers display for Rx buffer split
  2022-10-18 14:50 ` [PATCH v2] " Yuan Wang
  2022-10-28  2:04   ` Wang, YuanX
@ 2022-11-06  9:58   ` Andrew Rybchenko
  2022-11-07  5:55     ` Wang, YuanX
  1 sibling, 1 reply; 15+ messages in thread
From: Andrew Rybchenko @ 2022-11-06  9:58 UTC (permalink / raw)
  To: Yuan Wang, Aman Singh, Yuying Zhang; +Cc: dev, xuan.ding, yaqi.tang

On 10/18/22 17:50, Yuan Wang wrote:
> The "show config rxhdrs" cmd displays the configured protocol headers
> that are used for protocol-based buffer split.
> However, it shows "inner-ipv6" as "inner-ipv4".
> 
> This patch fixes that by adjusting the order of condition judgments.
> 
> Fixes: 52e2e7edcf48 ("app/testpmd: add protocol-based buffer split")
> 
> Signed-off-by: Yuan Wang <yuanx.wang@intel.com>
> Tested-by: Yaqi Tang <yaqi.tang@intel.com>
> 
> ---
> v2: add fixline.
> 
> ---
>   app/test-pmd/config.c | 24 ++++++++++++------------
>   1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 0f7dbd698f..82fbbc9944 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -4937,15 +4937,6 @@ static const char *get_ptype_str(uint32_t ptype)
>   	else if ((ptype & RTE_PTYPE_L2_ETHER) == RTE_PTYPE_L2_ETHER)
>   		return "eth";
>   
> -	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_TCP)) ==
> -		(RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_TCP))

I realize that the patch solves the problem, but it is still
wrong. Why are RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN and
RTE_PTYPE_INNER_L4_TCP used as masks instead of
RTE_PTYPE_L3_MASK and RTE_PTYPE_L4_MASK?

It is simply incorrect from the very beginning and all
conditions must be fixed to use appropriate masks instead.


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

* RE: [PATCH v2] app/testpmd: fix protocol headers display for Rx buffer split
  2022-11-06  9:58   ` Andrew Rybchenko
@ 2022-11-07  5:55     ` Wang, YuanX
  0 siblings, 0 replies; 15+ messages in thread
From: Wang, YuanX @ 2022-11-07  5:55 UTC (permalink / raw)
  To: Andrew Rybchenko, Singh, Aman Deep, Zhang, Yuying
  Cc: dev, Ding, Xuan, Tang,  Yaqi

Hi Andrew,

> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Sent: Sunday, November 6, 2022 5:58 PM
> To: Wang, YuanX <yuanx.wang@intel.com>; Singh, Aman Deep
> <aman.deep.singh@intel.com>; Zhang, Yuying <yuying.zhang@intel.com>
> Cc: dev@dpdk.org; Ding, Xuan <xuan.ding@intel.com>; Tang, Yaqi
> <yaqi.tang@intel.com>
> Subject: Re: [PATCH v2] app/testpmd: fix protocol headers display for Rx
> buffer split
> 
> On 10/18/22 17:50, Yuan Wang wrote:
> > The "show config rxhdrs" cmd displays the configured protocol headers
> > that are used for protocol-based buffer split.
> > However, it shows "inner-ipv6" as "inner-ipv4".
> >
> > This patch fixes that by adjusting the order of condition judgments.
> >
> > Fixes: 52e2e7edcf48 ("app/testpmd: add protocol-based buffer split")
> >
> > Signed-off-by: Yuan Wang <yuanx.wang@intel.com>
> > Tested-by: Yaqi Tang <yaqi.tang@intel.com>
> >
> > ---
> > v2: add fixline.
> >
> > ---
> >   app/test-pmd/config.c | 24 ++++++++++++------------
> >   1 file changed, 12 insertions(+), 12 deletions(-)
> >
> > diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index
> > 0f7dbd698f..82fbbc9944 100644
> > --- a/app/test-pmd/config.c
> > +++ b/app/test-pmd/config.c
> > @@ -4937,15 +4937,6 @@ static const char *get_ptype_str(uint32_t ptype)
> >   	else if ((ptype & RTE_PTYPE_L2_ETHER) == RTE_PTYPE_L2_ETHER)
> >   		return "eth";
> >
> > -	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN |
> RTE_PTYPE_INNER_L4_TCP)) ==
> > -		(RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN |
> RTE_PTYPE_INNER_L4_TCP))
> 
> I realize that the patch solves the problem, but it is still wrong. Why are
> RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN and
> RTE_PTYPE_INNER_L4_TCP used as masks instead of RTE_PTYPE_L3_MASK
> and RTE_PTYPE_L4_MASK?
> 
> It is simply incorrect from the very beginning and all conditions must be fixed
> to use appropriate masks instead.

Thanks for your comments. I will update the patch with the appropriate mask.

Thanks,
Yuan



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

* [PATCH v3] app/testpmd: fix protocol header display for Rx buffer split
  2022-10-17 17:40 [PATCH] app/testpmd: fix protocol headers display for Rx buffer split Yuan Wang
  2022-10-17 10:03 ` Tang, Yaqi
  2022-10-18 14:50 ` [PATCH v2] " Yuan Wang
@ 2022-11-07  8:45 ` Yuan Wang
  2022-11-07  8:51   ` Tang, Yaqi
  2022-11-07 11:31   ` Andrew Rybchenko
  2022-11-09  1:37 ` [PATCH v4] " Yuan Wang
  2022-11-10  8:20 ` [PATCH v5] " Yuan Wang
  4 siblings, 2 replies; 15+ messages in thread
From: Yuan Wang @ 2022-11-07  8:45 UTC (permalink / raw)
  To: andrew.rybchenko, aman.deep.singh, yuying.zhang
  Cc: xuan.ding, yaqi.tang, dev, Yuan Wang

The "show config rxhdrs" cmd displays the configured protocol headers
that are used for protocol-based buffer split.
However, it shows inner-ipv6 as inner-ipv4.

This patch fixes that by adjusting the order of condition judgments.
This patch also uses RTE_PTYPE_*_MASK as masks.

Fixes: 52e2e7edcf48 ("app/testpmd: add protocol-based buffer split")

Signed-off-by: Yuan Wang <yuanx.wang@intel.com>

---
v3: 
- use RTE_PTYPE_*_MASK as masks.
- refactor to use switch statement.
v2:
- add fixline.

---
 app/test-pmd/config.c | 89 +++++++++++++++++++++----------------------
 1 file changed, 44 insertions(+), 45 deletions(-)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index e8a1b77c2a..8638dfed11 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -5070,73 +5070,72 @@ show_rx_pkt_segments(void)
 
 static const char *get_ptype_str(uint32_t ptype)
 {
-	if ((ptype & (RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_TCP)) ==
-		(RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_TCP))
+	switch (ptype & (RTE_PTYPE_L3_MASK | RTE_PTYPE_L4_MASK)) {
+	case RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_TCP:
 		return "ipv4-tcp";
-	else if ((ptype & (RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_UDP)) ==
-		(RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_UDP))
+	case RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_UDP:
 		return "ipv4-udp";
-	else if ((ptype & (RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_SCTP)) ==
-		(RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_SCTP))
+	case RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_SCTP:
 		return "ipv4-sctp";
-	else if ((ptype & (RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_TCP)) ==
-		(RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_TCP))
+	case RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_TCP:
 		return "ipv6-tcp";
-	else if ((ptype & (RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_UDP)) ==
-		(RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_UDP))
+	case RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_UDP:
 		return "ipv6-udp";
-	else if ((ptype & (RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_SCTP)) ==
-		(RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_SCTP))
+	case RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_SCTP:
 		return "ipv6-sctp";
-	else if ((ptype & RTE_PTYPE_L4_TCP) == RTE_PTYPE_L4_TCP)
+	case RTE_PTYPE_L4_TCP:
 		return "tcp";
-	else if ((ptype & RTE_PTYPE_L4_UDP) == RTE_PTYPE_L4_UDP)
+	case RTE_PTYPE_L4_UDP:
 		return "udp";
-	else if ((ptype & RTE_PTYPE_L4_SCTP) == RTE_PTYPE_L4_SCTP)
+	case RTE_PTYPE_L4_SCTP:
 		return "sctp";
-	else if ((ptype & RTE_PTYPE_L3_IPV4_EXT_UNKNOWN) == RTE_PTYPE_L3_IPV4_EXT_UNKNOWN)
+	case RTE_PTYPE_L3_IPV4_EXT_UNKNOWN:
 		return "ipv4";
-	else if ((ptype & RTE_PTYPE_L3_IPV6_EXT_UNKNOWN) == RTE_PTYPE_L3_IPV6_EXT_UNKNOWN)
+	case RTE_PTYPE_L3_IPV6_EXT_UNKNOWN:
 		return "ipv6";
-	else if ((ptype & RTE_PTYPE_L2_ETHER) == RTE_PTYPE_L2_ETHER)
+	}
+
+	switch (ptype & RTE_PTYPE_L2_MASK) {
+	case RTE_PTYPE_L2_ETHER:
 		return "eth";
+	}
 
-	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_TCP)) ==
-		(RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_TCP))
-		return "inner-ipv4-tcp";
-	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_UDP)) ==
-		(RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_UDP))
-		return "inner-ipv4-udp";
-	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_SCTP)) ==
-		(RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_SCTP))
-		return "inner-ipv4-sctp";
-	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_TCP)) ==
-		(RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_TCP))
+	switch (ptype & (RTE_PTYPE_INNER_L3_MASK | RTE_PTYPE_INNER_L4_MASK)) {
+	case RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_TCP:
 		return "inner-ipv6-tcp";
-	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_UDP)) ==
-		(RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_UDP))
+	case RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_UDP:
 		return "inner-ipv6-udp";
-	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_SCTP)) ==
-		(RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_SCTP))
+	case RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_SCTP:
 		return "inner-ipv6-sctp";
-	else if ((ptype & RTE_PTYPE_INNER_L4_TCP) == RTE_PTYPE_INNER_L4_TCP)
+	case RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_TCP:
+		return "inner-ipv4-tcp";
+	case RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_UDP:
+		return "inner-ipv4-udp";
+	case RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_SCTP:
+		return "inner-ipv4-sctp";
+	case RTE_PTYPE_INNER_L4_TCP:
 		return "inner-tcp";
-	else if ((ptype & RTE_PTYPE_INNER_L4_UDP) == RTE_PTYPE_INNER_L4_UDP)
+	case RTE_PTYPE_INNER_L4_UDP:
 		return "inner-udp";
-	else if ((ptype & RTE_PTYPE_INNER_L4_SCTP) == RTE_PTYPE_INNER_L4_SCTP)
+	case RTE_PTYPE_INNER_L4_SCTP:
 		return "inner-sctp";
-	else if ((ptype & RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN) ==
-		RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN)
-		return "inner-ipv4";
-	else if ((ptype & RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN) ==
-		RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN)
+	case RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN:
 		return "inner-ipv6";
-	else if ((ptype & RTE_PTYPE_INNER_L2_ETHER) == RTE_PTYPE_INNER_L2_ETHER)
+	case RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN:
+		return "inner-ipv4";
+	}
+
+	switch (ptype & RTE_PTYPE_INNER_L2_MASK) {
+	case RTE_PTYPE_INNER_L2_ETHER:
 		return "inner-eth";
-	else if ((ptype & RTE_PTYPE_TUNNEL_GRENAT) == RTE_PTYPE_TUNNEL_GRENAT)
+	}
+
+	switch (ptype & RTE_PTYPE_TUNNEL_MASK) {
+	case RTE_PTYPE_TUNNEL_GRENAT:
 		return "grenat";
-	else
-		return "unsupported";
+	}
+
+	return "unsupported";
 }
 
 void
-- 
2.25.1


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

* RE: [PATCH v3] app/testpmd: fix protocol header display for Rx buffer split
  2022-11-07  8:45 ` [PATCH v3] app/testpmd: fix protocol header " Yuan Wang
@ 2022-11-07  8:51   ` Tang, Yaqi
  2022-11-07 11:31   ` Andrew Rybchenko
  1 sibling, 0 replies; 15+ messages in thread
From: Tang, Yaqi @ 2022-11-07  8:51 UTC (permalink / raw)
  To: Wang, YuanX, andrew.rybchenko, Singh, Aman Deep, Zhang, Yuying
  Cc: Ding, Xuan, dev


> -----Original Message-----
> From: Wang, YuanX <yuanx.wang@intel.com>
> Sent: Monday, November 7, 2022 4:45 PM
> To: andrew.rybchenko@oktetlabs.ru; Singh, Aman Deep
> <aman.deep.singh@intel.com>; Zhang, Yuying <yuying.zhang@intel.com>
> Cc: Ding, Xuan <xuan.ding@intel.com>; Tang, Yaqi <yaqi.tang@intel.com>;
> dev@dpdk.org; Wang, YuanX <yuanx.wang@intel.com>
> Subject: [PATCH v3] app/testpmd: fix protocol header display for Rx buffer
> split
> 
> The "show config rxhdrs" cmd displays the configured protocol headers that
> are used for protocol-based buffer split.
> However, it shows inner-ipv6 as inner-ipv4.
> 
> This patch fixes that by adjusting the order of condition judgments.
> This patch also uses RTE_PTYPE_*_MASK as masks.
> 
> Fixes: 52e2e7edcf48 ("app/testpmd: add protocol-based buffer split")
> 
> Signed-off-by: Yuan Wang <yuanx.wang@intel.com>
> 
> ---
> v3:
> - use RTE_PTYPE_*_MASK as masks.
> - refactor to use switch statement.
> v2:
> - add fixline.
> 
> ---

Tested-by: Yaqi Tang <yaqi.tang@intel.com>

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

* Re: [PATCH v3] app/testpmd: fix protocol header display for Rx buffer split
  2022-11-07  8:45 ` [PATCH v3] app/testpmd: fix protocol header " Yuan Wang
  2022-11-07  8:51   ` Tang, Yaqi
@ 2022-11-07 11:31   ` Andrew Rybchenko
  2022-11-08 13:53     ` Wang, YuanX
  1 sibling, 1 reply; 15+ messages in thread
From: Andrew Rybchenko @ 2022-11-07 11:31 UTC (permalink / raw)
  To: Yuan Wang, aman.deep.singh, yuying.zhang; +Cc: xuan.ding, yaqi.tang, dev

On 11/7/22 11:45, Yuan Wang wrote:
> The "show config rxhdrs" cmd displays the configured protocol headers
> that are used for protocol-based buffer split.
> However, it shows inner-ipv6 as inner-ipv4.
> 
> This patch fixes that by adjusting the order of condition judgments.
> This patch also uses RTE_PTYPE_*_MASK as masks.
> 
> Fixes: 52e2e7edcf48 ("app/testpmd: add protocol-based buffer split")
> 
> Signed-off-by: Yuan Wang <yuanx.wang@intel.com>
> 
> ---
> v3:
> - use RTE_PTYPE_*_MASK as masks.
> - refactor to use switch statement.
> v2:
> - add fixline.
> 
> ---
>   app/test-pmd/config.c | 89 +++++++++++++++++++++----------------------
>   1 file changed, 44 insertions(+), 45 deletions(-)
> 
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index e8a1b77c2a..8638dfed11 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -5070,73 +5070,72 @@ show_rx_pkt_segments(void)
>   
>   static const char *get_ptype_str(uint32_t ptype)
>   {
> -	if ((ptype & (RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_TCP)) ==
> -		(RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_TCP))
> +	switch (ptype & (RTE_PTYPE_L3_MASK | RTE_PTYPE_L4_MASK)) {
> +	case RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_TCP:
>   		return "ipv4-tcp";

If I map "ipv4-tcp" to packets types, I get:
RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_TCP
but vice versa it is sufficient to have just
RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_TCP
I think such asymmetry in mapping is bad.

> -	else if ((ptype & (RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_UDP)) ==
> -		(RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_UDP))
> +	case RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_UDP:
>   		return "ipv4-udp";
> -	else if ((ptype & (RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_SCTP)) ==
> -		(RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_SCTP))
> +	case RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_SCTP:
>   		return "ipv4-sctp";
> -	else if ((ptype & (RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_TCP)) ==
> -		(RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_TCP))
> +	case RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_TCP:
>   		return "ipv6-tcp";
> -	else if ((ptype & (RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_UDP)) ==
> -		(RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_UDP))
> +	case RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_UDP:
>   		return "ipv6-udp";
> -	else if ((ptype & (RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_SCTP)) ==
> -		(RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_SCTP))
> +	case RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_SCTP:
>   		return "ipv6-sctp";
> -	else if ((ptype & RTE_PTYPE_L4_TCP) == RTE_PTYPE_L4_TCP)
> +	case RTE_PTYPE_L4_TCP:
>   		return "tcp";
> -	else if ((ptype & RTE_PTYPE_L4_UDP) == RTE_PTYPE_L4_UDP)
> +	case RTE_PTYPE_L4_UDP:
>   		return "udp";
> -	else if ((ptype & RTE_PTYPE_L4_SCTP) == RTE_PTYPE_L4_SCTP)
> +	case RTE_PTYPE_L4_SCTP:
>   		return "sctp";
> -	else if ((ptype & RTE_PTYPE_L3_IPV4_EXT_UNKNOWN) == RTE_PTYPE_L3_IPV4_EXT_UNKNOWN)
> +	case RTE_PTYPE_L3_IPV4_EXT_UNKNOWN:
>   		return "ipv4";
> -	else if ((ptype & RTE_PTYPE_L3_IPV6_EXT_UNKNOWN) == RTE_PTYPE_L3_IPV6_EXT_UNKNOWN)
> +	case RTE_PTYPE_L3_IPV6_EXT_UNKNOWN:
>   		return "ipv6";
> -	else if ((ptype & RTE_PTYPE_L2_ETHER) == RTE_PTYPE_L2_ETHER)
> +	}
> +
> +	switch (ptype & RTE_PTYPE_L2_MASK) {

Having many switches here looks confusing. Who defines
priorities? IMHO it should be single switch here and
values should be in exactly the same order as get_ptype().
Ideally both function should be close to each other.

> +	case RTE_PTYPE_L2_ETHER:
>   		return "eth";
> +	}
>   
> -	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_TCP)) ==
> -		(RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_TCP))
> -		return "inner-ipv4-tcp";
> -	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_UDP)) ==
> -		(RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_UDP))
> -		return "inner-ipv4-udp";
> -	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_SCTP)) ==
> -		(RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_SCTP))
> -		return "inner-ipv4-sctp";
> -	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_TCP)) ==
> -		(RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_TCP))
> +	switch (ptype & (RTE_PTYPE_INNER_L3_MASK | RTE_PTYPE_INNER_L4_MASK)) {
> +	case RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_TCP:
>   		return "inner-ipv6-tcp";

get_ptype():
inner-ipv6-tcp -> RTE_PTYPE_TUNNEL_GRENAT | RTE_PTYPE_INNER_L2_ETHER | 
RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_TCP

So, mapping is asymmetric again.

Out of topic for the patch:
Also I'm wondering why inner-ipv6-tcp is a grenat. Why not
VxLAN, not GENEVE?

> -	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_UDP)) ==
> -		(RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_UDP))
> +	case RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_UDP:
>   		return "inner-ipv6-udp";
> -	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_SCTP)) ==
> -		(RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_SCTP))
> +	case RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_SCTP:
>   		return "inner-ipv6-sctp";
> -	else if ((ptype & RTE_PTYPE_INNER_L4_TCP) == RTE_PTYPE_INNER_L4_TCP)
> +	case RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_TCP:
> +		return "inner-ipv4-tcp";
> +	case RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_UDP:
> +		return "inner-ipv4-udp";
> +	case RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_SCTP:
> +		return "inner-ipv4-sctp";
> +	case RTE_PTYPE_INNER_L4_TCP:
>   		return "inner-tcp";
> -	else if ((ptype & RTE_PTYPE_INNER_L4_UDP) == RTE_PTYPE_INNER_L4_UDP)
> +	case RTE_PTYPE_INNER_L4_UDP:
>   		return "inner-udp";
> -	else if ((ptype & RTE_PTYPE_INNER_L4_SCTP) == RTE_PTYPE_INNER_L4_SCTP)
> +	case RTE_PTYPE_INNER_L4_SCTP:
>   		return "inner-sctp";
> -	else if ((ptype & RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN) ==
> -		RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN)
> -		return "inner-ipv4";
> -	else if ((ptype & RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN) ==
> -		RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN)
> +	case RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN:
>   		return "inner-ipv6";
> -	else if ((ptype & RTE_PTYPE_INNER_L2_ETHER) == RTE_PTYPE_INNER_L2_ETHER)
> +	case RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN:
> +		return "inner-ipv4";
> +	}
> +
> +	switch (ptype & RTE_PTYPE_INNER_L2_MASK) {
> +	case RTE_PTYPE_INNER_L2_ETHER:
>   		return "inner-eth";
> -	else if ((ptype & RTE_PTYPE_TUNNEL_GRENAT) == RTE_PTYPE_TUNNEL_GRENAT)
> +	}
> +
> +	switch (ptype & RTE_PTYPE_TUNNEL_MASK) {
> +	case RTE_PTYPE_TUNNEL_GRENAT:
>   		return "grenat";
> -	else
> -		return "unsupported";
> +	}
> +
> +	return "unsupported";
>   }
>   
>   void


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

* RE: [PATCH v3] app/testpmd: fix protocol header display for Rx buffer split
  2022-11-07 11:31   ` Andrew Rybchenko
@ 2022-11-08 13:53     ` Wang, YuanX
  0 siblings, 0 replies; 15+ messages in thread
From: Wang, YuanX @ 2022-11-08 13:53 UTC (permalink / raw)
  To: Andrew Rybchenko, Singh, Aman Deep, Zhang, Yuying
  Cc: Ding, Xuan, Tang, Yaqi, dev

Hi Andrew,

> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Sent: Monday, November 7, 2022 7:31 PM
> To: Wang, YuanX <yuanx.wang@intel.com>; Singh, Aman Deep
> <aman.deep.singh@intel.com>; Zhang, Yuying <yuying.zhang@intel.com>
> Cc: Ding, Xuan <xuan.ding@intel.com>; Tang, Yaqi <yaqi.tang@intel.com>;
> dev@dpdk.org
> Subject: Re: [PATCH v3] app/testpmd: fix protocol header display for Rx
> buffer split
> 
> On 11/7/22 11:45, Yuan Wang wrote:
> > The "show config rxhdrs" cmd displays the configured protocol headers
> > that are used for protocol-based buffer split.
> > However, it shows inner-ipv6 as inner-ipv4.
> >
> > This patch fixes that by adjusting the order of condition judgments.
> > This patch also uses RTE_PTYPE_*_MASK as masks.
> >
> > Fixes: 52e2e7edcf48 ("app/testpmd: add protocol-based buffer split")
> >
> > Signed-off-by: Yuan Wang <yuanx.wang@intel.com>
> >
> > ---
> > v3:
> > - use RTE_PTYPE_*_MASK as masks.
> > - refactor to use switch statement.
> > v2:
> > - add fixline.
> >
> > ---
> >   app/test-pmd/config.c | 89 +++++++++++++++++++++----------------------
> >   1 file changed, 44 insertions(+), 45 deletions(-)
> >
> > diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index
> > e8a1b77c2a..8638dfed11 100644
> > --- a/app/test-pmd/config.c
> > +++ b/app/test-pmd/config.c
> > @@ -5070,73 +5070,72 @@ show_rx_pkt_segments(void)
> >
> >   static const char *get_ptype_str(uint32_t ptype)
> >   {
> > -	if ((ptype & (RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
> RTE_PTYPE_L4_TCP)) ==
> > -		(RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
> RTE_PTYPE_L4_TCP))
> > +	switch (ptype & (RTE_PTYPE_L3_MASK | RTE_PTYPE_L4_MASK)) {
> > +	case RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_TCP:
> >   		return "ipv4-tcp";
> 
> If I map "ipv4-tcp" to packets types, I get:
> RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
> RTE_PTYPE_L4_TCP but vice versa it is sufficient to have just
> RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_TCP I think such
> asymmetry in mapping is bad.

The reason for the asymmetric is that the lib requires that each segment cannot be repeated with the previous one. For example "set rxhdrs eth,ipv4-udp", seg[0]=RTE_PTYPE_L2_ETHER,seg[1]=RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_UDP. 
In order not to duplicate, seg[1] is truncated to RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_UDP. 
In this way we can use the truncated seg[1] to map to "ipv4-udp". The same goes for tunneled packets.

> 
> > -	else if ((ptype & (RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
> RTE_PTYPE_L4_UDP)) ==
> > -		(RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
> RTE_PTYPE_L4_UDP))
> > +	case RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_UDP:
> >   		return "ipv4-udp";
> > -	else if ((ptype & (RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
> RTE_PTYPE_L4_SCTP)) ==
> > -		(RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
> RTE_PTYPE_L4_SCTP))
> > +	case RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_SCTP:
> >   		return "ipv4-sctp";
> > -	else if ((ptype & (RTE_PTYPE_L3_IPV6_EXT_UNKNOWN |
> RTE_PTYPE_L4_TCP)) ==
> > -		(RTE_PTYPE_L3_IPV6_EXT_UNKNOWN |
> RTE_PTYPE_L4_TCP))
> > +	case RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_TCP:
> >   		return "ipv6-tcp";
> > -	else if ((ptype & (RTE_PTYPE_L3_IPV6_EXT_UNKNOWN |
> RTE_PTYPE_L4_UDP)) ==
> > -		(RTE_PTYPE_L3_IPV6_EXT_UNKNOWN |
> RTE_PTYPE_L4_UDP))
> > +	case RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_UDP:
> >   		return "ipv6-udp";
> > -	else if ((ptype & (RTE_PTYPE_L3_IPV6_EXT_UNKNOWN |
> RTE_PTYPE_L4_SCTP)) ==
> > -		(RTE_PTYPE_L3_IPV6_EXT_UNKNOWN |
> RTE_PTYPE_L4_SCTP))
> > +	case RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_SCTP:
> >   		return "ipv6-sctp";
> > -	else if ((ptype & RTE_PTYPE_L4_TCP) == RTE_PTYPE_L4_TCP)
> > +	case RTE_PTYPE_L4_TCP:
> >   		return "tcp";
> > -	else if ((ptype & RTE_PTYPE_L4_UDP) == RTE_PTYPE_L4_UDP)
> > +	case RTE_PTYPE_L4_UDP:
> >   		return "udp";
> > -	else if ((ptype & RTE_PTYPE_L4_SCTP) == RTE_PTYPE_L4_SCTP)
> > +	case RTE_PTYPE_L4_SCTP:
> >   		return "sctp";
> > -	else if ((ptype & RTE_PTYPE_L3_IPV4_EXT_UNKNOWN) ==
> RTE_PTYPE_L3_IPV4_EXT_UNKNOWN)
> > +	case RTE_PTYPE_L3_IPV4_EXT_UNKNOWN:
> >   		return "ipv4";
> > -	else if ((ptype & RTE_PTYPE_L3_IPV6_EXT_UNKNOWN) ==
> RTE_PTYPE_L3_IPV6_EXT_UNKNOWN)
> > +	case RTE_PTYPE_L3_IPV6_EXT_UNKNOWN:
> >   		return "ipv6";
> > -	else if ((ptype & RTE_PTYPE_L2_ETHER) == RTE_PTYPE_L2_ETHER)
> > +	}
> > +
> > +	switch (ptype & RTE_PTYPE_L2_MASK) {
> 
> Having many switches here looks confusing. Who defines priorities? IMHO it
> should be single switch here and values should be in exactly the same order
> as get_ptype().
> Ideally both function should be close to each other.

The order of switch statement (and if statement as well) is to correctly map ptype to string. For example set rxhdrs ipv4-udp corresponds to RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN UNKNOWN | RTE_PTYPE_L4_UDP; we need to prioritize RTE_PTYPE_L3_IPV4_EXT_UNKNOWN|RTE_PTYPE_L4_UDP over RTE_PTYPE_L2_ETHER.

I have an idea to solve the multiple switches and asymmetry issue by adding a variable to hold the original ptypes. Please see v4.

> 
> > +	case RTE_PTYPE_L2_ETHER:
> >   		return "eth";
> > +	}
> >
> > -	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN |
> RTE_PTYPE_INNER_L4_TCP)) ==
> > -		(RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN |
> RTE_PTYPE_INNER_L4_TCP))
> > -		return "inner-ipv4-tcp";
> > -	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN |
> RTE_PTYPE_INNER_L4_UDP)) ==
> > -		(RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN |
> RTE_PTYPE_INNER_L4_UDP))
> > -		return "inner-ipv4-udp";
> > -	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN |
> RTE_PTYPE_INNER_L4_SCTP)) ==
> > -		(RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN |
> RTE_PTYPE_INNER_L4_SCTP))
> > -		return "inner-ipv4-sctp";
> > -	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN |
> RTE_PTYPE_INNER_L4_TCP)) ==
> > -		(RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN |
> RTE_PTYPE_INNER_L4_TCP))
> > +	switch (ptype & (RTE_PTYPE_INNER_L3_MASK |
> RTE_PTYPE_INNER_L4_MASK)) {
> > +	case RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN |
> RTE_PTYPE_INNER_L4_TCP:
> >   		return "inner-ipv6-tcp";
> 
> get_ptype():
> inner-ipv6-tcp -> RTE_PTYPE_TUNNEL_GRENAT |
> RTE_PTYPE_INNER_L2_ETHER |
> RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_TCP
> 
> So, mapping is asymmetric again.
> 
> Out of topic for the patch:
> Also I'm wondering why inner-ipv6-tcp is a grenat. Why not VxLAN, not
> GENEVE?

The reason inner-ipv6-tcp is grenat is to simplify the expression.
If add VxLAN/GENEVE, then the setting command needs to be changed to grenat-ipv6-tcp,VxLAN-ipv6-tcp,GENEVE-ipv6-tcp,
and so on grenat-ipv6,VxLANX-ipv6... This increases the complexity of the command.
So, for simplicity we choose grenat as the default for tunneled packets.
Any thoughts?

Thanks,
Yuan



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

* [PATCH v4] app/testpmd: fix protocol header display for Rx buffer split
  2022-10-17 17:40 [PATCH] app/testpmd: fix protocol headers display for Rx buffer split Yuan Wang
                   ` (2 preceding siblings ...)
  2022-11-07  8:45 ` [PATCH v3] app/testpmd: fix protocol header " Yuan Wang
@ 2022-11-09  1:37 ` Yuan Wang
  2022-11-10  6:55   ` Andrew Rybchenko
  2022-11-10  8:20 ` [PATCH v5] " Yuan Wang
  4 siblings, 1 reply; 15+ messages in thread
From: Yuan Wang @ 2022-11-09  1:37 UTC (permalink / raw)
  To: andrew.rybchenko, aman.deep.singh, yuying.zhang
  Cc: xuan.ding, yaqi.tang, dev, Yuan Wang

The "show config rxhdrs" cmd displays the configured protocol headers
that are used for protocol-based buffer split.
However, it shows inner-ipv6 as inner-ipv4.

This patch fixes that by redefining rx_pkt_hdr_protos to hold
the full ptypes, and the show and set commands therefore
remain symmetrical.

Fixes: 52e2e7edcf48 ("app/testpmd: add protocol-based buffer split")

Signed-off-by: Yuan Wang <yuanx.wang@intel.com>

---
v4:
- redefine rx_pkt_hdr_protos to hold the full ptypes.
- use single switch in get_ptype_str().
v3:
- use RTE_PTYPE_*_MASK as masks.
- refactor to use switch statement.
v2:
- add fixline.

---
 app/test-pmd/cmdline.c |  12 ++--
 app/test-pmd/config.c  | 141 +++++++++++++++++++++--------------------
 app/test-pmd/testpmd.c |   5 +-
 3 files changed, 82 insertions(+), 76 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 3fbcb6ca8f..f7df24f105 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -3443,20 +3443,16 @@ parse_hdrs_list(const char *str, const char *item_name, unsigned int max_items,
 				unsigned int *parsed_items)
 {
 	unsigned int nb_item;
-	char *cur;
-	char *tmp;
-	unsigned int cur_item, prev_items = 0;
+	char *cur, *tmp;
+	char *str2;
 
 	nb_item = 0;
-	char *str2 = strdup(str);
+	str2 = strdup(str);
 	cur = strtok_r(str2, ",", &tmp);
 	while (cur != NULL) {
-		cur_item = get_ptype(cur);
-		cur_item &= ~prev_items;
-		parsed_items[nb_item] = cur_item;
+		parsed_items[nb_item] = get_ptype(cur);
 		cur = strtok_r(NULL, ",", &tmp);
 		nb_item++;
-		prev_items |= cur_item;
 	}
 	if (nb_item > max_items)
 		fprintf(stderr, "Number of %s = %u > %u (maximum items)\n",
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index e8a1b77c2a..755728966b 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -5070,73 +5070,79 @@ show_rx_pkt_segments(void)
 
 static const char *get_ptype_str(uint32_t ptype)
 {
-	if ((ptype & (RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_TCP)) ==
-		(RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_TCP))
-		return "ipv4-tcp";
-	else if ((ptype & (RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_UDP)) ==
-		(RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_UDP))
-		return "ipv4-udp";
-	else if ((ptype & (RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_SCTP)) ==
-		(RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_SCTP))
-		return "ipv4-sctp";
-	else if ((ptype & (RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_TCP)) ==
-		(RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_TCP))
-		return "ipv6-tcp";
-	else if ((ptype & (RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_UDP)) ==
-		(RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_UDP))
-		return "ipv6-udp";
-	else if ((ptype & (RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_SCTP)) ==
-		(RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_SCTP))
-		return "ipv6-sctp";
-	else if ((ptype & RTE_PTYPE_L4_TCP) == RTE_PTYPE_L4_TCP)
-		return "tcp";
-	else if ((ptype & RTE_PTYPE_L4_UDP) == RTE_PTYPE_L4_UDP)
-		return "udp";
-	else if ((ptype & RTE_PTYPE_L4_SCTP) == RTE_PTYPE_L4_SCTP)
-		return "sctp";
-	else if ((ptype & RTE_PTYPE_L3_IPV4_EXT_UNKNOWN) == RTE_PTYPE_L3_IPV4_EXT_UNKNOWN)
-		return "ipv4";
-	else if ((ptype & RTE_PTYPE_L3_IPV6_EXT_UNKNOWN) == RTE_PTYPE_L3_IPV6_EXT_UNKNOWN)
-		return "ipv6";
-	else if ((ptype & RTE_PTYPE_L2_ETHER) == RTE_PTYPE_L2_ETHER)
-		return "eth";
-
-	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_TCP)) ==
-		(RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_TCP))
-		return "inner-ipv4-tcp";
-	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_UDP)) ==
-		(RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_UDP))
-		return "inner-ipv4-udp";
-	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_SCTP)) ==
-		(RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_SCTP))
-		return "inner-ipv4-sctp";
-	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_TCP)) ==
-		(RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_TCP))
-		return "inner-ipv6-tcp";
-	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_UDP)) ==
-		(RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_UDP))
-		return "inner-ipv6-udp";
-	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_SCTP)) ==
-		(RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_SCTP))
-		return "inner-ipv6-sctp";
-	else if ((ptype & RTE_PTYPE_INNER_L4_TCP) == RTE_PTYPE_INNER_L4_TCP)
-		return "inner-tcp";
-	else if ((ptype & RTE_PTYPE_INNER_L4_UDP) == RTE_PTYPE_INNER_L4_UDP)
-		return "inner-udp";
-	else if ((ptype & RTE_PTYPE_INNER_L4_SCTP) == RTE_PTYPE_INNER_L4_SCTP)
-		return "inner-sctp";
-	else if ((ptype & RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN) ==
-		RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN)
-		return "inner-ipv4";
-	else if ((ptype & RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN) ==
-		RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN)
-		return "inner-ipv6";
-	else if ((ptype & RTE_PTYPE_INNER_L2_ETHER) == RTE_PTYPE_INNER_L2_ETHER)
-		return "inner-eth";
-	else if ((ptype & RTE_PTYPE_TUNNEL_GRENAT) == RTE_PTYPE_TUNNEL_GRENAT)
-		return "grenat";
-	else
-		return "unsupported";
+	const char *str;
+
+	switch (ptype) {
+	case RTE_PTYPE_L2_ETHER:
+		str = "eth";
+		break;
+	case RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN:
+		str = "ipv4";
+		break;
+	case RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV6_EXT_UNKNOWN:
+		str = "ipv6";
+		break;
+	case RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_TCP:
+		str = "ipv4-tcp";
+		break;
+	case RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_UDP:
+		str = "ipv4-udp";
+		break;
+	case RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_SCTP:
+		str = "ipv4-sctp";
+		break;
+	case RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_TCP:
+		str = "ipv6-tcp";
+		break;
+	case RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_UDP:
+		str = "ipv6-udp";
+		break;
+	case RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_SCTP:
+		str = "ipv6-sctp";
+		break;
+	case RTE_PTYPE_TUNNEL_GRENAT:
+		str = "grenat";
+		break;
+	case RTE_PTYPE_TUNNEL_GRENAT | RTE_PTYPE_INNER_L2_ETHER:
+		str = "inner-eth";
+		break;
+	case RTE_PTYPE_TUNNEL_GRENAT | RTE_PTYPE_INNER_L2_ETHER
+			| RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN:
+		str = "inner-ipv4";
+		break;
+	case RTE_PTYPE_TUNNEL_GRENAT | RTE_PTYPE_INNER_L2_ETHER
+			| RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN:
+		str = "inner-ipv6";
+		break;
+	case RTE_PTYPE_TUNNEL_GRENAT | RTE_PTYPE_INNER_L2_ETHER |
+			RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_TCP:
+		str = "inner-ipv4-tcp";
+		break;
+	case RTE_PTYPE_TUNNEL_GRENAT | RTE_PTYPE_INNER_L2_ETHER |
+			RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_UDP:
+		str = "inner-ipv4-udp";
+		break;
+	case RTE_PTYPE_TUNNEL_GRENAT | RTE_PTYPE_INNER_L2_ETHER |
+			RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_SCTP:
+		str = "inner-ipv4-sctp";
+		break;
+	case RTE_PTYPE_TUNNEL_GRENAT | RTE_PTYPE_INNER_L2_ETHER |
+			RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_TCP:
+		str = "inner-ipv6-tcp";
+		break;
+	case RTE_PTYPE_TUNNEL_GRENAT | RTE_PTYPE_INNER_L2_ETHER |
+			RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_UDP:
+		str = "inner-ipv6-udp";
+		break;
+	case RTE_PTYPE_TUNNEL_GRENAT | RTE_PTYPE_INNER_L2_ETHER |
+			RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_SCTP:
+		str = "inner-ipv6-sctp";
+		break;
+	default:
+		str = "unsupported";
+	}
+
+	return str;
 }
 
 void
@@ -5169,6 +5175,7 @@ set_rx_pkt_hdrs(unsigned int *seg_hdrs, unsigned int nb_segs)
 
 	for (i = 0; i < nb_segs; i++)
 		rx_pkt_hdr_protos[i] = (uint32_t)seg_hdrs[i];
+
 	/*
 	 * We calculate the number of hdrs, but payload is not included,
 	 * so rx_pkt_nb_segs would increase 1.
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 7381dfd9e5..b4a1fb360f 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2654,6 +2654,7 @@ rx_queue_setup(uint16_t port_id, uint16_t rx_queue_id,
 {
 	union rte_eth_rxseg rx_useg[MAX_SEGS_BUFFER_SPLIT] = {};
 	unsigned int i, mp_n;
+	uint32_t prev_hdrs = 0;
 	int ret;
 
 	if (rx_pkt_nb_segs <= 1 ||
@@ -2668,6 +2669,7 @@ rx_queue_setup(uint16_t port_id, uint16_t rx_queue_id,
 	for (i = 0; i < rx_pkt_nb_segs; i++) {
 		struct rte_eth_rxseg_split *rx_seg = &rx_useg[i].split;
 		struct rte_mempool *mpx;
+
 		/*
 		 * Use last valid pool for the segments with number
 		 * exceeding the pool index.
@@ -2679,7 +2681,8 @@ rx_queue_setup(uint16_t port_id, uint16_t rx_queue_id,
 				   rx_pkt_seg_offsets[i] : 0;
 		rx_seg->mp = mpx ? mpx : mp;
 		if (rx_pkt_hdr_protos[i] != 0 && rx_pkt_seg_lengths[i] == 0) {
-			rx_seg->proto_hdr = rx_pkt_hdr_protos[i];
+			rx_seg->proto_hdr = rx_pkt_hdr_protos[i] & ~prev_hdrs;
+			prev_hdrs |= rx_seg->proto_hdr;
 		} else {
 			rx_seg->length = rx_pkt_seg_lengths[i] ?
 					rx_pkt_seg_lengths[i] :
-- 
2.25.1


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

* Re: [PATCH v4] app/testpmd: fix protocol header display for Rx buffer split
  2022-11-09  1:37 ` [PATCH v4] " Yuan Wang
@ 2022-11-10  6:55   ` Andrew Rybchenko
  2022-11-10  7:49     ` Wang, YuanX
  0 siblings, 1 reply; 15+ messages in thread
From: Andrew Rybchenko @ 2022-11-10  6:55 UTC (permalink / raw)
  To: Yuan Wang, aman.deep.singh, yuying.zhang; +Cc: xuan.ding, yaqi.tang, dev

On 11/9/22 04:37, Yuan Wang wrote:
> The "show config rxhdrs" cmd displays the configured protocol headers
> that are used for protocol-based buffer split.
> However, it shows inner-ipv6 as inner-ipv4.
> 
> This patch fixes that by redefining rx_pkt_hdr_protos to hold
> the full ptypes, and the show and set commands therefore
> remain symmetrical.
> 
> Fixes: 52e2e7edcf48 ("app/testpmd: add protocol-based buffer split")
> 
> Signed-off-by: Yuan Wang <yuanx.wang@intel.com>
> 
> ---
> v4:
> - redefine rx_pkt_hdr_protos to hold the full ptypes.
> - use single switch in get_ptype_str().
> v3:
> - use RTE_PTYPE_*_MASK as masks.
> - refactor to use switch statement.
> v2:
> - add fixline.
> 
> ---
>   app/test-pmd/cmdline.c |  12 ++--
>   app/test-pmd/config.c  | 141 +++++++++++++++++++++--------------------
>   app/test-pmd/testpmd.c |   5 +-
>   3 files changed, 82 insertions(+), 76 deletions(-)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index 3fbcb6ca8f..f7df24f105 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -3443,20 +3443,16 @@ parse_hdrs_list(const char *str, const char *item_name, unsigned int max_items,
>   				unsigned int *parsed_items)
>   {
>   	unsigned int nb_item;
> -	char *cur;
> -	char *tmp;
> -	unsigned int cur_item, prev_items = 0;
> +	char *cur, *tmp;

above line is unrelated changes

> +	char *str2;
>   
>   	nb_item = 0;
> -	char *str2 = strdup(str);
> +	str2 = strdup(str);

I've failed to find where str2 is freed.

>   	cur = strtok_r(str2, ",", &tmp);
>   	while (cur != NULL) {
> -		cur_item = get_ptype(cur);
> -		cur_item &= ~prev_items;
> -		parsed_items[nb_item] = cur_item;
> +		parsed_items[nb_item] = get_ptype(cur);
>   		cur = strtok_r(NULL, ",", &tmp);
>   		nb_item++;
> -		prev_items |= cur_item;
>   	}
>   	if (nb_item > max_items)
>   		fprintf(stderr, "Number of %s = %u > %u (maximum items)\n",
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index e8a1b77c2a..755728966b 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -5070,73 +5070,79 @@ show_rx_pkt_segments(void)
>   
>   static const char *get_ptype_str(uint32_t ptype)
>   {
> -	if ((ptype & (RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_TCP)) ==
> -		(RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_TCP))
> -		return "ipv4-tcp";
> -	else if ((ptype & (RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_UDP)) ==
> -		(RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_UDP))
> -		return "ipv4-udp";
> -	else if ((ptype & (RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_SCTP)) ==
> -		(RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_SCTP))
> -		return "ipv4-sctp";
> -	else if ((ptype & (RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_TCP)) ==
> -		(RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_TCP))
> -		return "ipv6-tcp";
> -	else if ((ptype & (RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_UDP)) ==
> -		(RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_UDP))
> -		return "ipv6-udp";
> -	else if ((ptype & (RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_SCTP)) ==
> -		(RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_SCTP))
> -		return "ipv6-sctp";
> -	else if ((ptype & RTE_PTYPE_L4_TCP) == RTE_PTYPE_L4_TCP)
> -		return "tcp";
> -	else if ((ptype & RTE_PTYPE_L4_UDP) == RTE_PTYPE_L4_UDP)
> -		return "udp";
> -	else if ((ptype & RTE_PTYPE_L4_SCTP) == RTE_PTYPE_L4_SCTP)
> -		return "sctp";
> -	else if ((ptype & RTE_PTYPE_L3_IPV4_EXT_UNKNOWN) == RTE_PTYPE_L3_IPV4_EXT_UNKNOWN)
> -		return "ipv4";
> -	else if ((ptype & RTE_PTYPE_L3_IPV6_EXT_UNKNOWN) == RTE_PTYPE_L3_IPV6_EXT_UNKNOWN)
> -		return "ipv6";
> -	else if ((ptype & RTE_PTYPE_L2_ETHER) == RTE_PTYPE_L2_ETHER)
> -		return "eth";
> -
> -	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_TCP)) ==
> -		(RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_TCP))
> -		return "inner-ipv4-tcp";
> -	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_UDP)) ==
> -		(RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_UDP))
> -		return "inner-ipv4-udp";
> -	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_SCTP)) ==
> -		(RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_SCTP))
> -		return "inner-ipv4-sctp";
> -	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_TCP)) ==
> -		(RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_TCP))
> -		return "inner-ipv6-tcp";
> -	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_UDP)) ==
> -		(RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_UDP))
> -		return "inner-ipv6-udp";
> -	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_SCTP)) ==
> -		(RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_SCTP))
> -		return "inner-ipv6-sctp";
> -	else if ((ptype & RTE_PTYPE_INNER_L4_TCP) == RTE_PTYPE_INNER_L4_TCP)
> -		return "inner-tcp";
> -	else if ((ptype & RTE_PTYPE_INNER_L4_UDP) == RTE_PTYPE_INNER_L4_UDP)
> -		return "inner-udp";
> -	else if ((ptype & RTE_PTYPE_INNER_L4_SCTP) == RTE_PTYPE_INNER_L4_SCTP)
> -		return "inner-sctp";
> -	else if ((ptype & RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN) ==
> -		RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN)
> -		return "inner-ipv4";
> -	else if ((ptype & RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN) ==
> -		RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN)
> -		return "inner-ipv6";
> -	else if ((ptype & RTE_PTYPE_INNER_L2_ETHER) == RTE_PTYPE_INNER_L2_ETHER)
> -		return "inner-eth";
> -	else if ((ptype & RTE_PTYPE_TUNNEL_GRENAT) == RTE_PTYPE_TUNNEL_GRENAT)
> -		return "grenat";
> -	else
> -		return "unsupported";
> +	const char *str;
> +
> +	switch (ptype) {
> +	case RTE_PTYPE_L2_ETHER:
> +		str = "eth";
> +		break;
> +	case RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN:
> +		str = "ipv4";
> +		break;
> +	case RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV6_EXT_UNKNOWN:
> +		str = "ipv6";
> +		break;
> +	case RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_TCP:
> +		str = "ipv4-tcp";
> +		break;
> +	case RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_UDP:
> +		str = "ipv4-udp";
> +		break;
> +	case RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_SCTP:
> +		str = "ipv4-sctp";
> +		break;
> +	case RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_TCP:
> +		str = "ipv6-tcp";
> +		break;
> +	case RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_UDP:
> +		str = "ipv6-udp";
> +		break;
> +	case RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_SCTP:
> +		str = "ipv6-sctp";
> +		break;
> +	case RTE_PTYPE_TUNNEL_GRENAT:
> +		str = "grenat";
> +		break;
> +	case RTE_PTYPE_TUNNEL_GRENAT | RTE_PTYPE_INNER_L2_ETHER:
> +		str = "inner-eth";
> +		break;
> +	case RTE_PTYPE_TUNNEL_GRENAT | RTE_PTYPE_INNER_L2_ETHER
> +			| RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN:
> +		str = "inner-ipv4";
> +		break;
> +	case RTE_PTYPE_TUNNEL_GRENAT | RTE_PTYPE_INNER_L2_ETHER
> +			| RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN:
> +		str = "inner-ipv6";
> +		break;
> +	case RTE_PTYPE_TUNNEL_GRENAT | RTE_PTYPE_INNER_L2_ETHER |
> +			RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_TCP:
> +		str = "inner-ipv4-tcp";
> +		break;
> +	case RTE_PTYPE_TUNNEL_GRENAT | RTE_PTYPE_INNER_L2_ETHER |
> +			RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_UDP:
> +		str = "inner-ipv4-udp";
> +		break;
> +	case RTE_PTYPE_TUNNEL_GRENAT | RTE_PTYPE_INNER_L2_ETHER |
> +			RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_SCTP:
> +		str = "inner-ipv4-sctp";
> +		break;
> +	case RTE_PTYPE_TUNNEL_GRENAT | RTE_PTYPE_INNER_L2_ETHER |
> +			RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_TCP:
> +		str = "inner-ipv6-tcp";
> +		break;
> +	case RTE_PTYPE_TUNNEL_GRENAT | RTE_PTYPE_INNER_L2_ETHER |
> +			RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_UDP:
> +		str = "inner-ipv6-udp";
> +		break;
> +	case RTE_PTYPE_TUNNEL_GRENAT | RTE_PTYPE_INNER_L2_ETHER |
> +			RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_SCTP:
> +		str = "inner-ipv6-sctp";
> +		break;
> +	default:
> +		str = "unsupported";
> +	}
> +
> +	return str;
>   }
>   
>   void
> @@ -5169,6 +5175,7 @@ set_rx_pkt_hdrs(unsigned int *seg_hdrs, unsigned int nb_segs)
>   
>   	for (i = 0; i < nb_segs; i++)
>   		rx_pkt_hdr_protos[i] = (uint32_t)seg_hdrs[i];
> +
>   	/*
>   	 * We calculate the number of hdrs, but payload is not included,
>   	 * so rx_pkt_nb_segs would increase 1.
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index 7381dfd9e5..b4a1fb360f 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -2654,6 +2654,7 @@ rx_queue_setup(uint16_t port_id, uint16_t rx_queue_id,
>   {
>   	union rte_eth_rxseg rx_useg[MAX_SEGS_BUFFER_SPLIT] = {};
>   	unsigned int i, mp_n;
> +	uint32_t prev_hdrs = 0;
>   	int ret;
>   
>   	if (rx_pkt_nb_segs <= 1 ||
> @@ -2668,6 +2669,7 @@ rx_queue_setup(uint16_t port_id, uint16_t rx_queue_id,
>   	for (i = 0; i < rx_pkt_nb_segs; i++) {
>   		struct rte_eth_rxseg_split *rx_seg = &rx_useg[i].split;
>   		struct rte_mempool *mpx;
> +

unrelated changes, avoid it

>   		/*
>   		 * Use last valid pool for the segments with number
>   		 * exceeding the pool index.
> @@ -2679,7 +2681,8 @@ rx_queue_setup(uint16_t port_id, uint16_t rx_queue_id,
>   				   rx_pkt_seg_offsets[i] : 0;
>   		rx_seg->mp = mpx ? mpx : mp;
>   		if (rx_pkt_hdr_protos[i] != 0 && rx_pkt_seg_lengths[i] == 0) {
> -			rx_seg->proto_hdr = rx_pkt_hdr_protos[i];
> +			rx_seg->proto_hdr = rx_pkt_hdr_protos[i] & ~prev_hdrs;
> +			prev_hdrs |= rx_seg->proto_hdr;
>   		} else {
>   			rx_seg->length = rx_pkt_seg_lengths[i] ?
>   					rx_pkt_seg_lengths[i] :


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

* RE: [PATCH v4] app/testpmd: fix protocol header display for Rx buffer split
  2022-11-10  6:55   ` Andrew Rybchenko
@ 2022-11-10  7:49     ` Wang, YuanX
  0 siblings, 0 replies; 15+ messages in thread
From: Wang, YuanX @ 2022-11-10  7:49 UTC (permalink / raw)
  To: Andrew Rybchenko, Singh, Aman Deep, Zhang, Yuying
  Cc: Ding, Xuan, Tang, Yaqi, dev

Hi Andrew,

> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Sent: Thursday, November 10, 2022 2:56 PM
> To: Wang, YuanX <yuanx.wang@intel.com>; Singh, Aman Deep
> <aman.deep.singh@intel.com>; Zhang, Yuying <yuying.zhang@intel.com>
> Cc: Ding, Xuan <xuan.ding@intel.com>; Tang, Yaqi <yaqi.tang@intel.com>;
> dev@dpdk.org
> Subject: Re: [PATCH v4] app/testpmd: fix protocol header display for Rx
> buffer split
> 
> On 11/9/22 04:37, Yuan Wang wrote:
> > The "show config rxhdrs" cmd displays the configured protocol headers
> > that are used for protocol-based buffer split.
> > However, it shows inner-ipv6 as inner-ipv4.
> >
> > This patch fixes that by redefining rx_pkt_hdr_protos to hold the full
> > ptypes, and the show and set commands therefore remain symmetrical.
> >
> > Fixes: 52e2e7edcf48 ("app/testpmd: add protocol-based buffer split")
> >
> > Signed-off-by: Yuan Wang <yuanx.wang@intel.com>
> >
> > ---
> > v4:
> > - redefine rx_pkt_hdr_protos to hold the full ptypes.
> > - use single switch in get_ptype_str().
> > v3:
> > - use RTE_PTYPE_*_MASK as masks.
> > - refactor to use switch statement.
> > v2:
> > - add fixline.
> >
> > ---
> >   app/test-pmd/cmdline.c |  12 ++--
> >   app/test-pmd/config.c  | 141 +++++++++++++++++++++--------------------
> >   app/test-pmd/testpmd.c |   5 +-
> >   3 files changed, 82 insertions(+), 76 deletions(-)
> >
> > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> > 3fbcb6ca8f..f7df24f105 100644
> > --- a/app/test-pmd/cmdline.c
> > +++ b/app/test-pmd/cmdline.c
> > @@ -3443,20 +3443,16 @@ parse_hdrs_list(const char *str, const char
> *item_name, unsigned int max_items,
> >   				unsigned int *parsed_items)
> >   {
> >   	unsigned int nb_item;
> > -	char *cur;
> > -	char *tmp;
> > -	unsigned int cur_item, prev_items = 0;
> > +	char *cur, *tmp;
> 
> above line is unrelated changes

Sorry for the unrelated changes, they will be removed in the new version.

> 
> > +	char *str2;
> >
> >   	nb_item = 0;
> > -	char *str2 = strdup(str);
> > +	str2 = strdup(str);
> 
> I've failed to find where str2 is freed.

This is also an unrelated change to make the code to follow the code style. It is not a new variable that has been freed in the code.
The change will be removed to make the patch clean.

> 
> >   	cur = strtok_r(str2, ",", &tmp);
> >   	while (cur != NULL) {
> > -		cur_item = get_ptype(cur);
> > -		cur_item &= ~prev_items;
> > -		parsed_items[nb_item] = cur_item;
> > +		parsed_items[nb_item] = get_ptype(cur);
> >   		cur = strtok_r(NULL, ",", &tmp);
> >   		nb_item++;
> > -		prev_items |= cur_item;
> >   	}
> >   	if (nb_item > max_items)
> >   		fprintf(stderr, "Number of %s = %u > %u (maximum
> items)\n", diff

[...]


> >
> >   void
> > @@ -5169,6 +5175,7 @@ set_rx_pkt_hdrs(unsigned int *seg_hdrs,
> unsigned
> > int nb_segs)
> >
> >   	for (i = 0; i < nb_segs; i++)
> >   		rx_pkt_hdr_protos[i] = (uint32_t)seg_hdrs[i];
> > +
> >   	/*
> >   	 * We calculate the number of hdrs, but payload is not included,
> >   	 * so rx_pkt_nb_segs would increase 1.
> > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> > 7381dfd9e5..b4a1fb360f 100644
> > --- a/app/test-pmd/testpmd.c
> > +++ b/app/test-pmd/testpmd.c
> > @@ -2654,6 +2654,7 @@ rx_queue_setup(uint16_t port_id, uint16_t
> rx_queue_id,
> >   {
> >   	union rte_eth_rxseg rx_useg[MAX_SEGS_BUFFER_SPLIT] = {};
> >   	unsigned int i, mp_n;
> > +	uint32_t prev_hdrs = 0;
> >   	int ret;
> >
> >   	if (rx_pkt_nb_segs <= 1 ||
> > @@ -2668,6 +2669,7 @@ rx_queue_setup(uint16_t port_id, uint16_t
> rx_queue_id,
> >   	for (i = 0; i < rx_pkt_nb_segs; i++) {
> >   		struct rte_eth_rxseg_split *rx_seg = &rx_useg[i].split;
> >   		struct rte_mempool *mpx;
> > +
> 
> unrelated changes, avoid it

Will fix in v5.

Thanks,
Yuan



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

* [PATCH v5] app/testpmd: fix protocol header display for Rx buffer split
  2022-10-17 17:40 [PATCH] app/testpmd: fix protocol headers display for Rx buffer split Yuan Wang
                   ` (3 preceding siblings ...)
  2022-11-09  1:37 ` [PATCH v4] " Yuan Wang
@ 2022-11-10  8:20 ` Yuan Wang
  2022-11-10  8:54   ` Andrew Rybchenko
  4 siblings, 1 reply; 15+ messages in thread
From: Yuan Wang @ 2022-11-10  8:20 UTC (permalink / raw)
  To: andrew.rybchenko, aman.deep.singh, yuying.zhang
  Cc: xuan.ding, yaqi.tang, dev, Yuan Wang

The "show config rxhdrs" cmd displays the configured protocol headers
that are used for protocol-based buffer split.
However, it shows inner-ipv6 as inner-ipv4.

This patch fixes that by redefining rx_pkt_hdr_protos to hold
the full ptypes, and the show and set commands therefore
remain symmetrical.

Fixes: 52e2e7edcf48 ("app/testpmd: add protocol-based buffer split")

Signed-off-by: Yuan Wang <yuanx.wang@intel.com>
---
v5:
- remove unrelated changes.
v4:
- redefine rx_pkt_hdr_protos to hold the full ptypes.
- use single switch in get_ptype_str().
v3:
- use RTE_PTYPE_*_MASK as masks.
- refactor to use switch statement.
v2:
- add fixline.

---
 app/test-pmd/cmdline.c |   6 +-
 app/test-pmd/config.c  | 140 +++++++++++++++++++++--------------------
 app/test-pmd/testpmd.c |   4 +-
 3 files changed, 77 insertions(+), 73 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 8dc60e9388..8ffd62cac9 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -3445,18 +3445,14 @@ parse_hdrs_list(const char *str, const char *item_name, unsigned int max_items,
 	unsigned int nb_item;
 	char *cur;
 	char *tmp;
-	unsigned int cur_item, prev_items = 0;
 
 	nb_item = 0;
 	char *str2 = strdup(str);
 	cur = strtok_r(str2, ",", &tmp);
 	while (cur != NULL) {
-		cur_item = get_ptype(cur);
-		cur_item &= ~prev_items;
-		parsed_items[nb_item] = cur_item;
+		parsed_items[nb_item] = get_ptype(cur);
 		cur = strtok_r(NULL, ",", &tmp);
 		nb_item++;
-		prev_items |= cur_item;
 	}
 	if (nb_item > max_items)
 		fprintf(stderr, "Number of %s = %u > %u (maximum items)\n",
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index e8a1b77c2a..84ebada8fd 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -5070,73 +5070,79 @@ show_rx_pkt_segments(void)
 
 static const char *get_ptype_str(uint32_t ptype)
 {
-	if ((ptype & (RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_TCP)) ==
-		(RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_TCP))
-		return "ipv4-tcp";
-	else if ((ptype & (RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_UDP)) ==
-		(RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_UDP))
-		return "ipv4-udp";
-	else if ((ptype & (RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_SCTP)) ==
-		(RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_SCTP))
-		return "ipv4-sctp";
-	else if ((ptype & (RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_TCP)) ==
-		(RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_TCP))
-		return "ipv6-tcp";
-	else if ((ptype & (RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_UDP)) ==
-		(RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_UDP))
-		return "ipv6-udp";
-	else if ((ptype & (RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_SCTP)) ==
-		(RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_SCTP))
-		return "ipv6-sctp";
-	else if ((ptype & RTE_PTYPE_L4_TCP) == RTE_PTYPE_L4_TCP)
-		return "tcp";
-	else if ((ptype & RTE_PTYPE_L4_UDP) == RTE_PTYPE_L4_UDP)
-		return "udp";
-	else if ((ptype & RTE_PTYPE_L4_SCTP) == RTE_PTYPE_L4_SCTP)
-		return "sctp";
-	else if ((ptype & RTE_PTYPE_L3_IPV4_EXT_UNKNOWN) == RTE_PTYPE_L3_IPV4_EXT_UNKNOWN)
-		return "ipv4";
-	else if ((ptype & RTE_PTYPE_L3_IPV6_EXT_UNKNOWN) == RTE_PTYPE_L3_IPV6_EXT_UNKNOWN)
-		return "ipv6";
-	else if ((ptype & RTE_PTYPE_L2_ETHER) == RTE_PTYPE_L2_ETHER)
-		return "eth";
-
-	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_TCP)) ==
-		(RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_TCP))
-		return "inner-ipv4-tcp";
-	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_UDP)) ==
-		(RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_UDP))
-		return "inner-ipv4-udp";
-	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_SCTP)) ==
-		(RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_SCTP))
-		return "inner-ipv4-sctp";
-	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_TCP)) ==
-		(RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_TCP))
-		return "inner-ipv6-tcp";
-	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_UDP)) ==
-		(RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_UDP))
-		return "inner-ipv6-udp";
-	else if ((ptype & (RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_SCTP)) ==
-		(RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_SCTP))
-		return "inner-ipv6-sctp";
-	else if ((ptype & RTE_PTYPE_INNER_L4_TCP) == RTE_PTYPE_INNER_L4_TCP)
-		return "inner-tcp";
-	else if ((ptype & RTE_PTYPE_INNER_L4_UDP) == RTE_PTYPE_INNER_L4_UDP)
-		return "inner-udp";
-	else if ((ptype & RTE_PTYPE_INNER_L4_SCTP) == RTE_PTYPE_INNER_L4_SCTP)
-		return "inner-sctp";
-	else if ((ptype & RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN) ==
-		RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN)
-		return "inner-ipv4";
-	else if ((ptype & RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN) ==
-		RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN)
-		return "inner-ipv6";
-	else if ((ptype & RTE_PTYPE_INNER_L2_ETHER) == RTE_PTYPE_INNER_L2_ETHER)
-		return "inner-eth";
-	else if ((ptype & RTE_PTYPE_TUNNEL_GRENAT) == RTE_PTYPE_TUNNEL_GRENAT)
-		return "grenat";
-	else
-		return "unsupported";
+	const char *str;
+
+	switch (ptype) {
+	case RTE_PTYPE_L2_ETHER:
+		str = "eth";
+		break;
+	case RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN:
+		str = "ipv4";
+		break;
+	case RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV6_EXT_UNKNOWN:
+		str = "ipv6";
+		break;
+	case RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_TCP:
+		str = "ipv4-tcp";
+		break;
+	case RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_UDP:
+		str = "ipv4-udp";
+		break;
+	case RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_SCTP:
+		str = "ipv4-sctp";
+		break;
+	case RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_TCP:
+		str = "ipv6-tcp";
+		break;
+	case RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_UDP:
+		str = "ipv6-udp";
+		break;
+	case RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_SCTP:
+		str = "ipv6-sctp";
+		break;
+	case RTE_PTYPE_TUNNEL_GRENAT:
+		str = "grenat";
+		break;
+	case RTE_PTYPE_TUNNEL_GRENAT | RTE_PTYPE_INNER_L2_ETHER:
+		str = "inner-eth";
+		break;
+	case RTE_PTYPE_TUNNEL_GRENAT | RTE_PTYPE_INNER_L2_ETHER
+			| RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN:
+		str = "inner-ipv4";
+		break;
+	case RTE_PTYPE_TUNNEL_GRENAT | RTE_PTYPE_INNER_L2_ETHER
+			| RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN:
+		str = "inner-ipv6";
+		break;
+	case RTE_PTYPE_TUNNEL_GRENAT | RTE_PTYPE_INNER_L2_ETHER |
+			RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_TCP:
+		str = "inner-ipv4-tcp";
+		break;
+	case RTE_PTYPE_TUNNEL_GRENAT | RTE_PTYPE_INNER_L2_ETHER |
+			RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_UDP:
+		str = "inner-ipv4-udp";
+		break;
+	case RTE_PTYPE_TUNNEL_GRENAT | RTE_PTYPE_INNER_L2_ETHER |
+			RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_SCTP:
+		str = "inner-ipv4-sctp";
+		break;
+	case RTE_PTYPE_TUNNEL_GRENAT | RTE_PTYPE_INNER_L2_ETHER |
+			RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_TCP:
+		str = "inner-ipv6-tcp";
+		break;
+	case RTE_PTYPE_TUNNEL_GRENAT | RTE_PTYPE_INNER_L2_ETHER |
+			RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_UDP:
+		str = "inner-ipv6-udp";
+		break;
+	case RTE_PTYPE_TUNNEL_GRENAT | RTE_PTYPE_INNER_L2_ETHER |
+			RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_SCTP:
+		str = "inner-ipv6-sctp";
+		break;
+	default:
+		str = "unsupported";
+	}
+
+	return str;
 }
 
 void
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index aa7ea29f15..401ac8b280 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2654,6 +2654,7 @@ rx_queue_setup(uint16_t port_id, uint16_t rx_queue_id,
 {
 	union rte_eth_rxseg rx_useg[MAX_SEGS_BUFFER_SPLIT] = {};
 	unsigned int i, mp_n;
+	uint32_t prev_hdrs = 0;
 	int ret;
 
 	if (rx_pkt_nb_segs <= 1 ||
@@ -2679,7 +2680,8 @@ rx_queue_setup(uint16_t port_id, uint16_t rx_queue_id,
 				   rx_pkt_seg_offsets[i] : 0;
 		rx_seg->mp = mpx ? mpx : mp;
 		if (rx_pkt_hdr_protos[i] != 0 && rx_pkt_seg_lengths[i] == 0) {
-			rx_seg->proto_hdr = rx_pkt_hdr_protos[i];
+			rx_seg->proto_hdr = rx_pkt_hdr_protos[i] & ~prev_hdrs;
+			prev_hdrs |= rx_seg->proto_hdr;
 		} else {
 			rx_seg->length = rx_pkt_seg_lengths[i] ?
 					rx_pkt_seg_lengths[i] :
-- 
2.25.1


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

* Re: [PATCH v5] app/testpmd: fix protocol header display for Rx buffer split
  2022-11-10  8:20 ` [PATCH v5] " Yuan Wang
@ 2022-11-10  8:54   ` Andrew Rybchenko
  0 siblings, 0 replies; 15+ messages in thread
From: Andrew Rybchenko @ 2022-11-10  8:54 UTC (permalink / raw)
  To: Yuan Wang, aman.deep.singh, yuying.zhang; +Cc: xuan.ding, yaqi.tang, dev

On 11/10/22 11:20, Yuan Wang wrote:
> The "show config rxhdrs" cmd displays the configured protocol headers
> that are used for protocol-based buffer split.
> However, it shows inner-ipv6 as inner-ipv4.
> 
> This patch fixes that by redefining rx_pkt_hdr_protos to hold
> the full ptypes, and the show and set commands therefore
> remain symmetrical.
> 
> Fixes: 52e2e7edcf48 ("app/testpmd: add protocol-based buffer split")
> 
> Signed-off-by: Yuan Wang <yuanx.wang@intel.com>

Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>

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



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

end of thread, other threads:[~2022-11-10  8:54 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-17 17:40 [PATCH] app/testpmd: fix protocol headers display for Rx buffer split Yuan Wang
2022-10-17 10:03 ` Tang, Yaqi
2022-10-18 14:50 ` [PATCH v2] " Yuan Wang
2022-10-28  2:04   ` Wang, YuanX
2022-11-06  9:58   ` Andrew Rybchenko
2022-11-07  5:55     ` Wang, YuanX
2022-11-07  8:45 ` [PATCH v3] app/testpmd: fix protocol header " Yuan Wang
2022-11-07  8:51   ` Tang, Yaqi
2022-11-07 11:31   ` Andrew Rybchenko
2022-11-08 13:53     ` Wang, YuanX
2022-11-09  1:37 ` [PATCH v4] " Yuan Wang
2022-11-10  6:55   ` Andrew Rybchenko
2022-11-10  7:49     ` Wang, YuanX
2022-11-10  8:20 ` [PATCH v5] " Yuan Wang
2022-11-10  8:54   ` Andrew Rybchenko

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