All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC 0/3] set protocol specific metadata using set_pkt_metadata API
@ 2018-01-22 13:11 Anoob Joseph
  2018-01-22 13:11 ` [RFC 1/3] lib/security: set/retrieve per packet protocol metadata Anoob Joseph
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Anoob Joseph @ 2018-01-22 13:11 UTC (permalink / raw)
  To: Akhil Goyal, Declan Doherty, Radu Nicolau, Sergio Gonzalez Monroy
  Cc: Anoob Joseph, Jerin Jacob, Narayana Prasad, Nelio Laranjeiro, dev

This series adds support for setting & retrieving per packet protocol specific
metadata. This is primarily required by the application to monitor sequence
number overflows in inline protocol processing.

The feature is added to the existing set_pkt_metadata API. The existing API
passes all arguments directly. This series introduces a new structure which
could be used to pass all metadata required in such cases.

The patch set adds the ability to both set & retrieve such parameters. The idea
is to make the application determine the sequence number to be used, where it
is supported. If the PMD doesn't support it that way (as in the parameters are
maintained by PMD/device), then application could just retrieve the value and
see if there is any overflow etc happening.

SA expiry/overflow monitoring requires knowing the latest sequence number
on an SA. So this change allows that ability - for now for the outbound SA.

Anoob Joseph (3):
  lib/security: set/retrieve per packet protocol metadata
  net/ixgbe: use structure for passing metadata
  examples/ipsec-secgw: support for setting seq no

 drivers/net/ixgbe/ixgbe_ipsec.c           |  5 ++-
 examples/ipsec-secgw/esp.h                |  9 +++++
 examples/ipsec-secgw/ipsec.c              | 42 +++++++++++++++++---
 lib/librte_security/rte_security.c        |  7 ++--
 lib/librte_security/rte_security.h        | 66 ++++++++++++++++++++++++++++---
 lib/librte_security/rte_security_driver.h |  3 +-
 6 files changed, 112 insertions(+), 20 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC 1/3] lib/security: set/retrieve per packet protocol metadata
  2018-01-22 13:11 [RFC 0/3] set protocol specific metadata using set_pkt_metadata API Anoob Joseph
@ 2018-01-22 13:11 ` Anoob Joseph
  2018-01-22 13:11 ` [RFC 2/3] net/ixgbe: use structure for passing metadata Anoob Joseph
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 14+ messages in thread
From: Anoob Joseph @ 2018-01-22 13:11 UTC (permalink / raw)
  To: Akhil Goyal, Declan Doherty, Radu Nicolau, Sergio Gonzalez Monroy
  Cc: Anoob Joseph, Jerin Jacob, Narayana Prasad, Nelio Laranjeiro, dev

This patch enables the application to set & retrieve per packet protocol
parameters like seq no, which is required in case of protocol offload. The
ability to set/retrieve such data is PMD dependent and the application is
expected to use "mdata_flags" while using such fields.

Retrieving the sequence number is required to monitor the sequence
number overflow in inline IPsec offload.

Signed-off-by: Anoob Joseph <anoob.joseph@caviumnetworks.com>
---
 lib/librte_security/rte_security.c        |  7 ++--
 lib/librte_security/rte_security.h        | 66 ++++++++++++++++++++++++++++---
 lib/librte_security/rte_security_driver.h |  3 +-
 3 files changed, 64 insertions(+), 12 deletions(-)

diff --git a/lib/librte_security/rte_security.c b/lib/librte_security/rte_security.c
index 5805051..508046b 100644
--- a/lib/librte_security/rte_security.c
+++ b/lib/librte_security/rte_security.c
@@ -100,12 +100,11 @@ rte_security_session_destroy(struct rte_security_ctx *instance,
 
 int
 rte_security_set_pkt_metadata(struct rte_security_ctx *instance,
-			      struct rte_security_session *sess,
-			      struct rte_mbuf *m, void *params)
+			      struct rte_security_mdata *mdata,
+			      struct rte_mbuf *m)
 {
 	RTE_FUNC_PTR_OR_ERR_RET(*instance->ops->set_pkt_metadata, -ENOTSUP);
-	return instance->ops->set_pkt_metadata(instance->device,
-					       sess, m, params);
+	return instance->ops->set_pkt_metadata(instance->device, mdata, m);
 }
 
 void *
diff --git a/lib/librte_security/rte_security.h b/lib/librte_security/rte_security.h
index 004a0eb..9d322a8 100644
--- a/lib/librte_security/rte_security.h
+++ b/lib/librte_security/rte_security.h
@@ -284,6 +284,48 @@ struct rte_security_session {
 	/**< Private session material */
 };
 
+/* IN/OUT flags for IPsec mdata */
+
+/**
+ * IN/OUT flag for sequence number
+ */
+#define RTE_SECURITY_IPSEC_MDATA_FLAGS_SEQ_NO   (1ULL << 0)
+
+/**
+ * Metadata for IPsec protocol offload
+ */
+struct rte_security_ipsec_mdata {
+	uint64_t seq_no;
+	/**< Sequence number */
+};
+
+/**
+ * Per packet metadata for protocol offload
+ */
+struct rte_security_mdata {
+	struct rte_security_session *sess;
+	/**< Security session */
+	union {
+		struct rte_security_ipsec_mdata ipsec;
+	};
+	/**< Protocol specific metadata. This field is IN/OUT, and could be
+	 * used for setting and retrieving per packet metadata.
+	 */
+	struct {
+		uint32_t set;
+		/**< Used by application to denote the fields it has set */
+		uint32_t get;
+		/**< Used by application to denote the fields PMD should
+		 * update back
+		 */
+		uint32_t updated;
+		/**< Used by PMD to denote the fields it has set */
+	} mdata_flags;
+	/**< Flags to denote the usage of various fields in metadata */
+	void *params;
+	/**< Device specific pointer */
+};
+
 /**
  * Create security session as specified by the session configuration
  *
@@ -331,13 +373,25 @@ rte_security_session_destroy(struct rte_security_ctx *instance,
 			     struct rte_security_session *sess);
 
 /**
- *  Updates the buffer with device-specific defined metadata
+ * Updates the buffer with the security metadata.
+ *
+ * This metadata could be used by the application to set some protocol defined
+ * fields per packet. For such protocol defined fields, application can only
+ * request the PMD to set various values, and it will be upto the PMD to
+ * decide whether the provided values should be used or not.
+ *
+ * In addition, this could be used by the application to probe such per packet
+ * fields used in inline offload case. PMD would update the metadata field with
+ * what it would use, if the corresponding "get" flag is set.
+ *
+ * E.g. for inline IPsec mode, application could request a sequence number by
+ * setting "rte_security_mdata.ipsec.seq_no" field and the corresponding flag.
+ * Additionally, "rte_security_mdata.mdata_flags.get" would give application
+ * the ability to check the sequence number selected for the packet.
  *
  * @param	instance	security instance
- * @param	sess		security session
+ * @param	mdata		security metadata
  * @param	mb		packet mbuf to set metadata on.
- * @param	params		device-specific defined parameters
- *				required for metadata
  *
  * @return
  *  - On success, zero.
@@ -345,8 +399,8 @@ rte_security_session_destroy(struct rte_security_ctx *instance,
  */
 int
 rte_security_set_pkt_metadata(struct rte_security_ctx *instance,
-			      struct rte_security_session *sess,
-			      struct rte_mbuf *mb, void *params);
+			      struct rte_security_mdata *mdata,
+			      struct rte_mbuf *mb);
 
 /**
  * Get userdata associated with the security session which processed the
diff --git a/lib/librte_security/rte_security_driver.h b/lib/librte_security/rte_security_driver.h
index bf0170e..662afa9 100644
--- a/lib/librte_security/rte_security_driver.h
+++ b/lib/librte_security/rte_security_driver.h
@@ -118,8 +118,7 @@ typedef int (*security_session_stats_get_t)(void *device,
  *  - Returns -ve value for errors.
  */
 typedef int (*security_set_pkt_metadata_t)(void *device,
-		struct rte_security_session *sess, struct rte_mbuf *m,
-		void *params);
+		struct rte_security_mdata *md, struct rte_mbuf *m);
 
 /**
  * Get application specific userdata associated with the security session which
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC 2/3] net/ixgbe: use structure for passing metadata
  2018-01-22 13:11 [RFC 0/3] set protocol specific metadata using set_pkt_metadata API Anoob Joseph
  2018-01-22 13:11 ` [RFC 1/3] lib/security: set/retrieve per packet protocol metadata Anoob Joseph
