DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
To: Akhil Goyal <akhil.goyal@nxp.com>, Thomas Monjalon <thomas@monjalon.net>
Cc: "Iremonger, Bernard" <bernard.iremonger@intel.com>,
	"dev@dpdk.org" <dev@dpdk.org>, Anoob Joseph <anoobj@marvell.com>,
	Jerin Jacob Kollanukkaran <jerinj@marvell.com>,
	Narayana Prasad Raju Athreya <pathreya@marvell.com>
Subject: Re: [dpdk-dev] [EXT] [PATCH] doc: deprecate legacy code path in ipsec-secgw
Date: Thu, 1 Aug 2019 07:34:14 +0000	[thread overview]
Message-ID: <2601191342CEEE43887BDE71AB9772580168A60822@irsmsx105.ger.corp.intel.com> (raw)
In-Reply-To: <VE1PR04MB6639AE1AC21A3A0D5C67AF73E6DF0@VE1PR04MB6639.eurprd04.prod.outlook.com>


Hi Akhil,
> >
> > > > > >
> > > > > > 30/07/2019 10:48, Akhil Goyal:
> > > > > > > > 30/07/2019 09:20, Akhil Goyal:
> > > > > > > > > > 30/07/2019 07:55, Akhil Goyal:
> > > > > > > > > > > > > > > All the functionality of the legacy code path in now
> > available
> > > > > > > > > > > > > > > in the librte_ipsec library.
> > > > > > > > > > > > > > > It is planned to deprecate the legacy code path in the
> > 19.11
> > > > > > > > > > > > > > > release and remove the legacy code path in the 20.02
> > > > release.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Signed-off-by: Bernard Iremonger
> > > > > > <bernard.iremonger@intel.com>
> > > > > > > > > > > > > > > Acked-by: Konstantin Ananyev
> > > > <konstantin.ananyev@intel.com>
> > > > > > > > > > > > > > > Acked-by: Fan Zhang <roy.fan.zhang@intel.com>
> > > > > > > > > > > > > > > Acked-by: Akhil Goyal <akhil.goyal@nxp.com>
> > > > > > > > > > > > > > > ---
> > > > > > > > > > > > > > >  doc/guides/rel_notes/deprecation.rst | 5 +++++
> > > > > > > > > > > > > > >  1 file changed, 5 insertions(+)
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > Acked-by: Anoob Joseph <anoobj@marvell.com>
> > > > > > > > > > > > >
> > > > > > > > > > > > > Applied to dpdk-next-crypto
> > > > > > > > > > > >
> > > > > > > > > > > > Why do we have a deprecation notice for some code path in
> > an
> > > > > > example?
> > > > > > > > > > > > The deprecation notices are for the API.
> > > > > > > > > > > >
> > > > > > > > > > > > I think you can drop the legacy code in 19.11,
> > > > > > > > > > > > and I don't merge this patch in master.
> > > > > > > > > > >
> > > > > > > > > > > We are planning to remove the original code and replace it with
> > > > IPSec
> > > > > > > > > > > library APIs which are still experimental.
> > > > > > > > > > > With this change there won't be any example of the legacy ipsec
> > > > code
> > > > > > path.
> > > > > > > >
> > > > > > > > That's good to drop old code.
> > > > > > > > If someone still wants to look at it, it is in old releases.
> > > > > > > >
> > > > > > > > > > > Applications over DPDK take ipsec-secgw as an example and
> > IPSec
> > > > > > > > > > > is a major use case for customers. There may also be
> > performance
> > > > > > > > > > > differences in the two code paths. Atleast on NXP platforms I
> > saw
> > > > > > > > > > > 5-7% drop when the patches were originally submitted.
> > > > > > > > > > > Not sure what is the current state.
> > > > > > > >
> > > > > > > > That's a different issue you need to solve in the library.
> > > > > > > >
> > > > > > > > > > > I feel it is worth notifying the users that the original codepath is
> > > > > > > > > > > getting deprecated, so that they can plan to move to new IPSec
> > > > APIs.
> > > > > > > >
> > > > > > > > I hope they already planned to move when they saw the new library.
> > > > > > > >
> > > > > > > > > > The deprecation notice is not the right place for a change in an
> > > > example.
> > > > > > > > > > What change is there in IPsec API? In which release?
> > > > > > > > >
> > > > > > > > > IPSec lib was introduced in 1902 release and a few enhancements
> > > > > > > > > are done thereafter.
> > > > > > > > > Previously all IPSec related stuff was done in the application,
> > > > > > > > > now we have IPSec Lib which perform similar work.
> > > > > > > > > There are changes both in datapath as well as control path.
> > > > > > > > > User need to adapt to the recent changes, as we may no longer
> > > > > > > > > support/maintain the datapath/control path which was done
> > previously
> > > > > > > > > and there may be some conflict.
> > > > > > > >
> > > > > > > > So the real DPDK change is to have a new library in 19.02.
> > > > > > > >
> > > > > > > > > If deprecation notice is not the right place,
> > > > > > > > > then where should it be notified before actually making the change.
> > > > > > > >
> > > > > > > > It has already been notified in "New Features" of 19.02
> > > > > > > > that there is an IPsec library. What do you want to notify more?
> > > > > > > > Again, the example is not supposed to be a real application.
> > > > > > > > If you want to maintain an IPsec application with better quality rules,
> > > > > > > > I suggest to start a new git repository for it.
> > > > > > >
> > > > > > > OK got your point, but in that case, I would say, legacy code shall not
> > be
> > > > > > removed
> > > > > > > Until we have the ipsec lib as experimental.
> > > > > > > User should have both the code paths as long as we have ipsec library
> > > > > > experimental.
> > > >
> > > > Not sure why it is needed?
> > > There is some performance gap.
> >
> > Ok, and you think that keeping legacy code-path  till 20.02
> > will provide enough time to analyze the perf drop for NXP devices?
> 
> Yes. We cannot commit for 19.11 timeframe to fix the performance issues in
> the new code. You can send the patches, we can try our best to incorporate it in
> 19.11 but we cannot commit at this moment.

