diff mbox series

[2/8] crypto/octeontx2: add lookaside SA context definitions

Message ID 20200623121228.10355-3-ktejasree@marvell.com (mailing list archive)
State Changes Requested, archived
Delegated to: akhil goyal
Headers
Series add OCTEON TX2 lookaside IPsec support |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation fail apply issues

Commit Message

Tejasree Kondoj June 23, 2020, 12:12 p.m. UTC 
  Signed-off-by: Vamsi Attunuru <vattunuru@marvell.com>
Signed-off-by: Tejasree Kondoj <ktejasree@marvell.com>
---
 drivers/crypto/octeontx2/otx2_cryptodev_sec.h |  52 ++++++++
 drivers/crypto/octeontx2/otx2_ipsec_po.h      | 119 ++++++++++++++++++
 drivers/crypto/octeontx2/otx2_security.h      |   2 +
 drivers/net/octeontx2/otx2_ethdev_sec.h       |   1 +
 4 files changed, 174 insertions(+)
 create mode 100644 drivers/crypto/octeontx2/otx2_cryptodev_sec.h
 create mode 100644 drivers/crypto/octeontx2/otx2_ipsec_po.h

Comments

Akhil Goyal July 1, 2020, 8:46 p.m. UTC | #1 
> Subject: [PATCH 2/8] crypto/octeontx2: add lookaside SA context definitions
> 
> Signed-off-by: Vamsi Attunuru <vattunuru@marvell.com>
> Signed-off-by: Tejasree Kondoj <ktejasree@marvell.com>
> ---
Please add appropriate description in all the patches. 

>  drivers/crypto/octeontx2/otx2_cryptodev_sec.h |  52 ++++++++
>  drivers/crypto/octeontx2/otx2_ipsec_po.h      | 119 ++++++++++++++++++
>  drivers/crypto/octeontx2/otx2_security.h      |   2 +
>  drivers/net/octeontx2/otx2_ethdev_sec.h       |   1 +
>  4 files changed, 174 insertions(+)
>  create mode 100644 drivers/crypto/octeontx2/otx2_cryptodev_sec.h
>  create mode 100644 drivers/crypto/octeontx2/otx2_ipsec_po.h
> 
> diff --git a/drivers/crypto/octeontx2/otx2_cryptodev_sec.h
> b/drivers/crypto/octeontx2/otx2_cryptodev_sec.h
> new file mode 100644
> index 0000000000..af62207d07
> --- /dev/null
> +++ b/drivers/crypto/octeontx2/otx2_cryptodev_sec.h
> @@ -0,0 +1,52 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright (C) 2020 Marvell International Ltd.
> + */
> +
> +#ifndef __OTX2_CRYPTODEV_SEC_H__
> +#define __OTX2_CRYPTODEV_SEC_H__
> +
> +#include "otx2_ipsec_po.h"
Why is it named like this? What is 'po' ?
Anoob Joseph July 2, 2020, 9:29 a.m. UTC | #2 
Hi Akhil,

Please see inline.

Thanks,
Anoob