@ 2018-01-22 13:11 ` Anoob Joseph
  2018-01-22 13:11 ` [RFC 3/3] examples/ipsec-secgw: support for setting seq no Anoob Joseph
  2018-01-25 17:13 ` [RFC 0/3] set protocol specific metadata using set_pkt_metadata API Anoob Joseph
  3 siblings, 0 replies; 14+ messages in thread
From: Anoob Joseph @ 2018-01-22 13:11 UTC (permalink / raw)
  To: Akhil Goyal, Declan Doherty, Radu Nicolau, Sergio Gonzalez Monroy
  Cc: Anoob Joseph, Jerin Jacob, Narayana Prasad, Nelio Laranjeiro, dev

Using structure to pass metadata

Signed-off-by: Anoob Joseph <anoob.joseph@caviumnetworks.com>
---
 drivers/net/ixgbe/ixgbe_ipsec.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ipsec.c b/drivers/net/ixgbe/ixgbe_ipsec.c
index 85305c6..6c8d6b4 100644
--- a/drivers/net/ixgbe/ixgbe_ipsec.c
+++ b/drivers/net/ixgbe/ixgbe_ipsec.c
@@ -444,9 +444,10 @@ ixgbe_crypto_compute_pad_len(struct rte_mbuf *m)
 
 static int
 ixgbe_crypto_update_mb(void *device __rte_unused,
-		struct rte_security_session *session,
-		       struct rte_mbuf *m, void *params __rte_unused)
+		       struct rte_security_mdata *sec_mdata,
+		       struct rte_mbuf *m)
 {
+	struct rte_security_session *session = sec_mdata->sess;
 	struct ixgbe_crypto_session *ic_session =
 			get_sec_session_private_data(session);
 	if (ic_session->op == IXGBE_OP_AUTHENTICATED_ENCRYPTION) {
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC 3/3] examples/ipsec-secgw: support for setting seq no
  2018-01-22 13:11 [RFC 0/3] set protocol specific metadata using set_pkt_metadata API Anoob Joseph
  2018-01-22 13:11 ` [RFC 1/3] lib/security: set/retrieve per packet protocol metadata Anoob Joseph
  2018-01-22 13:11 ` [RFC 2/3] net/ixgbe: use structure for passing metadata Anoob Joseph
@ 2018-01-22 13:11 ` Anoob Joseph
  2018-01-25 17:13 ` [RFC 0/3] set protocol specific metadata using set_pkt_metadata API Anoob Joseph
  3 siblings, 0 replies; 14+ messages in thread
From: Anoob Joseph @ 2018-01-22 13:11 UTC (permalink / raw)
  To: Akhil Goyal, Declan Doherty, Radu Nicolau, Sergio Gonzalez Monroy
  Cc: Anoob Joseph, Jerin Jacob, Narayana Prasad, Nelio Laranjeiro, dev

Adding support for setting sequence number for inline protocol processed
packets.

Signed-off-by: Anoob Joseph <anoob.joseph@caviumnetworks.com>
---
 examples/ipsec-secgw/esp.h   |  9 +++++++++
 examples/ipsec-secgw/ipsec.c | 42 ++++++++++++++++++++++++++++++++++++------
 2 files changed, 45 insertions(+), 6 deletions(-)

diff --git a/examples/ipsec-secgw/esp.h b/examples/ipsec-secgw/esp.h
index 792312c..ec9dbd1 100644
--- a/examples/ipsec-secgw/esp.h
+++ b/examples/ipsec-secgw/esp.h
@@ -6,6 +6,15 @@
 
 struct mbuf;
 
+static inline int
+esp_inline_protocol_fill_mdata(struct ipsec_sa *sa,
+			       struct rte_security_ipsec_mdata *md_ipsec)
+{
+	/* Set sequence number */
+	md_ipsec->seq_no = ++(sa->seq);
+
+	return 0;
+}
 
 int
 esp_inbound(struct rte_mbuf *m, struct ipsec_sa *sa,
diff --git a/examples/ipsec-secgw/ipsec.c b/examples/ipsec-secgw/ipsec.c
index 05e89a1..d602c6b 100644
--- a/examples/ipsec-secgw/ipsec.c
+++ b/examples/ipsec-secgw/ipsec.c
@@ -359,6 +359,40 @@ enqueue_cop(struct cdev_qp *cqp, struct rte_crypto_op *cop)
 	}
 }
 
+static inline int
+inline_protocol_set_pkt_metadata(struct ipsec_sa *sa, struct rte_mbuf *pkt)
+{
+	int ret;
+	struct rte_security_mdata md = { 0 };
+
+	md.sess = sa->sec_session;
+
+	ret = esp_inline_protocol_fill_mdata(sa, &md.ipsec);
+
+	if (ret != 0) {
+		RTE_LOG(ERR, IPSEC,
+			"Could not generate per packet metadata for IPsec offload\n");
+		return ret;
+	}
+
+	/* Update flags to hint the PMD to use seq_no provided */
+	md.mdata_flags.set = RTE_SECURITY_IPSEC_MDATA_FLAGS_SEQ_NO;
+
+	rte_security_set_pkt_metadata(sa->security_ctx, &md, pkt);
+
+	return 0;
+}
+
+static inline void
+inline_crypto_set_pkt_metadata(struct ipsec_sa *sa, struct rte_mbuf *pkt)
+{
+	struct rte_security_mdata mdata = { 0 };
+
+	mdata.sess = sa->sec_session;
+
+	rte_security_set_pkt_metadata(sa->security_ctx, &mdata, pkt);
+}
+
 static inline void
 ipsec_enqueue(ipsec_xform_fn xform_func, struct ipsec_ctx *ipsec_ctx,
 		struct rte_mbuf *pkts[], struct ipsec_sa *sas[],
@@ -434,9 +468,7 @@ ipsec_enqueue(ipsec_xform_fn xform_func, struct ipsec_ctx *ipsec_ctx,
 			cqp = &ipsec_ctx->tbl[sa->cdev_id_qp];
 			cqp->ol_pkts[cqp->ol_pkts_cnt++] = pkts[i];
 			if (sa->ol_flags & RTE_SECURITY_TX_OLOAD_NEED_MDATA)
-				rte_security_set_pkt_metadata(
-						sa->security_ctx,
-						sa->sec_session, pkts[i], NULL);
+				inline_protocol_set_pkt_metadata(sa, pkts[i]);
 			continue;
 		case RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO:
 			priv->cop.type = RTE_CRYPTO_OP_TYPE_SYMMETRIC;
@@ -462,9 +494,7 @@ ipsec_enqueue(ipsec_xform_fn xform_func, struct ipsec_ctx *ipsec_ctx,
 			cqp = &ipsec_ctx->tbl[sa->cdev_id_qp];
 			cqp->ol_pkts[cqp->ol_pkts_cnt++] = pkts[i];
 			if (sa->ol_flags & RTE_SECURITY_TX_OLOAD_NEED_MDATA)
-				rte_security_set_pkt_metadata(
-						sa->security_ctx,
-						sa->sec_session, pkts[i], NULL);
+				inline_crypto_set_pkt_metadata(sa, pkts[i]);
 			continue;
 		}
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [RFC 0/3] set protocol specific metadata using set_pkt_metadata API
  2018-01-22 13:11 [RFC 0/3] set protocol specific metadata using set_pkt_metadata API Anoob Joseph
                   ` (2 preceding siblings ...)
  2018-01-22 13:11 ` [RFC 3/3] examples/ipsec-secgw: support for setting seq no Anoob Joseph
@ 2018-01-25 17:13 ` Anoob Joseph
  2018-01-26 11:22   ` Nicolau, Radu
  3 siblings, 1 reply; 14+ messages in thread
From: Anoob Joseph @ 2018-01-25 17:13 UTC (permalink / raw)
  To: Akhil Goyal, Radu Nicolau
  Cc: Declan Doherty, Sergio Gonzalez Monroy, anoob.joseph,
	Jerin Jacob, Narayana Prasad, Nelio Laranjeiro, dev

Hi Akhil, Radu,

Could you review the patch and share your thoughts on the proposed change?

Thanks
Anoob

On 01/22/2018 06:41 PM, Anoob Joseph wrote:
> This series adds support for setting & retrieving per packet protocol specific
> metadata. This is primarily required by the application to monitor sequence
> number overflows in inline protocol processing.
>
> The feature is added to the existing set_pkt_metadata API. The existing API
> passes all arguments directly. This series introduces a new structure which
> could be used to pass all metadata required in such cases.
>
> The patch set adds the ability to both set & retrieve such parameters. The idea
> is to make the application determine the sequence number to be used, where it
> is supported. If the PMD doesn't support it that way (as in the parameters are
> maintained by PMD/device), then application could just retrieve the value and
> see if there is any overflow etc happening.
>
> SA expiry/overflow monitoring requires knowing the latest sequence number
> on an SA. So this change allows that ability - for now for the outbound SA.
>
> Anoob Joseph (3):
>    lib/security: set/retrieve per packet protocol metadata
>    net/ixgbe: use structure for passing metadata
>    examples/ipsec-secgw: support for setting seq no
>
>   drivers/net/ixgbe/ixgbe_ipsec.c           |  5 ++-
>   examples/ipsec-secgw/esp.h                |  9 +++++
>   examples/ipsec-secgw/ipsec.c              | 42 +++++++++++++++++---
>   lib/librte_security/rte_security.c        |  7 ++--
>   lib/librte_security/rte_security.h        | 66 ++++++++++++++++++++++++++++---
>   lib/librte_security/rte_security_driver.h |  3 +-
>   6 files changed, 112 insertions(+), 20 deletions(-)
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC 0/3] set protocol specific metadata using set_pkt_metadata API
  2018-01-25 17:13 ` [RFC 0/3] set protocol specific metadata using set_pkt_metadata API Anoob Joseph
