Hi Akhil, Konstantin, Please see inline.
Thanks, Anoob > -----Original Message----- > From: Ananyev, Konstantin <konstantin.anan...@intel.com> > Sent: Monday, November 4, 2019 8:55 PM > To: Akhil Goyal <akhil.go...@nxp.com>; Iremonger, Bernard > <bernard.iremon...@intel.com>; Thomas Monjalon <tho...@monjalon.net> > Cc: dev@dpdk.org; Anoob Joseph <ano...@marvell.com>; Jerin Jacob > Kollanukkaran <jer...@marvell.com>; dpdk-techboard <techbo...@dpdk.org> > Subject: [EXT] RE: [PATCH v2 0/3] examples/ipsec-secgw: set default > > External Email > > ---------------------------------------------------------------------- > Hi Akhil, > > > > > > > > > > > > > > > 11/10/2019 14:40, Akhil Goyal: > > > > > > > > Hi All, > > > > > > > > > > > > > > > > This patchset would need ack from more vendors as it will > > > > > > > > impact user > > > > > > > experience > > > > > > > > on a key example application which is normally > > > > > > > > demonstrated to > > > > > > customers. > > > > > > > > > > > > > > > > IPSec library is still evolving and there are new > > > > > > > > functionality added every > > > > > > > release. > > > > > > > > Atleast from NXP side we are not OK with this change. > > > > > > > > > > > > > > What can be changed in the library to make it acceptable as > > > > > > > a default in this example? > > > > > > > > > > > > > > > > > > > We are observing performance issues with ipsec library. So > > > > > > would request other Vendors to confirm if they are OK with the > > > > > > performance > > > > numbers. > > > > > > > > > > Could you give some details on the performance issues you are seeing. > > > > > > > > > > > > > We were observing about 4-5% drop when using the ipsec-lib instead > > > > of the Legacy code path. We would again measure it on RC1. That is > > > > why I say, I will Hold this patch till RC2, unless some other vendor > > > > also > confirms that. > > > > > > Is there any update on performance measurements on 19.11-rc1 ? > > > > > The performance impact of this patch is huge ~10% w.r.t. 19.11-rc1 base on > NXP hardware. > > > > We cannot merge this. Anoob also reported performance issues on Marvell > hardware. > > Sure, 10% is a lot, so more than understandable. > Though, I think we do need to decide our future goals for it. > I see two main options here: > 1. Make lib code-path on par with legacy one in terms of performance, > deprecate and then remove legacy code-path. > Till that happen (deprecation/removal) to minimize code divergence, > forbid to add new features to legacy code path only. > New features should be added to both paths, or library code path. > Obviously that one looks like a preferred option to me, but it requires some > effort from all interested parties (Intel, NXP, Marvell, ...). > If everyone is ok with it, then I think it would be good to have some draft > timeline here. > If you guys are not interested in this effort, then the only other approach I > can > think about: [Anoob] I would say this is the right option. But then, there are features getting added only to lib ipsec mode. There are features like "fallback session" or anti replay which is only added for lib ipsec mode and not for non lib ipsec mode. Such features are good to have but would cost extra cycles in the datapath. Since it is only added for lib ipsec mode, the perf divergence between lib ipsec mode and non lib ipsec mode is fairly obvious. So what is the solution? Both the modes need to be at par if our end strategy is going to deprecate one. If we need to reach a state where we can do apples to apples comparison, new features should be added to both modes and there should be a consensus on the feature implementations. Also, looking at the "fallback session" feature, the feature was "pushed" without properly addressing the issues. The solution was hardly suitable for inline ipsec and the comments were ignored. Marvell is not ok with adding checks in datapath for an incomplete feature. Marvell withdrew the objections when it was conveyed that the review comments were deemed invaluable. Also because it was added to lib ipsec path which is not being used currently for our benchmarks. Marvell will be submitting an RFC for supporting fallback session with few changes in the rte_security library. When we attempt that, we can take care of only non lib ipsec mode as lib ipsec mode already has one and Marvell cannot work on two different fronts, when the other contributors themselves don't care about other established modes. Also considering that lib ipsec is under constant change, (and is still experimental), I would not be too excited about making it the default immediately. I see that there are usages supported by the non lib ipsec mode, which is not supported by lib ipsec mode. For example, ipv4 & ipv6 using same SPI is valid for non lib ipsec, but not for lib ipsec. Because of this, the default conf doesn't even run when launched with lib ipsec mode. I'm not sure whether we should rush with making it the default when the whole idea is still "experimental". > 2. split ipsec-secgw app into 2 (one using librte_ipsec, second using raw > devices > (legacy one)). > We probably can still try to keep some code shared by 2 apps: > (configuration/initialization/session management (SAD, SPD)), > but actual packet processing path will be different. > I really don't like that option, but I think we need to come-up with clear > decision, one way or another. > > Thanks > Konstantin > > > > >
