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

["Minggang(Gavin) Li" <gavinl@nvidia.com>]

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@amd.com>
> Sent: Tuesday, July 23, 2024 4:06 PM
> To: Minggang(Gavin) Li <gavinl@nvidia.com>; Matan Azrad <matan@nvidia.com>;
> Slava Ovsiienko <viacheslavo@nvidia.com>; Ori Kam <orika@nvidia.com>; NBU-
> Contact-Thomas Monjalon (EXTERNAL) <thomas@monjalon.net>; Aman Singh
> <aman.deep.singh@intel.com>
> Cc: dev@dpdk.org; Raslan Darawsheh <rasland@nvidia.com>
> Subject: Re: [V2] app/testpmd: restore VXLAN-GPE support
> 
> 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.
ACK. I prefer the second one. 
Eg. ITEM_VXLAN_GPE_PROTO_IN_DEPRECATED_VXLAN_GPE_HDR.
What do you think---I'm not good at naming?
> 
> 
> 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.
ACK
> 
> 
> >  	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.