@ 2018-01-26 11:22   ` Nicolau, Radu
  2018-01-26 14:38     ` Anoob Joseph
  0 siblings, 1 reply; 14+ messages in thread
From: Nicolau, Radu @ 2018-01-26 11:22 UTC (permalink / raw)
  To: Anoob Joseph, Akhil Goyal
  Cc: Doherty, Declan, Gonzalez Monroy, Sergio, Jerin Jacob,
	Narayana Prasad, Nelio Laranjeiro, dev



> -----Original Message-----
> From: Anoob Joseph [mailto:anoob.joseph@caviumnetworks.com]
> Sent: Thursday, January 25, 2018 5:13 PM
> To: Akhil Goyal <akhil.goyal@nxp.com>; Nicolau, Radu
> <radu.nicolau@intel.com>
> Cc: Doherty, Declan <declan.doherty@intel.com>; Gonzalez Monroy, Sergio
> <sergio.gonzalez.monroy@intel.com>;
> anoob.joseph@caviumnetworks.com; Jerin Jacob
> <jerin.jacob@caviumnetworks.com>; Narayana Prasad
> <narayanaprasad.athreya@caviumnetworks.com>; Nelio Laranjeiro
> <nelio.laranjeiro@6wind.com>; dev@dpdk.org
> Subject: Re: [RFC 0/3] set protocol specific metadata using set_pkt_metadata
> API
> 
> Hi Akhil, Radu,
> 
> Could you review the patch and share your thoughts on the proposed
> change?
> 

Hi,

I've had a quick look. From what I can see you can do everything you do in this patch with the current API. For example you can store an internal struct pointer in the private section of the security context and you can increment the ESP SN with every tx or set metadata call.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC 0/3] set protocol specific metadata using set_pkt_metadata API
  2018-01-26 11:22   ` Nicolau, Radu
@ 2018-01-26 14:38     ` Anoob Joseph
  2018-01-26 15:08       ` Nicolau, Radu
  0 siblings, 1 reply; 14+ messages in thread
From: Anoob Joseph @ 2018-01-26 14:38 UTC (permalink / raw)
  To: Nicolau, Radu, Akhil Goyal
  Cc: anoob.joseph, Doherty, Declan, Gonzalez Monroy, Sergio,
	Jerin Jacob, Narayana Prasad, Nelio Laranjeiro, dev

Hi Radu,

On 01/26/2018 04:52 PM, Nicolau, Radu wrote:
>
>> -----Original Message-----
>> From: Anoob Joseph [mailto:anoob.joseph@caviumnetworks.com]
>> Sent: Thursday, January 25, 2018 5:13 PM
>> To: Akhil Goyal <akhil.goyal@nxp.com>; Nicolau, Radu
>> <radu.nicolau@intel.com>
>> Cc: Doherty, Declan <declan.doherty@intel.com>; Gonzalez Monroy, Sergio
>> <sergio.gonzalez.monroy@intel.com>;
>> anoob.joseph@caviumnetworks.com; Jerin Jacob
>> <jerin.jacob@caviumnetworks.com>; Narayana Prasad
>> <narayanaprasad.athreya@caviumnetworks.com>; Nelio Laranjeiro
>> <nelio.laranjeiro@6wind.com>; dev@dpdk.org
>> Subject: Re: [RFC 0/3] set protocol specific metadata using set_pkt_metadata
>> API
>>
>> Hi Akhil, Radu,
>>
>> Could you review the patch and share your thoughts on the proposed
>> change?
>>
> Hi,
>
> I've had a quick look. From what I can see you can do everything you do in this patch with the current API. For example you can store an internal struct pointer in the private section of the security context and you can increment the ESP SN with every tx or set metadata call.
With the current API, PMD could store the ESN with the security session, 
but there is no means for the application to read this. Application 
should be aware of the sequence number used per packet. This is required 
to monitor sequence number overflow.In the proposal, the sequence number 
field is IN-OUT. So application could either dictate the sequence 
number, or read the value from the PMD.

Thanks,
Anoob

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC 0/3] set protocol specific metadata using set_pkt_metadata API
  2018-01-26 14:38     ` Anoob Joseph
