All of lore.kernel.org
 help / color / mirror / Atom feed
* [net PATCH] octeontx2-pf: mcs: Generate hash key using ecb(aes)
@ 2023-07-14 16:22 Subbaraya Sundeep
  2023-07-16 10:49 ` Kalesh Anakkur Purayil
  0 siblings, 1 reply; 5+ messages in thread
From: Subbaraya Sundeep @ 2023-07-14 16:22 UTC (permalink / raw)
  To: netdev, linux-kernel
  Cc: kuba, davem, edumazet, pabeni, sgoutham, gakula, hkelam, naveenm,
	Subbaraya Sundeep

Hardware generated encryption and ICV tags are found to
be wrong when tested with IEEE MACSEC test vectors.
This is because as per the HRM, the hash key (derived by
AES-ECB block encryption of an all 0s block with the SAK)
has to be programmed by the software in
MCSX_RS_MCS_CPM_TX_SLAVE_SA_PLCY_MEM_4X register.
Hence fix this by generating hash key in software and
configuring in hardware.

Fixes: c54ffc73601c ("octeontx2-pf: mcs: Introduce MACSEC hardware offloading")
Signed-off-by: Subbaraya Sundeep <sbhatta@marvell.com>
---
 .../ethernet/marvell/octeontx2/nic/cn10k_macsec.c  | 132 +++++++++++++++------
 1 file changed, 95 insertions(+), 37 deletions(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_macsec.c b/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_macsec.c
index 6e2fb24..9f23118 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_macsec.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_macsec.c
@@ -4,6 +4,7 @@
  * Copyright (C) 2022 Marvell.
  */
 
+#include <crypto/skcipher.h>
 #include <linux/rtnetlink.h>
 #include <linux/bitfield.h>
 #include "otx2_common.h"
@@ -42,6 +43,51 @@
 #define MCS_TCI_E			0x08 /* encryption */
 #define MCS_TCI_C			0x04 /* changed text */
 
+#define CN10K_MAX_HASH_LEN		16
+#define CN10K_MAX_SAK_LEN		32
+
+static int cn10k_ecb_aes_encrypt(struct otx2_nic *pfvf, u8 *sak,
+				 u16 sak_len, u8 *hash)
+{
+	u8 data[CN10K_MAX_HASH_LEN] = { 0 };
+	struct skcipher_request *req = NULL;
+	struct scatterlist sg_src, sg_dst;
+	struct crypto_skcipher *tfm;
+	DECLARE_CRYPTO_WAIT(wait);
+	int err;
+
+	tfm = crypto_alloc_skcipher("ecb(aes)", 0, 0);
+	if (IS_ERR(tfm)) {
+		dev_err(pfvf->dev, "failed to allocate transform for ecb-aes\n");
+		return PTR_ERR(tfm);
+	}
+
+	req = skcipher_request_alloc(tfm, GFP_KERNEL);
+	if (!req) {
+		dev_err(pfvf->dev, "failed to allocate request for skcipher\n");
+		err = -ENOMEM;
+		goto out;
+	}
+
+	err = crypto_skcipher_setkey(tfm, sak, sak_len);
+
+	/* build sg list */
+	sg_init_one(&sg_src, data, CN10K_MAX_HASH_LEN);
+	sg_init_one(&sg_dst, hash, CN10K_MAX_HASH_LEN);
+
+	skcipher_request_set_callback(req, 0, crypto_req_done, &wait);
+	skcipher_request_set_crypt(req, &sg_src, &sg_dst,
+				   CN10K_MAX_HASH_LEN, NULL);
+
+	err = crypto_skcipher_encrypt(req);
+	err = crypto_wait_req(err, &wait);
+
+out:
+	skcipher_request_free(req);
+	crypto_free_skcipher(tfm);
+	return err;
+}
+
 static struct cn10k_mcs_txsc *cn10k_mcs_get_txsc(struct cn10k_mcs_cfg *cfg,
 						 struct macsec_secy *secy)
 {
@@ -330,19 +376,53 @@ static int cn10k_mcs_write_sc_cam(struct otx2_nic *pfvf,
 	return ret;
 }
 
+static int cn10k_mcs_write_keys(struct otx2_nic *pfvf,
+				struct macsec_secy *secy,
+				struct mcs_sa_plcy_write_req *req,
+				u8 *sak, u8 *salt, ssci_t ssci)
+{
+	u8 hash_rev[CN10K_MAX_HASH_LEN] = { 0 };
+	u8 sak_rev[CN10K_MAX_SAK_LEN] = { 0 };
+	u8 salt_rev[MACSEC_SALT_LEN] = { 0 };
+	u8 hash[CN10K_MAX_HASH_LEN] = { 0 };
+	u32 ssci_63_32;
+	int err, i;
+
+	err = cn10k_ecb_aes_encrypt(pfvf, sak, secy->key_len, hash);
+	if (err) {
+		dev_err(pfvf->dev, "Generating hash using ECB(AES) failed\n");
+		return err;
+	}
+
+	for (i = 0; i < secy->key_len; i++)
+		sak_rev[i] = sak[secy->key_len - 1 - i];
+
+	for (i = 0; i < CN10K_MAX_HASH_LEN; i++)
+		hash_rev[i] = hash[CN10K_MAX_HASH_LEN - 1 - i];
+
+	for (i = 0; i < MACSEC_SALT_LEN; i++)
+		salt_rev[i] = salt[MACSEC_SALT_LEN - 1 - i];
+
+	ssci_63_32 = (__force u32)cpu_to_be32((__force u32)ssci);
+
+	memcpy(&req->plcy[0][0], sak_rev, secy->key_len);
+	memcpy(&req->plcy[0][4], hash_rev, CN10K_MAX_HASH_LEN);
+	memcpy(&req->plcy[0][6], salt_rev, MACSEC_SALT_LEN);
+	req->plcy[0][7] |= (u64)ssci_63_32 << 32;
+
+	return 0;
+}
+
 static int cn10k_mcs_write_rx_sa_plcy(struct otx2_nic *pfvf,
 				      struct macsec_secy *secy,
 				      struct cn10k_mcs_rxsc *rxsc,
 				      u8 assoc_num, bool sa_in_use)
 {
-	unsigned char *src = rxsc->sa_key[assoc_num];
 	struct mcs_sa_plcy_write_req *plcy_req;
-	u8 *salt_p = rxsc->salt[assoc_num];
+	u8 *sak = rxsc->sa_key[assoc_num];
+	u8 *salt = rxsc->salt[assoc_num];
 	struct mcs_rx_sc_sa_map *map_req;
 	struct mbox *mbox = &pfvf->mbox;
-	u64 ssci_salt_95_64 = 0;
-	u8 reg, key_len;
-	u64 salt_63_0;
 	int ret;
 
 	mutex_lock(&mbox->lock);
@@ -360,20 +440,10 @@ static int cn10k_mcs_write_rx_sa_plcy(struct otx2_nic *pfvf,
 		goto fail;
 	}
 
-	for (reg = 0, key_len = 0; key_len < secy->key_len; key_len += 8) {
-		memcpy((u8 *)&plcy_req->plcy[0][reg],
-		       (src + reg * 8), 8);
-		reg++;
-	}
-
-	if (secy->xpn) {
-		memcpy((u8 *)&salt_63_0, salt_p, 8);
-		memcpy((u8 *)&ssci_salt_95_64, salt_p + 8, 4);
-		ssci_salt_95_64 |= (__force u64)rxsc->ssci[assoc_num] << 32;
-
-		plcy_req->plcy[0][6] = salt_63_0;
-		plcy_req->plcy[0][7] = ssci_salt_95_64;
-	}
+	ret = cn10k_mcs_write_keys(pfvf, secy, plcy_req, sak,
+				   salt, rxsc->ssci[assoc_num]);
+	if (ret)
+		goto fail;
 
 	plcy_req->sa_index[0] = rxsc->hw_sa_id[assoc_num];
 	plcy_req->sa_cnt = 1;
@@ -586,13 +656,10 @@ static int cn10k_mcs_write_tx_sa_plcy(struct otx2_nic *pfvf,
 				      struct cn10k_mcs_txsc *txsc,
 				      u8 assoc_num)
 {
-	unsigned char *src = txsc->sa_key[assoc_num];
 	struct mcs_sa_plcy_write_req *plcy_req;
-	u8 *salt_p = txsc->salt[assoc_num];
+	u8 *sak = txsc->sa_key[assoc_num];
+	u8 *salt = txsc->salt[assoc_num];
 	struct mbox *mbox = &pfvf->mbox;
-	u64 ssci_salt_95_64 = 0;
-	u8 reg, key_len;
-	u64 salt_63_0;
 	int ret;
 
 	mutex_lock(&mbox->lock);
@@ -603,19 +670,10 @@ static int cn10k_mcs_write_tx_sa_plcy(struct otx2_nic *pfvf,
 		goto fail;
 	}
 
-	for (reg = 0, key_len = 0; key_len < secy->key_len; key_len += 8) {
-		memcpy((u8 *)&plcy_req->plcy[0][reg], (src + reg * 8), 8);
-		reg++;
-	}
-
-	if (secy->xpn) {
-		memcpy((u8 *)&salt_63_0, salt_p, 8);
-		memcpy((u8 *)&ssci_salt_95_64, salt_p + 8, 4);
-		ssci_salt_95_64 |= (__force u64)txsc->ssci[assoc_num] << 32;
-
-		plcy_req->plcy[0][6] = salt_63_0;
-		plcy_req->plcy[0][7] = ssci_salt_95_64;
-	}
+	ret = cn10k_mcs_write_keys(pfvf, secy, plcy_req, sak,
+				   salt, txsc->ssci[assoc_num]);
+	if (ret)
+		goto fail;
 
 	plcy_req->plcy[0][8] = assoc_num;
 	plcy_req->sa_index[0] = txsc->hw_sa_id[assoc_num];
-- 
2.7.4


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

* Re: [net PATCH] octeontx2-pf: mcs: Generate hash key using ecb(aes)
  2023-07-14 16:22 [net PATCH] octeontx2-pf: mcs: Generate hash key using ecb(aes) Subbaraya Sundeep
@ 2023-07-16 10:49 ` Kalesh Anakkur Purayil
       [not found]   ` <CO1PR18MB4666877CBEAB2D8290C2ACB8A13BA@CO1PR18MB4666.namprd18.prod.outlook.com>
  0 siblings, 1 reply; 5+ messages in thread
From: Kalesh Anakkur Purayil @ 2023-07-16 10:49 UTC (permalink / raw)
  To: Subbaraya Sundeep
  Cc: netdev, linux-kernel, kuba, davem, edumazet, pabeni, sgoutham,
	gakula, hkelam, naveenm


[-- Attachment #1.1: Type: text/plain, Size: 8491 bytes --]

On Fri, Jul 14, 2023 at 9:53 PM Subbaraya Sundeep <sbhatta@marvell.com>
wrote:

> Hardware generated encryption and ICV tags are found to
> be wrong when tested with IEEE MACSEC test vectors.
> This is because as per the HRM, the hash key (derived by
> AES-ECB block encryption of an all 0s block with the SAK)
> has to be programmed by the software in
> MCSX_RS_MCS_CPM_TX_SLAVE_SA_PLCY_MEM_4X register.
> Hence fix this by generating hash key in software and
> configuring in hardware.
>
> Fixes: c54ffc73601c ("octeontx2-pf: mcs: Introduce MACSEC hardware
> offloading")
> Signed-off-by: Subbaraya Sundeep <sbhatta@marvell.com>
> ---
>  .../ethernet/marvell/octeontx2/nic/cn10k_macsec.c  | 132
> +++++++++++++++------
>  1 file changed, 95 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_macsec.c
> b/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_macsec.c
> index 6e2fb24..9f23118 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_macsec.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_macsec.c
> @@ -4,6 +4,7 @@
>   * Copyright (C) 2022 Marvell.
>   */
>
> +#include <crypto/skcipher.h>
>  #include <linux/rtnetlink.h>
>  #include <linux/bitfield.h>
>  #include "otx2_common.h"
> @@ -42,6 +43,51 @@
>  #define MCS_TCI_E                      0x08 /* encryption */
>  #define MCS_TCI_C                      0x04 /* changed text */
>
> +#define CN10K_MAX_HASH_LEN             16
> +#define CN10K_MAX_SAK_LEN              32
> +
> +static int cn10k_ecb_aes_encrypt(struct otx2_nic *pfvf, u8 *sak,
> +                                u16 sak_len, u8 *hash)
> +{
> +       u8 data[CN10K_MAX_HASH_LEN] = { 0 };
>
[Kalesh]: There is no need in 0 here, just use {}


> +       struct skcipher_request *req = NULL;
> +       struct scatterlist sg_src, sg_dst;
> +       struct crypto_skcipher *tfm;
> +       DECLARE_CRYPTO_WAIT(wait);
> +       int err;
> +
> +       tfm = crypto_alloc_skcipher("ecb(aes)", 0, 0);
> +       if (IS_ERR(tfm)) {
> +               dev_err(pfvf->dev, "failed to allocate transform for
> ecb-aes\n");
> +               return PTR_ERR(tfm);
> +       }
> +
> +       req = skcipher_request_alloc(tfm, GFP_KERNEL);
> +       if (!req) {
> +               dev_err(pfvf->dev, "failed to allocate request for
> skcipher\n");
> +               err = -ENOMEM;
> +               goto out;
> +       }
> +
> +       err = crypto_skcipher_setkey(tfm, sak, sak_len);
>
[Kalesh]: No need for a return value check here?

> +
> +       /* build sg list */
> +       sg_init_one(&sg_src, data, CN10K_MAX_HASH_LEN);
> +       sg_init_one(&sg_dst, hash, CN10K_MAX_HASH_LEN);
> +
> +       skcipher_request_set_callback(req, 0, crypto_req_done, &wait);
> +       skcipher_request_set_crypt(req, &sg_src, &sg_dst,
> +                                  CN10K_MAX_HASH_LEN, NULL);
> +
> +       err = crypto_skcipher_encrypt(req);
> +       err = crypto_wait_req(err, &wait);
> +
> +out:
> +       skcipher_request_free(req);
>
[Kalesh]: I think you should move the label here.

> +       crypto_free_skcipher(tfm);
> +       return err;
> +}
> +
>  static struct cn10k_mcs_txsc *cn10k_mcs_get_txsc(struct cn10k_mcs_cfg
> *cfg,
>                                                  struct macsec_secy *secy)
>  {
> @@ -330,19 +376,53 @@ static int cn10k_mcs_write_sc_cam(struct otx2_nic
> *pfvf,
>         return ret;
>  }
>
> +static int cn10k_mcs_write_keys(struct otx2_nic *pfvf,
> +                               struct macsec_secy *secy,
> +                               struct mcs_sa_plcy_write_req *req,
> +                               u8 *sak, u8 *salt, ssci_t ssci)
> +{
> +       u8 hash_rev[CN10K_MAX_HASH_LEN] = { 0 };
>
[Kalesh]: There is no need in 0 here, just use {}


> +       u8 sak_rev[CN10K_MAX_SAK_LEN] = { 0 };
> +       u8 salt_rev[MACSEC_SALT_LEN] = { 0 };
> +       u8 hash[CN10K_MAX_HASH_LEN] = { 0 };
> +       u32 ssci_63_32;
> +       int err, i;
> +
> +       err = cn10k_ecb_aes_encrypt(pfvf, sak, secy->key_len, hash);
> +       if (err) {
> +               dev_err(pfvf->dev, "Generating hash using ECB(AES)
> failed\n");
> +               return err;
> +       }
> +
> +       for (i = 0; i < secy->key_len; i++)
> +               sak_rev[i] = sak[secy->key_len - 1 - i];
> +
> +       for (i = 0; i < CN10K_MAX_HASH_LEN; i++)
> +               hash_rev[i] = hash[CN10K_MAX_HASH_LEN - 1 - i];
> +
> +       for (i = 0; i < MACSEC_SALT_LEN; i++)
> +               salt_rev[i] = salt[MACSEC_SALT_LEN - 1 - i];
> +
> +       ssci_63_32 = (__force u32)cpu_to_be32((__force u32)ssci);
> +
> +       memcpy(&req->plcy[0][0], sak_rev, secy->key_len);
> +       memcpy(&req->plcy[0][4], hash_rev, CN10K_MAX_HASH_LEN);
> +       memcpy(&req->plcy[0][6], salt_rev, MACSEC_SALT_LEN);
> +       req->plcy[0][7] |= (u64)ssci_63_32 << 32;
> +
> +       return 0;
> +}
> +
>  static int cn10k_mcs_write_rx_sa_plcy(struct otx2_nic *pfvf,
>                                       struct macsec_secy *secy,
>                                       struct cn10k_mcs_rxsc *rxsc,
>                                       u8 assoc_num, bool sa_in_use)
>  {
> -       unsigned char *src = rxsc->sa_key[assoc_num];
>         struct mcs_sa_plcy_write_req *plcy_req;
> -       u8 *salt_p = rxsc->salt[assoc_num];
> +       u8 *sak = rxsc->sa_key[assoc_num];
> +       u8 *salt = rxsc->salt[assoc_num];
>         struct mcs_rx_sc_sa_map *map_req;
>         struct mbox *mbox = &pfvf->mbox;
> -       u64 ssci_salt_95_64 = 0;
> -       u8 reg, key_len;
> -       u64 salt_63_0;
>         int ret;
>
>         mutex_lock(&mbox->lock);
> @@ -360,20 +440,10 @@ static int cn10k_mcs_write_rx_sa_plcy(struct
> otx2_nic *pfvf,
>                 goto fail;
>         }
>
> -       for (reg = 0, key_len = 0; key_len < secy->key_len; key_len += 8) {
> -               memcpy((u8 *)&plcy_req->plcy[0][reg],
> -                      (src + reg * 8), 8);
> -               reg++;
> -       }
> -
> -       if (secy->xpn) {
> -               memcpy((u8 *)&salt_63_0, salt_p, 8);
> -               memcpy((u8 *)&ssci_salt_95_64, salt_p + 8, 4);
> -               ssci_salt_95_64 |= (__force u64)rxsc->ssci[assoc_num] <<
> 32;
> -
> -               plcy_req->plcy[0][6] = salt_63_0;
> -               plcy_req->plcy[0][7] = ssci_salt_95_64;
> -       }
> +       ret = cn10k_mcs_write_keys(pfvf, secy, plcy_req, sak,
> +                                  salt, rxsc->ssci[assoc_num]);
> +       if (ret)
> +               goto fail;
>
>         plcy_req->sa_index[0] = rxsc->hw_sa_id[assoc_num];
>         plcy_req->sa_cnt = 1;
> @@ -586,13 +656,10 @@ static int cn10k_mcs_write_tx_sa_plcy(struct
> otx2_nic *pfvf,
>                                       struct cn10k_mcs_txsc *txsc,
>                                       u8 assoc_num)
>  {
> -       unsigned char *src = txsc->sa_key[assoc_num];
>         struct mcs_sa_plcy_write_req *plcy_req;
> -       u8 *salt_p = txsc->salt[assoc_num];
> +       u8 *sak = txsc->sa_key[assoc_num];
> +       u8 *salt = txsc->salt[assoc_num];
>         struct mbox *mbox = &pfvf->mbox;
> -       u64 ssci_salt_95_64 = 0;
> -       u8 reg, key_len;
> -       u64 salt_63_0;
>         int ret;
>
>         mutex_lock(&mbox->lock);
> @@ -603,19 +670,10 @@ static int cn10k_mcs_write_tx_sa_plcy(struct
> otx2_nic *pfvf,
>                 goto fail;
>         }
>
> -       for (reg = 0, key_len = 0; key_len < secy->key_len; key_len += 8) {
> -               memcpy((u8 *)&plcy_req->plcy[0][reg], (src + reg * 8), 8);
> -               reg++;
> -       }
> -
> -       if (secy->xpn) {
> -               memcpy((u8 *)&salt_63_0, salt_p, 8);
> -               memcpy((u8 *)&ssci_salt_95_64, salt_p + 8, 4);
> -               ssci_salt_95_64 |= (__force u64)txsc->ssci[assoc_num] <<
> 32;
> -
> -               plcy_req->plcy[0][6] = salt_63_0;
> -               plcy_req->plcy[0][7] = ssci_salt_95_64;
> -       }
> +       ret = cn10k_mcs_write_keys(pfvf, secy, plcy_req, sak,
> +                                  salt, txsc->ssci[assoc_num]);
> +       if (ret)
> +               goto fail;
>
>         plcy_req->plcy[0][8] = assoc_num;
>         plcy_req->sa_index[0] = txsc->hw_sa_id[assoc_num];
> --
> 2.7.4
>
>
>

-- 
Regards,
Kalesh A P

[-- Attachment #1.2: Type: text/html, Size: 11137 bytes --]

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4239 bytes --]

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

* RE: [EXT] Re: [net PATCH] octeontx2-pf: mcs: Generate hash key using ecb(aes)
       [not found]   ` <CO1PR18MB4666877CBEAB2D8290C2ACB8A13BA@CO1PR18MB4666.namprd18.prod.outlook.com>
