* [PATCH] app/testpmd: fix GENEVE parsing in csum forward mode
@ 2021-12-05 3:44 Raja Zidane
2022-01-18 9:51 ` Ferruh Yigit
2022-02-16 12:37 ` [PATCH V2] " Raja Zidane
0 siblings, 2 replies; 20+ messages in thread
From: Raja Zidane @ 2021-12-05 3:44 UTC (permalink / raw)
To: dev; +Cc: Matan Azrad, stable
The csum FWD mode parses any received packet to set mbuf offloads for the
transmitting burst, mainly in the checksum/TSO areas.
In the case of a tunnel header, the csum FWD tries to detect known tunnels
by the standard definition using the header'sdata and fallback to check the
packet type in the mbuf to see if the Rx port driver already sign the
packet as a tunnel.
In the fallback case, the csum assumes the tunnel is VXLAN and parses the
tunnel as VXLAN.
When the GENEVE tunnel was added to the known tunnels in csum, its parsing
trial was wrongly located after the pkt type detection, causing the csum to
parse the GENEVE header as VXLAN when the Rx port set the tunnel packet
type.
Locate the GENEVE parsing trial before the packet type detection.
Fixes: ea0e711b8ae0 ("app/testpmd: add GENEVE parsing")
Cc: stable@dpdk.org
Signed-off-by: Raja Zidane <rzidane@nvidia.com>
---
Acked-by: Matan Azrad <matan@nvidia.com>
app/test-pmd/csumonly.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
index 2aeea243b6..fe810fecdd 100644
--- a/app/test-pmd/csumonly.c
+++ b/app/test-pmd/csumonly.c
@@ -254,7 +254,10 @@ parse_gtp(struct rte_udp_hdr *udp_hdr,
info->l2_len += RTE_ETHER_GTP_HLEN;
}
-/* Parse a vxlan header */
+/*
+ * Parse a vxlan header.
+ * If a tunnel is detected in 'pkt_type' it will be parsed by default as vxlan.
+ */
static void
parse_vxlan(struct rte_udp_hdr *udp_hdr,
struct testpmd_offload_info *info,
@@ -912,17 +915,18 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
RTE_MBUF_F_TX_TUNNEL_VXLAN_GPE;
goto tunnel_update;
}
- parse_vxlan(udp_hdr, &info,
- m->packet_type);
+ parse_geneve(udp_hdr, &info);
if (info.is_tunnel) {
tx_ol_flags |=
- RTE_MBUF_F_TX_TUNNEL_VXLAN;
+ RTE_MBUF_F_TX_TUNNEL_GENEVE;
goto tunnel_update;
}
- parse_geneve(udp_hdr, &info);
+ /* Always keep last. */
+ parse_vxlan(udp_hdr, &info,
+ m->packet_type);
if (info.is_tunnel) {
tx_ol_flags |=
- RTE_MBUF_F_TX_TUNNEL_GENEVE;
+ RTE_MBUF_F_TX_TUNNEL_VXLAN;
goto tunnel_update;
}
} else if (info.l4_proto == IPPROTO_GRE) {
--
2.17.1
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] app/testpmd: fix GENEVE parsing in csum forward mode
2021-12-05 3:44 [PATCH] app/testpmd: fix GENEVE parsing in csum forward mode Raja Zidane
@ 2022-01-18 9:51 ` Ferruh Yigit
2022-01-18 11:27 ` Matan Azrad
2022-02-16 12:37 ` [PATCH V2] " Raja Zidane
1 sibling, 1 reply; 20+ messages in thread
From: Ferruh Yigit @ 2022-01-18 9:51 UTC (permalink / raw)
To: Raja Zidane, dev; +Cc: Matan Azrad, stable
On 12/5/2021 3:44 AM, Raja Zidane wrote:
> The csum FWD mode parses any received packet to set mbuf offloads for the
> transmitting burst, mainly in the checksum/TSO areas.
> In the case of a tunnel header, the csum FWD tries to detect known tunnels
> by the standard definition using the header'sdata and fallback to check the
> packet type in the mbuf to see if the Rx port driver already sign the
> packet as a tunnel.
> In the fallback case, the csum assumes the tunnel is VXLAN and parses the
> tunnel as VXLAN.
As far as I can see there is a VXLAN port check in 'parse_vxlan()', why it
is not helping?
> When the GENEVE tunnel was added to the known tunnels in csum, its parsing
> trial was wrongly located after the pkt type detection, causing the csum to
> parse the GENEVE header as VXLAN when the Rx port set the tunnel packet
> type.
>
> Locate the GENEVE parsing trial before the packet type detection.
>
> Fixes: ea0e711b8ae0 ("app/testpmd: add GENEVE parsing")
> Cc: stable@dpdk.org
>
> Signed-off-by: Raja Zidane <rzidane@nvidia.com>
> ---
> Acked-by: Matan Azrad <matan@nvidia.com>
Ack should be before '---' to be part of the commit log, otherwise it is dropped
when applied as comment.
> app/test-pmd/csumonly.c | 16 ++++++++++------
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
> index 2aeea243b6..fe810fecdd 100644
> --- a/app/test-pmd/csumonly.c
> +++ b/app/test-pmd/csumonly.c
> @@ -254,7 +254,10 @@ parse_gtp(struct rte_udp_hdr *udp_hdr,
> info->l2_len += RTE_ETHER_GTP_HLEN;
> }
>
> -/* Parse a vxlan header */
> +/*
> + * Parse a vxlan header.
> + * If a tunnel is detected in 'pkt_type' it will be parsed by default as vxlan.
> + */
> static void
> parse_vxlan(struct rte_udp_hdr *udp_hdr,
> struct testpmd_offload_info *info,
> @@ -912,17 +915,18 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
> RTE_MBUF_F_TX_TUNNEL_VXLAN_GPE;
> goto tunnel_update;
> }
> - parse_vxlan(udp_hdr, &info,
> - m->packet_type);
> + parse_geneve(udp_hdr, &info);
> if (info.is_tunnel) {
> tx_ol_flags |=
> - RTE_MBUF_F_TX_TUNNEL_VXLAN;
> + RTE_MBUF_F_TX_TUNNEL_GENEVE;
> goto tunnel_update;
> }
> - parse_geneve(udp_hdr, &info);
> + /* Always keep last. */
> + parse_vxlan(udp_hdr, &info,
> + m->packet_type);
> if (info.is_tunnel) {
> tx_ol_flags |=
> - RTE_MBUF_F_TX_TUNNEL_GENEVE;
> + RTE_MBUF_F_TX_TUNNEL_VXLAN;
> goto tunnel_update;
> }
> } else if (info.l4_proto == IPPROTO_GRE) {
^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [PATCH] app/testpmd: fix GENEVE parsing in csum forward mode
2022-01-18 9:51 ` Ferruh Yigit
@ 2022-01-18 11:27 ` Matan Azrad
2022-01-18 12:28 ` Ferruh Yigit
0 siblings, 1 reply; 20+ messages in thread
From: Matan Azrad @ 2022-01-18 11:27 UTC (permalink / raw)
To: Ferruh Yigit, Raja Zidane, dev; +Cc: stable
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Tuesday, January 18, 2022 11:52 AM
> To: Raja Zidane <rzidane@nvidia.com>; dev@dpdk.org
> Cc: Matan Azrad <matan@nvidia.com>; stable@dpdk.org
> Subject: Re: [PATCH] app/testpmd: fix GENEVE parsing in csum forward mode
>
> External email: Use caution opening links or attachments
>
>
> On 12/5/2021 3:44 AM, Raja Zidane wrote:
> > The csum FWD mode parses any received packet to set mbuf offloads for
> > the transmitting burst, mainly in the checksum/TSO areas.
> > In the case of a tunnel header, the csum FWD tries to detect known
> > tunnels by the standard definition using the header'sdata and fallback
> > to check the packet type in the mbuf to see if the Rx port driver
> > already sign the packet as a tunnel.
> > In the fallback case, the csum assumes the tunnel is VXLAN and parses
> > the tunnel as VXLAN.
>
> As far as I can see there is a VXLAN port check in 'parse_vxlan()', why it is not
> helping?
>
The problem is not the vxlan check but the tunnel type in mbuf that caused the packet to be detected as vxlan(default) before checking GENEVE tunnel case.
> > When the GENEVE tunnel was added to the known tunnels in csum, its
> > parsing trial was wrongly located after the pkt type detection,
> > causing the csum to parse the GENEVE header as VXLAN when the Rx port
> > set the tunnel packet type.
> >
> > Locate the GENEVE parsing trial before the packet type detection.
> >
> > Fixes: ea0e711b8ae0 ("app/testpmd: add GENEVE parsing")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Raja Zidane <rzidane@nvidia.com>
> > ---
> > Acked-by: Matan Azrad <matan@nvidia.com>
>
> Ack should be before '---' to be part of the commit log, otherwise it is dropped
> when applied as comment.
>
> > app/test-pmd/csumonly.c | 16 ++++++++++------
> > 1 file changed, 10 insertions(+), 6 deletions(-)
> >
> > diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c index
> > 2aeea243b6..fe810fecdd 100644
> > --- a/app/test-pmd/csumonly.c
> > +++ b/app/test-pmd/csumonly.c
> > @@ -254,7 +254,10 @@ parse_gtp(struct rte_udp_hdr *udp_hdr,
> > info->l2_len += RTE_ETHER_GTP_HLEN;
> > }
> >
> > -/* Parse a vxlan header */
> > +/*
> > + * Parse a vxlan header.
> > + * If a tunnel is detected in 'pkt_type' it will be parsed by default as vxlan.
> > + */
> > static void
> > parse_vxlan(struct rte_udp_hdr *udp_hdr,
> > struct testpmd_offload_info *info, @@ -912,17 +915,18 @@
> > pkt_burst_checksum_forward(struct fwd_stream *fs)
> > RTE_MBUF_F_TX_TUNNEL_VXLAN_GPE;
> > goto tunnel_update;
> > }
> > - parse_vxlan(udp_hdr, &info,
> > - m->packet_type);
> > + parse_geneve(udp_hdr, &info);
> > if (info.is_tunnel) {
> > tx_ol_flags |=
> > - RTE_MBUF_F_TX_TUNNEL_VXLAN;
> > +
> > + RTE_MBUF_F_TX_TUNNEL_GENEVE;
> > goto tunnel_update;
> > }
> > - parse_geneve(udp_hdr, &info);
> > + /* Always keep last. */
> > + parse_vxlan(udp_hdr, &info,
> > + m->packet_type);
> > if (info.is_tunnel) {
> > tx_ol_flags |=
> > - RTE_MBUF_F_TX_TUNNEL_GENEVE;
> > +
> > + RTE_MBUF_F_TX_TUNNEL_VXLAN;
> > goto tunnel_update;
> > }
> > } else if (info.l4_proto == IPPROTO_GRE) {
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] app/testpmd: fix GENEVE parsing in csum forward mode
2022-01-18 11:27 ` Matan Azrad
@ 2022-01-18 12:28 ` Ferruh Yigit
2022-01-18 12:55 ` Matan Azrad
0 siblings, 1 reply; 20+ messages in thread
From: Ferruh Yigit @ 2022-01-18 12:28 UTC (permalink / raw)
To: Matan Azrad, Raja Zidane, dev; +Cc: stable
On 1/18/2022 11:27 AM, Matan Azrad wrote:
>
>
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>> Sent: Tuesday, January 18, 2022 11:52 AM
>> To: Raja Zidane <rzidane@nvidia.com>; dev@dpdk.org
>> Cc: Matan Azrad <matan@nvidia.com>; stable@dpdk.org
>> Subject: Re: [PATCH] app/testpmd: fix GENEVE parsing in csum forward mode
>>
>> External email: Use caution opening links or attachments
>>
>>
>> On 12/5/2021 3:44 AM, Raja Zidane wrote:
>>> The csum FWD mode parses any received packet to set mbuf offloads for
>>> the transmitting burst, mainly in the checksum/TSO areas.
>>> In the case of a tunnel header, the csum FWD tries to detect known
>>> tunnels by the standard definition using the header'sdata and fallback
>>> to check the packet type in the mbuf to see if the Rx port driver
>>> already sign the packet as a tunnel.
>>> In the fallback case, the csum assumes the tunnel is VXLAN and parses
>>> the tunnel as VXLAN.
>>
>> As far as I can see there is a VXLAN port check in 'parse_vxlan()', why it is not
>> helping?
>>
>
> The problem is not the vxlan check but the tunnel type in mbuf that caused the packet to be detected as vxlan(default) before checking GENEVE tunnel case.
>
Check is as following:
if (udp_hdr->dst_port != _htons(RTE_VXLAN_DEFAULT_PORT) &&
RTE_ETH_IS_TUNNEL_PKT(pkt_type) == 0)
return;
Do you what is the intention for the "RTE_ETH_IS_TUNNEL_PKT(pkt_type) == 0" check?
Why vxlan parsing doesn't stop when it is not default port?
>>> When the GENEVE tunnel was added to the known tunnels in csum, its
>>> parsing trial was wrongly located after the pkt type detection,
>>> causing the csum to parse the GENEVE header as VXLAN when the Rx port
>>> set the tunnel packet type.
>>>
>>> Locate the GENEVE parsing trial before the packet type detection.
>>>
>>> Fixes: ea0e711b8ae0 ("app/testpmd: add GENEVE parsing")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Raja Zidane <rzidane@nvidia.com>
>>> ---
>>> Acked-by: Matan Azrad <matan@nvidia.com>
>>
>> Ack should be before '---' to be part of the commit log, otherwise it is dropped
>> when applied as comment.
>>
>>> app/test-pmd/csumonly.c | 16 ++++++++++------
>>> 1 file changed, 10 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c index
>>> 2aeea243b6..fe810fecdd 100644
>>> --- a/app/test-pmd/csumonly.c
>>> +++ b/app/test-pmd/csumonly.c
>>> @@ -254,7 +254,10 @@ parse_gtp(struct rte_udp_hdr *udp_hdr,
>>> info->l2_len += RTE_ETHER_GTP_HLEN;
>>> }
>>>
>>> -/* Parse a vxlan header */
>>> +/*
>>> + * Parse a vxlan header.
>>> + * If a tunnel is detected in 'pkt_type' it will be parsed by default as vxlan.
>>> + */
>>> static void
>>> parse_vxlan(struct rte_udp_hdr *udp_hdr,
>>> struct testpmd_offload_info *info, @@ -912,17 +915,18 @@
>>> pkt_burst_checksum_forward(struct fwd_stream *fs)
>>> RTE_MBUF_F_TX_TUNNEL_VXLAN_GPE;
>>> goto tunnel_update;
>>> }
>>> - parse_vxlan(udp_hdr, &info,
>>> - m->packet_type);
>>> + parse_geneve(udp_hdr, &info);
>>> if (info.is_tunnel) {
>>> tx_ol_flags |=
>>> - RTE_MBUF_F_TX_TUNNEL_VXLAN;
>>> +
>>> + RTE_MBUF_F_TX_TUNNEL_GENEVE;
>>> goto tunnel_update;
>>> }
>>> - parse_geneve(udp_hdr, &info);
>>> + /* Always keep last. */
>>> + parse_vxlan(udp_hdr, &info,
>>> + m->packet_type);
>>> if (info.is_tunnel) {
>>> tx_ol_flags |=
>>> - RTE_MBUF_F_TX_TUNNEL_GENEVE;
>>> +
>>> + RTE_MBUF_F_TX_TUNNEL_VXLAN;
>>> goto tunnel_update;
>>> }
>>> } else if (info.l4_proto == IPPROTO_GRE) {
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [PATCH] app/testpmd: fix GENEVE parsing in csum forward mode
2022-01-18 12:28 ` Ferruh Yigit
@ 2022-01-18 12:55 ` Matan Azrad
2022-01-18 13:03 ` Ferruh Yigit
0 siblings, 1 reply; 20+ messages in thread
From: Matan Azrad @ 2022-01-18 12:55 UTC (permalink / raw)
To: Ferruh Yigit, Raja Zidane, dev; +Cc: stable
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Tuesday, January 18, 2022 2:28 PM
> To: Matan Azrad <matan@nvidia.com>; Raja Zidane <rzidane@nvidia.com>;
> dev@dpdk.org
> Cc: stable@dpdk.org
> Subject: Re: [PATCH] app/testpmd: fix GENEVE parsing in csum forward mode
>
> External email: Use caution opening links or attachments
>
>
> On 1/18/2022 11:27 AM, Matan Azrad wrote:
> >
> >
> >> -----Original Message-----
> >> From: Ferruh Yigit <ferruh.yigit@intel.com>
> >> Sent: Tuesday, January 18, 2022 11:52 AM
> >> To: Raja Zidane <rzidane@nvidia.com>; dev@dpdk.org
> >> Cc: Matan Azrad <matan@nvidia.com>; stable@dpdk.org
> >> Subject: Re: [PATCH] app/testpmd: fix GENEVE parsing in csum forward
> >> mode
> >>
> >> External email: Use caution opening links or attachments
> >>
> >>
> >> On 12/5/2021 3:44 AM, Raja Zidane wrote:
> >>> The csum FWD mode parses any received packet to set mbuf offloads
> >>> for the transmitting burst, mainly in the checksum/TSO areas.
> >>> In the case of a tunnel header, the csum FWD tries to detect known
> >>> tunnels by the standard definition using the header'sdata and
> >>> fallback to check the packet type in the mbuf to see if the Rx port
> >>> driver already sign the packet as a tunnel.
> >>> In the fallback case, the csum assumes the tunnel is VXLAN and
> >>> parses the tunnel as VXLAN.
> >>
> >> As far as I can see there is a VXLAN port check in 'parse_vxlan()',
> >> why it is not helping?
> >>
> >
> > The problem is not the vxlan check but the tunnel type in mbuf that caused the
> packet to be detected as vxlan(default) before checking GENEVE tunnel case.
> >
>
> Check is as following:
>
> if (udp_hdr->dst_port != _htons(RTE_VXLAN_DEFAULT_PORT) &&
> RTE_ETH_IS_TUNNEL_PKT(pkt_type) == 0)
> return;
>
> Do you what is the intention for the "RTE_ETH_IS_TUNNEL_PKT(pkt_type) == 0"
> check?
> Why vxlan parsing doesn't stop when it is not default port?
Maybe some drivers set the tunnel type for vxlan packets coming after non-standard vxlan port.
>
> >>> When the GENEVE tunnel was added to the known tunnels in csum, its
> >>> parsing trial was wrongly located after the pkt type detection,
> >>> causing the csum to parse the GENEVE header as VXLAN when the Rx
> >>> port set the tunnel packet type.
> >>>
> >>> Locate the GENEVE parsing trial before the packet type detection.
> >>>
> >>> Fixes: ea0e711b8ae0 ("app/testpmd: add GENEVE parsing")
> >>> Cc: stable@dpdk.org
> >>>
> >>> Signed-off-by: Raja Zidane <rzidane@nvidia.com>
> >>> ---
> >>> Acked-by: Matan Azrad <matan@nvidia.com>
> >>
> >> Ack should be before '---' to be part of the commit log, otherwise it
> >> is dropped when applied as comment.
> >>
> >>> app/test-pmd/csumonly.c | 16 ++++++++++------
> >>> 1 file changed, 10 insertions(+), 6 deletions(-)
> >>>
> >>> diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c index
> >>> 2aeea243b6..fe810fecdd 100644
> >>> --- a/app/test-pmd/csumonly.c
> >>> +++ b/app/test-pmd/csumonly.c
> >>> @@ -254,7 +254,10 @@ parse_gtp(struct rte_udp_hdr *udp_hdr,
> >>> info->l2_len += RTE_ETHER_GTP_HLEN;
> >>> }
> >>>
> >>> -/* Parse a vxlan header */
> >>> +/*
> >>> + * Parse a vxlan header.
> >>> + * If a tunnel is detected in 'pkt_type' it will be parsed by default as vxlan.
> >>> + */
> >>> static void
> >>> parse_vxlan(struct rte_udp_hdr *udp_hdr,
> >>> struct testpmd_offload_info *info, @@ -912,17 +915,18 @@
> >>> pkt_burst_checksum_forward(struct fwd_stream *fs)
> >>> RTE_MBUF_F_TX_TUNNEL_VXLAN_GPE;
> >>> goto tunnel_update;
> >>> }
> >>> - parse_vxlan(udp_hdr, &info,
> >>> - m->packet_type);
> >>> + parse_geneve(udp_hdr, &info);
> >>> if (info.is_tunnel) {
> >>> tx_ol_flags |=
> >>> - RTE_MBUF_F_TX_TUNNEL_VXLAN;
> >>> +
> >>> + RTE_MBUF_F_TX_TUNNEL_GENEVE;
> >>> goto tunnel_update;
> >>> }
> >>> - parse_geneve(udp_hdr, &info);
> >>> + /* Always keep last. */
> >>> + parse_vxlan(udp_hdr, &info,
> >>> + m->packet_type);
> >>> if (info.is_tunnel) {
> >>> tx_ol_flags |=
> >>> - RTE_MBUF_F_TX_TUNNEL_GENEVE;
> >>> +
> >>> + RTE_MBUF_F_TX_TUNNEL_VXLAN;
> >>> goto tunnel_update;
> >>> }
> >>> } else if (info.l4_proto == IPPROTO_GRE) {
> >
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] app/testpmd: fix GENEVE parsing in csum forward mode
2022-01-18 12:55 ` Matan Azrad
@ 2022-01-18 13:03 ` Ferruh Yigit
2022-01-18 13:19 ` Matan Azrad
0 siblings, 1 reply; 20+ messages in thread
From: Ferruh Yigit @ 2022-01-18 13:03 UTC (permalink / raw)
To: Matan Azrad, Raja Zidane, dev; +Cc: stable
On 1/18/2022 12:55 PM, Matan Azrad wrote:
>
>
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>> Sent: Tuesday, January 18, 2022 2:28 PM
>> To: Matan Azrad <matan@nvidia.com>; Raja Zidane <rzidane@nvidia.com>;
>> dev@dpdk.org
>> Cc: stable@dpdk.org
>> Subject: Re: [PATCH] app/testpmd: fix GENEVE parsing in csum forward mode
>>
>> External email: Use caution opening links or attachments
>>
>>
>> On 1/18/2022 11:27 AM, Matan Azrad wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>>>> Sent: Tuesday, January 18, 2022 11:52 AM
>>>> To: Raja Zidane <rzidane@nvidia.com>; dev@dpdk.org
>>>> Cc: Matan Azrad <matan@nvidia.com>; stable@dpdk.org
>>>> Subject: Re: [PATCH] app/testpmd: fix GENEVE parsing in csum forward
>>>> mode
>>>>
>>>> External email: Use caution opening links or attachments
>>>>
>>>>
>>>> On 12/5/2021 3:44 AM, Raja Zidane wrote:
>>>>> The csum FWD mode parses any received packet to set mbuf offloads
>>>>> for the transmitting burst, mainly in the checksum/TSO areas.
>>>>> In the case of a tunnel header, the csum FWD tries to detect known
>>>>> tunnels by the standard definition using the header'sdata and
>>>>> fallback to check the packet type in the mbuf to see if the Rx port
>>>>> driver already sign the packet as a tunnel.
>>>>> In the fallback case, the csum assumes the tunnel is VXLAN and
>>>>> parses the tunnel as VXLAN.
>>>>
>>>> As far as I can see there is a VXLAN port check in 'parse_vxlan()',
>>>> why it is not helping?
>>>>
>>>
>>> The problem is not the vxlan check but the tunnel type in mbuf that caused the
>> packet to be detected as vxlan(default) before checking GENEVE tunnel case.
>>>
>>
>> Check is as following:
>>
>> if (udp_hdr->dst_port != _htons(RTE_VXLAN_DEFAULT_PORT) &&
>> RTE_ETH_IS_TUNNEL_PKT(pkt_type) == 0)
>> return;
>>
>> Do you what is the intention for the "RTE_ETH_IS_TUNNEL_PKT(pkt_type) == 0"
>> check?
>> Why vxlan parsing doesn't stop when it is not default port?
>
> Maybe some drivers set the tunnel type for vxlan packets coming after non-standard vxlan port.
>
But checking the tunnel flag to say that it is vxlan is too broad, isn't it?
And this is the problem you are having.
Can there be any way to detect and check non-standard vxlan port?
>>
>>>>> When the GENEVE tunnel was added to the known tunnels in csum, its
>>>>> parsing trial was wrongly located after the pkt type detection,
>>>>> causing the csum to parse the GENEVE header as VXLAN when the Rx
>>>>> port set the tunnel packet type.
>>>>>
>>>>> Locate the GENEVE parsing trial before the packet type detection.
>>>>>
>>>>> Fixes: ea0e711b8ae0 ("app/testpmd: add GENEVE parsing")
>>>>> Cc: stable@dpdk.org
>>>>>
>>>>> Signed-off-by: Raja Zidane <rzidane@nvidia.com>
>>>>> ---
>>>>> Acked-by: Matan Azrad <matan@nvidia.com>
>>>>
>>>> Ack should be before '---' to be part of the commit log, otherwise it
>>>> is dropped when applied as comment.
>>>>
>>>>> app/test-pmd/csumonly.c | 16 ++++++++++------
>>>>> 1 file changed, 10 insertions(+), 6 deletions(-)
>>>>>
>>>>> diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c index
>>>>> 2aeea243b6..fe810fecdd 100644
>>>>> --- a/app/test-pmd/csumonly.c
>>>>> +++ b/app/test-pmd/csumonly.c
>>>>> @@ -254,7 +254,10 @@ parse_gtp(struct rte_udp_hdr *udp_hdr,
>>>>> info->l2_len += RTE_ETHER_GTP_HLEN;
>>>>> }
>>>>>
>>>>> -/* Parse a vxlan header */
>>>>> +/*
>>>>> + * Parse a vxlan header.
>>>>> + * If a tunnel is detected in 'pkt_type' it will be parsed by default as vxlan.
>>>>> + */
>>>>> static void
>>>>> parse_vxlan(struct rte_udp_hdr *udp_hdr,
>>>>> struct testpmd_offload_info *info, @@ -912,17 +915,18 @@
>>>>> pkt_burst_checksum_forward(struct fwd_stream *fs)
>>>>> RTE_MBUF_F_TX_TUNNEL_VXLAN_GPE;
>>>>> goto tunnel_update;
>>>>> }
>>>>> - parse_vxlan(udp_hdr, &info,
>>>>> - m->packet_type);
>>>>> + parse_geneve(udp_hdr, &info);
>>>>> if (info.is_tunnel) {
>>>>> tx_ol_flags |=
>>>>> - RTE_MBUF_F_TX_TUNNEL_VXLAN;
>>>>> +
>>>>> + RTE_MBUF_F_TX_TUNNEL_GENEVE;
>>>>> goto tunnel_update;
>>>>> }
>>>>> - parse_geneve(udp_hdr, &info);
>>>>> + /* Always keep last. */
>>>>> + parse_vxlan(udp_hdr, &info,
>>>>> + m->packet_type);
>>>>> if (info.is_tunnel) {
>>>>> tx_ol_flags |=
>>>>> - RTE_MBUF_F_TX_TUNNEL_GENEVE;
>>>>> +
>>>>> + RTE_MBUF_F_TX_TUNNEL_VXLAN;
>>>>> goto tunnel_update;
>>>>> }
>>>>> } else if (info.l4_proto == IPPROTO_GRE) {
>>>
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [PATCH] app/testpmd: fix GENEVE parsing in csum forward mode
2022-01-18 13:03 ` Ferruh Yigit
@ 2022-01-18 13:19 ` Matan Azrad
2022-01-20 10:46 ` Singh, Aman Deep
0 siblings, 1 reply; 20+ messages in thread
From: Matan Azrad @ 2022-01-18 13:19 UTC (permalink / raw)
To: Ferruh Yigit, Raja Zidane, dev; +Cc: stable
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Tuesday, January 18, 2022 3:03 PM
> To: Matan Azrad <matan@nvidia.com>; Raja Zidane <rzidane@nvidia.com>;
> dev@dpdk.org
> Cc: stable@dpdk.org
> Subject: Re: [PATCH] app/testpmd: fix GENEVE parsing in csum forward mode
>
> External email: Use caution opening links or attachments
>
>
> On 1/18/2022 12:55 PM, Matan Azrad wrote:
> >
> >
> >> -----Original Message-----
> >> From: Ferruh Yigit <ferruh.yigit@intel.com>
> >> Sent: Tuesday, January 18, 2022 2:28 PM
> >> To: Matan Azrad <matan@nvidia.com>; Raja Zidane <rzidane@nvidia.com>;
> >> dev@dpdk.org
> >> Cc: stable@dpdk.org
> >> Subject: Re: [PATCH] app/testpmd: fix GENEVE parsing in csum forward
> >> mode
> >>
> >> External email: Use caution opening links or attachments
> >>
> >>
> >> On 1/18/2022 11:27 AM, Matan Azrad wrote:
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Ferruh Yigit <ferruh.yigit@intel.com>
> >>>> Sent: Tuesday, January 18, 2022 11:52 AM
> >>>> To: Raja Zidane <rzidane@nvidia.com>; dev@dpdk.org
> >>>> Cc: Matan Azrad <matan@nvidia.com>; stable@dpdk.org
> >>>> Subject: Re: [PATCH] app/testpmd: fix GENEVE parsing in csum
> >>>> forward mode
> >>>>
> >>>> External email: Use caution opening links or attachments
> >>>>
> >>>>
> >>>> On 12/5/2021 3:44 AM, Raja Zidane wrote:
> >>>>> The csum FWD mode parses any received packet to set mbuf offloads
> >>>>> for the transmitting burst, mainly in the checksum/TSO areas.
> >>>>> In the case of a tunnel header, the csum FWD tries to detect known
> >>>>> tunnels by the standard definition using the header'sdata and
> >>>>> fallback to check the packet type in the mbuf to see if the Rx
> >>>>> port driver already sign the packet as a tunnel.
> >>>>> In the fallback case, the csum assumes the tunnel is VXLAN and
> >>>>> parses the tunnel as VXLAN.
> >>>>
> >>>> As far as I can see there is a VXLAN port check in 'parse_vxlan()',
> >>>> why it is not helping?
> >>>>
> >>>
> >>> The problem is not the vxlan check but the tunnel type in mbuf that
> >>> caused the
> >> packet to be detected as vxlan(default) before checking GENEVE tunnel case.
> >>>
> >>
> >> Check is as following:
> >>
> >> if (udp_hdr->dst_port != _htons(RTE_VXLAN_DEFAULT_PORT) &&
> >> RTE_ETH_IS_TUNNEL_PKT(pkt_type) == 0)
> >> return;
> >>
> >> Do you what is the intention for the "RTE_ETH_IS_TUNNEL_PKT(pkt_type) ==
> 0"
> >> check?
> >> Why vxlan parsing doesn't stop when it is not default port?
> >
> > Maybe some drivers set the tunnel type for vxlan packets coming after non-
> standard vxlan port.
> >
>
> But checking the tunnel flag to say that it is vxlan is too broad, isn't it?
> And this is the problem you are having.
>
> Can there be any way to detect and check non-standard vxlan port?
Maybe yes, but it is probably more complex solusion.
See this patch:
https://patches.dpdk.org/project/dpdk/patch/1423819371-24222-13-git-send-email-olivier.matz@6wind.com/
comments there:
1. check udp destination port, 4789 is the default vxlan port (rfc7348).
2. currently, this flag is set by i40e only if the packet is vxlan
And maybe another driver assumes more ports here.
Collecting all the non-standard ports can start from i40 maintainers😊
> >>
> >>>>> When the GENEVE tunnel was added to the known tunnels in csum, its
> >>>>> parsing trial was wrongly located after the pkt type detection,
> >>>>> causing the csum to parse the GENEVE header as VXLAN when the Rx
> >>>>> port set the tunnel packet type.
> >>>>>
> >>>>> Locate the GENEVE parsing trial before the packet type detection.
> >>>>>
> >>>>> Fixes: ea0e711b8ae0 ("app/testpmd: add GENEVE parsing")
> >>>>> Cc: stable@dpdk.org
> >>>>>
> >>>>> Signed-off-by: Raja Zidane <rzidane@nvidia.com>
> >>>>> ---
> >>>>> Acked-by: Matan Azrad <matan@nvidia.com>
> >>>>
> >>>> Ack should be before '---' to be part of the commit log, otherwise
> >>>> it is dropped when applied as comment.
> >>>>
> >>>>> app/test-pmd/csumonly.c | 16 ++++++++++------
> >>>>> 1 file changed, 10 insertions(+), 6 deletions(-)
> >>>>>
> >>>>> diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
> >>>>> index 2aeea243b6..fe810fecdd 100644
> >>>>> --- a/app/test-pmd/csumonly.c
> >>>>> +++ b/app/test-pmd/csumonly.c
> >>>>> @@ -254,7 +254,10 @@ parse_gtp(struct rte_udp_hdr *udp_hdr,
> >>>>> info->l2_len += RTE_ETHER_GTP_HLEN;
> >>>>> }
> >>>>>
> >>>>> -/* Parse a vxlan header */
> >>>>> +/*
> >>>>> + * Parse a vxlan header.
> >>>>> + * If a tunnel is detected in 'pkt_type' it will be parsed by default as vxlan.
> >>>>> + */
> >>>>> static void
> >>>>> parse_vxlan(struct rte_udp_hdr *udp_hdr,
> >>>>> struct testpmd_offload_info *info, @@ -912,17 +915,18
> >>>>> @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
> >>>>> RTE_MBUF_F_TX_TUNNEL_VXLAN_GPE;
> >>>>> goto tunnel_update;
> >>>>> }
> >>>>> - parse_vxlan(udp_hdr, &info,
> >>>>> - m->packet_type);
> >>>>> + parse_geneve(udp_hdr, &info);
> >>>>> if (info.is_tunnel) {
> >>>>> tx_ol_flags |=
> >>>>> - RTE_MBUF_F_TX_TUNNEL_VXLAN;
> >>>>> +
> >>>>> + RTE_MBUF_F_TX_TUNNEL_GENEVE;
> >>>>> goto tunnel_update;
> >>>>> }
> >>>>> - parse_geneve(udp_hdr, &info);
> >>>>> + /* Always keep last. */
> >>>>> + parse_vxlan(udp_hdr, &info,
> >>>>> + m->packet_type);
> >>>>> if (info.is_tunnel) {
> >>>>> tx_ol_flags |=
> >>>>> - RTE_MBUF_F_TX_TUNNEL_GENEVE;
> >>>>> +
> >>>>> + RTE_MBUF_F_TX_TUNNEL_VXLAN;
> >>>>> goto tunnel_update;
> >>>>> }
> >>>>> } else if (info.l4_proto == IPPROTO_GRE) {
> >>>
> >
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] app/testpmd: fix GENEVE parsing in csum forward mode
2022-01-18 13:19 ` Matan Azrad
@ 2022-01-20 10:46 ` Singh, Aman Deep
2022-01-30 11:18 ` Raja Zidane
0 siblings, 1 reply; 20+ messages in thread
From: Singh, Aman Deep @ 2022-01-20 10:46 UTC (permalink / raw)
To: Matan Azrad, Ferruh Yigit, Raja Zidane, dev; +Cc: stable
On 1/18/2022 6:49 PM, Matan Azrad wrote:
>
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>> Sent: Tuesday, January 18, 2022 3:03 PM
>> To: Matan Azrad <matan@nvidia.com>; Raja Zidane <rzidane@nvidia.com>;
>> dev@dpdk.org
>> Cc: stable@dpdk.org
>> Subject: Re: [PATCH] app/testpmd: fix GENEVE parsing in csum forward mode
>>
>> External email: Use caution opening links or attachments
>>
>>
>> On 1/18/2022 12:55 PM, Matan Azrad wrote:
>>>
>>>> -----Original Message-----
>>>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>>>> Sent: Tuesday, January 18, 2022 2:28 PM
>>>> To: Matan Azrad <matan@nvidia.com>; Raja Zidane <rzidane@nvidia.com>;
>>>> dev@dpdk.org
>>>> Cc: stable@dpdk.org
>>>> Subject: Re: [PATCH] app/testpmd: fix GENEVE parsing in csum forward
>>>> mode
>>>>
>>>> External email: Use caution opening links or attachments
>>>>
>>>>
>>>> On 1/18/2022 11:27 AM, Matan Azrad wrote:
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>>>>>> Sent: Tuesday, January 18, 2022 11:52 AM
>>>>>> To: Raja Zidane <rzidane@nvidia.com>; dev@dpdk.org
>>>>>> Cc: Matan Azrad <matan@nvidia.com>; stable@dpdk.org
>>>>>> Subject: Re: [PATCH] app/testpmd: fix GENEVE parsing in csum
>>>>>> forward mode
>>>>>>
>>>>>> External email: Use caution opening links or attachments
>>>>>>
>>>>>>
>>>>>> On 12/5/2021 3:44 AM, Raja Zidane wrote:
>>>>>>> The csum FWD mode parses any received packet to set mbuf offloads
>>>>>>> for the transmitting burst, mainly in the checksum/TSO areas.
>>>>>>> In the case of a tunnel header, the csum FWD tries to detect known
>>>>>>> tunnels by the standard definition using the header'sdata and
>>>>>>> fallback to check the packet type in the mbuf to see if the Rx
>>>>>>> port driver already sign the packet as a tunnel.
>>>>>>> In the fallback case, the csum assumes the tunnel is VXLAN and
>>>>>>> parses the tunnel as VXLAN.
>>>>>> As far as I can see there is a VXLAN port check in 'parse_vxlan()',
>>>>>> why it is not helping?
>>>>>>
>>>>> The problem is not the vxlan check but the tunnel type in mbuf that
>>>>> caused the
>>>> packet to be detected as vxlan(default) before checking GENEVE tunnel case.
>>>> Check is as following:
>>>>
>>>> if (udp_hdr->dst_port != _htons(RTE_VXLAN_DEFAULT_PORT) &&
>>>> RTE_ETH_IS_TUNNEL_PKT(pkt_type) == 0)
>>>> return;
>>>>
>>>> Do you what is the intention for the "RTE_ETH_IS_TUNNEL_PKT(pkt_type) ==
>> 0"
>>>> check?
>>>> Why vxlan parsing doesn't stop when it is not default port?
>>> Maybe some drivers set the tunnel type for vxlan packets coming after non-
>> standard vxlan port.
>> But checking the tunnel flag to say that it is vxlan is too broad, isn't it?
>> And this is the problem you are having.
>>
>> Can there be any way to detect and check non-standard vxlan port?
> Maybe yes, but it is probably more complex solusion.
>
> See this patch:
> https://patches.dpdk.org/project/dpdk/patch/1423819371-24222-13-git-send-email-olivier.matz@6wind.com/
>
> comments there:
> 1. check udp destination port, 4789 is the default vxlan port (rfc7348).
> 2. currently, this flag is set by i40e only if the packet is vxlan
>
> And maybe another driver assumes more ports here.
>
> Collecting all the non-standard ports can start from i40 maintainers😊
I think, here we should check for supported tunnel types only. The i40
driver setting a Tunnel flag, does not means that it VxLan type only.
As in your case those were GENEVE packets getting treated as VxLan.
Making parse_vxlan() udp_dst_port specific can avoid this issue.
>>>>>>> When the GENEVE tunnel was added to the known tunnels in csum, its
>>>>>>> parsing trial was wrongly located after the pkt type detection,
>>>>>>> causing the csum to parse the GENEVE header as VXLAN when the Rx
>>>>>>> port set the tunnel packet type.
>>>>>>>
>>>>>>> Locate the GENEVE parsing trial before the packet type detection.
>>>>>>>
>>>>>>> Fixes: ea0e711b8ae0 ("app/testpmd: add GENEVE parsing")
>>>>>>> Cc: stable@dpdk.org
>>>>>>>
>>>>>>> Signed-off-by: Raja Zidane <rzidane@nvidia.com>
>>>>>>> ---
>>>>>>> Acked-by: Matan Azrad <matan@nvidia.com>
>>>>>> Ack should be before '---' to be part of the commit log, otherwise
>>>>>> it is dropped when applied as comment.
>>>>>>
>>>>>>> app/test-pmd/csumonly.c | 16 ++++++++++------
>>>>>>> 1 file changed, 10 insertions(+), 6 deletions(-)
>>>>>>>
>>>>>>> diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
>>>>>>> index 2aeea243b6..fe810fecdd 100644
>>>>>>> --- a/app/test-pmd/csumonly.c
>>>>>>> +++ b/app/test-pmd/csumonly.c
>>>>>>> @@ -254,7 +254,10 @@ parse_gtp(struct rte_udp_hdr *udp_hdr,
>>>>>>> info->l2_len += RTE_ETHER_GTP_HLEN;
>>>>>>> }
>>>>>>>
>>>>>>> -/* Parse a vxlan header */
>>>>>>> +/*
>>>>>>> + * Parse a vxlan header.
>>>>>>> + * If a tunnel is detected in 'pkt_type' it will be parsed by default as vxlan.
>>>>>>> + */
>>>>>>> static void
>>>>>>> parse_vxlan(struct rte_udp_hdr *udp_hdr,
>>>>>>> struct testpmd_offload_info *info, @@ -912,17 +915,18
>>>>>>> @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
>>>>>>> RTE_MBUF_F_TX_TUNNEL_VXLAN_GPE;
>>>>>>> goto tunnel_update;
>>>>>>> }
>>>>>>> - parse_vxlan(udp_hdr, &info,
>>>>>>> - m->packet_type);
>>>>>>> + parse_geneve(udp_hdr, &info);
>>>>>>> if (info.is_tunnel) {
>>>>>>> tx_ol_flags |=
>>>>>>> - RTE_MBUF_F_TX_TUNNEL_VXLAN;
>>>>>>> +
>>>>>>> + RTE_MBUF_F_TX_TUNNEL_GENEVE;
>>>>>>> goto tunnel_update;
>>>>>>> }
>>>>>>> - parse_geneve(udp_hdr, &info);
>>>>>>> + /* Always keep last. */
>>>>>>> + parse_vxlan(udp_hdr, &info,
>>>>>>> + m->packet_type);
>>>>>>> if (info.is_tunnel) {
>>>>>>> tx_ol_flags |=
>>>>>>> - RTE_MBUF_F_TX_TUNNEL_GENEVE;
>>>>>>> +
>>>>>>> + RTE_MBUF_F_TX_TUNNEL_VXLAN;
>>>>>>> goto tunnel_update;
>>>>>>> }
>>>>>>> } else if (info.l4_proto == IPPROTO_GRE) {
^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [PATCH] app/testpmd: fix GENEVE parsing in csum forward mode
2022-01-20 10:46 ` Singh, Aman Deep
@ 2022-01-30 11:18 ` Raja Zidane
2022-01-31 16:47 ` Singh, Aman Deep
0 siblings, 1 reply; 20+ messages in thread
From: Raja Zidane @ 2022-01-30 11:18 UTC (permalink / raw)
To: Singh, Aman Deep, Matan Azrad, Ferruh Yigit, dev; +Cc: stable
I didn't want to remove the default parsing of tunnel as VxLan because I thought it might be used,
Instead I moved it to the end, which makes it detect all supported tunnel through udp_dst_port,
And only if no tunnel was matched it would default to VxLan.
That was the reason geneve weren't detected and parsed as vxlan instead, which is the bug I was trying to solve.
-----Original Message-----
From: Singh, Aman Deep <aman.deep.singh@intel.com>
Sent: Thursday, January 20, 2022 12:47 PM
To: Matan Azrad <matan@nvidia.com>; Ferruh Yigit <ferruh.yigit@intel.com>; Raja Zidane <rzidane@nvidia.com>; dev@dpdk.org
Cc: stable@dpdk.org
Subject: Re: [PATCH] app/testpmd: fix GENEVE parsing in csum forward mode
External email: Use caution opening links or attachments
On 1/18/2022 6:49 PM, Matan Azrad wrote:
>
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>> Sent: Tuesday, January 18, 2022 3:03 PM
>> To: Matan Azrad <matan@nvidia.com>; Raja Zidane <rzidane@nvidia.com>;
>> dev@dpdk.org
>> Cc: stable@dpdk.org
>> Subject: Re: [PATCH] app/testpmd: fix GENEVE parsing in csum forward
>> mode
>>
>> External email: Use caution opening links or attachments
>>
>>
>> On 1/18/2022 12:55 PM, Matan Azrad wrote:
>>>
>>>> -----Original Message-----
>>>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>>>> Sent: Tuesday, January 18, 2022 2:28 PM
>>>> To: Matan Azrad <matan@nvidia.com>; Raja Zidane
>>>> <rzidane@nvidia.com>; dev@dpdk.org
>>>> Cc: stable@dpdk.org
>>>> Subject: Re: [PATCH] app/testpmd: fix GENEVE parsing in csum
>>>> forward mode
>>>>
>>>> External email: Use caution opening links or attachments
>>>>
>>>>
>>>> On 1/18/2022 11:27 AM, Matan Azrad wrote:
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>>>>>> Sent: Tuesday, January 18, 2022 11:52 AM
>>>>>> To: Raja Zidane <rzidane@nvidia.com>; dev@dpdk.org
>>>>>> Cc: Matan Azrad <matan@nvidia.com>; stable@dpdk.org
>>>>>> Subject: Re: [PATCH] app/testpmd: fix GENEVE parsing in csum
>>>>>> forward mode
>>>>>>
>>>>>> External email: Use caution opening links or attachments
>>>>>>
>>>>>>
>>>>>> On 12/5/2021 3:44 AM, Raja Zidane wrote:
>>>>>>> The csum FWD mode parses any received packet to set mbuf
>>>>>>> offloads for the transmitting burst, mainly in the checksum/TSO areas.
>>>>>>> In the case of a tunnel header, the csum FWD tries to detect
>>>>>>> known tunnels by the standard definition using the header'sdata
>>>>>>> and fallback to check the packet type in the mbuf to see if the
>>>>>>> Rx port driver already sign the packet as a tunnel.
>>>>>>> In the fallback case, the csum assumes the tunnel is VXLAN and
>>>>>>> parses the tunnel as VXLAN.
>>>>>> As far as I can see there is a VXLAN port check in
>>>>>> 'parse_vxlan()', why it is not helping?
>>>>>>
>>>>> The problem is not the vxlan check but the tunnel type in mbuf
>>>>> that caused the
>>>> packet to be detected as vxlan(default) before checking GENEVE tunnel case.
>>>> Check is as following:
>>>>
>>>> if (udp_hdr->dst_port != _htons(RTE_VXLAN_DEFAULT_PORT) &&
>>>> RTE_ETH_IS_TUNNEL_PKT(pkt_type) == 0)
>>>> return;
>>>>
>>>> Do you what is the intention for the
>>>> "RTE_ETH_IS_TUNNEL_PKT(pkt_type) ==
>> 0"
>>>> check?
>>>> Why vxlan parsing doesn't stop when it is not default port?
>>> Maybe some drivers set the tunnel type for vxlan packets coming
>>> after non-
>> standard vxlan port.
>> But checking the tunnel flag to say that it is vxlan is too broad, isn't it?
>> And this is the problem you are having.
>>
>> Can there be any way to detect and check non-standard vxlan port?
> Maybe yes, but it is probably more complex solusion.
>
> See this patch:
> https://patches.dpdk.org/project/dpdk/patch/1423819371-24222-13-git-se
> nd-email-olivier.matz@6wind.com/
>
> comments there:
> 1. check udp destination port, 4789 is the default vxlan port (rfc7348).
> 2. currently, this flag is set by i40e only if the packet is vxlan
>
> And maybe another driver assumes more ports here.
>
> Collecting all the non-standard ports can start from i40 maintainers😊
I think, here we should check for supported tunnel types only. The i40 driver setting a Tunnel flag, does not means that it VxLan type only.
As in your case those were GENEVE packets getting treated as VxLan.
Making parse_vxlan() udp_dst_port specific can avoid this issue.
>>>>>>> When the GENEVE tunnel was added to the known tunnels in csum,
>>>>>>> its parsing trial was wrongly located after the pkt type
>>>>>>> detection, causing the csum to parse the GENEVE header as VXLAN
>>>>>>> when the Rx port set the tunnel packet type.
>>>>>>>
>>>>>>> Locate the GENEVE parsing trial before the packet type detection.
>>>>>>>
>>>>>>> Fixes: ea0e711b8ae0 ("app/testpmd: add GENEVE parsing")
>>>>>>> Cc: stable@dpdk.org
>>>>>>>
>>>>>>> Signed-off-by: Raja Zidane <rzidane@nvidia.com>
>>>>>>> ---
>>>>>>> Acked-by: Matan Azrad <matan@nvidia.com>
>>>>>> Ack should be before '---' to be part of the commit log,
>>>>>> otherwise it is dropped when applied as comment.
>>>>>>
>>>>>>> app/test-pmd/csumonly.c | 16 ++++++++++------
>>>>>>> 1 file changed, 10 insertions(+), 6 deletions(-)
>>>>>>>
>>>>>>> diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
>>>>>>> index 2aeea243b6..fe810fecdd 100644
>>>>>>> --- a/app/test-pmd/csumonly.c
>>>>>>> +++ b/app/test-pmd/csumonly.c
>>>>>>> @@ -254,7 +254,10 @@ parse_gtp(struct rte_udp_hdr *udp_hdr,
>>>>>>> info->l2_len += RTE_ETHER_GTP_HLEN;
>>>>>>> }
>>>>>>>
>>>>>>> -/* Parse a vxlan header */
>>>>>>> +/*
>>>>>>> + * Parse a vxlan header.
>>>>>>> + * If a tunnel is detected in 'pkt_type' it will be parsed by default as vxlan.
>>>>>>> + */
>>>>>>> static void
>>>>>>> parse_vxlan(struct rte_udp_hdr *udp_hdr,
>>>>>>> struct testpmd_offload_info *info, @@ -912,17
>>>>>>> +915,18 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
>>>>>>> RTE_MBUF_F_TX_TUNNEL_VXLAN_GPE;
>>>>>>> goto tunnel_update;
>>>>>>> }
>>>>>>> - parse_vxlan(udp_hdr, &info,
>>>>>>> - m->packet_type);
>>>>>>> + parse_geneve(udp_hdr, &info);
>>>>>>> if (info.is_tunnel) {
>>>>>>> tx_ol_flags |=
>>>>>>> - RTE_MBUF_F_TX_TUNNEL_VXLAN;
>>>>>>> +
>>>>>>> + RTE_MBUF_F_TX_TUNNEL_GENEVE;
>>>>>>> goto tunnel_update;
>>>>>>> }
>>>>>>> - parse_geneve(udp_hdr, &info);
>>>>>>> + /* Always keep last. */
>>>>>>> + parse_vxlan(udp_hdr, &info,
>>>>>>> + m->packet_type);
>>>>>>> if (info.is_tunnel) {
>>>>>>> tx_ol_flags |=
>>>>>>> - RTE_MBUF_F_TX_TUNNEL_GENEVE;
>>>>>>> +
>>>>>>> + RTE_MBUF_F_TX_TUNNEL_VXLAN;
>>>>>>> goto tunnel_update;
>>>>>>> }
>>>>>>> } else if (info.l4_proto ==
>>>>>>> IPPROTO_GRE) {
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] app/testpmd: fix GENEVE parsing in csum forward mode
2022-01-30 11:18 ` Raja Zidane
@ 2022-01-31 16:47 ` Singh, Aman Deep
2022-02-15 14:31 ` Raja Zidane
0 siblings, 1 reply; 20+ messages in thread
From: Singh, Aman Deep @ 2022-01-31 16:47 UTC (permalink / raw)
To: Raja Zidane, Matan Azrad, Ferruh Yigit, dev, Beilei Xing, Qi Zhang; +Cc: stable
[-- Attachment #1: Type: text/plain, Size: 7540 bytes --]
On 1/30/2022 4:48 PM, Raja Zidane wrote:
> I didn't want to remove the default parsing of tunnel as VxLan because I thought it might be used,
> Instead I moved it to the end, which makes it detect all supported tunnel through udp_dst_port,
> And only if no tunnel was matched it would default to VxLan.
> That was the reason geneve weren't detected and parsed as vxlan instead, which is the bug I was trying to solve.
We can take help/input from i40 maintainers for it.
Hi Beilei Xing,
For setting packet_type as Tunnel, what criteria is used by i40 driver. Is it only udp_dst port or any other parameters also.
>
> -----Original Message-----
> From: Singh, Aman Deep<aman.deep.singh@intel.com>
> Sent: Thursday, January 20, 2022 12:47 PM
> To: Matan Azrad<matan@nvidia.com>; Ferruh Yigit<ferruh.yigit@intel.com>; Raja Zidane<rzidane@nvidia.com>;dev@dpdk.org
> Cc:stable@dpdk.org
> Subject: Re: [PATCH] app/testpmd: fix GENEVE parsing in csum forward mode
>
> External email: Use caution opening links or attachments
>
>
> On 1/18/2022 6:49 PM, Matan Azrad wrote:
> app/test-pmd/csumonly.c | 16 ++++++++++------
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
> index 2aeea243b6..fe810fecdd 100644
> --- a/app/test-pmd/csumonly.c
> +++ b/app/test-pmd/csumonly.c
> @@ -254,7 +254,10 @@ parse_gtp(struct rte_udp_hdr *udp_hdr,
> info->l2_len += RTE_ETHER_GTP_HLEN;
> }
>
> -/* Parse a vxlan header */
> +/*
> + * Parse a vxlan header.
> + * If a tunnel is detected in 'pkt_type' it will be parsed by default as vxlan.
> + */
> static void
> parse_vxlan(struct rte_udp_hdr *udp_hdr,
> struct testpmd_offload_info *info, @@ -912,17
> +915,18 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
> RTE_MBUF_F_TX_TUNNEL_VXLAN_GPE;
> goto tunnel_update;
> }
> - parse_vxlan(udp_hdr, &info,
> - m->packet_type);
> + parse_geneve(udp_hdr, &info);
> if (info.is_tunnel) {
> tx_ol_flags |=
> - RTE_MBUF_F_TX_TUNNEL_VXLAN;
> +
> + RTE_MBUF_F_TX_TUNNEL_GENEVE;
> goto tunnel_update;
> }
> - parse_geneve(udp_hdr, &info);
> + /* Always keep last. */
> + parse_vxlan(udp_hdr, &info,
> + m->packet_type);
> if (info.is_tunnel) {
> tx_ol_flags |=
> - RTE_MBUF_F_TX_TUNNEL_GENEVE;
> +
> + RTE_MBUF_F_TX_TUNNEL_VXLAN;
> goto tunnel_update;
> }
> } else if (info.l4_proto ==
> IPPROTO_GRE) {
>>> -----Original Message-----
>>> From: Ferruh Yigit<ferruh.yigit@intel.com>
>>> Sent: Tuesday, January 18, 2022 3:03 PM
>>> To: Matan Azrad<matan@nvidia.com>; Raja Zidane<rzidane@nvidia.com>;
>>> dev@dpdk.org
>>> Cc:stable@dpdk.org
>>> Subject: Re: [PATCH] app/testpmd: fix GENEVE parsing in csum forward
>>> mode
>>>
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> On 1/18/2022 12:55 PM, Matan Azrad wrote:
>>>>> -----Original Message-----
>>>>> From: Ferruh Yigit<ferruh.yigit@intel.com>
>>>>> Sent: Tuesday, January 18, 2022 2:28 PM
>>>>> To: Matan Azrad<matan@nvidia.com>; Raja Zidane
>>>>> <rzidane@nvidia.com>;dev@dpdk.org
>>>>> Cc:stable@dpdk.org
>>>>> Subject: Re: [PATCH] app/testpmd: fix GENEVE parsing in csum
>>>>> forward mode
>>>>>
>>>>> External email: Use caution opening links or attachments
>>>>>
>>>>>
>>>>> On 1/18/2022 11:27 AM, Matan Azrad wrote:
>>>>>>> -----Original Message-----
>>>>>>> From: Ferruh Yigit<ferruh.yigit@intel.com>
>>>>>>> Sent: Tuesday, January 18, 2022 11:52 AM
>>>>>>> To: Raja Zidane<rzidane@nvidia.com>;dev@dpdk.org
>>>>>>> Cc: Matan Azrad<matan@nvidia.com>;stable@dpdk.org
>>>>>>> Subject: Re: [PATCH] app/testpmd: fix GENEVE parsing in csum
>>>>>>> forward mode
>>>>>>>
>>>>>>> External email: Use caution opening links or attachments
>>>>>>>
>>>>>>>
>>>>>>> On 12/5/2021 3:44 AM, Raja Zidane wrote:
>>>>>>>> The csum FWD mode parses any received packet to set mbuf
>>>>>>>> offloads for the transmitting burst, mainly in the checksum/TSO areas.
>>>>>>>> In the case of a tunnel header, the csum FWD tries to detect
>>>>>>>> known tunnels by the standard definition using the header'sdata
>>>>>>>> and fallback to check the packet type in the mbuf to see if the
>>>>>>>> Rx port driver already sign the packet as a tunnel.
>>>>>>>> In the fallback case, the csum assumes the tunnel is VXLAN and
>>>>>>>> parses the tunnel as VXLAN.
>>>>>>> As far as I can see there is a VXLAN port check in
>>>>>>> 'parse_vxlan()', why it is not helping?
>>>>>>>
>>>>>> The problem is not the vxlan check but the tunnel type in mbuf
>>>>>> that caused the
>>>>> packet to be detected as vxlan(default) before checking GENEVE tunnel case.
>>>>> Check is as following:
>>>>>
>>>>> if (udp_hdr->dst_port != _htons(RTE_VXLAN_DEFAULT_PORT) &&
>>>>> RTE_ETH_IS_TUNNEL_PKT(pkt_type) == 0)
>>>>> return;
>>>>>
>>>>> Do you what is the intention for the
>>>>> "RTE_ETH_IS_TUNNEL_PKT(pkt_type) ==
>>> 0"
>>>>> check?
>>>>> Why vxlan parsing doesn't stop when it is not default port?
>>>> Maybe some drivers set the tunnel type for vxlan packets coming
>>>> after non-
>>> standard vxlan port.
>>> But checking the tunnel flag to say that it is vxlan is too broad, isn't it?
>>> And this is the problem you are having.
>>>
>>> Can there be any way to detect and check non-standard vxlan port?
>> Maybe yes, but it is probably more complex solusion.
>>
>> See this patch:
>> https://patches.dpdk.org/project/dpdk/patch/1423819371-24222-13-git-se
>> nd-email-olivier.matz@6wind.com/
>>
>> comments there:
>> 1. check udp destination port, 4789 is the default vxlan port (rfc7348).
>> 2. currently, this flag is set by i40e only if the packet is vxlan
>>
>> And maybe another driver assumes more ports here.
>>
>> Collecting all the non-standard ports can start from i40 maintainers😊
> I think, here we should check for supported tunnel types only. The i40 driver setting a Tunnel flag, does not means that it VxLan type only.
> As in your case those were GENEVE packets getting treated as VxLan.
>
> Making parse_vxlan() udp_dst_port specific can avoid this issue.
>
>>>>>>>> When the GENEVE tunnel was added to the known tunnels in csum,
>>>>>>>> its parsing trial was wrongly located after the pkt type
>>>>>>>> detection, causing the csum to parse the GENEVE header as VXLAN
>>>>>>>> when the Rx port set the tunnel packet type.
>>>>>>>>
>>>>>>>> Locate the GENEVE parsing trial before the packet type detection.
>>>>>>>>
>>>>>>>> Fixes: ea0e711b8ae0 ("app/testpmd: add GENEVE parsing")
>>>>>>>> Cc:stable@dpdk.org
>>>>>>>>
>>>>>>>> Signed-off-by: Raja Zidane<rzidane@nvidia.com>
>>>>>>>> ---
>>>>>>>> Acked-by: Matan Azrad<matan@nvidia.com>
>>>>>>> Ack should be before '---' to be part of the commit log,
>>>>>>> otherwise it is dropped when applied as comment.
>>>>>>>
[-- Attachment #2: Type: text/html, Size: 11602 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [PATCH] app/testpmd: fix GENEVE parsing in csum forward mode
2022-01-31 16:47 ` Singh, Aman Deep
@ 2022-02-15 14:31 ` Raja Zidane
2022-02-15 14:43 ` Singh, Aman Deep
2022-02-16 2:02 ` Xing, Beilei
0 siblings, 2 replies; 20+ messages in thread
From: Raja Zidane @ 2022-02-15 14:31 UTC (permalink / raw)
To: Singh, Aman Deep, Matan Azrad, Ferruh Yigit, dev, Beilei Xing, Qi Zhang
Cc: stable
[-- Attachment #1: Type: text/plain, Size: 8708 bytes --]
Hi all,
reviving the discussion.
@Beilei Xing<mailto:beilei.xing@intel.com> could you please provide info on what UDP destination ports are used for VxLan by i40 driver?
if its just the default then we can remove "RTE_ETH_IS_TUNNEL_PKT(pkt_type) == 0"
From: Singh, Aman Deep <aman.deep.singh@intel.com>
Sent: Monday, January 31, 2022 6:48 PM
To: Raja Zidane <rzidane@nvidia.com>; Matan Azrad <matan@nvidia.com>; Ferruh Yigit <ferruh.yigit@intel.com>; dev@dpdk.org; Beilei Xing <beilei.xing@intel.com>; Qi Zhang <qi.z.zhang@intel.com>
Cc: stable@dpdk.org
Subject: Re: [PATCH] app/testpmd: fix GENEVE parsing in csum forward mode
External email: Use caution opening links or attachments
On 1/30/2022 4:48 PM, Raja Zidane wrote:
I didn't want to remove the default parsing of tunnel as VxLan because I thought it might be used,
Instead I moved it to the end, which makes it detect all supported tunnel through udp_dst_port,
And only if no tunnel was matched it would default to VxLan.
That was the reason geneve weren't detected and parsed as vxlan instead, which is the bug I was trying to solve.
We can take help/input from i40 maintainers for it.
Hi Beilei Xing,
For setting packet_type as Tunnel, what criteria is used by i40 driver. Is it only udp_dst port or any other parameters also.
-----Original Message-----
From: Singh, Aman Deep <aman.deep.singh@intel.com><mailto:aman.deep.singh@intel.com>
Sent: Thursday, January 20, 2022 12:47 PM
To: Matan Azrad <matan@nvidia.com><mailto:matan@nvidia.com>; Ferruh Yigit <ferruh.yigit@intel.com><mailto:ferruh.yigit@intel.com>; Raja Zidane <rzidane@nvidia.com><mailto:rzidane@nvidia.com>; dev@dpdk.org<mailto:dev@dpdk.org>
Cc: stable@dpdk.org<mailto:stable@dpdk.org>
Subject: Re: [PATCH] app/testpmd: fix GENEVE parsing in csum forward mode
External email: Use caution opening links or attachments
On 1/18/2022 6:49 PM, Matan Azrad wrote:
app/test-pmd/csumonly.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
index 2aeea243b6..fe810fecdd 100644
--- a/app/test-pmd/csumonly.c
+++ b/app/test-pmd/csumonly.c
@@ -254,7 +254,10 @@ parse_gtp(struct rte_udp_hdr *udp_hdr,
info->l2_len += RTE_ETHER_GTP_HLEN;
}
-/* Parse a vxlan header */
+/*
+ * Parse a vxlan header.
+ * If a tunnel is detected in 'pkt_type' it will be parsed by default as vxlan.
+ */
static void
parse_vxlan(struct rte_udp_hdr *udp_hdr,
struct testpmd_offload_info *info, @@ -912,17
+915,18 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
RTE_MBUF_F_TX_TUNNEL_VXLAN_GPE;
goto tunnel_update;
}
- parse_vxlan(udp_hdr, &info,
- m->packet_type);
+ parse_geneve(udp_hdr, &info);
if (info.is_tunnel) {
tx_ol_flags |=
- RTE_MBUF_F_TX_TUNNEL_VXLAN;
+
+ RTE_MBUF_F_TX_TUNNEL_GENEVE;
goto tunnel_update;
}
- parse_geneve(udp_hdr, &info);
+ /* Always keep last. */
+ parse_vxlan(udp_hdr, &info,
+ m->packet_type);
if (info.is_tunnel) {
tx_ol_flags |=
- RTE_MBUF_F_TX_TUNNEL_GENEVE;
+
+ RTE_MBUF_F_TX_TUNNEL_VXLAN;
goto tunnel_update;
}
} else if (info.l4_proto ==
IPPROTO_GRE) {
-----Original Message-----
From: Ferruh Yigit <ferruh.yigit@intel.com><mailto:ferruh.yigit@intel.com>
Sent: Tuesday, January 18, 2022 3:03 PM
To: Matan Azrad <matan@nvidia.com><mailto:matan@nvidia.com>; Raja Zidane <rzidane@nvidia.com><mailto:rzidane@nvidia.com>;
dev@dpdk.org<mailto:dev@dpdk.org>
Cc: stable@dpdk.org<mailto:stable@dpdk.org>
Subject: Re: [PATCH] app/testpmd: fix GENEVE parsing in csum forward
mode
External email: Use caution opening links or attachments
On 1/18/2022 12:55 PM, Matan Azrad wrote:
-----Original Message-----
From: Ferruh Yigit <ferruh.yigit@intel.com><mailto:ferruh.yigit@intel.com>
Sent: Tuesday, January 18, 2022 2:28 PM
To: Matan Azrad <matan@nvidia.com><mailto:matan@nvidia.com>; Raja Zidane
<rzidane@nvidia.com><mailto:rzidane@nvidia.com>; dev@dpdk.org<mailto:dev@dpdk.org>
Cc: stable@dpdk.org<mailto:stable@dpdk.org>
Subject: Re: [PATCH] app/testpmd: fix GENEVE parsing in csum
forward mode
External email: Use caution opening links or attachments
On 1/18/2022 11:27 AM, Matan Azrad wrote:
-----Original Message-----
From: Ferruh Yigit <ferruh.yigit@intel.com><mailto:ferruh.yigit@intel.com>
Sent: Tuesday, January 18, 2022 11:52 AM
To: Raja Zidane <rzidane@nvidia.com><mailto:rzidane@nvidia.com>; dev@dpdk.org<mailto:dev@dpdk.org>
Cc: Matan Azrad <matan@nvidia.com><mailto:matan@nvidia.com>; stable@dpdk.org<mailto:stable@dpdk.org>
Subject: Re: [PATCH] app/testpmd: fix GENEVE parsing in csum
forward mode
External email: Use caution opening links or attachments
On 12/5/2021 3:44 AM, Raja Zidane wrote:
The csum FWD mode parses any received packet to set mbuf
offloads for the transmitting burst, mainly in the checksum/TSO areas.
In the case of a tunnel header, the csum FWD tries to detect
known tunnels by the standard definition using the header'sdata
and fallback to check the packet type in the mbuf to see if the
Rx port driver already sign the packet as a tunnel.
In the fallback case, the csum assumes the tunnel is VXLAN and
parses the tunnel as VXLAN.
As far as I can see there is a VXLAN port check in
'parse_vxlan()', why it is not helping?
The problem is not the vxlan check but the tunnel type in mbuf
that caused the
packet to be detected as vxlan(default) before checking GENEVE tunnel case.
Check is as following:
if (udp_hdr->dst_port != _htons(RTE_VXLAN_DEFAULT_PORT) &&
RTE_ETH_IS_TUNNEL_PKT(pkt_type) == 0)
return;
Do you what is the intention for the
"RTE_ETH_IS_TUNNEL_PKT(pkt_type) ==
0"
check?
Why vxlan parsing doesn't stop when it is not default port?
Maybe some drivers set the tunnel type for vxlan packets coming
after non-
standard vxlan port.
But checking the tunnel flag to say that it is vxlan is too broad, isn't it?
And this is the problem you are having.
Can there be any way to detect and check non-standard vxlan port?
Maybe yes, but it is probably more complex solusion.
See this patch:
https://patches.dpdk.org/project/dpdk/patch/1423819371-24222-13-git-se
nd-email-olivier.matz@6wind.com/<mailto:nd-email-olivier.matz@6wind.com/>
comments there:
1. check udp destination port, 4789 is the default vxlan port (rfc7348).
2. currently, this flag is set by i40e only if the packet is vxlan
And maybe another driver assumes more ports here.
Collecting all the non-standard ports can start from i40 maintainers😊
I think, here we should check for supported tunnel types only. The i40 driver setting a Tunnel flag, does not means that it VxLan type only.
As in your case those were GENEVE packets getting treated as VxLan.
Making parse_vxlan() udp_dst_port specific can avoid this issue.
When the GENEVE tunnel was added to the known tunnels in csum,
its parsing trial was wrongly located after the pkt type
detection, causing the csum to parse the GENEVE header as VXLAN
when the Rx port set the tunnel packet type.
Locate the GENEVE parsing trial before the packet type detection.
Fixes: ea0e711b8ae0 ("app/testpmd: add GENEVE parsing")
Cc: stable@dpdk.org<mailto:stable@dpdk.org>
Signed-off-by: Raja Zidane <rzidane@nvidia.com><mailto:rzidane@nvidia.com>
---
Acked-by: Matan Azrad <matan@nvidia.com><mailto:matan@nvidia.com>
Ack should be before '---' to be part of the commit log,
otherwise it is dropped when applied as comment.
[-- Attachment #2: Type: text/html, Size: 21631 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] app/testpmd: fix GENEVE parsing in csum forward mode
2022-02-15 14:31 ` Raja Zidane
@ 2022-02-15 14:43 ` Singh, Aman Deep
2022-02-16 2:02 ` Xing, Beilei
1 sibling, 0 replies; 20+ messages in thread
From: Singh, Aman Deep @ 2022-02-15 14:43 UTC (permalink / raw)
To: Raja Zidane, dev, Beilei Xing; +Cc: stable, Qi Zhang, Matan Azrad, Ferruh Yigit
[-- Attachment #1: Type: text/plain, Size: 11613 bytes --]
Hi Raja Zidane,
On 2/15/2022 8:01 PM, Raja Zidane wrote:
>
> Hi all,
> reviving the discussion.
>
> @Beilei Xing <mailto:beilei.xing@intel.com> could you please provide
> info on what UDP destination ports are used for VxLan by i40 driver?
> if its just the default then we can remove "RTE_ETH_IS_TUNNEL_PKT(pkt_type) == 0"
Right, we would like to remove the check "RTE_ETH_IS_TUNNEL_PKT(pkt_type) == 0" from parse_vxlan function.
And rather, add a default case with this check "RTE_ETH_IS_TUNNEL_PKT(pkt_type) == 0" at the end.
This default case can print an error stating "unknown tunnel pkt" with its udp_port parameters.
> *From:* Singh, Aman Deep <aman.deep.singh@intel.com>
> *Sent:* Monday, January 31, 2022 6:48 PM
> *To:* Raja Zidane <rzidane@nvidia.com>; Matan Azrad
> <matan@nvidia.com>; Ferruh Yigit <ferruh.yigit@intel.com>;
> dev@dpdk.org; Beilei Xing <beilei.xing@intel.com>; Qi Zhang
> <qi.z.zhang@intel.com>
> *Cc:* stable@dpdk.org
> *Subject:* Re: [PATCH] app/testpmd: fix GENEVE parsing in csum forward
> mode
>
> *External email: Use caution opening links or attachments*
>
> On 1/30/2022 4:48 PM, Raja Zidane wrote:
>
> I didn't want to remove the default parsing of tunnel as VxLan because I thought it might be used,
>
> Instead I moved it to the end, which makes it detect all supported tunnel through udp_dst_port,
>
> And only if no tunnel was matched it would default to VxLan.
>
> That was the reason geneve weren't detected and parsed as vxlan instead, which is the bug I was trying to solve.
>
> We can take help/input from i40 maintainers for it.
> Hi Beilei Xing,
> For setting packet_type as Tunnel, what criteria is used by i40 driver. Is it only udp_dst port or any other parameters also.
>
> -----Original Message-----
>
> From: Singh, Aman Deep<aman.deep.singh@intel.com> <mailto:aman.deep.singh@intel.com>
>
> Sent: Thursday, January 20, 2022 12:47 PM
>
> To: Matan Azrad<matan@nvidia.com> <mailto:matan@nvidia.com>; Ferruh Yigit<ferruh.yigit@intel.com> <mailto:ferruh.yigit@intel.com>; Raja Zidane<rzidane@nvidia.com> <mailto:rzidane@nvidia.com>;dev@dpdk.org
>
> Cc:stable@dpdk.org
>
> Subject: Re: [PATCH] app/testpmd: fix GENEVE parsing in csum forward mode
>
> External email: Use caution opening links or attachments
>
> On 1/18/2022 6:49 PM, Matan Azrad wrote:
>
> app/test-pmd/csumonly.c | 16 ++++++++++------
>
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
>
> index 2aeea243b6..fe810fecdd 100644
>
> --- a/app/test-pmd/csumonly.c
>
> +++ b/app/test-pmd/csumonly.c
>
> @@ -254,7 +254,10 @@ parse_gtp(struct rte_udp_hdr *udp_hdr,
>
> info->l2_len += RTE_ETHER_GTP_HLEN;
>
> }
>
> -/* Parse a vxlan header */
>
> +/*
>
> + * Parse a vxlan header.
>
> + * If a tunnel is detected in 'pkt_type' it will be parsed by default as vxlan.
>
> + */
>
> static void
>
> parse_vxlan(struct rte_udp_hdr *udp_hdr,
>
> struct testpmd_offload_info *info, @@ -912,17
>
> +915,18 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
>
> RTE_MBUF_F_TX_TUNNEL_VXLAN_GPE;
>
> goto tunnel_update;
>
> }
>
> - parse_vxlan(udp_hdr, &info,
>
> - m->packet_type);
>
> + parse_geneve(udp_hdr, &info);
>
> if (info.is_tunnel) {
>
> tx_ol_flags |=
>
> - RTE_MBUF_F_TX_TUNNEL_VXLAN;
>
> +
>
> + RTE_MBUF_F_TX_TUNNEL_GENEVE;
>
> goto tunnel_update;
>
> }
>
> - parse_geneve(udp_hdr, &info);
>
> + /* Always keep last. */
>
> + parse_vxlan(udp_hdr, &info,
>
> + m->packet_type);
>
> if (info.is_tunnel) {
>
> tx_ol_flags |=
>
> - RTE_MBUF_F_TX_TUNNEL_GENEVE;
>
> +
>
> + RTE_MBUF_F_TX_TUNNEL_VXLAN;
>
> goto tunnel_update;
>
> }
>
> } else if (info.l4_proto ==
>
> IPPROTO_GRE) {
>
> -----Original Message-----
>
> From: Ferruh Yigit<ferruh.yigit@intel.com> <mailto:ferruh.yigit@intel.com>
>
> Sent: Tuesday, January 18, 2022 3:03 PM
>
> To: Matan Azrad<matan@nvidia.com> <mailto:matan@nvidia.com>; Raja Zidane<rzidane@nvidia.com> <mailto:rzidane@nvidia.com>;
>
> dev@dpdk.org
>
> Cc:stable@dpdk.org
>
> Subject: Re: [PATCH] app/testpmd: fix GENEVE parsing in csum forward
>
> mode
>
> External email: Use caution opening links or attachments
>
> On 1/18/2022 12:55 PM, Matan Azrad wrote:
>
> -----Original Message-----
>
> From: Ferruh Yigit<ferruh.yigit@intel.com> <mailto:ferruh.yigit@intel.com>
>
> Sent: Tuesday, January 18, 2022 2:28 PM
>
> To: Matan Azrad<matan@nvidia.com> <mailto:matan@nvidia.com>; Raja Zidane
>
> <rzidane@nvidia.com> <mailto:rzidane@nvidia.com>;dev@dpdk.org
>
> Cc:stable@dpdk.org
>
> Subject: Re: [PATCH] app/testpmd: fix GENEVE parsing in csum
>
> forward mode
>
> External email: Use caution opening links or attachments
>
> On 1/18/2022 11:27 AM, Matan Azrad wrote:
>
> -----Original Message-----
>
> From: Ferruh Yigit<ferruh.yigit@intel.com> <mailto:ferruh.yigit@intel.com>
>
> Sent: Tuesday, January 18, 2022 11:52 AM
>
> To: Raja Zidane<rzidane@nvidia.com> <mailto:rzidane@nvidia.com>;dev@dpdk.org
>
> Cc: Matan Azrad<matan@nvidia.com> <mailto:matan@nvidia.com>;stable@dpdk.org
>
> Subject: Re: [PATCH] app/testpmd: fix GENEVE parsing in csum
>
> forward mode
>
> External email: Use caution opening links or attachments
>
> On 12/5/2021 3:44 AM, Raja Zidane wrote:
>
> The csum FWD mode parses any received packet to set mbuf
>
> offloads for the transmitting burst, mainly in the checksum/TSO areas.
>
> In the case of a tunnel header, the csum FWD tries to detect
>
> known tunnels by the standard definition using the header'sdata
>
> and fallback to check the packet type in the mbuf to see if the
>
> Rx port driver already sign the packet as a tunnel.
>
> In the fallback case, the csum assumes the tunnel is VXLAN and
>
> parses the tunnel as VXLAN.
>
> As far as I can see there is a VXLAN port check in
>
> 'parse_vxlan()', why it is not helping?
>
> The problem is not the vxlan check but the tunnel type in mbuf
>
> that caused the
>
> packet to be detected as vxlan(default) before checking GENEVE tunnel case.
>
> Check is as following:
>
> if (udp_hdr->dst_port != _htons(RTE_VXLAN_DEFAULT_PORT) &&
>
> RTE_ETH_IS_TUNNEL_PKT(pkt_type) == 0)
>
> return;
>
> Do you what is the intention for the
>
> "RTE_ETH_IS_TUNNEL_PKT(pkt_type) ==
>
> 0"
>
> check?
>
> Why vxlan parsing doesn't stop when it is not default port?
>
> Maybe some drivers set the tunnel type for vxlan packets coming
>
> after non-
>
> standard vxlan port.
>
> But checking the tunnel flag to say that it is vxlan is too broad, isn't it?
>
> And this is the problem you are having.
>
> Can there be any way to detect and check non-standard vxlan port?
>
> Maybe yes, but it is probably more complex solusion.
>
> See this patch:
>
> https://patches.dpdk.org/project/dpdk/patch/1423819371-24222-13-git-se
>
> nd-email-olivier.matz@6wind.com/
>
> comments there:
>
> 1. check udp destination port, 4789 is the default vxlan port (rfc7348).
>
> 2. currently, this flag is set by i40e only if the packet is vxlan
>
> And maybe another driver assumes more ports here.
>
> Collecting all the non-standard ports can start from i40 maintainers😊
>
> I think, here we should check for supported tunnel types only. The i40 driver setting a Tunnel flag, does not means that it VxLan type only.
>
> As in your case those were GENEVE packets getting treated as VxLan.
>
> Making parse_vxlan() udp_dst_port specific can avoid this issue.
>
> When the GENEVE tunnel was added to the known tunnels in csum,
>
> its parsing trial was wrongly located after the pkt type
>
> detection, causing the csum to parse the GENEVE header as VXLAN
>
> when the Rx port set the tunnel packet type.
>
> Locate the GENEVE parsing trial before the packet type detection.
>
> Fixes: ea0e711b8ae0 ("app/testpmd: add GENEVE parsing")
>
> Cc:stable@dpdk.org
>
> Signed-off-by: Raja Zidane<rzidane@nvidia.com> <mailto:rzidane@nvidia.com>
>
> ---
>
> Acked-by: Matan Azrad<matan@nvidia.com> <mailto:matan@nvidia.com>
>
> Ack should be before '---' to be part of the commit log,
>
> otherwise it is dropped when applied as comment.
>
[-- Attachment #2: Type: text/html, Size: 27270 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [PATCH] app/testpmd: fix GENEVE parsing in csum forward mode
2022-02-15 14:31 ` Raja Zidane
2022-02-15 14:43 ` Singh, Aman Deep
@ 2022-02-16 2:02 ` Xing, Beilei
1 sibling, 0 replies; 20+ messages in thread
From: Xing, Beilei @ 2022-02-16 2:02 UTC (permalink / raw)
To: Raja Zidane, Singh, Aman Deep, Matan Azrad, Yigit, Ferruh, dev,
Zhang, Qi Z
Cc: stable
[-- Attachment #1: Type: text/plain, Size: 9391 bytes --]
Hi Zidane,
I40e used UDP dst port 4789 for Vxlan.
BR,
Beilei
From: Raja Zidane <rzidane@nvidia.com>
Sent: Tuesday, February 15, 2022 10:31 PM
To: Singh, Aman Deep <aman.deep.singh@intel.com>; Matan Azrad <matan@nvidia.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; dev@dpdk.org; Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
Cc: stable@dpdk.org
Subject: RE: [PATCH] app/testpmd: fix GENEVE parsing in csum forward mode
Hi all,
reviving the discussion.
@Beilei Xing<mailto:beilei.xing@intel.com> could you please provide info on what UDP destination ports are used for VxLan by i40 driver?
if its just the default then we can remove "RTE_ETH_IS_TUNNEL_PKT(pkt_type) == 0"
From: Singh, Aman Deep <aman.deep.singh@intel.com<mailto:aman.deep.singh@intel.com>>
Sent: Monday, January 31, 2022 6:48 PM
To: Raja Zidane <rzidane@nvidia.com<mailto:rzidane@nvidia.com>>; Matan Azrad <matan@nvidia.com<mailto:matan@nvidia.com>>; Ferruh Yigit <ferruh.yigit@intel.com<mailto:ferruh.yigit@intel.com>>; dev@dpdk.org<mailto:dev@dpdk.org>; Beilei Xing <beilei.xing@intel.com<mailto:beilei.xing@intel.com>>; Qi Zhang <qi.z.zhang@intel.com<mailto:qi.z.zhang@intel.com>>
Cc: stable@dpdk.org<mailto:stable@dpdk.org>
Subject: Re: [PATCH] app/testpmd: fix GENEVE parsing in csum forward mode
External email: Use caution opening links or attachments
On 1/30/2022 4:48 PM, Raja Zidane wrote:
I didn't want to remove the default parsing of tunnel as VxLan because I thought it might be used,
Instead I moved it to the end, which makes it detect all supported tunnel through udp_dst_port,
And only if no tunnel was matched it would default to VxLan.
That was the reason geneve weren't detected and parsed as vxlan instead, which is the bug I was trying to solve.
We can take help/input from i40 maintainers for it.
Hi Beilei Xing,
For setting packet_type as Tunnel, what criteria is used by i40 driver. Is it only udp_dst port or any other parameters also.
-----Original Message-----
From: Singh, Aman Deep <aman.deep.singh@intel.com><mailto:aman.deep.singh@intel.com>
Sent: Thursday, January 20, 2022 12:47 PM
To: Matan Azrad <matan@nvidia.com><mailto:matan@nvidia.com>; Ferruh Yigit <ferruh.yigit@intel.com><mailto:ferruh.yigit@intel.com>; Raja Zidane <rzidane@nvidia.com><mailto:rzidane@nvidia.com>; dev@dpdk.org<mailto:dev@dpdk.org>
Cc: stable@dpdk.org<mailto:stable@dpdk.org>
Subject: Re: [PATCH] app/testpmd: fix GENEVE parsing in csum forward mode
External email: Use caution opening links or attachments
On 1/18/2022 6:49 PM, Matan Azrad wrote:
app/test-pmd/csumonly.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
index 2aeea243b6..fe810fecdd 100644
--- a/app/test-pmd/csumonly.c
+++ b/app/test-pmd/csumonly.c
@@ -254,7 +254,10 @@ parse_gtp(struct rte_udp_hdr *udp_hdr,
info->l2_len += RTE_ETHER_GTP_HLEN;
}
-/* Parse a vxlan header */
+/*
+ * Parse a vxlan header.
+ * If a tunnel is detected in 'pkt_type' it will be parsed by default as vxlan.
+ */
static void
parse_vxlan(struct rte_udp_hdr *udp_hdr,
struct testpmd_offload_info *info, @@ -912,17
+915,18 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
RTE_MBUF_F_TX_TUNNEL_VXLAN_GPE;
goto tunnel_update;
}
- parse_vxlan(udp_hdr, &info,
- m->packet_type);
+ parse_geneve(udp_hdr, &info);
if (info.is_tunnel) {
tx_ol_flags |=
- RTE_MBUF_F_TX_TUNNEL_VXLAN;
+
+ RTE_MBUF_F_TX_TUNNEL_GENEVE;
goto tunnel_update;
}
- parse_geneve(udp_hdr, &info);
+ /* Always keep last. */
+ parse_vxlan(udp_hdr, &info,
+ m->packet_type);
if (info.is_tunnel) {
tx_ol_flags |=
- RTE_MBUF_F_TX_TUNNEL_GENEVE;
+
+ RTE_MBUF_F_TX_TUNNEL_VXLAN;
goto tunnel_update;
}
} else if (info.l4_proto ==
IPPROTO_GRE) {
-----Original Message-----
From: Ferruh Yigit <ferruh.yigit@intel.com><mailto:ferruh.yigit@intel.com>
Sent: Tuesday, January 18, 2022 3:03 PM
To: Matan Azrad <matan@nvidia.com><mailto:matan@nvidia.com>; Raja Zidane <rzidane@nvidia.com><mailto:rzidane@nvidia.com>;
dev@dpdk.org<mailto:dev@dpdk.org>
Cc: stable@dpdk.org<mailto:stable@dpdk.org>
Subject: Re: [PATCH] app/testpmd: fix GENEVE parsing in csum forward
mode
External email: Use caution opening links or attachments
On 1/18/2022 12:55 PM, Matan Azrad wrote:
-----Original Message-----
From: Ferruh Yigit <ferruh.yigit@intel.com><mailto:ferruh.yigit@intel.com>
Sent: Tuesday, January 18, 2022 2:28 PM
To: Matan Azrad <matan@nvidia.com><mailto:matan@nvidia.com>; Raja Zidane
<rzidane@nvidia.com><mailto:rzidane@nvidia.com>; dev@dpdk.org<mailto:dev@dpdk.org>
Cc: stable@dpdk.org<mailto:stable@dpdk.org>
Subject: Re: [PATCH] app/testpmd: fix GENEVE parsing in csum
forward mode
External email: Use caution opening links or attachments
On 1/18/2022 11:27 AM, Matan Azrad wrote:
-----Original Message-----
From: Ferruh Yigit <ferruh.yigit@intel.com><mailto:ferruh.yigit@intel.com>
Sent: Tuesday, January 18, 2022 11:52 AM
To: Raja Zidane <rzidane@nvidia.com><mailto:rzidane@nvidia.com>; dev@dpdk.org<mailto:dev@dpdk.org>
Cc: Matan Azrad <matan@nvidia.com><mailto:matan@nvidia.com>; stable@dpdk.org<mailto:stable@dpdk.org>
Subject: Re: [PATCH] app/testpmd: fix GENEVE parsing in csum
forward mode
External email: Use caution opening links or attachments
On 12/5/2021 3:44 AM, Raja Zidane wrote:
The csum FWD mode parses any received packet to set mbuf
offloads for the transmitting burst, mainly in the checksum/TSO areas.
In the case of a tunnel header, the csum FWD tries to detect
known tunnels by the standard definition using the header'sdata
and fallback to check the packet type in the mbuf to see if the
Rx port driver already sign the packet as a tunnel.
In the fallback case, the csum assumes the tunnel is VXLAN and
parses the tunnel as VXLAN.
As far as I can see there is a VXLAN port check in
'parse_vxlan()', why it is not helping?
The problem is not the vxlan check but the tunnel type in mbuf
that caused the
packet to be detected as vxlan(default) before checking GENEVE tunnel case.
Check is as following:
if (udp_hdr->dst_port != _htons(RTE_VXLAN_DEFAULT_PORT) &&
RTE_ETH_IS_TUNNEL_PKT(pkt_type) == 0)
return;
Do you what is the intention for the
"RTE_ETH_IS_TUNNEL_PKT(pkt_type) ==
0"
check?
Why vxlan parsing doesn't stop when it is not default port?
Maybe some drivers set the tunnel type for vxlan packets coming
after non-
standard vxlan port.
But checking the tunnel flag to say that it is vxlan is too broad, isn't it?
And this is the problem you are having.
Can there be any way to detect and check non-standard vxlan port?
Maybe yes, but it is probably more complex solusion.
See this patch:
https://patches.dpdk.org/project/dpdk/patch/1423819371-24222-13-git-se
nd-email-olivier.matz@6wind.com/<mailto:nd-email-olivier.matz@6wind.com/>
comments there:
1. check udp destination port, 4789 is the default vxlan port (rfc7348).
2. currently, this flag is set by i40e only if the packet is vxlan
And maybe another driver assumes more ports here.
Collecting all the non-standard ports can start from i40 maintainers😊
I think, here we should check for supported tunnel types only. The i40 driver setting a Tunnel flag, does not means that it VxLan type only.
As in your case those were GENEVE packets getting treated as VxLan.
Making parse_vxlan() udp_dst_port specific can avoid this issue.
When the GENEVE tunnel was added to the known tunnels in csum,
its parsing trial was wrongly located after the pkt type
detection, causing the csum to parse the GENEVE header as VXLAN
when the Rx port set the tunnel packet type.
Locate the GENEVE parsing trial before the packet type detection.
Fixes: ea0e711b8ae0 ("app/testpmd: add GENEVE parsing")
Cc: stable@dpdk.org<mailto:stable@dpdk.org>
Signed-off-by: Raja Zidane <rzidane@nvidia.com><mailto:rzidane@nvidia.com>
---
Acked-by: Matan Azrad <matan@nvidia.com><mailto:matan@nvidia.com>
Ack should be before '---' to be part of the commit log,
otherwise it is dropped when applied as comment.
[-- Attachment #2: Type: text/html, Size: 23193 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH V2] app/testpmd: fix GENEVE parsing in csum forward mode
2021-12-05 3:44 [PATCH] app/testpmd: fix GENEVE parsing in csum forward mode Raja Zidane
2022-01-18 9:51 ` Ferruh Yigit
@ 2022-02-16 12:37 ` Raja Zidane
2022-02-18 9:09 ` Singh, Aman Deep
2022-02-20 12:09 ` [PATCH V3] " Raja Zidane
1 sibling, 2 replies; 20+ messages in thread
From: Raja Zidane @ 2022-02-16 12:37 UTC (permalink / raw)
To: dev; +Cc: Matan Azrad, stable
The csum FWD mode parses any received packet to set mbuf offloads for the
transmitting burst, mainly in the checksum/TSO areas.
In the case of a tunnel header, the csum FWD tries to detect known tunnels
by the standard definition using the header'sdata and fallback to check the
packet type in the mbuf to see if the Rx port driver already sign the
packet as a tunnel.
In the fallback case, the csum assumes the tunnel is VXLAN and parses the
tunnel as VXLAN.
When the GENEVE tunnel was added to the known tunnels in csum, its parsing
trial was wrongly located after the pkt type detection, causing the csum to
parse the GENEVE header as VXLAN when the Rx port set the tunnel packet
type.
Remove the fall back case to VxLan.
Log error of unrecognized tunnel if no tunnel was parsed successfully.
Fixes: ea0e711b8ae0 ("app/testpmd: add GENEVE parsing")
Cc: stable@dpdk.org
Signed-off-by: Raja Zidane <rzidane@nvidia.com>
---
V2: Log error when an unrecognized tunnel is found (unknown UDP dst port), instead of parsing it as VxLan by default.
app/test-pmd/csumonly.c | 23 +++++++++++++----------
1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
index 02bc3929c7..210f4263f8 100644
--- a/app/test-pmd/csumonly.c
+++ b/app/test-pmd/csumonly.c
@@ -255,11 +255,10 @@ parse_gtp(struct rte_udp_hdr *udp_hdr,
info->l2_len += RTE_ETHER_GTP_HLEN;
}
-/* Parse a vxlan header */
+/* Parse a vxlan header. */
static void
parse_vxlan(struct rte_udp_hdr *udp_hdr,
- struct testpmd_offload_info *info,
- uint32_t pkt_type)
+ struct testpmd_offload_info *info)
{
struct rte_ether_hdr *eth_hdr;
@@ -267,8 +266,7 @@ parse_vxlan(struct rte_udp_hdr *udp_hdr,
* default vxlan port (rfc7348) or that the rx offload flag is set
* (i40e only currently)
*/
- if (udp_hdr->dst_port != _htons(RTE_VXLAN_DEFAULT_PORT) &&
- RTE_ETH_IS_TUNNEL_PKT(pkt_type) == 0)
+ if (udp_hdr->dst_port != _htons(RTE_VXLAN_DEFAULT_PORT))
return;
update_tunnel_outer(info);
@@ -922,19 +920,24 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
RTE_MBUF_F_TX_TUNNEL_VXLAN_GPE;
goto tunnel_update;
}
- parse_vxlan(udp_hdr, &info,
- m->packet_type);
+ parse_geneve(udp_hdr, &info);
if (info.is_tunnel) {
tx_ol_flags |=
- RTE_MBUF_F_TX_TUNNEL_VXLAN;
+ RTE_MBUF_F_TX_TUNNEL_GENEVE;
goto tunnel_update;
}
- parse_geneve(udp_hdr, &info);
+ parse_vxlan(udp_hdr, &info);
if (info.is_tunnel) {
tx_ol_flags |=
- RTE_MBUF_F_TX_TUNNEL_GENEVE;
+ RTE_MBUF_F_TX_TUNNEL_VXLAN;
goto tunnel_update;
}
+ /* Always keep last. */
+ if (unlikely(RTE_ETH_IS_TUNNEL_PKT(
+ m->packet_type) != 0)) {
+ TESTPMD_LOG(ERR, "Unknown tunnel packet. UDP dst port: %hu",
+ udp_hdr->dst_port);
+ }
} else if (info.l4_proto == IPPROTO_GRE) {
struct simple_gre_hdr *gre_hdr;
--
2.21.0
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH V2] app/testpmd: fix GENEVE parsing in csum forward mode
2022-02-16 12:37 ` [PATCH V2] " Raja Zidane
@ 2022-02-18 9:09 ` Singh, Aman Deep
2022-02-20 12:09 ` [PATCH V3] " Raja Zidane
1 sibling, 0 replies; 20+ messages in thread
From: Singh, Aman Deep @ 2022-02-18 9:09 UTC (permalink / raw)
To: Raja Zidane, dev; +Cc: Matan Azrad, stable
[-- Attachment #1: Type: text/plain, Size: 3300 bytes --]
On 2/16/2022 6:07 PM, Raja Zidane wrote:
> The csum FWD mode parses any received packet to set mbuf offloads for the
> transmitting burst, mainly in the checksum/TSO areas.
> In the case of a tunnel header, the csum FWD tries to detect known tunnels
> by the standard definition using the header'sdata and fallback to check the
> packet type in the mbuf to see if the Rx port driver already sign the
> packet as a tunnel.
> In the fallback case, the csum assumes the tunnel is VXLAN and parses the
> tunnel as VXLAN.
> When the GENEVE tunnel was added to the known tunnels in csum, its parsing
> trial was wrongly located after the pkt type detection, causing the csum to
> parse the GENEVE header as VXLAN when the Rx port set the tunnel packet
> type.
>
> Remove the fall back case to VxLan.
> Log error of unrecognized tunnel if no tunnel was parsed successfully.
>
> Fixes: ea0e711b8ae0 ("app/testpmd: add GENEVE parsing")
> Cc:stable@dpdk.org
>
> Signed-off-by: Raja Zidane<rzidane@nvidia.com>
> ---
> V2: Log error when an unrecognized tunnel is found (unknown UDP dst port), instead of parsing it as VxLan by default.
>
> app/test-pmd/csumonly.c | 23 +++++++++++++----------
> 1 file changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
> index 02bc3929c7..210f4263f8 100644
> --- a/app/test-pmd/csumonly.c
> +++ b/app/test-pmd/csumonly.c
> @@ -255,11 +255,10 @@ parse_gtp(struct rte_udp_hdr *udp_hdr,
> info->l2_len += RTE_ETHER_GTP_HLEN;
> }
>
> -/* Parse a vxlan header */
> +/* Parse a vxlan header. */
> static void
> parse_vxlan(struct rte_udp_hdr *udp_hdr,
> - struct testpmd_offload_info *info,
> - uint32_t pkt_type)
> + struct testpmd_offload_info *info)
> {
> struct rte_ether_hdr *eth_hdr;
>
> @@ -267,8 +266,7 @@ parse_vxlan(struct rte_udp_hdr *udp_hdr,
> * default vxlan port (rfc7348) or that the rx offload flag is set
> * (i40e only currently)
> */
> - if (udp_hdr->dst_port != _htons(RTE_VXLAN_DEFAULT_PORT) &&
> - RTE_ETH_IS_TUNNEL_PKT(pkt_type) == 0)
> + if (udp_hdr->dst_port != _htons(RTE_VXLAN_DEFAULT_PORT))
> return;
>
> update_tunnel_outer(info);
> @@ -922,19 +920,24 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
> RTE_MBUF_F_TX_TUNNEL_VXLAN_GPE;
> goto tunnel_update;
> }
> - parse_vxlan(udp_hdr, &info,
> - m->packet_type);
> + parse_geneve(udp_hdr, &info);
> if (info.is_tunnel) {
> tx_ol_flags |=
> - RTE_MBUF_F_TX_TUNNEL_VXLAN;
> + RTE_MBUF_F_TX_TUNNEL_GENEVE;
> goto tunnel_update;
> }
> - parse_geneve(udp_hdr, &info);
> + parse_vxlan(udp_hdr, &info);
> if (info.is_tunnel) {
> tx_ol_flags |=
> - RTE_MBUF_F_TX_TUNNEL_GENEVE;
> + RTE_MBUF_F_TX_TUNNEL_VXLAN;
After adding default case, i suppose swapping of parse_vxlan & parse_geneve is not needed.
Can we revert these above changes.
> goto tunnel_update;
> }
> + /* Always keep last. */
> + if (unlikely(RTE_ETH_IS_TUNNEL_PKT(
> + m->packet_type) != 0)) {
> + TESTPMD_LOG(ERR, "Unknown tunnel packet. UDP dst port: %hu",
> + udp_hdr->dst_port);
> + }
> } else if (info.l4_proto == IPPROTO_GRE) {
> struct simple_gre_hdr *gre_hdr;
>
[-- Attachment #2: Type: text/html, Size: 3826 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH V3] app/testpmd: fix GENEVE parsing in csum forward mode
2022-02-16 12:37 ` [PATCH V2] " Raja Zidane
2022-02-18 9:09 ` Singh, Aman Deep
@ 2022-02-20 12:09 ` Raja Zidane
2022-02-21 10:24 ` Singh, Aman Deep
` (2 more replies)
1 sibling, 3 replies; 20+ messages in thread
From: Raja Zidane @ 2022-02-20 12:09 UTC (permalink / raw)
To: dev; +Cc: matan, stable
The csum FWD mode parses any received packet to set mbuf offloads for the
transmitting burst, mainly in the checksum/TSO areas.
In the case of a tunnel header, the csum FWD tries to detect known tunnels
by the standard definition using the header'sdata and fallback to check the
packet type in the mbuf to see if the Rx port driver already sign the
packet as a tunnel.
In the fallback case, the csum assumes the tunnel is VXLAN and parses the
tunnel as VXLAN.
When the GENEVE tunnel was added to the known tunnels in csum, its parsing
trial was wrongly located after the pkt type detection, causing the csum to
parse the GENEVE header as VXLAN when the Rx port set the tunnel packet
type.
Remove the fall back case to VxLan.
Log error of unrecognized tunnel if no tunnel was parsed successfully.
Fixes: c10a026c3b03 ("app/testpmd: introduce vxlan parsing function in csum fwd engine")
Cc: stable@dpdk.org
Signed-off-by: Raja Zidane <rzidane@nvidia.com>
---
V2: Log error when an unrecognized tunnel is found (unknown UDP dst port), instead of parsing it as VxLan by default.
V3: revert unneeded changes (swapping parse_geneve & parse_vxlan).
app/test-pmd/csumonly.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
index 02bc3929c7..e9ade3b864 100644
--- a/app/test-pmd/csumonly.c
+++ b/app/test-pmd/csumonly.c
@@ -258,8 +258,7 @@ parse_gtp(struct rte_udp_hdr *udp_hdr,
/* Parse a vxlan header */
static void
parse_vxlan(struct rte_udp_hdr *udp_hdr,
- struct testpmd_offload_info *info,
- uint32_t pkt_type)
+ struct testpmd_offload_info *info)
{
struct rte_ether_hdr *eth_hdr;
@@ -267,8 +266,7 @@ parse_vxlan(struct rte_udp_hdr *udp_hdr,
* default vxlan port (rfc7348) or that the rx offload flag is set
* (i40e only currently)
*/
- if (udp_hdr->dst_port != _htons(RTE_VXLAN_DEFAULT_PORT) &&
- RTE_ETH_IS_TUNNEL_PKT(pkt_type) == 0)
+ if (udp_hdr->dst_port != _htons(RTE_VXLAN_DEFAULT_PORT))
return;
update_tunnel_outer(info);
@@ -922,8 +920,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
RTE_MBUF_F_TX_TUNNEL_VXLAN_GPE;
goto tunnel_update;
}
- parse_vxlan(udp_hdr, &info,
- m->packet_type);
+ parse_vxlan(udp_hdr, &info);
if (info.is_tunnel) {
tx_ol_flags |=
RTE_MBUF_F_TX_TUNNEL_VXLAN;
@@ -935,6 +932,12 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
RTE_MBUF_F_TX_TUNNEL_GENEVE;
goto tunnel_update;
}
+ /* Always keep last. */
+ if (unlikely(RTE_ETH_IS_TUNNEL_PKT(
+ m->packet_type) != 0)) {
+ TESTPMD_LOG(ERR, "Unknown tunnel packet. UDP dst port: %hu",
+ udp_hdr->dst_port);
+ }
} else if (info.l4_proto == IPPROTO_GRE) {
struct simple_gre_hdr *gre_hdr;
--
2.21.0
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH V3] app/testpmd: fix GENEVE parsing in csum forward mode
2022-02-20 12:09 ` [PATCH V3] " Raja Zidane
@ 2022-02-21 10:24 ` Singh, Aman Deep
2022-02-21 12:08 ` Ferruh Yigit
2022-02-21 13:24 ` [PATCH V4] " Raja Zidane
2 siblings, 0 replies; 20+ messages in thread
From: Singh, Aman Deep @ 2022-02-21 10:24 UTC (permalink / raw)
To: Raja Zidane, dev; +Cc: matan, stable
[-- Attachment #1: Type: text/plain, Size: 1397 bytes --]
On 2/20/2022 5:39 PM, Raja Zidane wrote:
> The csum FWD mode parses any received packet to set mbuf offloads for the
> transmitting burst, mainly in the checksum/TSO areas.
> In the case of a tunnel header, the csum FWD tries to detect known tunnels
> by the standard definition using the header'sdata and fallback to check the
> packet type in the mbuf to see if the Rx port driver already sign the
> packet as a tunnel.
> In the fallback case, the csum assumes the tunnel is VXLAN and parses the
> tunnel as VXLAN.
> When the GENEVE tunnel was added to the known tunnels in csum, its parsing
> trial was wrongly located after the pkt type detection, causing the csum to
> parse the GENEVE header as VXLAN when the Rx port set the tunnel packet
> type.
>
> Remove the fall back case to VxLan.
> Log error of unrecognized tunnel if no tunnel was parsed successfully.
>
> Fixes: c10a026c3b03 ("app/testpmd: introduce vxlan parsing function in csum fwd engine")
> Cc:stable@dpdk.org
>
> Signed-off-by: Raja Zidane<rzidane@nvidia.com>
> ---
> V2: Log error when an unrecognized tunnel is found (unknown UDP dst port), instead of parsing it as VxLan by default.
> V3: revert unneeded changes (swapping parse_geneve & parse_vxlan).
> app/test-pmd/csumonly.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> Acked-by: Aman Singh <aman.deep.singh@intel.com>
<snip>
>
[-- Attachment #2: Type: text/html, Size: 2359 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH V3] app/testpmd: fix GENEVE parsing in csum forward mode
2022-02-20 12:09 ` [PATCH V3] " Raja Zidane
2022-02-21 10:24 ` Singh, Aman Deep
@ 2022-02-21 12:08 ` Ferruh Yigit
2022-02-21 13:24 ` [PATCH V4] " Raja Zidane
2 siblings, 0 replies; 20+ messages in thread
From: Ferruh Yigit @ 2022-02-21 12:08 UTC (permalink / raw)
To: Raja Zidane, dev; +Cc: matan, stable
On 2/20/2022 12:09 PM, Raja Zidane wrote:
> The csum FWD mode parses any received packet to set mbuf offloads for the
> transmitting burst, mainly in the checksum/TSO areas.
> In the case of a tunnel header, the csum FWD tries to detect known tunnels
> by the standard definition using the header'sdata and fallback to check the
> packet type in the mbuf to see if the Rx port driver already sign the
> packet as a tunnel.
> In the fallback case, the csum assumes the tunnel is VXLAN and parses the
> tunnel as VXLAN.
> When the GENEVE tunnel was added to the known tunnels in csum, its parsing
> trial was wrongly located after the pkt type detection, causing the csum to
> parse the GENEVE header as VXLAN when the Rx port set the tunnel packet
> type.
>
> Remove the fall back case to VxLan.
> Log error of unrecognized tunnel if no tunnel was parsed successfully.
>
> Fixes: c10a026c3b03 ("app/testpmd: introduce vxlan parsing function in csum fwd engine")
> Cc: stable@dpdk.org
>
> Signed-off-by: Raja Zidane <rzidane@nvidia.com>
> ---
> V2: Log error when an unrecognized tunnel is found (unknown UDP dst port), instead of parsing it as VxLan by default.
> V3: revert unneeded changes (swapping parse_geneve & parse_vxlan).
> app/test-pmd/csumonly.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
> index 02bc3929c7..e9ade3b864 100644
> --- a/app/test-pmd/csumonly.c
> +++ b/app/test-pmd/csumonly.c
> @@ -258,8 +258,7 @@ parse_gtp(struct rte_udp_hdr *udp_hdr,
> /* Parse a vxlan header */
> static void
> parse_vxlan(struct rte_udp_hdr *udp_hdr,
> - struct testpmd_offload_info *info,
> - uint32_t pkt_type)
> + struct testpmd_offload_info *info)
> {
> struct rte_ether_hdr *eth_hdr;
>
> @@ -267,8 +266,7 @@ parse_vxlan(struct rte_udp_hdr *udp_hdr,
> * default vxlan port (rfc7348) or that the rx offload flag is set
> * (i40e only currently)
> */
> - if (udp_hdr->dst_port != _htons(RTE_VXLAN_DEFAULT_PORT) &&
> - RTE_ETH_IS_TUNNEL_PKT(pkt_type) == 0)
> + if (udp_hdr->dst_port != _htons(RTE_VXLAN_DEFAULT_PORT))
> return;
>
> update_tunnel_outer(info);
> @@ -922,8 +920,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
> RTE_MBUF_F_TX_TUNNEL_VXLAN_GPE;
> goto tunnel_update;
> }
> - parse_vxlan(udp_hdr, &info,
> - m->packet_type);
> + parse_vxlan(udp_hdr, &info);
> if (info.is_tunnel) {
> tx_ol_flags |=
> RTE_MBUF_F_TX_TUNNEL_VXLAN;
> @@ -935,6 +932,12 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
> RTE_MBUF_F_TX_TUNNEL_GENEVE;
> goto tunnel_update;
> }
> + /* Always keep last. */
> + if (unlikely(RTE_ETH_IS_TUNNEL_PKT(
> + m->packet_type) != 0)) {
> + TESTPMD_LOG(ERR, "Unknown tunnel packet. UDP dst port: %hu",
> + udp_hdr->dst_port);
ERR type log can in datapath cause too much log, and not sure
what user can do with this error log.
But I can see tunnel type set by PMD but not parsed goes through
silently also not good.
So what about making log type DEBUG, at least it will be visible
to who wants to get more information about what is going on.
> + }
> } else if (info.l4_proto == IPPROTO_GRE) {
> struct simple_gre_hdr *gre_hdr;
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH V4] app/testpmd: fix GENEVE parsing in csum forward mode
2022-02-20 12:09 ` [PATCH V3] " Raja Zidane
2022-02-21 10:24 ` Singh, Aman Deep
2022-02-21 12:08 ` Ferruh Yigit
@ 2022-02-21 13:24 ` Raja Zidane
2022-02-21 17:36 ` Ferruh Yigit
2 siblings, 1 reply; 20+ messages in thread
From: Raja Zidane @ 2022-02-21 13:24 UTC (permalink / raw)
To: dev; +Cc: matan, stable
The csum FWD mode parses any received packet to set mbuf offloads for the
transmitting burst, mainly in the checksum/TSO areas.
In the case of a tunnel header, the csum FWD tries to detect known tunnels
by the standard definition using the header'sdata and fallback to check the
packet type in the mbuf to see if the Rx port driver already sign the
packet as a tunnel.
In the fallback case, the csum assumes the tunnel is VXLAN and parses the
tunnel as VXLAN.
When the GENEVE tunnel was added to the known tunnels in csum, its parsing
trial was wrongly located after the pkt type detection, causing the csum to
parse the GENEVE header as VXLAN when the Rx port set the tunnel packet
type.
Remove the fall back case to VxLan.
Log error of unrecognized tunnel if no tunnel was parsed successfully.
Fixes: c10a026c3b03 ("app/testpmd: introduce vxlan parsing function in csum fwd engine")
Cc: stable@dpdk.org
Signed-off-by: Raja Zidane <rzidane@nvidia.com>
---
V2: Log error when an unrecognized tunnel is found (unknown UDP dst port), instead of parsing it as VxLan by default.
V3: revert unneeded changes (swapping parse_geneve & parse_vxlan).
V4: Log unknown tunnel error in debug mode.
app/test-pmd/csumonly.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
index 02bc3929c7..5274d498ee 100644
--- a/app/test-pmd/csumonly.c
+++ b/app/test-pmd/csumonly.c
@@ -258,8 +258,7 @@ parse_gtp(struct rte_udp_hdr *udp_hdr,
/* Parse a vxlan header */
static void
parse_vxlan(struct rte_udp_hdr *udp_hdr,
- struct testpmd_offload_info *info,
- uint32_t pkt_type)
+ struct testpmd_offload_info *info)
{
struct rte_ether_hdr *eth_hdr;
@@ -267,8 +266,7 @@ parse_vxlan(struct rte_udp_hdr *udp_hdr,
* default vxlan port (rfc7348) or that the rx offload flag is set
* (i40e only currently)
*/
- if (udp_hdr->dst_port != _htons(RTE_VXLAN_DEFAULT_PORT) &&
- RTE_ETH_IS_TUNNEL_PKT(pkt_type) == 0)
+ if (udp_hdr->dst_port != _htons(RTE_VXLAN_DEFAULT_PORT))
return;
update_tunnel_outer(info);
@@ -922,8 +920,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
RTE_MBUF_F_TX_TUNNEL_VXLAN_GPE;
goto tunnel_update;
}
- parse_vxlan(udp_hdr, &info,
- m->packet_type);
+ parse_vxlan(udp_hdr, &info);
if (info.is_tunnel) {
tx_ol_flags |=
RTE_MBUF_F_TX_TUNNEL_VXLAN;
@@ -935,6 +932,12 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
RTE_MBUF_F_TX_TUNNEL_GENEVE;
goto tunnel_update;
}
+ /* Always keep last. */
+ if (unlikely(RTE_ETH_IS_TUNNEL_PKT(
+ m->packet_type) != 0)) {
+ TESTPMD_LOG(DEBUG, "Unknown tunnel packet. UDP dst port: %hu",
+ udp_hdr->dst_port);
+ }
} else if (info.l4_proto == IPPROTO_GRE) {
struct simple_gre_hdr *gre_hdr;
--
2.21.0
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH V4] app/testpmd: fix GENEVE parsing in csum forward mode
2022-02-21 13:24 ` [PATCH V4] " Raja Zidane
@ 2022-02-21 17:36 ` Ferruh Yigit
0 siblings, 0 replies; 20+ messages in thread
From: Ferruh Yigit @ 2022-02-21 17:36 UTC (permalink / raw)
To: Raja Zidane, dev; +Cc: matan, stable
On 2/21/2022 1:24 PM, Raja Zidane wrote:
> The csum FWD mode parses any received packet to set mbuf offloads for the
> transmitting burst, mainly in the checksum/TSO areas.
> In the case of a tunnel header, the csum FWD tries to detect known tunnels
> by the standard definition using the header'sdata and fallback to check the
> packet type in the mbuf to see if the Rx port driver already sign the
> packet as a tunnel.
> In the fallback case, the csum assumes the tunnel is VXLAN and parses the
> tunnel as VXLAN.
> When the GENEVE tunnel was added to the known tunnels in csum, its parsing
> trial was wrongly located after the pkt type detection, causing the csum to
> parse the GENEVE header as VXLAN when the Rx port set the tunnel packet
> type.
>
> Remove the fall back case to VxLan.
> Log error of unrecognized tunnel if no tunnel was parsed successfully.
>
> Fixes: c10a026c3b03 ("app/testpmd: introduce vxlan parsing function in csum fwd engine")
> Cc:stable@dpdk.org
>
> Signed-off-by: Raja Zidane<rzidane@nvidia.com>
Moving Aman's ack from previous version:
Acked-by: Aman Singh <aman.deep.singh@intel.com>
Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>
Applied to dpdk-next-net/main, thanks.
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2022-02-21 17:36 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-05 3:44 [PATCH] app/testpmd: fix GENEVE parsing in csum forward mode Raja Zidane
2022-01-18 9:51 ` Ferruh Yigit
2022-01-18 11:27 ` Matan Azrad
2022-01-18 12:28 ` Ferruh Yigit
2022-01-18 12:55 ` Matan Azrad
2022-01-18 13:03 ` Ferruh Yigit
2022-01-18 13:19 ` Matan Azrad
2022-01-20 10:46 ` Singh, Aman Deep
2022-01-30 11:18 ` Raja Zidane
2022-01-31 16:47 ` Singh, Aman Deep
2022-02-15 14:31 ` Raja Zidane
2022-02-15 14:43 ` Singh, Aman Deep
2022-02-16 2:02 ` Xing, Beilei
2022-02-16 12:37 ` [PATCH V2] " Raja Zidane
2022-02-18 9:09 ` Singh, Aman Deep
2022-02-20 12:09 ` [PATCH V3] " Raja Zidane
2022-02-21 10:24 ` Singh, Aman Deep
2022-02-21 12:08 ` Ferruh Yigit
2022-02-21 13:24 ` [PATCH V4] " Raja Zidane
2022-02-21 17:36 ` Ferruh Yigit
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).