Ok, then let's put 20.02 as a preliminary target for removal experimental tag and legacy code-path.
As you said, we can later consider to backport ipsec-secgw changes into 19.11.1 or so.
Konstantin


> 
> >
> > > Original Idea for ipsec lib was to have
> > > better performing APIs.
> >
> > Not only.
> > Main reasons were to hide from user complexity of IPsec packet processing
> > and avoid necessity for each user to re-implement it.
> > Also legacy code doesn't provide many important features
> > (replay window, ESN, multi-seg support, etc.)
> >
> > > How much gain is observed using these APIs on intel?
> >
> > It depends.
> > From my last observations for inline case library code-path is 10-15%
> > faster than legacy one. Though I think reason for that in not in the library
> > itself, but that legacy code handles inline traffic in suboptimal way.
> > For lookaside - in most cases library and legacy code performance is about the
> > same,
> > except the case when we have a burst of packets where each packet belongs to
> > different SA.
> > In that case new code-path is 3-5% slower.
> > As I understand, main reason is again not the library itself,
> > but that in new code we try to group packets that belongs
> > to the same SA both before and after crypto-dev enqueue/dequeue.
> > Usually that helps, but for that case it just adds extra overhead (no grouping is
> > possible).
> >
> > > Do we have data? Atleast on NXP devices, these are not performing better.
> > >
> > > > Why DPDK sample app can't use DPDK experimental API as it is,
> > > > without some alternate code-path?
> > >
> > > Sample app can use experimental APIs as is. There is no issue with that.
> > > The intent is just to notify the users that it will be removed so that they
> > > can be ready for change in their code.
> >
> > Ok, but if we don't submit deprecation notice (as Thomas suggested)
> > how they'll be notified?
> As Thomas said, there is no need for sending the deprecation notice for application.
> So I have to hold my comment back.
> 
> > Would there be a separate RN when we'll remove experimental tag
> > and legacy code?
> There will be a RN entry when we remove the experimental tag and the legacy code.
> >
> > >
> > > >
> > > > > >
> > > > > > That's your take.
> > > > > > When do you plan to remove experimental status of IPsec library?
> > > > > >
> > > > > There have been addition of some functionality in this release cycle. I
> > would
> > > > say we
> > > > > can wait for 1 release cycle for some fixes or changes which may be
> > required.
> > > > > If it looks stable in next release cycle, we can make formal in DPDK 2002.
> > > >
> > > > If we'll leave legacy code in 19.11, does it mean we'll have to
> > > > support it for next 2 years (LTS cycle)?
> > >
> > > The original intent of this patch was also to remove the legacy code in 2002
> > release and
> > > not in 1911. Moreover, it is the application code that we need to remove. We
> > can get that patch
> > > backported to the 19.11.1 release as well. Then it would be same.
> > >
> >
> >
> >


      reply	other threads:[~2019-08-01  7:34 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-11  9:05 [dpdk-dev] [RFC] " Bernard Iremonger
2019-07-19 11:23 ` Ananyev, Konstantin
2019-07-19 14:55 ` Zhang, Roy Fan
2019-07-24  9:47 ` Akhil Goyal
2019-07-24 10:12 ` [dpdk-dev] [PATCH] " Bernard Iremonger
2019-07-24 10:14   ` Akhil Goyal
2019-07-26 14:06     ` Akhil Goyal
2019-07-26 14:45   ` [dpdk-dev] [EXT] " Anoob Joseph
2019-07-29  8:49     ` Akhil Goyal
2019-07-29 16:33       ` Thomas Monjalon
2019-07-29 20:07         ` Ananyev, Konstantin
2019-07-30  5:55         ` Akhil Goyal
2019-07-30  7:09           ` Thomas Monjalon
2019-07-30  7:20             ` Akhil Goyal
2019-07-30  7:54               ` Thomas Monjalon
2019-07-30  8:48                 ` Akhil Goyal
2019-07-30  8:56                   ` Thomas Monjalon
2019-07-30  9:03                     ` Akhil Goyal
2019-07-30  9:25                       ` Ananyev, Konstantin
2019-07-30 11:06                         ` Akhil Goyal
2019-07-31 10:19                           ` Ananyev, Konstantin
2019-07-31 11:26                             ` Akhil Goyal
2019-08-01  7:34                               ` Ananyev, Konstantin [this message]

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=2601191342CEEE43887BDE71AB9772580168A60822@irsmsx105.ger.corp.intel.com \
    --to=konstantin.ananyev@intel.com \
    --cc=akhil.goyal@nxp.com \
    --cc=anoobj@marvell.com \
    --cc=bernard.iremonger@intel.com \
    --cc=dev@dpdk.org \
    --cc=jerinj@marvell.com \
    --cc=pathreya@marvell.com \
    --cc=thomas@monjalon.net \
    /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).