[V1] app/testpmd: restore VXLAN-GPE support
Checks
Commit Message
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.
Fixes: da118115d95c ("app/testpmd: support matching any VXLAN field")
Signed-off-by: Gavin Li <gavinl@nvidia.com>
---
app/test-pmd/cmdline_flow.c | 65 +++++++++++++++++++++
doc/guides/testpmd_app_ug/testpmd_funcs.rst | 21 +++++--
2 files changed, 80 insertions(+), 6 deletions(-)
Comments
On 7/17/2024 8:11 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.
>
> Fixes: da118115d95c ("app/testpmd: support matching any VXLAN field")
> Signed-off-by: Gavin Li <gavinl@nvidia.com>
>
Hi Gavin,
The original patch was from you, right? What went wrong?
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@amd.com>
> Sent: Saturday, July 20, 2024 4:25 AM
> 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: [V1] app/testpmd: restore VXLAN-GPE support
>
> On 7/17/2024 8:11 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.
> >
> > Fixes: da118115d95c ("app/testpmd: support matching any VXLAN field")
> > Signed-off-by: Gavin Li <gavinl@nvidia.com>
> >
>
> Hi Gavin,
>
> The original patch was from you, right? What went wrong?
The remove of VXLAN-GPE from testpmd is too aggressive since there are
drivers which are not migrated are still using VXLAN-GPE. It's better to
keep it till the day to remove the RTE item of VXLAN-GPE from DPDK.
On 7/22/2024 8:10 AM, Minggang(Gavin) Li wrote:
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@amd.com>
>> Sent: Saturday, July 20, 2024 4:25 AM
>> 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: [V1] app/testpmd: restore VXLAN-GPE support
>>
>> On 7/17/2024 8:11 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.
>>>
>>> Fixes: da118115d95c ("app/testpmd: support matching any VXLAN field")
>>> Signed-off-by: Gavin Li <gavinl@nvidia.com>
>>>
>>
>> Hi Gavin,
>>
>> The original patch was from you, right? What went wrong?
> The remove of VXLAN-GPE from testpmd is too aggressive since there are
> drivers which are not migrated are still using VXLAN-GPE. It's better to
> keep it till the day to remove the RTE item of VXLAN-GPE from DPDK.
>
Sorry, I was not clear enough maybe, I was asking more details on the
problem?
With a net/vxlan commit [1] in this release, (this commit is part of
-rc1), VXLAN & VXLAN-GPE headers combined and VXLAN-GBP header added to
this combined struct. VXLAN-GPE header is marked as deprecated.
Testpmd is also updated to use new headers, that is the commit in the
fixes tag of this patch.
But drivers using old, now depreciated, VXLAN structs won't able to use
testpmd to test, so I agree, may be I merged the patch too early.
As this patch was part of -rc1, I wonder why we didn't get any complaint
about not able to test VXLAN-GPE?
Btw, if we revert this patch, is there a way to test VXLAN-GBP? Because
it only exists as part of new combined VXLAN struct?
Instead of reverting the commit all together, is there way to keep old
capability in testpmd, but add feature to test VXLAN-GBP?
And another issue is, there can still some users of the VXLAN-GPE header
in the net library, perhaps that also deprecated immaturely.
Can you please send a deprecation note for combining VXLAN headers and
removing VXLAN-GPE in v24.11? Please CC all drivers implementing this
flow pattern. This can be way to highlight the issue to driver
maintainers and communicate the change with end users.
[1]
77cb7b18ad9b ("net: extend VXLAN header to support more extensions")
22/07/2024 11:36, Ferruh Yigit:
> On 7/22/2024 8:10 AM, Minggang(Gavin) Li wrote:
> > From: Ferruh Yigit <ferruh.yigit@amd.com>
> >> On 7/17/2024 8:11 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.
> >>>
> >>> Fixes: da118115d95c ("app/testpmd: support matching any VXLAN field")
> >>> Signed-off-by: Gavin Li <gavinl@nvidia.com>
> >>>
> >>
> >> Hi Gavin,
> >>
> >> The original patch was from you, right? What went wrong?
> > The remove of VXLAN-GPE from testpmd is too aggressive since there are
> > drivers which are not migrated are still using VXLAN-GPE. It's better to
> > keep it till the day to remove the RTE item of VXLAN-GPE from DPDK.
> >
>
> Sorry, I was not clear enough maybe, I was asking more details on the
> problem?
I remember I was suggesting Gavin to remove testpmd code based on the old API.
And he realized it was too much agressive later.
> With a net/vxlan commit [1] in this release, (this commit is part of
> -rc1), VXLAN & VXLAN-GPE headers combined and VXLAN-GBP header added to
> this combined struct. VXLAN-GPE header is marked as deprecated.
> Testpmd is also updated to use new headers, that is the commit in the
> fixes tag of this patch.
>
> But drivers using old, now depreciated, VXLAN structs won't able to use
> testpmd to test, so I agree, may be I merged the patch too early.
Yes, so this patch re-add testpmd code for the old API.
> As this patch was part of -rc1, I wonder why we didn't get any complaint
> about not able to test VXLAN-GPE?
Maybe it is tested only with mlx5.
> Btw, if we revert this patch, is there a way to test VXLAN-GBP? Because
> it only exists as part of new combined VXLAN struct?
> Instead of reverting the commit all together, is there way to keep old
> capability in testpmd, but add feature to test VXLAN-GBP?
It is not a complete revert, we keep GBP in the new API.
> And another issue is, there can still some users of the VXLAN-GPE header
> in the net library, perhaps that also deprecated immaturely.
It is just marked as deprecated.
> Can you please send a deprecation note for combining VXLAN headers and
> removing VXLAN-GPE in v24.11? Please CC all drivers implementing this
> flow pattern. This can be way to highlight the issue to driver
> maintainers and communicate the change with end users.
24.11 is probably too early.
I propose to keep it as deprecated for compatibility,
and maybe remove in one year?
On 7/22/2024 2:04 PM, Thomas Monjalon wrote:
> 22/07/2024 11:36, Ferruh Yigit:
>> On 7/22/2024 8:10 AM, Minggang(Gavin) Li wrote:
>>> From: Ferruh Yigit <ferruh.yigit@amd.com>
>>>> On 7/17/2024 8:11 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.
>>>>>
>>>>> Fixes: da118115d95c ("app/testpmd: support matching any VXLAN field")
>>>>> Signed-off-by: Gavin Li <gavinl@nvidia.com>
>>>>>
>>>>
>>>> Hi Gavin,
>>>>
>>>> The original patch was from you, right? What went wrong?
>>> The remove of VXLAN-GPE from testpmd is too aggressive since there are
>>> drivers which are not migrated are still using VXLAN-GPE. It's better to
>>> keep it till the day to remove the RTE item of VXLAN-GPE from DPDK.
>>>
>>
>> Sorry, I was not clear enough maybe, I was asking more details on the
>> problem?
>
> I remember I was suggesting Gavin to remove testpmd code based on the old API.
> And he realized it was too much agressive later.
>
>> With a net/vxlan commit [1] in this release, (this commit is part of
>> -rc1), VXLAN & VXLAN-GPE headers combined and VXLAN-GBP header added to
>> this combined struct. VXLAN-GPE header is marked as deprecated.
>> Testpmd is also updated to use new headers, that is the commit in the
>> fixes tag of this patch.
>>
>> But drivers using old, now depreciated, VXLAN structs won't able to use
>> testpmd to test, so I agree, may be I merged the patch too early.
>
> Yes, so this patch re-add testpmd code for the old API.
>
>> As this patch was part of -rc1, I wonder why we didn't get any complaint
>> about not able to test VXLAN-GPE?
>
> Maybe it is tested only with mlx5.
>
>> Btw, if we revert this patch, is there a way to test VXLAN-GBP? Because
>> it only exists as part of new combined VXLAN struct?
>> Instead of reverting the commit all together, is there way to keep old
>> capability in testpmd, but add feature to test VXLAN-GBP?
>
> It is not a complete revert, we keep GBP in the new API.
>
So what I was asking done already, let me check the patch again.
>> And another issue is, there can still some users of the VXLAN-GPE header
>> in the net library, perhaps that also deprecated immaturely.
>
> It is just marked as deprecated.
>
>> Can you please send a deprecation note for combining VXLAN headers and
>> removing VXLAN-GPE in v24.11? Please CC all drivers implementing this
>> flow pattern. This can be way to highlight the issue to driver
>> maintainers and communicate the change with end users.
>
> 24.11 is probably too early.
> I propose to keep it as deprecated for compatibility,
> and maybe remove in one year?
>
No strong opinion, both v24.11 or v25.11 is OK.
Normally I would go with v25.11, but for VXLAN I expect only small users
will be impacted and only a few driver needs update, so perhaps v24.11
also can be a valid target.
We can get comment from driver maintainers to the deprecation notice,
but not able to get this from potential users.
On 7/17/2024 8:11 AM, Gavin Li wrote:
> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> index a76b44bf39..51a8d4993e 100644
> --- a/app/test-pmd/cmdline_flow.c
> +++ b/app/test-pmd/cmdline_flow.c
> @@ -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_PROTOCOL,
> + ITEM_VXLAN_GPE_FLAGS,
> + ITEM_VXLAN_GPE_RSVD0,
> + ITEM_VXLAN_GPE_RSVD1,
>
With this addition we have both of following enum items, right?
'ITEM_VXLAN_GPE_PROTOCOL'
'ITEM_VXLAN_GPE_PROTO'
'ITEM_VXLAN_GPE_PROTOCOL' is for the old usage, which was previously
'ITEM_VXLAN_GPE_PROTO'.
And 'ITEM_VXLAN_GPE_PROTO' is now for the new usage.
This is confusing, and looks like it may live with us for a while if we
remove them on v25.11.
Does it make sense to keep 'ITEM_VXLAN_GPE_PROTO' as it is, add new one
with a name that is more obvious that it is for new VXLAN struct, and
some more comment to explain the reasoning of this redundant enum items?
As these are testpmd internal, when old VXLAN structs removed, I assume
we can easily rename new enum item back to 'ITEM_VXLAN_GPE_PROTO'. What
do you think?
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@amd.com>
> Sent: Monday, July 22, 2024 10:44 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: [V1] app/testpmd: restore VXLAN-GPE support
>
> On 7/17/2024 8:11 AM, Gavin Li wrote:
> > diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> > index a76b44bf39..51a8d4993e 100644
> > --- a/app/test-pmd/cmdline_flow.c
> > +++ b/app/test-pmd/cmdline_flow.c
> > @@ -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_PROTOCOL,
> > + ITEM_VXLAN_GPE_FLAGS,
> > + ITEM_VXLAN_GPE_RSVD0,
> > + ITEM_VXLAN_GPE_RSVD1,
> >
>
> With this addition we have both of following enum items, right?
> 'ITEM_VXLAN_GPE_PROTOCOL'
> 'ITEM_VXLAN_GPE_PROTO'
>
> 'ITEM_VXLAN_GPE_PROTOCOL' is for the old usage, which was previously
> 'ITEM_VXLAN_GPE_PROTO'.
>
> And 'ITEM_VXLAN_GPE_PROTO' is now for the new usage.
>
> This is confusing, and looks like it may live with us for a while if we remove them
> on v25.11.
>
>
> Does it make sense to keep 'ITEM_VXLAN_GPE_PROTO' as it is, add new one with
> a name that is more obvious that it is for new VXLAN struct, and some more
> comment to explain the reasoning of this redundant enum items?
>
> As these are testpmd internal, when old VXLAN structs removed, I assume we can
> easily rename new enum item back to 'ITEM_VXLAN_GPE_PROTO'. What do you
> think?
Sounds good. ACK
@@ -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_PROTOCOL,
+ 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,
@@ -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_PROTOCOL,
+ 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,
@@ -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_PROTOCOL] = {
+ .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",
@@ -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.