> -----Original Message-----
> From: Ferruh Yigit <ferruh.yi...@amd.com>
> Sent: Tuesday, July 23, 2024 4:06 PM
> To: Minggang(Gavin) Li <gav...@nvidia.com>; Matan Azrad <ma...@nvidia.com>;
> Slava Ovsiienko <viachesl...@nvidia.com>; Ori Kam <or...@nvidia.com>; NBU-
> Contact-Thomas Monjalon (EXTERNAL) <tho...@monjalon.net>; Aman Singh
> <aman.deep.si...@intel.com>
> Cc: dev@dpdk.org; Raslan Darawsheh <rasl...@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 <gav...@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.
