Re: [V2] app/testpmd: restore VXLAN-GPE support

[Ferruh Yigit <ferruh.yigit@amd.com>]

On 7/23/2024 8:37 AM, Gavin Li wrote:
> VXLAN-GPE support was removed from testpmd recently. Drivers which are
> not migrated are still using VXLAN-GPE in tests.
> 
> This commit is to restore the support for VXLAN-GPE in testpmd.
> 
> After this change, there are two command line items with the same name, ie.
> "protocol". One is for the new VXLAN extensions and the other is for the
> deprecated VXLAN-GPE. Add a new one that is more obvious for the new VXLAN
> structure since the two have different command line context.
> 
> Fixes: da118115d95c ("app/testpmd: support matching any VXLAN field")
> Signed-off-by: Gavin Li <gavinl@nvidia.com>
> ---
>  app/test-pmd/cmdline_flow.c                 | 71 ++++++++++++++++++++-
>  doc/guides/testpmd_app_ug/testpmd_funcs.rst | 21 ++++--
>  2 files changed, 83 insertions(+), 9 deletions(-)
> 
> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> index a76b44bf39..2c5a0491d2 100644
> --- a/app/test-pmd/cmdline_flow.c
> +++ b/app/test-pmd/cmdline_flow.c
> @@ -391,7 +391,7 @@ enum index {
>  	ITEM_VXLAN_FLAG_D,
>  	ITEM_VXLAN_FLAG_A,
>  	ITEM_VXLAN_GBP_ID,
> -	ITEM_VXLAN_GPE_PROTO,
> +	ITEM_VXLAN_GPE_PROTOCOL,
>

With more obvious naming I was suggesting something like:
ITEM_VXLAN_GPE_PROTO_IN_COMBINED_STRUCT

Or we can go the other way around, give obvious name to the other one,
and when it is removed, this saves us some work to rename. Whichever you
thik better is good.


Also can you please add code comment, I think at least giving struct
names can be useful, something like:
/* Used for "struct rte_vxlan_gpe_hdr", deprecated, prefer ... */

Similar comment to the both can help.


>  	ITEM_VXLAN_FIRST_RSVD,
>  	ITEM_VXLAN_SECND_RSVD,
>  	ITEM_VXLAN_THIRD_RSVD,
> @@ -423,6 +423,12 @@ enum index {
>  	ITEM_GENEVE_VNI,
>  	ITEM_GENEVE_PROTO,
>  	ITEM_GENEVE_OPTLEN,
> +	ITEM_VXLAN_GPE,
> +	ITEM_VXLAN_GPE_VNI,
> +	ITEM_VXLAN_GPE_PROTO,
> +	ITEM_VXLAN_GPE_FLAGS,
> +	ITEM_VXLAN_GPE_RSVD0,
> +	ITEM_VXLAN_GPE_RSVD1,
>  	ITEM_ARP_ETH_IPV4,
>  	ITEM_ARP_ETH_IPV4_SHA,
>  	ITEM_ARP_ETH_IPV4_SPA,
> @@ -1612,6 +1618,7 @@ static const enum index next_item[] = {
>  	ITEM_GTPC,
>  	ITEM_GTPU,
>  	ITEM_GENEVE,
> +	ITEM_VXLAN_GPE,
>  	ITEM_ARP_ETH_IPV4,
>  	ITEM_IPV6_EXT,
>  	ITEM_IPV6_FRAG_EXT,
> @@ -1794,7 +1801,7 @@ static const enum index item_vxlan[] = {
>  	ITEM_VXLAN_FLAG_D,
>  	ITEM_VXLAN_FLAG_A,
>  	ITEM_VXLAN_GBP_ID,
> -	ITEM_VXLAN_GPE_PROTO,
> +	ITEM_VXLAN_GPE_PROTOCOL,
>  	ITEM_VXLAN_FIRST_RSVD,
>  	ITEM_VXLAN_SECND_RSVD,
>  	ITEM_VXLAN_THIRD_RSVD,
> @@ -1864,6 +1871,16 @@ static const enum index item_geneve[] = {
>  	ZERO,
>  };
>  
> +static const enum index item_vxlan_gpe[] = {
> +	ITEM_VXLAN_GPE_VNI,
> +	ITEM_VXLAN_GPE_PROTO,
> +	ITEM_VXLAN_GPE_FLAGS,
> +	ITEM_VXLAN_GPE_RSVD0,
> +	ITEM_VXLAN_GPE_RSVD1,
> +	ITEM_NEXT,
> +	ZERO,
> +};
> +
>  static const enum index item_arp_eth_ipv4[] = {
>  	ITEM_ARP_ETH_IPV4_SHA,
>  	ITEM_ARP_ETH_IPV4_SPA,
> @@ -4992,7 +5009,7 @@ static const struct token token_list[] = {
>  		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_vxlan,
>  					     hdr.policy_id)),
>  	},
> -	[ITEM_VXLAN_GPE_PROTO] = {
> +	[ITEM_VXLAN_GPE_PROTOCOL] = {
>  		.name = "protocol",
>  		.help = "VXLAN GPE next protocol",
>  		.next = NEXT(item_vxlan, NEXT_ENTRY(COMMON_UNSIGNED),
> @@ -5241,6 +5258,54 @@ static const struct token token_list[] = {
>  						  ver_opt_len_o_c_rsvd0,
>  						  "\x3f\x00")),
>  	},
> +	[ITEM_VXLAN_GPE] = {
> +		.name = "vxlan-gpe",
> +		.help = "match VXLAN-GPE header",
> +		.priv = PRIV_ITEM(VXLAN_GPE,
> +				  sizeof(struct rte_flow_item_vxlan_gpe)),
> +		.next = NEXT(item_vxlan_gpe),
> +		.call = parse_vc,
> +	},
> +	[ITEM_VXLAN_GPE_VNI] = {
> +		.name = "vni",
> +		.help = "VXLAN-GPE identifier",
> +		.next = NEXT(item_vxlan_gpe, NEXT_ENTRY(COMMON_UNSIGNED),
> +			     item_param),
> +		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_vxlan_gpe,
> +					     hdr.vni)),
> +	},
> +	[ITEM_VXLAN_GPE_PROTO] = {
> +		.name = "protocol",
> +		.help = "VXLAN-GPE next protocol",
> +		.next = NEXT(item_vxlan_gpe, NEXT_ENTRY(COMMON_UNSIGNED),
> +			     item_param),
> +		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_vxlan_gpe,
> +					     protocol)),
> +	},
> +	[ITEM_VXLAN_GPE_FLAGS] = {
> +		.name = "flags",
> +		.help = "VXLAN-GPE flags",
> +		.next = NEXT(item_vxlan_gpe, NEXT_ENTRY(COMMON_UNSIGNED),
> +			     item_param),
> +		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_vxlan_gpe,
> +					     flags)),
> +	},
> +	[ITEM_VXLAN_GPE_RSVD0] = {
> +		.name = "rsvd0",
> +		.help = "VXLAN-GPE rsvd0",
> +		.next = NEXT(item_vxlan_gpe, NEXT_ENTRY(COMMON_UNSIGNED),
> +			     item_param),
> +		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_vxlan_gpe,
> +					     rsvd0)),
> +	},
> +	[ITEM_VXLAN_GPE_RSVD1] = {
> +		.name = "rsvd1",
> +		.help = "VXLAN-GPE rsvd1",
> +		.next = NEXT(item_vxlan_gpe, NEXT_ENTRY(COMMON_UNSIGNED),
> +			     item_param),
> +		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_vxlan_gpe,
> +					     rsvd1)),
> +	},
>  	[ITEM_ARP_ETH_IPV4] = {
>  		.name = "arp_eth_ipv4",
>  		.help = "match ARP header for Ethernet/IPv4",
> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> index c19b4f8958..f00ab07605 100644
> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> @@ -214,6 +214,7 @@ For example:
>       vxlan
>       geneve
>       nvgre
> +     vxlan-gpe
>  
>  show port (module_eeprom|eeprom)
>  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> @@ -1103,12 +1104,12 @@ Where:
>  * ``ip|udp|tcp|sctp`` always relate to  the inner layer.
>  
>  * ``outer-ip`` relates to the outer IP layer (only for IPv4) in the case where the packet is recognized
> -  as a tunnel packet by the forwarding engine (geneve, gre, gtp, ipip and vxlan are supported).
> -  See also the ``csum parse-tunnel`` command.
> +  as a tunnel packet by the forwarding engine (geneve, gre, gtp, ipip, vxlan and vxlan-gpe are
> +  supported). See also the ``csum parse-tunnel`` command.
>  
>  * ``outer-udp`` relates to the outer UDP layer in the case where the packet is recognized
> -  as a tunnel packet by the forwarding engine (geneve, gtp and vxlan are supported).
> -  See also the ``csum parse-tunnel`` command.
> +  as a tunnel packet by the forwarding engine (geneve, gtp, vxlan and vxlan-gpe are
> +  supported). See also the ``csum parse-tunnel`` command.
>  
>  .. note::
>  
> @@ -1123,7 +1124,7 @@ engine::
>     testpmd> csum parse-tunnel (on|off) (tx_port_id)
>  
>  If enabled, the csum forward engine will try to recognize supported
> -tunnel headers (geneve, gtp, gre, ipip, vxlan).
> +tunnel headers (geneve, gtp, gre, ipip, vxlan, vxlan-gpe).
>  
>  If disabled, treat tunnel packets as non-tunneled packets (a inner
>  header is handled as a packet payload).
> @@ -2221,7 +2222,7 @@ port config udp_tunnel_port
>  
>  Add/remove UDP tunnel port for VXLAN/GENEVE tunneling protocols::
>  
> -    testpmd> port config (port_id) udp_tunnel_port add|rm vxlan|geneve|ecpri (udp_port)
> +    testpmd> port config (port_id) udp_tunnel_port add|rm vxlan|geneve|vxlan-gpe|ecpri (udp_port)
>  
>  port config tx_metadata
>  ~~~~~~~~~~~~~~~~~~~~~~~
> @@ -3745,6 +3746,14 @@ This section lists supported pattern items and their attributes, if any.
>    - ``data {hex string}``: GENEVE option data, the length is defined by
>      ``length`` field.
>  
> +- ``vxlan-gpe``: match VXLAN-GPE header.
> +
> +  - ``vni {unsigned}``: VXLAN-GPE identifier.
> +  - ``flags {unsigned}``: VXLAN-GPE flags.
> +  - ``protocol {unsigned}`` : VXLAN-GPE next protocol.
> +  - ``rsvd0 {unsigned}``: VXLAN-GPE reserved field 0.
> +  - ``rsvd1 {unsigned}``: VXLAN-GPE reserved field 1.
> +
>  - ``arp_eth_ipv4``: match ARP header for Ethernet/IPv4.
>  
>    - ``sha {MAC-48}``: sender hardware address.