> -----Original Message-----
> From: Ferruh Yigit <ferruh.yi...@intel.com>
> Sent: Tuesday, January 18, 2022 2:28 PM
> To: Matan Azrad <ma...@nvidia.com>; Raja Zidane <rzid...@nvidia.com>;
> dev@dpdk.org
> Cc: sta...@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.yi...@intel.com>
> >> Sent: Tuesday, January 18, 2022 11:52 AM
> >> To: Raja Zidane <rzid...@nvidia.com>; dev@dpdk.org
> >> Cc: Matan Azrad <ma...@nvidia.com>; sta...@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: sta...@dpdk.org
> >>>
> >>> Signed-off-by: Raja Zidane <rzid...@nvidia.com>
> >>> ---
> >>> Acked-by: Matan Azrad <ma...@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) {
> >
