Re: [dpdk-dev] [PATCH v2 2/2] examples/ipsec-secgw: attach session-qp

[Akhil Goyal]

On 3/23/2017 4:13 PM, Sergio Gonzalez Monroy wrote:
On 23/03/2017 10:38, Akhil Goyal wrote:
On 3/23/2017 4:00 PM, Sergio Gonzalez Monroy wrote:
That was simpler than I thought.
For some reason I understood that the device will support thousands of
queues and single session per queue which would have needed more app
changes.

On 23/03/2017 08:06, akhil.go...@nxp.com wrote:
From: Akhil Goyal <akhil.go...@nxp.com>

adding support for attaching session to queue pairs.
This is required as underlying crypto driver may only
support limited number of sessions per queue pair
if max_nb_sessions_per_qp > 0, session should be
attached to a particular qp.

Signed-off-by: Akhil Goyal <akhil.go...@nxp.com>
---
  examples/ipsec-secgw/ipsec.c | 12 ++++++++++++
  1 file changed, 12 insertions(+)

diff --git a/examples/ipsec-secgw/ipsec.c
b/examples/ipsec-secgw/ipsec.c
index 144f0aa..b35b30f 100644
--- a/examples/ipsec-secgw/ipsec.c
+++ b/examples/ipsec-secgw/ipsec.c
@@ -47,6 +47,7 @@
  static inline int
  create_session(struct ipsec_ctx *ipsec_ctx __rte_unused, struct
ipsec_sa *sa)
  {
+    struct rte_cryptodev_info cdev_info;
      unsigned long cdev_id_qp = 0;
      int32_t ret;
      struct cdev_key key = { 0 };
@@ -73,6 +74,17 @@ create_session(struct ipsec_ctx *ipsec_ctx
__rte_unused, struct ipsec_sa *sa)
      sa->crypto_session = rte_cryptodev_sym_session_create(
              ipsec_ctx->tbl[cdev_id_qp].id, sa->xforms);
  + rte_cryptodev_info_get(ipsec_ctx->tbl[cdev_id_qp].id, &cdev_info);
+    if (cdev_info.sym.max_nb_sessions_per_qp > 0) {
+        ret = rte_cryptodev_queue_pair_attach_sym_session(
+                ipsec_ctx->tbl[cdev_id_qp].qp,
+                sa->crypto_session);
+        if (ret < 0) {
+            RTE_LOG(ERR, IPSEC, "Session cannot be attached"
+                " to qp %u ", ipsec_ctx->tbl[cdev_id_qp].qp);

Guideline is to keep error strings in single line to facilitate grep.
Other than that:

Acked-by: Sergio Gonzalez Monroy <sergio.gonzalez.mon...@intel.com>

+            return -1;
+        }
+    }
      sa->cdev_id_qp = cdev_id_qp;
        return 0;



Hi Sergio,

Similar error string is mentioned above my change also in the same
function. I deliberately did that as per the error strings in the file.

Thanks,
Akhil


That means that we need to update those strings too, but new code should
try to keep error string in single line.

Thanks,
Sergio

Ok I would correct the string. Will send the v3 with this change only if 
there is no more comments from Fiona.

Thanks,
Akhil