> -----Original Message-----
> From: Akhil Goyal <akhil.goyal@nxp.com>
> Sent: Thursday, July 2, 2020 2:17 AM
> To: Tejasree Kondoj <ktejasree@marvell.com>; Radu Nicolau
> <radu.nicolau@intel.com>
> Cc: Narayana Prasad Raju Athreya <pathreya@marvell.com>; Anoob Joseph
> <anoobj@marvell.com>; Vamsi Krishna Attunuru <vattunuru@marvell.com>;
> dev@dpdk.org
> Subject: [EXT] RE: [PATCH 2/8] crypto/octeontx2: add lookaside SA context
> definitions
> 
> External Email
> 
> ----------------------------------------------------------------------
> 
> > Subject: [PATCH 2/8] crypto/octeontx2: add lookaside SA context
> definitions
> >
> > Signed-off-by: Vamsi Attunuru <vattunuru@marvell.com>
> > Signed-off-by: Tejasree Kondoj <ktejasree@marvell.com>
> > ---
> Please add appropriate description in all the patches.
> 
> >  drivers/crypto/octeontx2/otx2_cryptodev_sec.h |  52 ++++++++
> >  drivers/crypto/octeontx2/otx2_ipsec_po.h      | 119
> ++++++++++++++++++
> >  drivers/crypto/octeontx2/otx2_security.h      |   2 +
> >  drivers/net/octeontx2/otx2_ethdev_sec.h       |   1 +
> >  4 files changed, 174 insertions(+)
> >  create mode 100644 drivers/crypto/octeontx2/otx2_cryptodev_sec.h
> >  create mode 100644 drivers/crypto/octeontx2/otx2_ipsec_po.h
> >
> > diff --git a/drivers/crypto/octeontx2/otx2_cryptodev_sec.h
> > b/drivers/crypto/octeontx2/otx2_cryptodev_sec.h
> > new file mode 100644
> > index 0000000000..af62207d07
> > --- /dev/null
> > +++ b/drivers/crypto/octeontx2/otx2_cryptodev_sec.h
> > @@ -0,0 +1,52 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause
> > + * Copyright (C) 2020 Marvell International Ltd.
> > + */
> > +
> > +#ifndef __OTX2_CRYPTODEV_SEC_H__
> > +#define __OTX2_CRYPTODEV_SEC_H__
> > +
> > +#include "otx2_ipsec_po.h"
> Why is it named like this? What is 'po' ?

[Anoob] OCTEON TX2 firmware supports two opcodes. One specific for inline and one for lookaside. The one for inline is FP and the one for lookaside is PO (Protocol Offload).
Akhil Goyal July 2, 2020, 9:38 a.m. UTC | #3 
> > > +#include "otx2_ipsec_po.h"
> > Why is it named like this? What is 'po' ?
> 
> [Anoob] OCTEON TX2 firmware supports two opcodes. One specific for inline
> and one for lookaside. The one for inline is FP and the one for lookaside is PO
> (Protocol Offload).
> 
Isn't it better to write "inline" in place of "fp" and "offload" in place of "po"?

Fp and po looks very cryptic.
Anoob Joseph July 2, 2020, 10 a.m. UTC | #4 
Hi Akhil,

> 
> ----------------------------------------------------------------------
> > > > +#include "otx2_ipsec_po.h"
> > > Why is it named like this? What is 'po' ?
> >
> > [Anoob] OCTEON TX2 firmware supports two opcodes. One specific for
> > inline and one for lookaside. The one for inline is FP and the one for
> > lookaside is PO (Protocol Offload).
> >
> Isn't it better to write "inline" in place of "fp" and "offload" in place of "po"?
> 
> Fp and po looks very cryptic.

[Anoob] Yes. But since it is firmware specific, these need to be added for all structures etc. And these opcodes can be used interchangeably as well (as in PO opcode can be used for inline and FP opcodes can be used for lookaside, but both with certain limitations). Hence we have tried to separate it out this way. Larger names would mean longer function names and structure names etc. We tried few other names as well, but then it was conflicting with other opcodes. I do agree that it is slightly cryptic, but we have tried to use the names consistently to avoid confusions. 

Thanks,
Anoob
Akhil Goyal July 2, 2020, 10:40 a.m. UTC | #5 
> > ----------------------------------------------------------------------
> > > > > +#include "otx2_ipsec_po.h"
> > > > Why is it named like this? What is 'po' ?
> > >
> > > [Anoob] OCTEON TX2 firmware supports two opcodes. One specific for
> > > inline and one for lookaside. The one for inline is FP and the one for
> > > lookaside is PO (Protocol Offload).
> > >
> > Isn't it better to write "inline" in place of "fp" and "offload" in place of "po"?
> >
> > Fp and po looks very cryptic.
> 
> [Anoob] Yes. But since it is firmware specific, these need to be added for all
> structures etc. And these opcodes can be used interchangeably as well (as in PO
> opcode can be used for inline and FP opcodes can be used for lookaside, but
> both with certain limitations). Hence we have tried to separate it out this way.
> Larger names would mean longer function names and structure names etc. We
> tried few other names as well, but then it was conflicting with other opcodes. I
> do agree that it is slightly cryptic, but we have tried to use the names
> consistently to avoid confusions.
> 
I think you can use the opcode as fp and po but the function/file names should be readable.
However, it is internal to PMD, I don't have a strong opinion on this but it would be good if
Names can be made readable.

