[PATCH] app/testpmd: fix GENEVE parsing in csum forward mode
Singh, Aman Deep
aman.deep.singh at intel.com
Tue Feb 15 15:43:58 CET 2022
Hi Raja Zidane,
On 2/15/2022 8:01 PM, Raja Zidane wrote:
>
> Hi all,
> reviving the discussion.
>
> @Beilei Xing <mailto:beilei.xing at 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 at intel.com>
> *Sent:* Monday, January 31, 2022 6:48 PM
> *To:* Raja Zidane <rzidane at nvidia.com>; Matan Azrad
> <matan at nvidia.com>; Ferruh Yigit <ferruh.yigit at intel.com>;
> dev at dpdk.org; Beilei Xing <beilei.xing at intel.com>; Qi Zhang
> <qi.z.zhang at intel.com>
> *Cc:* stable at 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 at intel.com> <mailto:aman.deep.singh at intel.com>
>
> Sent: Thursday, January 20, 2022 12:47 PM
>
> To: Matan Azrad<matan at nvidia.com> <mailto:matan at nvidia.com>; Ferruh Yigit<ferruh.yigit at intel.com> <mailto:ferruh.yigit at intel.com>; Raja Zidane<rzidane at nvidia.com> <mailto:rzidane at nvidia.com>;dev at dpdk.org
>
> Cc:stable at 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 at intel.com> <mailto:ferruh.yigit at intel.com>
>
> Sent: Tuesday, January 18, 2022 3:03 PM
>
> To: Matan Azrad<matan at nvidia.com> <mailto:matan at nvidia.com>; Raja Zidane<rzidane at nvidia.com> <mailto:rzidane at nvidia.com>;
>
> dev at dpdk.org
>
> Cc:stable at 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 at intel.com> <mailto:ferruh.yigit at intel.com>
>
> Sent: Tuesday, January 18, 2022 2:28 PM
>
> To: Matan Azrad<matan at nvidia.com> <mailto:matan at nvidia.com>; Raja Zidane
>
> <rzidane at nvidia.com> <mailto:rzidane at nvidia.com>;dev at dpdk.org
>
> Cc:stable at 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 at intel.com> <mailto:ferruh.yigit at intel.com>
>
> Sent: Tuesday, January 18, 2022 11:52 AM
>
> To: Raja Zidane<rzidane at nvidia.com> <mailto:rzidane at nvidia.com>;dev at dpdk.org
>
> Cc: Matan Azrad<matan at nvidia.com> <mailto:matan at nvidia.com>;stable at 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 at 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 at dpdk.org
>
> Signed-off-by: Raja Zidane<rzidane at nvidia.com> <mailto:rzidane at nvidia.com>
>
> ---
>
> Acked-by: Matan Azrad<matan at nvidia.com> <mailto:matan at nvidia.com>
>
> Ack should be before '---' to be part of the commit log,
>
> otherwise it is dropped when applied as comment.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mails.dpdk.org/archives/stable/attachments/20220215/b30e8f30/attachment.htm>
More information about the stable
mailing list