@ 2018-01-26 15:08       ` Nicolau, Radu
  2018-01-29  7:32         ` Akhil Goyal
  0 siblings, 1 reply; 14+ messages in thread
From: Nicolau, Radu @ 2018-01-26 15:08 UTC (permalink / raw)
  To: Anoob Joseph, Akhil Goyal
  Cc: Doherty, Declan, Gonzalez Monroy, Sergio, Jerin Jacob,
	Narayana Prasad, Nelio Laranjeiro, dev



> -----Original Message-----
> From: Anoob Joseph [mailto:anoob.joseph@caviumnetworks.com]
> Sent: Friday, January 26, 2018 2:38 PM
> To: Nicolau, Radu <radu.nicolau@intel.com>; Akhil Goyal
> <akhil.goyal@nxp.com>
> Cc: anoob.joseph@caviumnetworks.com; Doherty, Declan
> <declan.doherty@intel.com>; Gonzalez Monroy, Sergio
> <sergio.gonzalez.monroy@intel.com>; Jerin Jacob
> <jerin.jacob@caviumnetworks.com>; Narayana Prasad
> <narayanaprasad.athreya@caviumnetworks.com>; Nelio Laranjeiro
> <nelio.laranjeiro@6wind.com>; dev@dpdk.org
> Subject: Re: [RFC 0/3] set protocol specific metadata using set_pkt_metadata
> API
> 
> Hi Radu,
> 
> On 01/26/2018 04:52 PM, Nicolau, Radu wrote:
> >
> >> -----Original Message-----
> >> From: Anoob Joseph [mailto:anoob.joseph@caviumnetworks.com]
> >> Sent: Thursday, January 25, 2018 5:13 PM
> >> To: Akhil Goyal <akhil.goyal@nxp.com>; Nicolau, Radu
> >> <radu.nicolau@intel.com>
> >> Cc: Doherty, Declan <declan.doherty@intel.com>; Gonzalez Monroy,
> >> Sergio <sergio.gonzalez.monroy@intel.com>;
> >> anoob.joseph@caviumnetworks.com; Jerin Jacob
> >> <jerin.jacob@caviumnetworks.com>; Narayana Prasad
> >> <narayanaprasad.athreya@caviumnetworks.com>; Nelio Laranjeiro
> >> <nelio.laranjeiro@6wind.com>; dev@dpdk.org
> >> Subject: Re: [RFC 0/3] set protocol specific metadata using
> >> set_pkt_metadata API
> >>
> >> Hi Akhil, Radu,
> >>
> >> Could you review the patch and share your thoughts on the proposed
> >> change?
> >>
> > Hi,
> >
> > I've had a quick look. From what I can see you can do everything you do in
> this patch with the current API. For example you can store an internal struct
> pointer in the private section of the security context and you can increment
> the ESP SN with every tx or set metadata call.
> With the current API, PMD could store the ESN with the security session, but
> there is no means for the application to read this. Application should be
> aware of the sequence number used per packet. This is required to monitor
> sequence number overflow.In the proposal, the sequence number field is
> IN-OUT. So application could either dictate the sequence number, or read
> the value from the PMD.
> 
> Thanks,
> Anoob

My concern is that we are adding too much and too specific to the security API.
Overflow situation can be monitored with a tx callback event or a crypto callback event, depending on the device type.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC 0/3] set protocol specific metadata using set_pkt_metadata API
  2018-01-26 15:08       ` Nicolau, Radu
@ 2018-01-29  7:32         ` Akhil Goyal
  2018-01-29  8:03           ` Anoob Joseph
  0 siblings, 1 reply; 14+ messages in thread
From: Akhil Goyal @ 2018-01-29  7:32 UTC (permalink / raw)
  To: Nicolau, Radu, Anoob Joseph
  Cc: Doherty, Declan, Gonzalez Monroy, Sergio, Jerin Jacob,
	Narayana Prasad, Nelio Laranjeiro, dev

On 1/26/2018 8:38 PM, Nicolau, Radu wrote:
> 
> 
>> -----Original Message-----
>> From: Anoob Joseph [mailto:anoob.joseph@caviumnetworks.com]
>> Sent: Friday, January 26, 2018 2:38 PM
>> To: Nicolau, Radu <radu.nicolau@intel.com>; Akhil Goyal
>> <akhil.goyal@nxp.com>
>> Cc: anoob.joseph@caviumnetworks.com; Doherty, Declan
>> <declan.doherty@intel.com>; Gonzalez Monroy, Sergio
>> <sergio.gonzalez.monroy@intel.com>; Jerin Jacob
>> <jerin.jacob@caviumnetworks.com>; Narayana Prasad
>> <narayanaprasad.athreya@caviumnetworks.com>; Nelio Laranjeiro
>> <nelio.laranjeiro@6wind.com>; dev@dpdk.org
>> Subject: Re: [RFC 0/3] set protocol specific metadata using set_pkt_metadata
>> API
>>
>> Hi Radu,
>>
>> On 01/26/2018 04:52 PM, Nicolau, Radu wrote:
>>>
>>>> -----Original Message-----
>>>> From: Anoob Joseph [mailto:anoob.joseph@caviumnetworks.com]
>>>> Sent: Thursday, January 25, 2018 5:13 PM
>>>> To: Akhil Goyal <akhil.goyal@nxp.com>; Nicolau, Radu
>>>> <radu.nicolau@intel.com>
>>>> Cc: Doherty, Declan <declan.doherty@intel.com>; Gonzalez Monroy,
>>>> Sergio <sergio.gonzalez.monroy@intel.com>;
>>>> anoob.joseph@caviumnetworks.com; Jerin Jacob
>>>> <jerin.jacob@caviumnetworks.com>; Narayana Prasad
>>>> <narayanaprasad.athreya@caviumnetworks.com>; Nelio Laranjeiro
>>>> <nelio.laranjeiro@6wind.com>; dev@dpdk.org
>>>> Subject: Re: [RFC 0/3] set protocol specific metadata using
>>>> set_pkt_metadata API
>>>>
>>>> Hi Akhil, Radu,
>>>>
>>>> Could you review the patch and share your thoughts on the proposed
>>>> change?
>>>>
>>> Hi,
>>>
>>> I've had a quick look. From what I can see you can do everything you do in
>> this patch with the current API. For example you can store an internal struct
>> pointer in the private section of the security context and you can increment
>> the ESP SN with every tx or set metadata call.
>> With the current API, PMD could store the ESN with the security session, but
>> there is no means for the application to read this. Application should be
>> aware of the sequence number used per packet. This is required to monitor
>> sequence number overflow.In the proposal, the sequence number field is
>> IN-OUT. So application could either dictate the sequence number, or read
>> the value from the PMD.
>>
>> Thanks,
>> Anoob
> 
> My concern is that we are adding too much and too specific to the security API.
> Overflow situation can be monitored with a tx callback event or a crypto callback event, depending on the device type.
> 
Agreed with Radu, this looks too specific information.
Instead, we can do overflow checking in the driver and add a macro in 
rte_crypto_op_status for overflow.

-Akhil

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC 0/3] set protocol specific metadata using set_pkt_metadata API
  2018-01-29  7:32         ` Akhil Goyal
