diff mbox series

[v3] app/testpmd: fix protocol header display for Rx buffer split

Message ID 20221107084506.1896422-1-yuanx.wang@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Andrew Rybchenko
Headers
Series [v3] app/testpmd: fix protocol header display for Rx buffer split |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/github-robot: build success github build: passed
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS

Commit Message

Wang, YuanX Nov. 7, 2022, 8:45 a.m. UTC 
  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(-)

Comments

Yaqi Tang Nov. 7, 2022, 8:51 a.m. UTC | #1 
> -----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>
Andrew Rybchenko Nov. 7, 2022, 11:31 a.m. UTC | #2 
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
Wang, YuanX Nov. 8, 2022, 1:53 p.m. UTC | #3 
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
diff mbox series

Patch

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