@ 2023-07-17  4:50     ` Subbaraya Sundeep Bhatta
  2023-07-17  5:19       ` Kalesh Anakkur Purayil
  0 siblings, 1 reply; 5+ messages in thread
From: Subbaraya Sundeep Bhatta @ 2023-07-17  4:50 UTC (permalink / raw)
  To: Kalesh Anakkur Purayil
  Cc: netdev, linux-kernel, kuba, davem, edumazet, pabeni,
	Sunil Kovvuri Goutham, Geethasowjanya Akula, Hariprasad Kelam,
	Naveen Mamindlapalli

Hi,

>From: Kalesh Anakkur Purayil <kalesh-anakkur.purayil@broadcom.com>
>Sent: Sunday, July 16, 2023 4:20 PM
>To: Subbaraya Sundeep Bhatta <sbhatta@marvell.com>
>Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; kuba@kernel.org;
>davem@davemloft.net; edumazet@google.com; pabeni@redhat.com; Sunil
>Kovvuri Goutham <sgoutham@marvell.com>; Geethasowjanya Akula
><gakula@marvell.com>; Hariprasad Kelam <hkelam@marvell.com>; Naveen
>Mamindlapalli <naveenm@marvell.com>
>Subject: Re: [net PATCH] octeontx2-pf: mcs: Generate hash key using ecb(aes)
>
>
>On Fri, Jul 14, 2023 at 9:53 PM Subbaraya Sundeep
><mailto:sbhatta@marvell.com> wrote:
>Hardware generated encryption and ICV tags are found to
>be wrong when tested with IEEE MACSEC test vectors.
>This is because as per the HRM, the hash key (derived by
>AES-ECB block encryption of an all 0s block with the SAK)
>has to be programmed by the software in
>MCSX_RS_MCS_CPM_TX_SLAVE_SA_PLCY_MEM_4X register.
>Hence fix this by generating hash key in software and
>configuring in hardware.
>
>Fixes: c54ffc73601c ("octeontx2-pf: mcs: Introduce MACSEC hardware
>offloading")
>Signed-off-by: Subbaraya Sundeep <mailto:sbhatta@marvell.com>
>---
> .../ethernet/marvell/octeontx2/nic/cn10k_macsec.c  | 132 +++++++++++++++---
>---
> 1 file changed, 95 insertions(+), 37 deletions(-)
>
>diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_macsec.c
>b/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_macsec.c
>index 6e2fb24..9f23118 100644
>--- a/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_macsec.c
>+++ b/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_macsec.c
>@@ -4,6 +4,7 @@
>  * Copyright (C) 2022 Marvell.
>  */
>
>+#include <crypto/skcipher.h>
> #include <linux/rtnetlink.h>
> #include <linux/bitfield.h>
> #include "otx2_common.h"
>@@ -42,6 +43,51 @@
> #define MCS_TCI_E                      0x08 /* encryption */
> #define MCS_TCI_C                      0x04 /* changed text */
>
>+#define CN10K_MAX_HASH_LEN             16
>+#define CN10K_MAX_SAK_LEN              32
>+
>+static int cn10k_ecb_aes_encrypt(struct otx2_nic *pfvf, u8 *sak,
>+                                u16 sak_len, u8 *hash)
>+{
>+       u8 data[CN10K_MAX_HASH_LEN] = { 0 };
>[Kalesh]: There is no need in 0 here, just use {}
>
Input has to be all zeroes. AES-ECB block encryption of an all 0s block with the SAK key.
Hence needed.

>+       struct skcipher_request *req = NULL;
>+       struct scatterlist sg_src, sg_dst;
>+       struct crypto_skcipher *tfm;
>+       DECLARE_CRYPTO_WAIT(wait);
>+       int err;
>+
>+       tfm = crypto_alloc_skcipher("ecb(aes)", 0, 0);
>+       if (IS_ERR(tfm)) {
>+               dev_err(pfvf->dev, "failed to allocate transform for ecb-aes\n");
>+               return PTR_ERR(tfm);
>+       }
>+
>+       req = skcipher_request_alloc(tfm, GFP_KERNEL);
>+       if (!req) {
>+               dev_err(pfvf->dev, "failed to allocate request for skcipher\n");
>+               err = -ENOMEM;
>+               goto out;
>+       }
>+
>+       err = crypto_skcipher_setkey(tfm, sak, sak_len);
>[Kalesh]: No need for a return value check here?
Missed it. Will add.

>+
>+       /* build sg list */
>+       sg_init_one(&sg_src, data, CN10K_MAX_HASH_LEN);
>+       sg_init_one(&sg_dst, hash, CN10K_MAX_HASH_LEN);
>+
>+       skcipher_request_set_callback(req, 0, crypto_req_done, &wait);
>+       skcipher_request_set_crypt(req, &sg_src, &sg_dst,
>+                                  CN10K_MAX_HASH_LEN, NULL);
>+
>+       err = crypto_skcipher_encrypt(req);
>+       err = crypto_wait_req(err, &wait);
>+
>+out:
>+       skcipher_request_free(req);
>[Kalesh]: I think you should move the label here.

No. After adding the new check, label must be above only. 

>+       crypto_free_skcipher(tfm);
>+       return err;
>+}
>+
> static struct cn10k_mcs_txsc *cn10k_mcs_get_txsc(struct cn10k_mcs_cfg *cfg,
>                                                 struct macsec_secy *secy)
> {
>@@ -330,19 +376,53 @@ static int cn10k_mcs_write_sc_cam(struct otx2_nic
>*pfvf,
>        return ret;
> }
>
>+static int cn10k_mcs_write_keys(struct otx2_nic *pfvf,
>+                               struct macsec_secy *secy,
>+                               struct mcs_sa_plcy_write_req *req,
>+                               u8 *sak, u8 *salt, ssci_t ssci)
>+{
>+       u8 hash_rev[CN10K_MAX_HASH_LEN] = { 0 };
>[Kalesh]: There is no need in 0 here, just use {}
>
Okay

Thanks,
Sundeep

>+       u8 sak_rev[CN10K_MAX_SAK_LEN] = { 0 };
>+       u8 salt_rev[MACSEC_SALT_LEN] = { 0 };
>+       u8 hash[CN10K_MAX_HASH_LEN] = { 0 };
>+       u32 ssci_63_32;
>+       int err, i;
>+
>+       err = cn10k_ecb_aes_encrypt(pfvf, sak, secy->key_len, hash);
>+       if (err) {
>+               dev_err(pfvf->dev, "Generating hash using ECB(AES) failed\n");
>+               return err;
>+       }
>+
>+       for (i = 0; i < secy->key_len; i++)
>+               sak_rev[i] = sak[secy->key_len - 1 - i];
>+
>+       for (i = 0; i < CN10K_MAX_HASH_LEN; i++)
>+               hash_rev[i] = hash[CN10K_MAX_HASH_LEN - 1 - i];
>+
>+       for (i = 0; i < MACSEC_SALT_LEN; i++)
>+               salt_rev[i] = salt[MACSEC_SALT_LEN - 1 - i];
>+
>+       ssci_63_32 = (__force u32)cpu_to_be32((__force u32)ssci);
>+
>+       memcpy(&req->plcy[0][0], sak_rev, secy->key_len);
>+       memcpy(&req->plcy[0][4], hash_rev, CN10K_MAX_HASH_LEN);
>+       memcpy(&req->plcy[0][6], salt_rev, MACSEC_SALT_LEN);
>+       req->plcy[0][7] |= (u64)ssci_63_32 << 32;
>+
>+       return 0;
>+}
>+
> static int cn10k_mcs_write_rx_sa_plcy(struct otx2_nic *pfvf,
>                                      struct macsec_secy *secy,
>                                      struct cn10k_mcs_rxsc *rxsc,
>                                      u8 assoc_num, bool sa_in_use)
> {
>-       unsigned char *src = rxsc->sa_key[assoc_num];
>        struct mcs_sa_plcy_write_req *plcy_req;
>-       u8 *salt_p = rxsc->salt[assoc_num];
>+       u8 *sak = rxsc->sa_key[assoc_num];
>+       u8 *salt = rxsc->salt[assoc_num];
>        struct mcs_rx_sc_sa_map *map_req;
>        struct mbox *mbox = &pfvf->mbox;
>-       u64 ssci_salt_95_64 = 0;
>-       u8 reg, key_len;
>-       u64 salt_63_0;
>        int ret;
>
>        mutex_lock(&mbox->lock);
>@@ -360,20 +440,10 @@ static int cn10k_mcs_write_rx_sa_plcy(struct otx2_nic
>*pfvf,
>                goto fail;
>        }
>
>-       for (reg = 0, key_len = 0; key_len < secy->key_len; key_len += 8) {
>-               memcpy((u8 *)&plcy_req->plcy[0][reg],
>-                      (src + reg * 8), 8);
>-               reg++;
>-       }
>-
>-       if (secy->xpn) {
>-               memcpy((u8 *)&salt_63_0, salt_p, 8);
>-               memcpy((u8 *)&ssci_salt_95_64, salt_p + 8, 4);
>-               ssci_salt_95_64 |= (__force u64)rxsc->ssci[assoc_num] << 32;
>-
>-               plcy_req->plcy[0][6] = salt_63_0;
>-               plcy_req->plcy[0][7] = ssci_salt_95_64;
>-       }
>+       ret = cn10k_mcs_write_keys(pfvf, secy, plcy_req, sak,
>+                                  salt, rxsc->ssci[assoc_num]);
>+       if (ret)
>+               goto fail;
>
>        plcy_req->sa_index[0] = rxsc->hw_sa_id[assoc_num];
>        plcy_req->sa_cnt = 1;
>@@ -586,13 +656,10 @@ static int cn10k_mcs_write_tx_sa_plcy(struct otx2_nic
>*pfvf,
>                                      struct cn10k_mcs_txsc *txsc,
>                                      u8 assoc_num)
> {
>-       unsigned char *src = txsc->sa_key[assoc_num];
>        struct mcs_sa_plcy_write_req *plcy_req;
>-       u8 *salt_p = txsc->salt[assoc_num];
>+       u8 *sak = txsc->sa_key[assoc_num];
>+       u8 *salt = txsc->salt[assoc_num];
>        struct mbox *mbox = &pfvf->mbox;
>-       u64 ssci_salt_95_64 = 0;
>-       u8 reg, key_len;
>-       u64 salt_63_0;
>        int ret;
>
>        mutex_lock(&mbox->lock);
>@@ -603,19 +670,10 @@ static int cn10k_mcs_write_tx_sa_plcy(struct otx2_nic
>*pfvf,
>                goto fail;
>        }
>
>-       for (reg = 0, key_len = 0; key_len < secy->key_len; key_len += 8) {
>-               memcpy((u8 *)&plcy_req->plcy[0][reg], (src + reg * 8), 8);
>-               reg++;
>-       }
>-
>-       if (secy->xpn) {
>-               memcpy((u8 *)&salt_63_0, salt_p, 8);
>-               memcpy((u8 *)&ssci_salt_95_64, salt_p + 8, 4);
>-               ssci_salt_95_64 |= (__force u64)txsc->ssci[assoc_num] << 32;
>-
>-               plcy_req->plcy[0][6] = salt_63_0;
>-               plcy_req->plcy[0][7] = ssci_salt_95_64;
>-       }
>+       ret = cn10k_mcs_write_keys(pfvf, secy, plcy_req, sak,
>+                                  salt, txsc->ssci[assoc_num]);
>+       if (ret)
>+               goto fail;
>
>        plcy_req->plcy[0][8] = assoc_num;
>        plcy_req->sa_index[0] = txsc->hw_sa_id[assoc_num];
>--
>2.7.4
>
>
>
>
>--
>Regards,
>Kalesh A P

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

* Re: [EXT] Re: [net PATCH] octeontx2-pf: mcs: Generate hash key using ecb(aes)
  2023-07-17  4:50     ` [EXT] " Subbaraya Sundeep Bhatta
@ 2023-07-17  5:19       ` Kalesh Anakkur Purayil
       [not found]         ` <CO1PR18MB4666108ABAC5C1033B45FA52A13BA@CO1PR18MB4666.namprd18.prod.outlook.com>
  0 siblings, 1 reply; 5+ messages in thread
From: Kalesh Anakkur Purayil @ 2023-07-17  5:19 UTC (permalink / raw)
  To: Subbaraya Sundeep Bhatta
  Cc: netdev, linux-kernel, kuba, davem, edumazet, pabeni,
	Sunil Kovvuri Goutham, Geethasowjanya Akula, Hariprasad Kelam,
	Naveen Mamindlapalli


[-- Attachment #1.1: Type: text/plain, Size: 9895 bytes --]

On Mon, Jul 17, 2023 at 10:20 AM Subbaraya Sundeep Bhatta <
sbhatta@marvell.com> wrote:

> Hi,
>
> >From: Kalesh Anakkur Purayil <kalesh-anakkur.purayil@broadcom.com>
> >Sent: Sunday, July 16, 2023 4:20 PM
> >To: Subbaraya Sundeep Bhatta <sbhatta@marvell.com>
> >Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; kuba@kernel.org
> ;
> >davem@davemloft.net; edumazet@google.com; pabeni@redhat.com; Sunil
> >Kovvuri Goutham <sgoutham@marvell.com>; Geethasowjanya Akula
> ><gakula@marvell.com>; Hariprasad Kelam <hkelam@marvell.com>; Naveen
> >Mamindlapalli <naveenm@marvell.com>
> >Subject: Re: [net PATCH] octeontx2-pf: mcs: Generate hash key using
> ecb(aes)
> >
> >
> >On Fri, Jul 14, 2023 at 9:53 PM Subbaraya Sundeep
> ><mailto:sbhatta@marvell.com> wrote:
> >Hardware generated encryption and ICV tags are found to
> >be wrong when tested with IEEE MACSEC test vectors.
> >This is because as per the HRM, the hash key (derived by
> >AES-ECB block encryption of an all 0s block with the SAK)
> >has to be programmed by the software in
> >MCSX_RS_MCS_CPM_TX_SLAVE_SA_PLCY_MEM_4X register.
> >Hence fix this by generating hash key in software and
> >configuring in hardware.
> >
> >Fixes: c54ffc73601c ("octeontx2-pf: mcs: Introduce MACSEC hardware
> >offloading")
> >Signed-off-by: Subbaraya Sundeep <mailto:sbhatta@marvell.com>
> >---
> > .../ethernet/marvell/octeontx2/nic/cn10k_macsec.c  | 132
> +++++++++++++++---
> >---
> > 1 file changed, 95 insertions(+), 37 deletions(-)
> >
> >diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_macsec.c
> >b/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_macsec.c
> >index 6e2fb24..9f23118 100644
> >--- a/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_macsec.c
> >+++ b/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_macsec.c
> >@@ -4,6 +4,7 @@
> >  * Copyright (C) 2022 Marvell.
> >  */
> >
> >+#include <crypto/skcipher.h>
> > #include <linux/rtnetlink.h>
> > #include <linux/bitfield.h>
> > #include "otx2_common.h"
> >@@ -42,6 +43,51 @@
> > #define MCS_TCI_E                      0x08 /* encryption */
> > #define MCS_TCI_C                      0x04 /* changed text */
> >
> >+#define CN10K_MAX_HASH_LEN             16
> >+#define CN10K_MAX_SAK_LEN              32
> >+
> >+static int cn10k_ecb_aes_encrypt(struct otx2_nic *pfvf, u8 *sak,
> >+                                u16 sak_len, u8 *hash)
> >+{
> >+       u8 data[CN10K_MAX_HASH_LEN] = { 0 };
> >[Kalesh]: There is no need in 0 here, just use {}
> >
> Input has to be all zeroes. AES-ECB block encryption of an all 0s block
> with the SAK key.
> Hence needed.
>

> >+       struct skcipher_request *req = NULL;
> >+       struct scatterlist sg_src, sg_dst;
> >+       struct crypto_skcipher *tfm;
> >+       DECLARE_CRYPTO_WAIT(wait);
> >+       int err;
> >+
> >+       tfm = crypto_alloc_skcipher("ecb(aes)", 0, 0);
> >+       if (IS_ERR(tfm)) {
> >+               dev_err(pfvf->dev, "failed to allocate transform for
> ecb-aes\n");
> >+               return PTR_ERR(tfm);
> >+       }
> >+
> >+       req = skcipher_request_alloc(tfm, GFP_KERNEL);
> >+       if (!req) {
> >+               dev_err(pfvf->dev, "failed to allocate request for
> skcipher\n");
> >+               err = -ENOMEM;
> >+               goto out;
> >+       }
> >+
> >+       err = crypto_skcipher_setkey(tfm, sak, sak_len);
> >[Kalesh]: No need for a return value check here?
> Missed it. Will add.
>
> >+
> >+       /* build sg list */
> >+       sg_init_one(&sg_src, data, CN10K_MAX_HASH_LEN);
> >+       sg_init_one(&sg_dst, hash, CN10K_MAX_HASH_LEN);
> >+
> >+       skcipher_request_set_callback(req, 0, crypto_req_done, &wait);
> >+       skcipher_request_set_crypt(req, &sg_src, &sg_dst,
> >+                                  CN10K_MAX_HASH_LEN, NULL);
> >+
> >+       err = crypto_skcipher_encrypt(req);
> >+       err = crypto_wait_req(err, &wait);
> >+
> >+out:
> >+       skcipher_request_free(req);
> >[Kalesh]: I think you should move the label here.
>
> No. After adding the new check, label must be above only.

[Kalesh] Sorry, I did not get that.

Why to invoke skcipher_request_free() when skcipher_request_alloc() fails?
Am I missing something here?

>
>
> >+       crypto_free_skcipher(tfm);
> >+       return err;
> >+}
> >+
> > static struct cn10k_mcs_txsc *cn10k_mcs_get_txsc(struct cn10k_mcs_cfg
> *cfg,
> >                                                 struct macsec_secy *secy)
> > {
> >@@ -330,19 +376,53 @@ static int cn10k_mcs_write_sc_cam(struct otx2_nic
> >*pfvf,
> >        return ret;
> > }
> >
> >+static int cn10k_mcs_write_keys(struct otx2_nic *pfvf,
> >+                               struct macsec_secy *secy,
> >+                               struct mcs_sa_plcy_write_req *req,
> >+                               u8 *sak, u8 *salt, ssci_t ssci)
> >+{
> >+       u8 hash_rev[CN10K_MAX_HASH_LEN] = { 0 };
> >[Kalesh]: There is no need in 0 here, just use {}
> >
> Okay
>
> Thanks,
> Sundeep
>
> >+       u8 sak_rev[CN10K_MAX_SAK_LEN] = { 0 };
> >+       u8 salt_rev[MACSEC_SALT_LEN] = { 0 };
> >+       u8 hash[CN10K_MAX_HASH_LEN] = { 0 };
> >+       u32 ssci_63_32;
> >+       int err, i;
> >+
> >+       err = cn10k_ecb_aes_encrypt(pfvf, sak, secy->key_len, hash);
> >+       if (err) {
> >+               dev_err(pfvf->dev, "Generating hash using ECB(AES)
> failed\n");
> >+               return err;
> >+       }
> >+
> >+       for (i = 0; i < secy->key_len; i++)
> >+               sak_rev[i] = sak[secy->key_len - 1 - i];
> >+
> >+       for (i = 0; i < CN10K_MAX_HASH_LEN; i++)
> >+               hash_rev[i] = hash[CN10K_MAX_HASH_LEN - 1 - i];
> >+
> >+       for (i = 0; i < MACSEC_SALT_LEN; i++)
> >+               salt_rev[i] = salt[MACSEC_SALT_LEN - 1 - i];
> >+
> >+       ssci_63_32 = (__force u32)cpu_to_be32((__force u32)ssci);
> >+
> >+       memcpy(&req->plcy[0][0], sak_rev, secy->key_len);
> >+       memcpy(&req->plcy[0][4], hash_rev, CN10K_MAX_HASH_LEN);
> >+       memcpy(&req->plcy[0][6], salt_rev, MACSEC_SALT_LEN);
> >+       req->plcy[0][7] |= (u64)ssci_63_32 << 32;
> >+
> >+       return 0;
> >+}
> >+
> > static int cn10k_mcs_write_rx_sa_plcy(struct otx2_nic *pfvf,
> >                                      struct macsec_secy *secy,
> >                                      struct cn10k_mcs_rxsc *rxsc,
> >                                      u8 assoc_num, bool sa_in_use)
> > {
> >-       unsigned char *src = rxsc->sa_key[assoc_num];
> >        struct mcs_sa_plcy_write_req *plcy_req;
> >-       u8 *salt_p = rxsc->salt[assoc_num];
> >+       u8 *sak = rxsc->sa_key[assoc_num];
> >+       u8 *salt = rxsc->salt[assoc_num];
> >        struct mcs_rx_sc_sa_map *map_req;
> >        struct mbox *mbox = &pfvf->mbox;
> >-       u64 ssci_salt_95_64 = 0;
> >-       u8 reg, key_len;
> >-       u64 salt_63_0;
> >        int ret;
> >
> >        mutex_lock(&mbox->lock);
> >@@ -360,20 +440,10 @@ static int cn10k_mcs_write_rx_sa_plcy(struct
> otx2_nic
> >*pfvf,
> >                goto fail;
> >        }
> >
> >-       for (reg = 0, key_len = 0; key_len < secy->key_len; key_len += 8)
> {
> >-               memcpy((u8 *)&plcy_req->plcy[0][reg],
> >-                      (src + reg * 8), 8);
> >-               reg++;
> >-       }
> >-
> >-       if (secy->xpn) {
> >-               memcpy((u8 *)&salt_63_0, salt_p, 8);
> >-               memcpy((u8 *)&ssci_salt_95_64, salt_p + 8, 4);
> >-               ssci_salt_95_64 |= (__force u64)rxsc->ssci[assoc_num] <<
> 32;
> >-
> >-               plcy_req->plcy[0][6] = salt_63_0;
> >-               plcy_req->plcy[0][7] = ssci_salt_95_64;
> >-       }
> >+       ret = cn10k_mcs_write_keys(pfvf, secy, plcy_req, sak,
> >+                                  salt, rxsc->ssci[assoc_num]);
> >+       if (ret)
> >+               goto fail;
> >
> >        plcy_req->sa_index[0] = rxsc->hw_sa_id[assoc_num];
> >        plcy_req->sa_cnt = 1;
> >@@ -586,13 +656,10 @@ static int cn10k_mcs_write_tx_sa_plcy(struct
> otx2_nic
> >*pfvf,
> >                                      struct cn10k_mcs_txsc *txsc,
> >                                      u8 assoc_num)
> > {
> >-       unsigned char *src = txsc->sa_key[assoc_num];
> >        struct mcs_sa_plcy_write_req *plcy_req;
> >-       u8 *salt_p = txsc->salt[assoc_num];
> >+       u8 *sak = txsc->sa_key[assoc_num];
> >+       u8 *salt = txsc->salt[assoc_num];
> >        struct mbox *mbox = &pfvf->mbox;
> >-       u64 ssci_salt_95_64 = 0;
> >-       u8 reg, key_len;
> >-       u64 salt_63_0;
> >        int ret;
> >
> >        mutex_lock(&mbox->lock);
> >@@ -603,19 +670,10 @@ static int cn10k_mcs_write_tx_sa_plcy(struct
> otx2_nic
> >*pfvf,
> >                goto fail;
> >        }
> >
> >-       for (reg = 0, key_len = 0; key_len < secy->key_len; key_len += 8)
> {
> >-               memcpy((u8 *)&plcy_req->plcy[0][reg], (src + reg * 8), 8);
> >-               reg++;
> >-       }
> >-
> >-       if (secy->xpn) {
> >-               memcpy((u8 *)&salt_63_0, salt_p, 8);
> >-               memcpy((u8 *)&ssci_salt_95_64, salt_p + 8, 4);
> >-               ssci_salt_95_64 |= (__force u64)txsc->ssci[assoc_num] <<
> 32;
> >-
> >-               plcy_req->plcy[0][6] = salt_63_0;
> >-               plcy_req->plcy[0][7] = ssci_salt_95_64;
> >-       }
> >+       ret = cn10k_mcs_write_keys(pfvf, secy, plcy_req, sak,
> >+                                  salt, txsc->ssci[assoc_num]);
> >+       if (ret)
> >+               goto fail;
> >
> >        plcy_req->plcy[0][8] = assoc_num;
> >        plcy_req->sa_index[0] = txsc->hw_sa_id[assoc_num];
> >--
> >2.7.4
> >
> >
> >
> >
> >--
> >Regards,
> >Kalesh A P
>


-- 
Regards,
Kalesh A P

[-- Attachment #1.2: Type: text/html, Size: 13830 bytes --]

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4239 bytes --]

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

* RE: [EXT] Re: [net PATCH] octeontx2-pf: mcs: Generate hash key using ecb(aes)
       [not found]           ` <CO1PR18MB46667EB15473D055F56A4971A13BA@CO1PR18MB4666.namprd18.prod.outlook.com>
@ 2023-07-17  5:39             ` Subbaraya Sundeep Bhatta
  0 siblings, 0 replies; 5+ messages in thread
From: Subbaraya Sundeep Bhatta @ 2023-07-17  5:39 UTC (permalink / raw)
  To: Kalesh Anakkur Purayil
  Cc: netdev, linux-kernel, kuba, davem, edumazet, pabeni,
	Sunil Kovvuri Goutham, Geethasowjanya Akula, Hariprasad Kelam,
	Naveen Mamindlapalli

Hi,

>From: Kalesh Anakkur Purayil <mailto:kalesh-anakkur.purayil@broadcom.com>
>Sent: Monday, July 17, 2023 10:50 AM
>To: Subbaraya Sundeep Bhatta <mailto:sbhatta@marvell.com>
>Cc: mailto:netdev@vger.kernel.org; mailto:linux-kernel@vger.kernel.org;
>mailto:kuba@kernel.org; mailto:davem@davemloft.net;
>mailto:edumazet@google.com; mailto:pabeni@redhat.com; Sunil Kovvuri
>Goutham <mailto:sgoutham@marvell.com>; Geethasowjanya Akula
><mailto:gakula@marvell.com>; Hariprasad Kelam
><mailto:hkelam@marvell.com>; Naveen Mamindlapalli
><mailto:naveenm@marvell.com>
>Subject: Re: [net PATCH] octeontx2-pf: mcs: Generate hash key using ecb(aes)
>
>
>
>On Mon, Jul 17, 2023 at 10:20 AM Subbaraya Sundeep Bhatta
><mailto:sbhatta@marvell.com> wrote:
>Hi,
>
>>From: Kalesh Anakkur Purayil <mailto:kalesh-anakkur.purayil@broadcom.com>
>>Sent: Sunday, July 16, 2023 4:20 PM
>>To: Subbaraya Sundeep Bhatta <mailto:sbhatta@marvell.com>
>>Cc: mailto:netdev@vger.kernel.org; mailto:linux-kernel@vger.kernel.org;
>mailto:kuba@kernel.org;
>>mailto:davem@davemloft.net; mailto:edumazet@google.com;
>mailto:pabeni@redhat.com; Sunil
>>Kovvuri Goutham <mailto:sgoutham@marvell.com>; Geethasowjanya Akula
>><mailto:gakula@marvell.com>; Hariprasad Kelam
><mailto:hkelam@marvell.com>; Naveen
>>Mamindlapalli <mailto:naveenm@marvell.com>
>>Subject: Re: [net PATCH] octeontx2-pf: mcs: Generate hash key using ecb(aes)
>>
>>
>>On Fri, Jul 14, 2023 at 9:53 PM Subbaraya Sundeep
>><mailto:mailto:sbhatta@marvell.com> wrote:
>>Hardware generated encryption and ICV tags are found to
>>be wrong when tested with IEEE MACSEC test vectors.
>>This is because as per the HRM, the hash key (derived by
>>AES-ECB block encryption of an all 0s block with the SAK)
>>has to be programmed by the software in
>>MCSX_RS_MCS_CPM_TX_SLAVE_SA_PLCY_MEM_4X register.
>>Hence fix this by generating hash key in software and
>>configuring in hardware.
>>
>>Fixes: c54ffc73601c ("octeontx2-pf: mcs: Introduce MACSEC hardware
>>offloading")
>>Signed-off-by: Subbaraya Sundeep <mailto:mailto:sbhatta@marvell.com>
>>---
>> .../ethernet/marvell/octeontx2/nic/cn10k_macsec.c  | 132 +++++++++++++++--
>-
>>---
>> 1 file changed, 95 insertions(+), 37 deletions(-)
>>
>>diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_macsec.c
>>b/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_macsec.c
>>index 6e2fb24..9f23118 100644
>>--- a/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_macsec.c
>>+++ b/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_macsec.c
>>@@ -4,6 +4,7 @@
>>  * Copyright (C) 2022 Marvell.
>>  */
>>
>>+#include <crypto/skcipher.h>
>> #include <linux/rtnetlink.h>
>> #include <linux/bitfield.h>
>> #include "otx2_common.h"
>>@@ -42,6 +43,51 @@
>> #define MCS_TCI_E                      0x08 /* encryption */
>> #define MCS_TCI_C                      0x04 /* changed text */
>>
>>+#define CN10K_MAX_HASH_LEN             16
>>+#define CN10K_MAX_SAK_LEN              32
>>+
>>+static int cn10k_ecb_aes_encrypt(struct otx2_nic *pfvf, u8 *sak,
>>+                                u16 sak_len, u8 *hash)
>>+{
>>+       u8 data[CN10K_MAX_HASH_LEN] = { 0 };
>>[Kalesh]: There is no need in 0 here, just use {}
>>
>Input has to be all zeroes. AES-ECB block encryption of an all 0s block with the
>SAK key.
>Hence needed.
>
>>+       struct skcipher_request *req = NULL;
>>+       struct scatterlist sg_src, sg_dst;
>>+       struct crypto_skcipher *tfm;
>>+       DECLARE_CRYPTO_WAIT(wait);
>>+       int err;
>>+
>>+       tfm = crypto_alloc_skcipher("ecb(aes)", 0, 0);
>>+       if (IS_ERR(tfm)) {
>>+               dev_err(pfvf->dev, "failed to allocate transform for ecb-aes\n");
>>+               return PTR_ERR(tfm);
>>+       }
>>+
>>+       req = skcipher_request_alloc(tfm, GFP_KERNEL);
>>+       if (!req) {
>>+               dev_err(pfvf->dev, "failed to allocate request for skcipher\n");
>>+               err = -ENOMEM;
>>+               goto out;
>>+       }
>>+
>>+       err = crypto_skcipher_setkey(tfm, sak, sak_len);
>>[Kalesh]: No need for a return value check here?
>Missed it. Will add.
>
>>+
>>+       /* build sg list */
>>+       sg_init_one(&sg_src, data, CN10K_MAX_HASH_LEN);
>>+       sg_init_one(&sg_dst, hash, CN10K_MAX_HASH_LEN);
>>+
>>+       skcipher_request_set_callback(req, 0, crypto_req_done, &wait);
>>+       skcipher_request_set_crypt(req, &sg_src, &sg_dst,
>>+                                  CN10K_MAX_HASH_LEN, NULL);
>>+
>>+       err = crypto_skcipher_encrypt(req);
>>+       err = crypto_wait_req(err, &wait);
>>+
>>+out:
>>+       skcipher_request_free(req);
>>[Kalesh]: I think you should move the label here.
>>
>>No. After adding the new check, label must be above only.
>[Kalesh] Sorry, I did not get that.
>Why to invoke skcipher_request_free() when skcipher_request_alloc() fails? Am I
>missing something here?
>

I avoided another label since we can call skcipher_request_free even input req is NULL.
Anyway will add new label so that it is easier to understand and send v2. Thanks for review.

Sundeep

>>+       crypto_free_skcipher(tfm);
>>+       return err;
>>+}
>>+
>> static struct cn10k_mcs_txsc *cn10k_mcs_get_txsc(struct cn10k_mcs_cfg *cfg,
>>                                                 struct macsec_secy *secy)
>> {
>>@@ -330,19 +376,53 @@ static int cn10k_mcs_write_sc_cam(struct otx2_nic
>>*pfvf,
>>        return ret;
>> }
>>
>>+static int cn10k_mcs_write_keys(struct otx2_nic *pfvf,
>>+                               struct macsec_secy *secy,
>>+                               struct mcs_sa_plcy_write_req *req,
>>+                               u8 *sak, u8 *salt, ssci_t ssci)
>>+{
>>+       u8 hash_rev[CN10K_MAX_HASH_LEN] = { 0 };
>>[Kalesh]: There is no need in 0 here, just use {}
>>
>Okay
>
>Thanks,
>Sundeep
>
>>+       u8 sak_rev[CN10K_MAX_SAK_LEN] = { 0 };
>>+       u8 salt_rev[MACSEC_SALT_LEN] = { 0 };
>>+       u8 hash[CN10K_MAX_HASH_LEN] = { 0 };
>>+       u32 ssci_63_32;
>>+       int err, i;
>>+
>>+       err = cn10k_ecb_aes_encrypt(pfvf, sak, secy->key_len, hash);
>>+       if (err) {
>>+               dev_err(pfvf->dev, "Generating hash using ECB(AES) failed\n");
>>+               return err;
>>+       }
>>+
>>+       for (i = 0; i < secy->key_len; i++)
>>+               sak_rev[i] = sak[secy->key_len - 1 - i];
>>+
>>+       for (i = 0; i < CN10K_MAX_HASH_LEN; i++)
>>+               hash_rev[i] = hash[CN10K_MAX_HASH_LEN - 1 - i];
>>+
>>+       for (i = 0; i < MACSEC_SALT_LEN; i++)
>>+               salt_rev[i] = salt[MACSEC_SALT_LEN - 1 - i];
>>+
>>+       ssci_63_32 = (__force u32)cpu_to_be32((__force u32)ssci);
>>+
>>+       memcpy(&req->plcy[0][0], sak_rev, secy->key_len);
>>+       memcpy(&req->plcy[0][4], hash_rev, CN10K_MAX_HASH_LEN);
>>+       memcpy(&req->plcy[0][6], salt_rev, MACSEC_SALT_LEN);
>>+       req->plcy[0][7] |= (u64)ssci_63_32 << 32;
>>+
>>+       return 0;
>>+}
>>+
>> static int cn10k_mcs_write_rx_sa_plcy(struct otx2_nic *pfvf,
>>                                      struct macsec_secy *secy,
>>                                      struct cn10k_mcs_rxsc *rxsc,
>>                                      u8 assoc_num, bool sa_in_use)
>> {
>>-       unsigned char *src = rxsc->sa_key[assoc_num];
>>        struct mcs_sa_plcy_write_req *plcy_req;
>>-       u8 *salt_p = rxsc->salt[assoc_num];
>>+       u8 *sak = rxsc->sa_key[assoc_num];
>>+       u8 *salt = rxsc->salt[assoc_num];
>>        struct mcs_rx_sc_sa_map *map_req;
>>        struct mbox *mbox = &pfvf->mbox;
>>-       u64 ssci_salt_95_64 = 0;
>>-       u8 reg, key_len;
>>-       u64 salt_63_0;
>>        int ret;
>>
>>        mutex_lock(&mbox->lock);
>>@@ -360,20 +440,10 @@ static int cn10k_mcs_write_rx_sa_plcy(struct
>otx2_nic
>>*pfvf,
>>                goto fail;
>>        }
>>
>>-       for (reg = 0, key_len = 0; key_len < secy->key_len; key_len += 8) {
>>-               memcpy((u8 *)&plcy_req->plcy[0][reg],
>>-                      (src + reg * 8), 8);
>>-               reg++;
>>-       }
>>-
>>-       if (secy->xpn) {
>>-               memcpy((u8 *)&salt_63_0, salt_p, 8);
>>-               memcpy((u8 *)&ssci_salt_95_64, salt_p + 8, 4);
>>-               ssci_salt_95_64 |= (__force u64)rxsc->ssci[assoc_num] << 32;
>>-
>>-               plcy_req->plcy[0][6] = salt_63_0;
>>-               plcy_req->plcy[0][7] = ssci_salt_95_64;
>>-       }
>>+       ret = cn10k_mcs_write_keys(pfvf, secy, plcy_req, sak,
>>+                                  salt, rxsc->ssci[assoc_num]);
>>+       if (ret)
>>+               goto fail;
>>
>>        plcy_req->sa_index[0] = rxsc->hw_sa_id[assoc_num];
>>        plcy_req->sa_cnt = 1;
>>@@ -586,13 +656,10 @@ static int cn10k_mcs_write_tx_sa_plcy(struct
>otx2_nic
>>*pfvf,
>>                                      struct cn10k_mcs_txsc *txsc,
>>                                      u8 assoc_num)
>> {
>>-       unsigned char *src = txsc->sa_key[assoc_num];
>>        struct mcs_sa_plcy_write_req *plcy_req;
>>-       u8 *salt_p = txsc->salt[assoc_num];
>>+       u8 *sak = txsc->sa_key[assoc_num];
>>+       u8 *salt = txsc->salt[assoc_num];
>>        struct mbox *mbox = &pfvf->mbox;
>>-       u64 ssci_salt_95_64 = 0;
>>-       u8 reg, key_len;
>>-       u64 salt_63_0;
>>        int ret;
>>
>>        mutex_lock(&mbox->lock);
>>@@ -603,19 +670,10 @@ static int cn10k_mcs_write_tx_sa_plcy(struct
>otx2_nic
>>*pfvf,
>>                goto fail;
>>        }
>>
>>-       for (reg = 0, key_len = 0; key_len < secy->key_len; key_len += 8) {
>>-               memcpy((u8 *)&plcy_req->plcy[0][reg], (src + reg * 8), 8);
>>-               reg++;
>>-       }
>>-
>>-       if (secy->xpn) {
>>-               memcpy((u8 *)&salt_63_0, salt_p, 8);
>>-               memcpy((u8 *)&ssci_salt_95_64, salt_p + 8, 4);
>>-               ssci_salt_95_64 |= (__force u64)txsc->ssci[assoc_num] << 32;
>>-
>>-               plcy_req->plcy[0][6] = salt_63_0;
>>-               plcy_req->plcy[0][7] = ssci_salt_95_64;
>>-       }
>>+       ret = cn10k_mcs_write_keys(pfvf, secy, plcy_req, sak,
>>+                                  salt, txsc->ssci[assoc_num]);
>>+       if (ret)
>>+               goto fail;
>>
>>        plcy_req->plcy[0][8] = assoc_num;
>>        plcy_req->sa_index[0] = txsc->hw_sa_id[assoc_num];
>>--
>>2.7.4
>>
>>
>>
>>
>>--
>>Regards,
>>Kalesh A P
>
>
>
>--
>Regards,
>Kalesh A P

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

end of thread, other threads:[~2023-07-17  5:39 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-14 16:22 [net PATCH] octeontx2-pf: mcs: Generate hash key using ecb(aes) Subbaraya Sundeep
2023-07-16 10:49 ` Kalesh Anakkur Purayil
     [not found]   ` <CO1PR18MB4666877CBEAB2D8290C2ACB8A13BA@CO1PR18MB4666.namprd18.prod.outlook.com>
2023-07-17  4:50     ` [EXT] " Subbaraya Sundeep Bhatta
2023-07-17  5:19       ` Kalesh Anakkur Purayil
     [not found]         ` <CO1PR18MB4666108ABAC5C1033B45FA52A13BA@CO1PR18MB4666.namprd18.prod.outlook.com>
     [not found]           ` <CO1PR18MB46667EB15473D055F56A4971A13BA@CO1PR18MB4666.namprd18.prod.outlook.com>
2023-07-17  5:39             ` Subbaraya Sundeep Bhatta

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.