@ 2018-01-29  8:03           ` Anoob Joseph
  2018-01-29  9:08             ` Akhil Goyal
  2018-01-29 10:01             ` Nicolau, Radu
  0 siblings, 2 replies; 14+ messages in thread
From: Anoob Joseph @ 2018-01-29  8:03 UTC (permalink / raw)
  To: Akhil Goyal, Nicolau, Radu
  Cc: anoob.joseph, Doherty, Declan, Gonzalez Monroy, Sergio,
	Jerin Jacob, Narayana Prasad, Nelio Laranjeiro, dev

Hi Akhil, Radu,


On 01/29/2018 01:02 PM, Akhil Goyal wrote:
> On 1/26/2018 8:38 PM, Nicolau, Radu wrote:
>>
>>
>>> -----Original Message-----
>>> From: Anoob Joseph [mailto:anoob.joseph@caviumnetworks.com]
>>> Sent: Friday, January 26, 2018 2:38 PM
>>> To: Nicolau, Radu <radu.nicolau@intel.com>; Akhil Goyal
>>> <akhil.goyal@nxp.com>
>>> Cc: anoob.joseph@caviumnetworks.com; Doherty, Declan
>>> <declan.doherty@intel.com>; Gonzalez Monroy, Sergio
>>> <sergio.gonzalez.monroy@intel.com>; Jerin Jacob
>>> <jerin.jacob@caviumnetworks.com>; Narayana Prasad
>>> <narayanaprasad.athreya@caviumnetworks.com>; Nelio Laranjeiro
>>> <nelio.laranjeiro@6wind.com>; dev@dpdk.org
>>> Subject: Re: [RFC 0/3] set protocol specific metadata using 
>>> set_pkt_metadata
>>> API
>>>
>>> Hi Radu,
>>>
>>> On 01/26/2018 04:52 PM, Nicolau, Radu wrote:
>>>>
>>>>> -----Original Message-----
>>>>> From: Anoob Joseph [mailto:anoob.joseph@caviumnetworks.com]
>>>>> Sent: Thursday, January 25, 2018 5:13 PM
>>>>> To: Akhil Goyal <akhil.goyal@nxp.com>; Nicolau, Radu
>>>>> <radu.nicolau@intel.com>
>>>>> Cc: Doherty, Declan <declan.doherty@intel.com>; Gonzalez Monroy,
>>>>> Sergio <sergio.gonzalez.monroy@intel.com>;
>>>>> anoob.joseph@caviumnetworks.com; Jerin Jacob
>>>>> <jerin.jacob@caviumnetworks.com>; Narayana Prasad
>>>>> <narayanaprasad.athreya@caviumnetworks.com>; Nelio Laranjeiro
>>>>> <nelio.laranjeiro@6wind.com>; dev@dpdk.org
>>>>> Subject: Re: [RFC 0/3] set protocol specific metadata using
>>>>> set_pkt_metadata API
>>>>>
>>>>> Hi Akhil, Radu,
>>>>>
>>>>> Could you review the patch and share your thoughts on the proposed
>>>>> change?
>>>>>
>>>> Hi,
>>>>
>>>> I've had a quick look. From what I can see you can do everything 
>>>> you do in
>>> this patch with the current API. For example you can store an 
>>> internal struct
>>> pointer in the private section of the security context and you can 
>>> increment
>>> the ESP SN with every tx or set metadata call.
>>> With the current API, PMD could store the ESN with the security 
>>> session, but
>>> there is no means for the application to read this. Application 
>>> should be
>>> aware of the sequence number used per packet. This is required to 
>>> monitor
>>> sequence number overflow.In the proposal, the sequence number field is
>>> IN-OUT. So application could either dictate the sequence number, or 
>>> read
>>> the value from the PMD.
>>>
>>> Thanks,
>>> Anoob
>>
>> My concern is that we are adding too much and too specific to the 
>> security API.
>> Overflow situation can be monitored with a tx callback event or a 
>> crypto callback event, depending on the device type.
>>
> Agreed with Radu, this looks too specific information.
> Instead, we can do overflow checking in the driver and add a macro in 
> rte_crypto_op_status for overflow.
We could do the callback when sequence number over flow happens, and 
IPsec processing fails subsequently. But ideally, application should be 
able to detect that the sequence number is about to over flow and 
renegotiate the SA while the original SA is still valid. I agree that we 
would be better off by handling this in the driver. But application 
would need some sort of event which would say, "sequence number is about 
to overflow, renegotiate SA", before the current SA becomes invalid.

Do we have any mechanism to register a callback (acting on mbuf), when a 
particular event occurs (without dropping the mbuf)? If yes, we could 
move to that approach.

rte_crypto_op_status could be leveraged for lookaside_protocol, but can 
we do something similar for inline protocol? Thoughts?

Anoob
>
> -Akhil

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC 0/3] set protocol specific metadata using set_pkt_metadata API
  2018-01-29  8:03           ` Anoob Joseph
