Hi, replies inline:

On 9/3/2019 11:04 AM, Anoob Joseph wrote:
Hi Radu,

Please see inline.

Thanks,
Anoob

-----Original Message-----
From: dev <dev-boun...@dpdk.org> On Behalf Of Radu Nicolau
Sent: Tuesday, September 3, 2019 3:12 PM
To: dev@dpdk.org
Cc: akhil.go...@nxp.com; konstantin.anan...@intel.com;
bernard.iremon...@intel.com; declan.dohe...@intel.com;
step...@networkplumber.org; Radu Nicolau <radu.nico...@intel.com>
Subject: [dpdk-dev] [PATCH v2] security: add statistics definitions and update
API

Update IPsec statistics struct definition, add per SA statistics collection 
enable
flag.

Signed-off-by: Radu Nicolau <radu.nico...@intel.com>
---
v2: added second reserved field

  lib/librte_security/rte_security.h | 24 ++++++++++++++++++++----
  1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/lib/librte_security/rte_security.h
b/lib/librte_security/rte_security.h
index 96806e3..21bbee2 100644
--- a/lib/librte_security/rte_security.h
+++ b/lib/librte_security/rte_security.h
@@ -172,6 +172,14 @@ struct rte_security_ipsec_sa_options {
         * * 0: Inner/outer header are not modified.
         */
        uint32_t ecn : 1;
+
+       /**< Security statistics
+        *
+        * * 1: Enable per session security statistics collection for
+        *      this SA, if supported by the driver.
+        * * 0: Disable per session security statistics collection for this SA.
+        */
[Anoob] I believe you will have to add the above description after the item. 
Else the documentation generated could end up wrong. Description of all items 
of this structure is actually wrong.
https://doc.dpdk.org/api/structrte__security__ipsec__sa__options.html
It is wrong indeed, I will fix it for the whole struct.
+       uint32_t stats : 1;
  };

  /** IPSec security association direction */ @@ -482,8 +490,14 @@ struct
rte_security_macsec_stats {  };

  struct rte_security_ipsec_stats {
-       uint64_t reserved;
-
+       uint64_t ipackets;  /**< Successfully received IPsec packets. */
+       uint64_t opackets;  /**< Successfully transmitted IPsec packets.*/
+       uint64_t ibytes;    /**< Successfully received IPsec bytes. */
+       uint64_t obytes;    /**< Successfully transmitted IPsec bytes. */
+       uint64_t ierrors;   /**< IPsec packets receive/decrypt errors. */
+       uint64_t oerrors;   /**< IPsec packets transmit/encrypt errors. */
+       uint64_t reserved1; /**< Reserved for future use. */
+       uint64_t reserved2; /**< Reserved for future use. */
  };

  struct rte_security_pdcp_stats {
@@ -507,10 +521,12 @@ struct rte_security_stats {
   *
   * @param     instance        security instance
   * @param     sess            security session
+ * If security session is NULL then global (per security instance)
+ statistics
+ * will be retrieved, if supported
[Anoob] With NULL as security session, do we expect PMDs to return stats for 
all sessions or only for the ones 'stats' is enabled?
We expect global stats, not a sum of per session stats, and independent of the per session configuration - that is, if there are 2 sessions and the per seesion stats are enabled for only one, this will still return the global total. I will make a note of this in the doc.
   * @param     stats           statistics
   * @return
- *  - On success return 0
- *  - On failure errno
+ *  - On success, return 0
+ *  - On failure, a negative value
[Anoob] PMDs which doesn't support this would return ENOTSUP, right? Do you 
think we should document that?
I don't think we need to explicitly document it, as the ENOTSUP is quite self explanatory.
   */
  __rte_experimental
  int
--
2.7.4

Reply via email to