RE: [PATCH] app/testpmd: fix GENEVE parsing in csum forward mode

[Matan Azrad <matan@nvidia.com>]

> -----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) {