@ 2018-01-29  9:08             ` Akhil Goyal
  2018-01-29 11:44               ` Anoob Joseph
  2018-01-29 10:01             ` Nicolau, Radu
  1 sibling, 1 reply; 14+ messages in thread
From: Akhil Goyal @ 2018-01-29  9:08 UTC (permalink / raw)
  To: Anoob Joseph, Nicolau, Radu
  Cc: Doherty, Declan, Gonzalez Monroy, Sergio, Jerin Jacob,
	Narayana Prasad, Nelio Laranjeiro, dev

On 1/29/2018 1:33 PM, Anoob Joseph wrote:
> Hi Akhil, Radu,
> 
> 
> On 01/29/2018 01:02 PM, Akhil Goyal wrote:
>> On 1/26/2018 8:38 PM, Nicolau, Radu wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Anoob Joseph [mailto:anoob.joseph@caviumnetworks.com]
>>>> Sent: Friday, January 26, 2018 2:38 PM
>>>> To: Nicolau, Radu <radu.nicolau@intel.com>; Akhil Goyal
>>>> <akhil.goyal@nxp.com>
>>>> Cc: anoob.joseph@caviumnetworks.com; Doherty, Declan
>>>> <declan.doherty@intel.com>; Gonzalez Monroy, Sergio
>>>> <sergio.gonzalez.monroy@intel.com>; Jerin Jacob
>>>> <jerin.jacob@caviumnetworks.com>; Narayana Prasad
>>>> <narayanaprasad.athreya@caviumnetworks.com>; Nelio Laranjeiro
>>>> <nelio.laranjeiro@6wind.com>; dev@dpdk.org
>>>> Subject: Re: [RFC 0/3] set protocol specific metadata using 
>>>> set_pkt_metadata
>>>> API
>>>>
>>>> Hi Radu,
>>>>
>>>> On 01/26/2018 04:52 PM, Nicolau, Radu wrote:
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Anoob Joseph [mailto:anoob.joseph@caviumnetworks.com]
>>>>>> Sent: Thursday, January 25, 2018 5:13 PM
>>>>>> To: Akhil Goyal <akhil.goyal@nxp.com>; Nicolau, Radu
>>>>>> <radu.nicolau@intel.com>
>>>>>> Cc: Doherty, Declan <declan.doherty@intel.com>; Gonzalez Monroy,
>>>>>> Sergio <sergio.gonzalez.monroy@intel.com>;
>>>>>> anoob.joseph@caviumnetworks.com; Jerin Jacob
>>>>>> <jerin.jacob@caviumnetworks.com>; Narayana Prasad
>>>>>> <narayanaprasad.athreya@caviumnetworks.com>; Nelio Laranjeiro
>>>>>> <nelio.laranjeiro@6wind.com>; dev@dpdk.org
>>>>>> Subject: Re: [RFC 0/3] set protocol specific metadata using
>>>>>> set_pkt_metadata API
>>>>>>
>>>>>> Hi Akhil, Radu,
>>>>>>
>>>>>> Could you review the patch and share your thoughts on the proposed
>>>>>> change?
>>>>>>
>>>>> Hi,
>>>>>
>>>>> I've had a quick look. From what I can see you can do everything 
>>>>> you do in
>>>> this patch with the current API. For example you can store an 
>>>> internal struct
>>>> pointer in the private section of the security context and you can 
>>>> increment
>>>> the ESP SN with every tx or set metadata call.
>>>> With the current API, PMD could store the ESN with the security 
>>>> session, but
>>>> there is no means for the application to read this. Application 
>>>> should be
>>>> aware of the sequence number used per packet. This is required to 
>>>> monitor
>>>> sequence number overflow.In the proposal, the sequence number field is
>>>> IN-OUT. So application could either dictate the sequence number, or 
>>>> read
>>>> the value from the PMD.
>>>>
>>>> Thanks,
>>>> Anoob
>>>
>>> My concern is that we are adding too much and too specific to the 
>>> security API.
>>> Overflow situation can be monitored with a tx callback event or a 
>>> crypto callback event, depending on the device type.
>>>
>> Agreed with Radu, this looks too specific information.
>> Instead, we can do overflow checking in the driver and add a macro in 
>> rte_crypto_op_status for overflow.
> We could do the callback when sequence number over flow happens, and 
> IPsec processing fails subsequently. But ideally, application should be 
> able to detect that the sequence number is about to over flow and 
> renegotiate the SA while the original SA is still valid. I agree that we 
> would be better off by handling this in the driver. But application 
> would need some sort of event which would say, "sequence number is about 
> to overflow, renegotiate SA", before the current SA becomes invalid.
> 
> Do we have any mechanism to register a callback (acting on mbuf), when a 
> particular event occurs (without dropping the mbuf)? If yes, we could 
> move to that approach.
> 
> rte_crypto_op_status could be leveraged for lookaside_protocol, but can 
> we do something similar for inline protocol? Thoughts?

Even in case of inline protocol, what is the issue in doing that?
You can write a similar code in the driver(if hardware doesn't support 
that) instead of application for handling the sequence number overflow 
as well as anti-replay. Both of these errors are protocol specific and 
for full protocol offload, application need not bother about this.
Application should be as clean as possible in case of protocol offload.

- Akhil

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC 0/3] set protocol specific metadata using set_pkt_metadata API
  2018-01-29  8:03           ` Anoob Joseph
  2018-01-29  9:08             ` Akhil Goyal
@ 2018-01-29 10:01             ` Nicolau, Radu
  2018-01-29 18:01               ` Anoob Joseph
  1 sibling, 1 reply; 14+ messages in thread
From: Nicolau, Radu @ 2018-01-29 10:01 UTC (permalink / raw)
  To: Anoob Joseph, Akhil Goyal
  Cc: Doherty, Declan, Gonzalez Monroy, Sergio, Jerin Jacob,
	Narayana Prasad, Nelio Laranjeiro, dev



> -----Original Message-----
> From: Anoob Joseph [mailto:anoob.joseph@caviumnetworks.com]
> Sent: Monday, January 29, 2018 8:04 AM
> To: Akhil Goyal <akhil.goyal@nxp.com>; Nicolau, Radu
> <radu.nicolau@intel.com>
> Cc: anoob.joseph@caviumnetworks.com; Doherty, Declan
> <declan.doherty@intel.com>; Gonzalez Monroy, Sergio
> <sergio.gonzalez.monroy@intel.com>; Jerin Jacob
> <jerin.jacob@caviumnetworks.com>; Narayana Prasad
> <narayanaprasad.athreya@caviumnetworks.com>; Nelio Laranjeiro
> <nelio.laranjeiro@6wind.com>; dev@dpdk.org
> Subject: Re: [RFC 0/3] set protocol specific metadata using set_pkt_metadata
> API
> 
> Hi Akhil, Radu,
> 
> 
> On 01/29/2018 01:02 PM, Akhil Goyal wrote:
> > On 1/26/2018 8:38 PM, Nicolau, Radu wrote:
> >>
> >>
> >>> -----Original Message-----
> >>> From: Anoob Joseph [mailto:anoob.joseph@caviumnetworks.com]
> >>> Sent: Friday, January 26, 2018 2:38 PM
> >>> To: Nicolau, Radu <radu.nicolau@intel.com>; Akhil Goyal
> >>> <akhil.goyal@nxp.com>
> >>> Cc: anoob.joseph@caviumnetworks.com; Doherty, Declan
> >>> <declan.doherty@intel.com>; Gonzalez Monroy, Sergio
> >>> <sergio.gonzalez.monroy@intel.com>; Jerin Jacob
> >>> <jerin.jacob@caviumnetworks.com>; Narayana Prasad
> >>> <narayanaprasad.athreya@caviumnetworks.com>; Nelio Laranjeiro
> >>> <nelio.laranjeiro@6wind.com>; dev@dpdk.org
> >>> Subject: Re: [RFC 0/3] set protocol specific metadata using
> >>> set_pkt_metadata API
> >>>
> >>> Hi Radu,
> >>>
> >>> On 01/26/2018 04:52 PM, Nicolau, Radu wrote:
> >>>>
> >>>>> -----Original Message-----
> >>>>> From: Anoob Joseph [mailto:anoob.joseph@caviumnetworks.com]
> >>>>> Sent: Thursday, January 25, 2018 5:13 PM
> >>>>> To: Akhil Goyal <akhil.goyal@nxp.com>; Nicolau, Radu
> >>>>> <radu.nicolau@intel.com>
> >>>>> Cc: Doherty, Declan <declan.doherty@intel.com>; Gonzalez Monroy,
> >>>>> Sergio <sergio.gonzalez.monroy@intel.com>;
> >>>>> anoob.joseph@caviumnetworks.com; Jerin Jacob
> >>>>> <jerin.jacob@caviumnetworks.com>; Narayana Prasad
> >>>>> <narayanaprasad.athreya@caviumnetworks.com>; Nelio Laranjeiro
> >>>>> <nelio.laranjeiro@6wind.com>; dev@dpdk.org
> >>>>> Subject: Re: [RFC 0/3] set protocol specific metadata using
> >>>>> set_pkt_metadata API
> >>>>>
> >>>>> Hi Akhil, Radu,
> >>>>>
> >>>>> Could you review the patch and share your thoughts on the proposed
> >>>>> change?
> >>>>>
> >>>> Hi,
> >>>>
> >>>> I've had a quick look. From what I can see you can do everything
> >>>> you do in
> >>> this patch with the current API. For example you can store an
> >>> internal struct pointer in the private section of the security
> >>> context and you can increment the ESP SN with every tx or set
> >>> metadata call.
> >>> With the current API, PMD could store the ESN with the security
> >>> session, but there is no means for the application to read this.
> >>> Application should be aware of the sequence number used per packet.
> >>> This is required to monitor sequence number overflow.In the
> >>> proposal, the sequence number field is IN-OUT. So application could
> >>> either dictate the sequence number, or read the value from the PMD.
> >>>
> >>> Thanks,
> >>> Anoob
> >>
> >> My concern is that we are adding too much and too specific to the
> >> security API.
> >> Overflow situation can be monitored with a tx callback event or a
> >> crypto callback event, depending on the device type.
> >>
> > Agreed with Radu, this looks too specific information.
> > Instead, we can do overflow checking in the driver and add a macro in
> > rte_crypto_op_status for overflow.
> We could do the callback when sequence number over flow happens, and
> IPsec processing fails subsequently. But ideally, application should be able to
> detect that the sequence number is about to over flow and renegotiate the
> SA while the original SA is still valid. I agree that we would be better off by
> handling this in the driver. But application would need some sort of event
> which would say, "sequence number is about to overflow, renegotiate SA",
> before the current SA becomes invalid.
> 
> Do we have any mechanism to register a callback (acting on mbuf), when a
> particular event occurs (without dropping the mbuf)? If yes, we could move
> to that approach.
> 
> rte_crypto_op_status could be leveraged for lookaside_protocol, but can we
> do something similar for inline protocol? Thoughts?

You can look at rx/tx callbacks (http://dpdk.org/doc/api/examples_2rxtx_callbacks_2main_8c-example.html#a26) but probably events are more suitable: http://dpdk.org/doc/api/rte__ethdev_8h.html#ac0bef2920a6ade4041cab5103f4700d9 There is already a "RTE_ETH_EVENT_MACSEC MACsec offload related event" so you can add a security related event.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC 0/3] set protocol specific metadata using set_pkt_metadata API
  2018-01-29  9:08             ` Akhil Goyal
