DPDK patches and discussions
 help / color / mirror / Atom feed
From: Kevin Traynor <ktraynor@redhat.com>
To: "Zeng, ZhichaoX" <zhichaox.zeng@intel.com>,
	"Xu, HailinX" <hailinx.xu@intel.com>,
	Xueming Li <xuemingl@nvidia.com>,
	"stable@dpdk.org" <stable@dpdk.org>,
	"Wu, Jingjing" <jingjing.wu@intel.com>,
	"Xing, Beilei" <beilei.xing@intel.com>,
	"Xu, Ke1" <ke1.xu@intel.com>,
	"Zhang, Qi Z" <qi.z.zhang@intel.com>
Cc: "xuemingl@nvdia.com" <xuemingl@nvdia.com>,
	"dev@dpdk.org" <dev@dpdk.org>,
	 "Stokes, Ian" <ian.stokes@intel.com>,
	"Mcnamara, John" <john.mcnamara@intel.com>,
	Luca Boccassi <bluca@debian.org>,
	"Xu, Qian Q" <qian.q.xu@intel.com>,
	Thomas Monjalon <thomas@monjalon.net>,
	"Peng, Yuan" <yuan.peng@intel.com>,
	"Chen, Zhaoyan" <zhaoyan.chen@intel.com>
Subject: Re: 22.11.3 patches review and test
Date: Mon, 4 Sep 2023 15:15:26 +0100	[thread overview]
Message-ID: <307bf0a5-b0a6-b602-64b8-8eff3dcf3905@redhat.com> (raw)
In-Reply-To: <ef36b678-3ed2-f543-a3b5-850a98bf867c@redhat.com>

