Hi Raja Zidane,

On 2/15/2022 8:01 PM, Raja Zidane wrote:

Hi all,
reviving the discussion.

@Beilei Xing <mailto:beilei.x...@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.si...@intel.com>
*Sent:* Monday, January 31, 2022 6:48 PM
*To:* Raja Zidane <rzid...@nvidia.com>; Matan Azrad <ma...@nvidia.com>; Ferruh Yigit <ferruh.yi...@intel.com>; dev@dpdk.org; Beilei Xing <beilei.x...@intel.com>; Qi Zhang <qi.z.zh...@intel.com>
*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/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.si...@intel.com> <mailto:aman.deep.si...@intel.com>
    Sent: Thursday, January 20, 2022 12:47 PM

    To: Matan Azrad<ma...@nvidia.com>  <mailto:ma...@nvidia.com>; Ferruh 
Yigit<ferruh.yi...@intel.com>  <mailto:ferruh.yi...@intel.com>; Raja Zidane<rzid...@nvidia.com>  
<mailto: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 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.yi...@intel.com>  
<mailto:ferruh.yi...@intel.com>

            Sent: Tuesday, January 18, 2022 3:03 PM

            To: Matan Azrad<ma...@nvidia.com>  <mailto:ma...@nvidia.com>; Raja 
Zidane<rzid...@nvidia.com>  <mailto: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 12:55 PM, Matan Azrad wrote:

                    -----Original Message-----

                    From: Ferruh Yigit<ferruh.yi...@intel.com>  
<mailto:ferruh.yi...@intel.com>

                    Sent: Tuesday, January 18, 2022 2:28 PM

                    To: Matan Azrad<ma...@nvidia.com>  
<mailto:ma...@nvidia.com>; Raja Zidane

                    <rzid...@nvidia.com>  
<mailto: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>  
<mailto:ferruh.yi...@intel.com>

                            Sent: Tuesday, January 18, 2022 11:52 AM

                            To: Raja Zidane<rzid...@nvidia.com>  
<mailto:rzid...@nvidia.com>;dev@dpdk.org

                            Cc: Matan Azrad<ma...@nvidia.com>  
<mailto: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.

            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.m...@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:sta...@dpdk.org

                                Signed-off-by: Raja Zidane<rzid...@nvidia.com>  
<mailto:rzid...@nvidia.com>

                                ---

                                Acked-by: Matan Azrad<ma...@nvidia.com>  
<mailto:ma...@nvidia.com>

                            Ack should be before '---' to be part of the commit 
log,

                            otherwise it is dropped when applied as comment.

Reply via email to