Regards,
Akhil
diff mbox series

Patch

diff --git a/drivers/crypto/octeontx2/otx2_cryptodev_sec.h b/drivers/crypto/octeontx2/otx2_cryptodev_sec.h
new file mode 100644
index 0000000000..af62207d07
--- /dev/null
+++ b/drivers/crypto/octeontx2/otx2_cryptodev_sec.h
@@ -0,0 +1,52 @@ 
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright (C) 2020 Marvell International Ltd.
+ */
+
+#ifndef __OTX2_CRYPTODEV_SEC_H__
+#define __OTX2_CRYPTODEV_SEC_H__
+
+#include "otx2_ipsec_po.h"
+
+struct otx2_sec_session_ipsec_lp {
+	RTE_STD_C11
+	union {
+		/* Inbound SA */
+		struct otx2_ipsec_po_in_sa in_sa;
+		/* Outbound SA */
+		struct otx2_ipsec_po_out_sa out_sa;
+	};
+
+	uint64_t ucmd_w3;
+	uint16_t ucmd_opcode;
+	uint16_t ucmd_param1;
+	uint16_t ucmd_param2;
+
+	uint8_t partial_len;
+	uint8_t roundup_len;
+	uint8_t roundup_byte;
+	uint16_t ip_id;
+	union {
+		uint64_t esn;
+		struct {
+			uint32_t seq_lo;
+			uint32_t seq_hi;
+		};
+	};
+
+	/** Context length in 8-byte words */
+	size_t ctx_len;
+	/** Auth IV offset in bytes */
+	uint16_t auth_iv_offset;
+	/** IV offset in bytes */
+	uint16_t iv_offset;
+	/** AAD length */
+	uint16_t aad_length;
+	/** MAC len in bytes */
+	uint8_t mac_len;
+	/** IV length in bytes */
+	uint8_t iv_length;
+	/** Auth IV length in bytes */
+	uint8_t auth_iv_length;
+};
+
+#endif /* __OTX2_CRYPTODEV_SEC_H__ */
diff --git a/drivers/crypto/octeontx2/otx2_ipsec_po.h b/drivers/crypto/octeontx2/otx2_ipsec_po.h
new file mode 100644
index 0000000000..602b9d10e2
--- /dev/null
+++ b/drivers/crypto/octeontx2/otx2_ipsec_po.h
@@ -0,0 +1,119 @@ 
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(C) 2020 Marvell International Ltd.
+ */
+
+#ifndef __OTX2_IPSEC_PO_H__
+#define __OTX2_IPSEC_PO_H__
+
+#include <rte_crypto_sym.h>
+#include <rte_ip.h>
+#include <rte_security.h>
+
+union bit_perfect_iv {
+	uint8_t aes_iv[16];
+	uint8_t des_iv[8];
+	struct {
+		uint8_t nonce[4];
+		uint8_t iv[8];
+		uint8_t counter[4];
+	} misc; /* For GCM/GMAC/CTR/CCM */
+};
+
+struct ip_selector {
+	uint8_t src_port[4];
+	uint8_t dest_port[4];
+	RTE_STD_C11
+	union {
+		struct {
+			uint8_t src_addr[8];
+			uint8_t dest_addr[8];
+		} ipv4;
+		struct {
+			uint8_t src_addr[32];
+			uint8_t dest_addr[32];
+		} ipv6;
+	};
+};
+
+struct otx2_ipsec_po_sa_ctl {
+	rte_be32_t spi          : 32;
+	uint64_t exp_proto_inter_frag : 8;
+	uint64_t rsvd_42_40   : 3;
+	uint64_t esn_en       : 1;
+	uint64_t rsvd_45_44   : 2;
+	uint64_t encap_type   : 2;
+	uint64_t enc_type     : 3;
+	uint64_t rsvd_48      : 1;
+	uint64_t auth_type    : 4;
+	uint64_t valid        : 1;
+	uint64_t direction    : 1;
+	uint64_t outer_ip_ver : 1;
+	uint64_t inner_ip_ver : 1;
+	uint64_t ipsec_mode   : 1;
+	uint64_t ipsec_proto  : 1;
+	uint64_t aes_key_len  : 2;
+};
+
+struct otx2_ipsec_po_in_sa {
+	/* w0 */
+	struct otx2_ipsec_po_sa_ctl ctl;
+
+	/* w1-w4 */
+	uint8_t cipher_key[32];
+
+	/* w5-w6 */
+	union bit_perfect_iv iv;
+
+	/* w7 */
+	uint32_t esn_hi;
+	uint32_t esn_low;
+
+	/* w8 */
+	uint8_t udp_encap[8];
+
+	/* w9-w23 */
+	RTE_STD_C11
+	struct {
+		uint8_t hmac_key[48];
+		struct ip_selector sel_checks;
+	} aes_gcm;
+};
+
+struct ip_template {
+	union {
+		RTE_STD_C11
+		uint8_t raw[252];
+		struct {
+			struct rte_ipv4_hdr hdr;
+			uint8_t unused[40];
+		} ipv4;
+
+		struct {
+			struct rte_ipv6_hdr hdr;
+			uint8_t unused[208];
+		} ipv6;
+	};
+};
+
+struct otx2_ipsec_po_out_sa {
+	/* w0 */
+	struct otx2_ipsec_po_sa_ctl ctl;
+
+	/* w1-w4 */
+	uint8_t cipher_key[32];
+
+	/* w5-w6 */
+	union bit_perfect_iv iv;
+
+	/* w7 */
+	uint32_t esn_hi;
+	uint32_t esn_low;
+
+	/* w8-w39 */
+	RTE_STD_C11
+	struct ip_template templt;
+	uint16_t udp_src;
+	uint16_t udp_dst;
+};
+
+#endif /* __OTX2_IPSEC_PO_H__ */
diff --git a/drivers/crypto/octeontx2/otx2_security.h b/drivers/crypto/octeontx2/otx2_security.h
index 275d69b6a5..e76cd843c7 100644
--- a/drivers/crypto/octeontx2/otx2_security.h
+++ b/drivers/crypto/octeontx2/otx2_security.h
@@ -5,11 +5,13 @@ 
 #ifndef __OTX2_SECURITY_H__
 #define __OTX2_SECURITY_H__
 
+#include "otx2_cryptodev_sec.h"
 #include "otx2_ethdev_sec.h"
 #include "otx2_ipsec_fp.h"
 
 union otx2_sec_session_ipsec {
 	struct otx2_sec_session_ipsec_ip ip;
+	struct otx2_sec_session_ipsec_lp lp;
 };
 
 struct otx2_sec_session {
diff --git a/drivers/net/octeontx2/otx2_ethdev_sec.h b/drivers/net/octeontx2/otx2_ethdev_sec.h
index 22025d0d0c..298b00bf89 100644
--- a/drivers/net/octeontx2/otx2_ethdev_sec.h
+++ b/drivers/net/octeontx2/otx2_ethdev_sec.h
@@ -8,6 +8,7 @@ 
 #include <rte_ethdev.h>
 
 #include "otx2_ipsec_fp.h"
+#include "otx2_ipsec_po.h"
 
 #define OTX2_CPT_RES_ALIGN		16
 #define OTX2_NIX_SEND_DESC_ALIGN	16