@ 2018-01-29 11:44               ` Anoob Joseph
  0 siblings, 0 replies; 14+ messages in thread
From: Anoob Joseph @ 2018-01-29 11:44 UTC (permalink / raw)
  To: Akhil Goyal, Nicolau, Radu
  Cc: anoob.joseph, Doherty, Declan, Gonzalez Monroy, Sergio,
	Jerin Jacob, Narayana Prasad, Nelio Laranjeiro, dev

Hi Akhil,


On 01/29/2018 02:38 PM, Akhil Goyal wrote:
> On 1/29/2018 1:33 PM, Anoob Joseph wrote:
>> Hi Akhil, Radu,
>>
>>
>> On 01/29/2018 01:02 PM, Akhil Goyal wrote:
>>> On 1/26/2018 8:38 PM, Nicolau, Radu wrote:
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Anoob Joseph [mailto:anoob.joseph@caviumnetworks.com]
>>>>> Sent: Friday, January 26, 2018 2:38 PM
>>>>> To: Nicolau, Radu <radu.nicolau@intel.com>; Akhil Goyal
>>>>> <akhil.goyal@nxp.com>
>>>>> Cc: anoob.joseph@caviumnetworks.com; Doherty, Declan
>>>>> <declan.doherty@intel.com>; Gonzalez Monroy, Sergio
>>>>> <sergio.gonzalez.monroy@intel.com>; Jerin Jacob
>>>>> <jerin.jacob@caviumnetworks.com>; Narayana Prasad
>>>>> <narayanaprasad.athreya@caviumnetworks.com>; Nelio Laranjeiro
>>>>> <nelio.laranjeiro@6wind.com>; dev@dpdk.org
>>>>> Subject: Re: [RFC 0/3] set protocol specific metadata using 
>>>>> set_pkt_metadata
>>>>> API
>>>>>
>>>>> Hi Radu,
>>>>>
>>>>> On 01/26/2018 04:52 PM, Nicolau, Radu wrote:
>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Anoob Joseph [mailto:anoob.joseph@caviumnetworks.com]
>>>>>>> Sent: Thursday, January 25, 2018 5:13 PM
>>>>>>> To: Akhil Goyal <akhil.goyal@nxp.com>; Nicolau, Radu
>>>>>>> <radu.nicolau@intel.com>
>>>>>>> Cc: Doherty, Declan <declan.doherty@intel.com>; Gonzalez Monroy,
>>>>>>> Sergio <sergio.gonzalez.monroy@intel.com>;
>>>>>>> anoob.joseph@caviumnetworks.com; Jerin Jacob
>>>>>>> <jerin.jacob@caviumnetworks.com>; Narayana Prasad
>>>>>>> <narayanaprasad.athreya@caviumnetworks.com>; Nelio Laranjeiro
>>>>>>> <nelio.laranjeiro@6wind.com>; dev@dpdk.org
>>>>>>> Subject: Re: [RFC 0/3] set protocol specific metadata using
>>>>>>> set_pkt_metadata API
>>>>>>>
>>>>>>> Hi Akhil, Radu,
>>>>>>>
>>>>>>> Could you review the patch and share your thoughts on the proposed
>>>>>>> change?
>>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> I've had a quick look. From what I can see you can do everything 
>>>>>> you do in
>>>>> this patch with the current API. For example you can store an 
>>>>> internal struct
>>>>> pointer in the private section of the security context and you can 
>>>>> increment
>>>>> the ESP SN with every tx or set metadata call.
>>>>> With the current API, PMD could store the ESN with the security 
>>>>> session, but
>>>>> there is no means for the application to read this. Application 
>>>>> should be
>>>>> aware of the sequence number used per packet. This is required to 
>>>>> monitor
>>>>> sequence number overflow.In the proposal, the sequence number 
>>>>> field is
>>>>> IN-OUT. So application could either dictate the sequence number, 
>>>>> or read
>>>>> the value from the PMD.
>>>>>
>>>>> Thanks,
>>>>> Anoob
>>>>
>>>> My concern is that we are adding too much and too specific to the 
>>>> security API.
>>>> Overflow situation can be monitored with a tx callback event or a 
>>>> crypto callback event, depending on the device type.
>>>>
>>> Agreed with Radu, this looks too specific information.
>>> Instead, we can do overflow checking in the driver and add a macro 
>>> in rte_crypto_op_status for overflow.
>> We could do the callback when sequence number over flow happens, and 
>> IPsec processing fails subsequently. But ideally, application should 
>> be able to detect that the sequence number is about to over flow and 
>> renegotiate the SA while the original SA is still valid. I agree that 
>> we would be better off by handling this in the driver. But 
>> application would need some sort of event which would say, "sequence 
>> number is about to overflow, renegotiate SA", before the current SA 
>> becomes invalid.
>>
>> Do we have any mechanism to register a callback (acting on mbuf), 
>> when a particular event occurs (without dropping the mbuf)? If yes, 
>> we could move to that approach.
>>
>> rte_crypto_op_status could be leveraged for lookaside_protocol, but 
>> can we do something similar for inline protocol? Thoughts?
>
> Even in case of inline protocol, what is the issue in doing that?
> You can write a similar code in the driver(if hardware doesn't support 
> that) instead of application for handling the sequence number overflow 
> as well as anti-replay. Both of these errors are protocol specific and 
> for full protocol offload, application need not bother about this.
> Application should be as clean as possible in case of protocol offload.
When SA expires, IKE needs to be notified to initiate new SAs. So there 
has to be an event from PMD to application indicating SA expiry. This 
needs to happen independently for outbound and inbound SAs. Even with 
inline protocol, IKE is still handled by application.

And, in inline_protocol, the protocol processing is done after the 
packet is submitted to eth_dev. Unlike crypto dev, there is no status of 
operation returned which could be leveraged to check about sequence 
number over flow. We need a way to know about overflow, when the packet 
is successfully processed and sent.

Anoob

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC 0/3] set protocol specific metadata using set_pkt_metadata API
  2018-01-29 10:01             ` Nicolau, Radu
