Re: [dpdk-dev] [PATCH] net/ixgbe: add security statistics

["Ananyev, Konstantin" <konstantin.ananyev@intel.com>]

Hi Radu,

> >
> >> Update IXGBE PMD with support for IPsec statistics.
> > The proposed approach - have a new hash table per device,
> > plus parse  each packet on RX and TX and do hash lookup
> > seems way too heavy to put it into PMD.
> 
> It is indeed heavy, but it's disabled by default and it only uses the
> heavy section when the app is enabling per-session statistics

As I can read  the code, disruption will be much more significant - 
even if user will enable statistics just for one SA (which might never be used),
that will affect whole device: all RX (or all TX) queues will experience a slowdown.
Even RX/TX for queues without any IPsec packets will be affected
(though not that severe as ones with mix of IPsec and non-IPsec or pure IPsec packets).   

> 
> > Wonder why we need to do that?
> > If HW doesn't provide such statistic info, why not to leave it
> > to the upper layer to collect/maintain?
> > In many cases SW will already have that infrastructure
> > and I don't see to put all these heavy-weight operations
> > into ixgbe rx/tx path.
> 
> The reason is to use the rte_security API consistently and take
> advantage of the hardware backed statistics when they are available.

AFAIK, right now in rte_security API there is no option to request statistics per session,
and struct rte_security_ipsec_stats is literally empty.
That's an extension you proposed by the patch:
http://patches.dpdk.org/patch/58462/
But as I can see it is not backed up by any real HW implementation.
Yours current patch provides pure SW implementation,
and I don't see absolutely no reason to push it into the ixgbe PMD.
Same statistics could be collected by dozen different and probably more effective ways without affecting the actual ixgbe driver
(RX/TX callback or  new option in librte_ipsec or new function that would be called by app directly or ...).
So my suggestion let's postpone introducing new API here till we'll have real HW that supports it.

Konstantin