Hi Raja Zidane, On 2/15/2022 8:01 PM, Raja Zidane wrote: > > Hi all, > reviving the discussion. > > @Beilei Xing 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 > *Sent:* Monday, January 31, 2022 6:48 PM > *To:* Raja Zidane ; Matan Azrad > ; Ferruh Yigit ; > dev@dpdk.org; Beilei Xing ; Qi Zhang > > *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 > > Sent: Thursday, January 20, 2022 12:47 PM > > To: Matan Azrad ; Ferruh Yigit ; Raja Zidane ;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 > > Sent: Tuesday, January 18, 2022 3:03 PM > > To: Matan Azrad ; Raja Zidane ; > > 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 > > Sent: Tuesday, January 18, 2022 2:28 PM > > To: Matan Azrad ; Raja Zidane > > ;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 > > Sent: Tuesday, January 18, 2022 11:52 AM > > To: Raja Zidane ;dev@dpdk.org > > Cc: Matan Azrad ;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 > > --- > > Acked-by: Matan Azrad > > Ack should be before '---' to be part of the commit log, > > otherwise it is dropped when applied as comment. >