RE: [PATCH v4] net/ice: improve performance of RX timestamp offload

["Zhang, Qi Z" <qi.z.zhang@intel.com>]

> -----Original Message-----
> From: Kevin Traynor <ktraynor@redhat.com>
> Sent: Thursday, March 24, 2022 9:45 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Wu, Wenjun1
> <wenjun1.wu@intel.com>; dev@dpdk.org; Yang, Qiming
> <qiming.yang@intel.com>
> Cc: Van Haaren, Harry <harry.van.haaren@intel.com>; Su, Simei
> <simei.su@intel.com>; Luca Boccassi <bluca@debian.org>; Christian Ehrhardt
> <christian.ehrhardt@canonical.com>
> Subject: Re: [PATCH v4] net/ice: improve performance of RX timestamp
> offload
> 
> On 24/03/2022 13:05, Zhang, Qi Z wrote:
> >
> >
> >> -----Original Message-----
> >> From: Kevin Traynor <ktraynor@redhat.com>
> >> Sent: Thursday, March 24, 2022 8:18 PM
> >> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Wu, Wenjun1
> >> <wenjun1.wu@intel.com>; dev@dpdk.org; Yang, Qiming
> >> <qiming.yang@intel.com>
> >> Cc: Van Haaren, Harry <harry.van.haaren@intel.com>; Su, Simei
> >> <simei.su@intel.com>; Luca Boccassi <bluca@debian.org>; Christian
> >> Ehrhardt <christian.ehrhardt@canonical.com>
> >> Subject: Re: [PATCH v4] net/ice: improve performance of RX timestamp
> >> offload
> >>
> >> On 24/03/2022 11:51, Zhang, Qi Z wrote:
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Kevin Traynor <ktraynor@redhat.com>
> >>>> Sent: Thursday, March 24, 2022 7:17 PM
> >>>> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Wu, Wenjun1
> >>>> <wenjun1.wu@intel.com>; dev@dpdk.org; Yang, Qiming
> >>>> <qiming.yang@intel.com>
> >>>> Cc: Van Haaren, Harry <harry.van.haaren@intel.com>; Su, Simei
> >>>> <simei.su@intel.com>; Luca Boccassi <bluca@debian.org>; Christian
> >>>> Ehrhardt <christian.ehrhardt@canonical.com>
> >>>> Subject: Re: [PATCH v4] net/ice: improve performance of RX
> >>>> timestamp offload
> >>>>
> >>>> On 24/03/2022 09:09, Zhang, Qi Z wrote:
> >>>>>
> >>>>>
> >>>>>> -----Original Message-----
> >>>>>> From: Wu, Wenjun1 <wenjun1.wu@intel.com>
> >>>>>> Sent: Monday, February 28, 2022 3:36 PM
> >>>>>> To: dev@dpdk.org; Zhang, Qi Z <qi.z.zhang@intel.com>; Yang,
> >>>>>> Qiming <qiming.yang@intel.com>
> >>>>>> Cc: Van Haaren, Harry <harry.van.haaren@intel.com>; Su, Simei
> >>>>>> <simei.su@intel.com>; Wu, Wenjun1 <wenjun1.wu@intel.com>
> >>>>>> Subject: [PATCH v4] net/ice: improve performance of RX timestamp
> >>>>>> offload
> >>>>>>
> >>>>>> Previously, each time a burst of packets is received, SW reads HW
> >>>>>> register and assembles it and the timestamp from descriptor
> >>>>>> together to get the complete 64 bits timestamp.
> >>>>>>
> >>>>>> This patch optimizes the algorithm. The SW only needs to check
> >>>>>> the monotonicity of the low 32bits timestamp to avoid crossing
> borders.
> >>>>>> Each time before SW receives a burst of packets, it should check
> >>>>>> the time difference between current time and last update time to
> >>>>>> avoid the low 32 bits timestamp cycling twice.
> >>>>>>
> >>>>>> Signed-off-by: Wenjun Wu <wenjun1.wu@intel.com>
> >>>>>
> >>>>> Added cc stable
> >>>>>
> >>>>
> >>>> Hi Qi. The DPDK documentation has guidance about what should be
> >>>> backported to LTS [0] and distinguishes between fixes and
> >>>> performance improvements. Please try and stick with this when
> >>>> applying patches or let LTS maintainers know if there is a debatable
> case.
> >>>
> >>> Thanks for the comments
> >>> Yes, actually this is about a 50% ~ 70% performance improvement,
> >>> which maybe critical for some performance sensitive use cases.(e.g.
> >>> network
> >>> forensics) So I'd like to defend with below case
> >>>
> >>> An existing feature in LTS is not usable as intended without it.
> >>>
> >>
> >> If that is the case, then I think the commitlog should be re-written.
> >> It just talks about the code changes, there's nothing about an impact
> >> to an existing use case that was unusuable and is now fixed.
> >
> > OK, I updated with below commit log in dpdk-next-net-intel.
> > Kevin , Wenjun, let me know if anything I missed.
> >
> >      Previously, each time a burst of packets is received, SW reads HW
> >      register and assembles it and the timestamp from descriptor together to
> >      get the complete 64 bits timestamp.
> >
> >      This patch optimizes the algorithm. The SW only needs to check the
> >      monotonicity of the low 32bits timestamp to avoid crossing borders.
> >      Each time before SW receives a burst of packets, it should check the
> >      time difference between current time and last update time to avoid
> >      the low 32 bits timestamp cycling twice.
> >
> >      The patch proved a 50% ~ 70% single core performance improvement on
> a
> >      main stream Xeon server, and it is necessary to be backport to LTS
> release,
> >      as this fix the performance gap for some use cases.
> >
> 
> The comment about backport to LTS can be removed. That is an operational
> comment, so not really relevant to a future reader trying to understand the
> commit.

Yes, removed the unnecessary comment.

The patch proved a 50% ~ 70% single core performance improvement on a
main stream Xeon server, this fix the performance gap for some use cases.

> 
> thanks,
> Kevin.
> 
> > Thanks
> > Qi
> >
> >>
> >>> Thanks
> >>> Qi
> >>>
> >>>>
> >>>> thanks,
> >>>> Kevin.
> >>>>
> >>>> [0]
> >>>> http://doc.dpdk.org/guides-21.11/contributing/stable.html#what-chan
> >>>> ge
> >>>> s-
> >>>> should-be-backported
> >>>
> >>>>
> >>>>> Acked-by: Qi Zhang <qi.z.zhang@intel.com>
> >>>>>
> >>>>> Applied to dpdk-next-net-intel.
> >>>>>
> >>>>> Thanks
> >>>>> Qi
> >>>>>
> >>>>
> >>>
> >