DPDK patches and discussions
 help / color / mirror / Atom feed
From: Matan Azrad <matan@nvidia.com>
To: Ferruh Yigit <ferruh.yigit@intel.com>,
	Raja Zidane <rzidane@nvidia.com>, "dev@dpdk.org" <dev@dpdk.org>
Cc: "stable@dpdk.org" <stable@dpdk.org>
Subject: RE: [PATCH] app/testpmd: fix GENEVE parsing in csum forward mode
Date: Tue, 18 Jan 2022 13:19:08 +0000	[thread overview]
Message-ID: <DM4PR12MB53890E072ED957BB05E03171DF589@DM4PR12MB5389.namprd12.prod.outlook.com> (raw)
In-Reply-To: <ce053b33-733f-c607-7a8e-2dbeccc27998@intel.com>



> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Tuesday, January 18, 2022 3:03 PM
> To: Matan Azrad <matan@nvidia.com>; Raja Zidane <rzidane@nvidia.com>;
> 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 <ferruh.yigit@intel.com>
> >> Sent: Tuesday, January 18, 2022 2:28 PM
> >> To: Matan Azrad <matan@nvidia.com>; Raja Zidane <rzidane@nvidia.com>;
> >> 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 <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.
> >>>
> >>
> >> 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-send-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😊




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


  reply	other threads:[~2022-01-18 13:19 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-05  3:44 Raja Zidane
2022-01-18  9:51 ` Ferruh Yigit
2022-01-18 11:27   ` Matan Azrad
2022-01-18 12:28     ` Ferruh Yigit
2022-01-18 12:55       ` Matan Azrad
2022-01-18 13:03         ` Ferruh Yigit
2022-01-18 13:19           ` Matan Azrad [this message]
2022-01-20 10:46             ` Singh, Aman Deep
2022-01-30 11:18               ` Raja Zidane
2022-01-31 16:47                 ` Singh, Aman Deep
2022-02-15 14:31                   ` Raja Zidane
2022-02-15 14:43                     ` Singh, Aman Deep
2022-02-16  2:02                     ` Xing, Beilei
2022-02-16 12:37 ` [PATCH V2] " Raja Zidane
2022-02-18  9:09   ` Singh, Aman Deep
2022-02-20 12:09   ` [PATCH V3] " Raja Zidane
2022-02-21 10:24     ` Singh, Aman Deep
2022-02-21 12:08     ` Ferruh Yigit
2022-02-21 13:24     ` [PATCH V4] " Raja Zidane
2022-02-21 17:36       ` Ferruh Yigit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DM4PR12MB53890E072ED957BB05E03171DF589@DM4PR12MB5389.namprd12.prod.outlook.com \
    --to=matan@nvidia.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=rzidane@nvidia.com \
    --cc=stable@dpdk.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).