@ 2018-01-29 18:01               ` Anoob Joseph
  0 siblings, 0 replies; 14+ messages in thread
From: Anoob Joseph @ 2018-01-29 18:01 UTC (permalink / raw)
  To: Nicolau, Radu, Akhil Goyal
  Cc: anoob.joseph, Doherty, Declan, Gonzalez Monroy, Sergio,
	Jerin Jacob, Narayana Prasad, Nelio Laranjeiro, dev

Hi Radu,


On 01/29/2018 03:31 PM, Nicolau, Radu wrote:
>
>> -----Original Message-----
>> From: Anoob Joseph [mailto:anoob.joseph@caviumnetworks.com]
>> Sent: Monday, January 29, 2018 8:04 AM
>> To: Akhil Goyal <akhil.goyal@nxp.com>; Nicolau, Radu
>> <radu.nicolau@intel.com>
>> Cc: anoob.joseph@caviumnetworks.com; Doherty, Declan
>> <declan.doherty@intel.com>; Gonzalez Monroy, Sergio
>> <sergio.gonzalez.monroy@intel.com>; Jerin Jacob
>> <jerin.jacob@caviumnetworks.com>; Narayana Prasad
>> <narayanaprasad.athreya@caviumnetworks.com>; Nelio Laranjeiro
>> <nelio.laranjeiro@6wind.com>; dev@dpdk.org
>> Subject: Re: [RFC 0/3] set protocol specific metadata using set_pkt_metadata
>> API
>>
>> Hi Akhil, Radu,
>>
>>
>> On 01/29/2018 01:02 PM, Akhil Goyal wrote:
>>> On 1/26/2018 8:38 PM, Nicolau, Radu wrote:
>>>>
>>>>> -----Original Message-----
>>>>> From: Anoob Joseph [mailto:anoob.joseph@caviumnetworks.com]
>>>>> Sent: Friday, January 26, 2018 2:38 PM
>>>>> To: Nicolau, Radu <radu.nicolau@intel.com>; Akhil Goyal
>>>>> <akhil.goyal@nxp.com>
>>>>> Cc: anoob.joseph@caviumnetworks.com; Doherty, Declan
>>>>> <declan.doherty@intel.com>; Gonzalez Monroy, Sergio
>>>>> <sergio.gonzalez.monroy@intel.com>; Jerin Jacob
>>>>> <jerin.jacob@caviumnetworks.com>; Narayana Prasad
>>>>> <narayanaprasad.athreya@caviumnetworks.com>; Nelio Laranjeiro
>>>>> <nelio.laranjeiro@6wind.com>; dev@dpdk.org
>>>>> Subject: Re: [RFC 0/3] set protocol specific metadata using
>>>>> set_pkt_metadata API
>>>>>
>>>>> Hi Radu,
>>>>>
>>>>> On 01/26/2018 04:52 PM, Nicolau, Radu wrote:
>>>>>>> -----Original Message-----
>>>>>>> From: Anoob Joseph [mailto:anoob.joseph@caviumnetworks.com]
>>>>>>> Sent: Thursday, January 25, 2018 5:13 PM
>>>>>>> To: Akhil Goyal <akhil.goyal@nxp.com>; Nicolau, Radu
>>>>>>> <radu.nicolau@intel.com>
>>>>>>> Cc: Doherty, Declan <declan.doherty@intel.com>; Gonzalez Monroy,
>>>>>>> Sergio <sergio.gonzalez.monroy@intel.com>;
>>>>>>> anoob.joseph@caviumnetworks.com; Jerin Jacob
>>>>>>> <jerin.jacob@caviumnetworks.com>; Narayana Prasad
>>>>>>> <narayanaprasad.athreya@caviumnetworks.com>; Nelio Laranjeiro
>>>>>>> <nelio.laranjeiro@6wind.com>; dev@dpdk.org
>>>>>>> Subject: Re: [RFC 0/3] set protocol specific metadata using
>>>>>>> set_pkt_metadata API
>>>>>>>
>>>>>>> Hi Akhil, Radu,
>>>>>>>
>>>>>>> Could you review the patch and share your thoughts on the proposed
>>>>>>> change?
>>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> I've had a quick look. From what I can see you can do everything
>>>>>> you do in
>>>>> this patch with the current API. For example you can store an
>>>>> internal struct pointer in the private section of the security
>>>>> context and you can increment the ESP SN with every tx or set
>>>>> metadata call.
>>>>> With the current API, PMD could store the ESN with the security
>>>>> session, but there is no means for the application to read this.
>>>>> Application should be aware of the sequence number used per packet.
>>>>> This is required to monitor sequence number overflow.In the
>>>>> proposal, the sequence number field is IN-OUT. So application could
>>>>> either dictate the sequence number, or read the value from the PMD.
>>>>>
>>>>> Thanks,
>>>>> Anoob
>>>> My concern is that we are adding too much and too specific to the
>>>> security API.
>>>> Overflow situation can be monitored with a tx callback event or a
>>>> crypto callback event, depending on the device type.
>>>>
>>> Agreed with Radu, this looks too specific information.
>>> Instead, we can do overflow checking in the driver and add a macro in
>>> rte_crypto_op_status for overflow.
>> We could do the callback when sequence number over flow happens, and
>> IPsec processing fails subsequently. But ideally, application should be able to
>> detect that the sequence number is about to over flow and renegotiate the
>> SA while the original SA is still valid. I agree that we would be better off by
>> handling this in the driver. But application would need some sort of event
>> which would say, "sequence number is about to overflow, renegotiate SA",
>> before the current SA becomes invalid.
>>
>> Do we have any mechanism to register a callback (acting on mbuf), when a
>> particular event occurs (without dropping the mbuf)? If yes, we could move
>> to that approach.
>>
>> rte_crypto_op_status could be leveraged for lookaside_protocol, but can we
>> do something similar for inline protocol? Thoughts?
> You can look at rx/tx callbacks (http://dpdk.org/doc/api/examples_2rxtx_callbacks_2main_8c-example.html#a26) but probably events are more suitable: http://dpdk.org/doc/api/rte__ethdev_8h.html#ac0bef2920a6ade4041cab5103f4700d9 There is already a "RTE_ETH_EVENT_MACSEC MACsec offload related event" so you can add a security related event.
Rx/tx callbacks would be either for all packets or for packets which 
encountered error. Neither is our case. So rx/tx callbacks might not fit 
in our model.

I need to attempt the implementation with events to see if that will 
solve the issue. The implementation could be a bit complex as 
"eth_event" is mostly to control the state of ports, rather than acting 
on packets. In that case, PMD could return a security session in the 
callback argument, and the application would be required to find out the 
SA from the security session. I'll give this a shot and see if this can 
solve the problem.

Thanks,
Anoob

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2018-01-29 18:01 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-22 13:11 [RFC 0/3] set protocol specific metadata using set_pkt_metadata API Anoob Joseph
2018-01-22 13:11 ` [RFC 1/3] lib/security: set/retrieve per packet protocol metadata Anoob Joseph
2018-01-22 13:11 ` [RFC 2/3] net/ixgbe: use structure for passing metadata Anoob Joseph
2018-01-22 13:11 ` [RFC 3/3] examples/ipsec-secgw: support for setting seq no Anoob Joseph
2018-01-25 17:13 ` [RFC 0/3] set protocol specific metadata using set_pkt_metadata API Anoob Joseph
2018-01-26 11:22   ` Nicolau, Radu
2018-01-26 14:38     ` Anoob Joseph
2018-01-26 15:08       ` Nicolau, Radu
2018-01-29  7:32         ` Akhil Goyal
2018-01-29  8:03           ` Anoob Joseph
2018-01-29  9:08             ` Akhil Goyal
2018-01-29 11:44               ` Anoob Joseph
2018-01-29 10:01             ` Nicolau, Radu
2018-01-29 18:01               ` Anoob Joseph

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.