On 04/09/2023 10:32, Kevin Traynor wrote:
> On 01/09/2023 04:23, Zeng, ZhichaoX wrote:
>>> -----Original Message-----
>>> From: Kevin Traynor <ktraynor@redhat.com>
>>> Sent: Thursday, August 31, 2023 8:18 PM
>>> To: Xu, HailinX <hailinx.xu@intel.com>; Xueming Li <xuemingl@nvidia.com>;
>>> stable@dpdk.org; Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei
>>> <beilei.xing@intel.com>; Xu, Ke1 <ke1.xu@intel.com>; Zeng, ZhichaoX
>>> <zhichaox.zeng@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
>>> Cc: xuemingl@nvdia.com; dev@dpdk.org; Stokes, Ian <ian.stokes@intel.com>;
>>> Mcnamara, John <john.mcnamara@intel.com>; Luca Boccassi
>>> <bluca@debian.org>; Xu, Qian Q <qian.q.xu@intel.com>; Thomas Monjalon
>>> <thomas@monjalon.net>; Peng, Yuan <yuan.peng@intel.com>; Chen,
>>> Zhaoyan <zhaoyan.chen@intel.com>
>>> Subject: Re: 22.11.3 patches review and test
>>>
>>> On 30/08/2023 17:25, Kevin Traynor wrote:
>>>> On 29/08/2023 09:22, Xu, HailinX wrote:
>>>>>> -----Original Message-----
>>>>>> From: Xueming Li <xuemingl@nvidia.com>
>>>>>> Sent: Thursday, August 17, 2023 2:14 PM
>>>>>> To: stable@dpdk.org
>>>>>> Cc: xuemingl@nvdia.com; dev@dpdk.org; Abhishek Marathe
>>>>>> <Abhishek.Marathe@microsoft.com>; Ali Alnubani <alialnu@nvidia.com>;
>>>>>> Walker, Benjamin <benjamin.walker@intel.com>; David Christensen
>>>>>> <drc@linux.vnet.ibm.com>; Hemant Agrawal
>>> <hemant.agrawal@nxp.com>;
>>>>>> Stokes, Ian <ian.stokes@intel.com>; Jerin Jacob
>>>>>> <jerinj@marvell.com>; Mcnamara, John <john.mcnamara@intel.com>;
>>>>>> Ju-Hyoung Lee <juhlee@microsoft.com>; Kevin Traynor
>>>>>> <ktraynor@redhat.com>; Luca Boccassi <bluca@debian.org>; Pei Zhang
>>>>>> <pezhang@redhat.com>; Xu, Qian Q <qian.q.xu@intel.com>; Raslan
>>>>>> Darawsheh <rasland@nvidia.com>; Thomas Monjalon
>>>>>> <thomas@monjalon.net>; Yanghang Liu <yanghliu@redhat.com>; Peng,
>>>>>> Yuan <yuan.peng@intel.com>; Chen, Zhaoyan
>>> <zhaoyan.chen@intel.com>
>>>>>> Subject: 22.11.3 patches review and test
>>>>>>
>>>>>> Hi all,
>>>>>>
>>>>>> Here is a list of patches targeted for stable release 22.11.3.
>>>>>>
>>>>>> The planned date for the final release is 31th August.
>>>>>>
>>>>>> Please help with testing and validation of your use cases and report
>>>>>> any issues/results with reply-all to this mail. For the final
>>>>>> release the fixes and reported validations will be added to the release
>>> notes.
>>>>>>
>>>>>> A release candidate tarball can be found at:
>>>>>>
>>>>>>         https://dpdk.org/browse/dpdk-stable/tag/?id=v22.11.3-rc1
>>>>>>
>>>>>> These patches are located at branch 22.11 of dpdk-stable repo:
>>>>>>         https://dpdk.org/browse/dpdk-stable/
>>>>>>
>>>>>> Thanks.
>>>>>
>>>>> We are conducting DPDK testing and have found two issues.
>>>>>
>>>>> 1. The startup speed of testpmd is significantly slower in the os of SUSE
>>>>>       This issue fix patch has been merged into main, But it has not backported
>>> to 22.11.3.
>>>>>       Fix patch commit id on DPDK main:
>>>>> 7e7b6762eac292e78c77ad37ec0973c0c944b845
>>>>>
>>>>> 2. The SCTP tunnel packet of iavf cannot be forwarded in avx512 mode
>>>
>>> Need to clarify this sentence. It looks like it is not a functional bug where
>>> avx512 mode is selected and then an SCTP tunnel packet cannot be sent.
>>> Instead, it is a possible performance issue that avx512 mode will not be
>>> selected where it might have been due to unneeded additions
>>> (RTE_ETH_TX_OFFLOAD_*_TNL_TSO) to IAVF_TX_NO_VECTOR_FLAGS.
>>>
>>> @IAVF maintainers - please confirm my analysis is correct ?
>>>
>>> In that case, as it is a possible performance issue in a specific case for a single
>>> driver I think it is non-critical for 21.11 and we can just revert the patch on the
>>> branch and wait for 21.11.6 release in December.
>>
>> Hi Kevin,
>>
>> Since the LTS version of the IAVF driver does not support avx512 checksum offload,
>> the scalar path should be selected, but this patch makes it incorrectly select the
>> avx512 path, and the SCTP tunnel packets can't be forwarded properly.
>>
> 
> ok, let's take a look at the patch and usage.
> 
> diff --git a/drivers/net/iavf/iavf_rxtx.h b/drivers/net/iavf/iavf_rxtx.h
> index 8d4a77271a..605ea3f824 100644
> --- a/drivers/net/iavf/iavf_rxtx.h
> +++ b/drivers/net/iavf/iavf_rxtx.h
> @@ -32,4 +32,8 @@
>                   RTE_ETH_TX_OFFLOAD_MULTI_SEGS |          \
>                   RTE_ETH_TX_OFFLOAD_TCP_TSO |             \
> +               RTE_ETH_TX_OFFLOAD_VXLAN_TNL_TSO |       \
> +               RTE_ETH_TX_OFFLOAD_GRE_TNL_TSO |         \
> +               RTE_ETH_TX_OFFLOAD_IPIP_TNL_TSO |        \
> +               RTE_ETH_TX_OFFLOAD_GENEVE_TNL_TSO |      \
>                   RTE_ETH_TX_OFFLOAD_SECURITY)
> 
> <snip>
> 
> So we now have:
> #define IAVF_TX_NO_VECTOR_FLAGS (				 \
> 		RTE_ETH_TX_OFFLOAD_VLAN_INSERT |		 \
> 		RTE_ETH_TX_OFFLOAD_QINQ_INSERT |		 \
> 		RTE_ETH_TX_OFFLOAD_MULTI_SEGS |		 \
> 		RTE_ETH_TX_OFFLOAD_TCP_TSO |		 \
> 		RTE_ETH_TX_OFFLOAD_VXLAN_TNL_TSO |	 \
> 		RTE_ETH_TX_OFFLOAD_GRE_TNL_TSO |	 \
> 		RTE_ETH_TX_OFFLOAD_IPIP_TNL_TSO |	 \
> 		RTE_ETH_TX_OFFLOAD_GENEVE_TNL_TSO |	 \
> 		RTE_ETH_TX_OFFLOAD_SECURITY)
> 
> <snip>
> 
> static inline int
> iavf_tx_vec_queue_default(struct iavf_tx_queue *txq)
> {
> 	if (!txq)
> 		return -1;
> 
> 	if (txq->rs_thresh < IAVF_VPMD_TX_MAX_BURST ||
> 	    txq->rs_thresh > IAVF_VPMD_TX_MAX_FREE_BUF)
> 		return -1;
> 
> 	if (txq->offloads & IAVF_TX_NO_VECTOR_FLAGS)
> 		return -1;
>               ^^^ Adding the extra bits to IAVF_TX_NO_VECTOR_FLAGS gives
> *more* reasons for why this statement might not be true, so returning -1
> indicating that vector cannot be used for tx queue
> 

typo here - just to clarify, the new flags give more reasons for this 
statement to be true, so returning -1.

> 
> <snip>
> 
> static inline bool
> check_tx_vec_allow(struct iavf_tx_queue *txq)
> {
> 	if (!(txq->offloads & IAVF_TX_NO_VECTOR_FLAGS) &&
> 
>               ^^^ Adding the extra bits to IAVF_TX_NO_VECTOR_FLAGS gives
> *more* reason for this statement will be false and then false returned
> indicating that vector cannot be used.
> 
> 	    txq->rs_thresh >= IAVF_VPMD_TX_MAX_BURST &&
> 	    txq->rs_thresh <= IAVF_VPMD_TX_MAX_FREE_BUF) {
> 		PMD_INIT_LOG(DEBUG, "Vector tx can be enabled on this txq.");
> 		return true;
> 	}
> 	PMD_INIT_LOG(DEBUG, "Vector Tx cannot be enabled on this txq.");
> 	return false;
> }
> 
> --
> 
> It looks like that adding the extra bits gives it less reasons to select
> vector mode. However, you are saying that this patch means there is a
> case where it now selects vector where it should not, meaning additional
> reason to select vector mode. So maybe I miss something ?
> 
>> Yes, we can revert this commit for 21.11.6 release, thanks.
>>
>> Regards
>> Zhichao
>>
>>> thanks,
>>> Kevin.
>>>
>>>>>       commit 9b7215f150d0bfc5cb00fce68ff08e5217c7f2b3 on v22.11.3-
>>> rc1.
>>>>>       This commit is for the new feature (avx512 checksum offload) in DPDK
>>> 23.03, which should not be backported to the LTS version since avx512
>>> checksum offload is not supported in v22.11.3 LTS.
>>>>>
>>>>
>>>> Thanks for flagging Xueming.
>>>>
>>>> The issue is that it was listed as fixing 059f18ae2aec ("net/iavf: add
>>>> offload path for Tx AVX512") which goes back to 21.05.
>>>>
>>>> This could have been avoided if the 'Fixes:' tag was correct, or if
>>>> the authors replied to the email about queued backports :/
>>>>
>>>> Requesting iavf/next-net-intel maintainers to check Fixes: tags are
>>>> correct before merging.
>>>>
>>>> DPDK 21.11.5 is already released with this patch. Any idea why it did
>>>> not show up in validation for 21.11.5 ? Is it an issue for 21.11.5 ?
>>>> How critical is it ?
>>>>
>>>> I can revert it on the 21.11 branch, but it will need to wait until
>>>> 21.11.6 in December before it will be reverted in a released version.
>>>>
>>>> thanks,
>>>> Kevin.
>>>>
>>>>> Regards,
>>>>> Xu, Hailin
>>>>>
>>>>
>>
> 


  reply	other threads:[~2023-09-04 14:15 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-17  6:13 Xueming Li
2023-08-24 16:02 ` Ali Alnubani
2023-08-25  6:41   ` Xueming(Steven) Li
2023-08-29  8:22 ` Xu, HailinX
2023-08-29  8:56   ` Xueming(Steven) Li
2023-08-31  1:50     ` Xu, HailinX
2023-08-31  6:36       ` Xueming(Steven) Li
2023-08-30 16:25   ` Kevin Traynor
2023-08-31 12:17     ` Kevin Traynor
2023-09-01  3:23       ` Zeng, ZhichaoX
2023-09-04  9:32         ` Kevin Traynor
2023-09-04 14:15           ` Kevin Traynor [this message]
2023-09-05  1:51             ` Zeng, ZhichaoX
2023-09-05  8:49               ` Kevin Traynor
2023-09-05  9:18                 ` Xueming(Steven) Li
2023-09-05  5:37             ` Zeng, ZhichaoX
2023-08-29 16:03 ` YangHang Liu
2023-08-30  5:52   ` Xueming(Steven) Li

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=307bf0a5-b0a6-b602-64b8-8eff3dcf3905@redhat.com \
    --to=ktraynor@redhat.com \
    --cc=beilei.xing@intel.com \
    --cc=bluca@debian.org \
    --cc=dev@dpdk.org \
    --cc=hailinx.xu@intel.com \
    --cc=ian.stokes@intel.com \
    --cc=jingjing.wu@intel.com \
    --cc=john.mcnamara@intel.com \
    --cc=ke1.xu@intel.com \
    --cc=qi.z.zhang@intel.com \
    --cc=qian.q.xu@intel.com \
    --cc=stable@dpdk.org \
    --cc=thomas@monjalon.net \
    --cc=xuemingl@nvdia.com \
    --cc=xuemingl@nvidia.com \
    --cc=yuan.peng@intel.com \
    --cc=zhaoyan.chen@intel.com \
    --cc=zhichaox.zeng@intel.com \
    /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).