diff mbox series

[net-next] octeontx2-pf: mcs: Support VLAN in clear text

Message ID 1684148326-29569-1-git-send-email-sbhatta@marvell.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [net-next] octeontx2-pf: mcs: Support VLAN in clear text | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 8 this patch: 8
netdev/cc_maintainers success CCed 9 of 9 maintainers
netdev/build_clang success Errors and warnings before: 8 this patch: 8
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 20 this patch: 20
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 26 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Subbaraya Sundeep May 15, 2023, 10:58 a.m. UTC
Detect whether macsec secy is running on top of VLAN
which implies transmitting VLAN tag in clear text before
macsec SecTag. In this case configure hardware to insert
SecTag after VLAN tag.

Signed-off-by: Subbaraya Sundeep <sbhatta@marvell.com>
Signed-off-by: Sunil Kovvuri Goutham <sgoutham@marvell.com>
---
 drivers/net/ethernet/marvell/octeontx2/nic/cn10k_macsec.c | 7 +++++--
 drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h  | 1 +
 2 files changed, 6 insertions(+), 2 deletions(-)

Comments

Simon Horman May 15, 2023, 2:09 p.m. UTC | #1
On Mon, May 15, 2023 at 04:28:46PM +0530, Subbaraya Sundeep wrote:
> Detect whether macsec secy is running on top of VLAN
> which implies transmitting VLAN tag in clear text before
> macsec SecTag. In this case configure hardware to insert
> SecTag after VLAN tag.
> 
> Signed-off-by: Subbaraya Sundeep <sbhatta@marvell.com>
> Signed-off-by: Sunil Kovvuri Goutham <sgoutham@marvell.com>
> ---
>  drivers/net/ethernet/marvell/octeontx2/nic/cn10k_macsec.c | 7 +++++--
>  drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h  | 1 +
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_macsec.c b/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_macsec.c
> index b59532c..c5e6d57 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_macsec.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_macsec.c
> @@ -426,8 +426,10 @@ static int cn10k_mcs_write_tx_secy(struct otx2_nic *pfvf,
>  	struct mcs_secy_plcy_write_req *req;
>  	struct mbox *mbox = &pfvf->mbox;
>  	struct macsec_tx_sc *sw_tx_sc;
> -	/* Insert SecTag after 12 bytes (DA+SA)*/
> -	u8 tag_offset = 12;
> +	/* Insert SecTag after 12 bytes (DA+SA) or 16 bytes
> +	 * if VLAN tag needs to be sent in clear text.
> +	 */
> +	u8 tag_offset = txsc->vlan_dev ? 16 : 12;
>  	u8 sectag_tci = 0;
>  	u64 policy;
>  	u8 cipher;

For networking code, please arrange local variables in reverse xmas tree
order - longest line to shortest.

I would suggest in this case something like:

        struct mcs_secy_plcy_write_req *req;
        struct mbox *mbox = &pfvf->mbox;
        struct macsec_tx_sc *sw_tx_sc;
        u8 sectag_tci = 0;
        u8 tag_offset
        u64 policy;
        u8 cipher;
        int ret;

        /* Insert SecTag after 12 bytes (DA+SA)*/
        tag_offset = txsc->vlan_dev ? 16 : 12;

> @@ -1163,6 +1165,7 @@ static int cn10k_mdo_add_secy(struct macsec_context *ctx)
>  	txsc->encoding_sa = secy->tx_sc.encoding_sa;
>  	txsc->last_validate_frames = secy->validate_frames;
>  	txsc->last_replay_protect = secy->replay_protect;
> +	txsc->vlan_dev = is_vlan_dev(ctx->netdev);
>  
>  	list_add(&txsc->entry, &cfg->txsc_list);
>  
> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
> index 0f2b2a9..b2267c8 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
> @@ -419,6 +419,7 @@ struct cn10k_mcs_txsc {
>  	u8 encoding_sa;
>  	u8 salt[CN10K_MCS_SA_PER_SC][MACSEC_SALT_LEN];
>  	ssci_t ssci[CN10K_MCS_SA_PER_SC];
> +	bool vlan_dev; /* macsec running on VLAN ? */

I think it would be good, as a follow-up, to consider adding
a kdoc for this structure.

>  };
>  
>  struct cn10k_mcs_rxsc {

--
pw-bot: cr
Subbaraya Sundeep May 16, 2023, 10:25 a.m. UTC | #2
Hi Simon

>-----Original Message-----
>From: Simon Horman <simon.horman@corigine.com>
>Sent: Monday, May 15, 2023 7:39 PM
>To: Subbaraya Sundeep Bhatta <sbhatta@marvell.com>
>Cc: netdev@vger.kernel.org; davem@davemloft.net;
>edumazet@google.com; kuba@kernel.org; pabeni@redhat.com;
>Geethasowjanya Akula <gakula@marvell.com>; Naveen Mamindlapalli
><naveenm@marvell.com>; Hariprasad Kelam <hkelam@marvell.com>; Linu
>Cherian <lcherian@marvell.com>; Sunil Kovvuri Goutham
><sgoutham@marvell.com>
>Subject: Re: [net-next PATCH] octeontx2-pf: mcs: Support VLAN in clear
>text
>
>On Mon, May 15, 2023 at 04:28:46PM +0530, Subbaraya Sundeep wrote:
>> Detect whether macsec secy is running on top of VLAN
>> which implies transmitting VLAN tag in clear text before
>> macsec SecTag. In this case configure hardware to insert
>> SecTag after VLAN tag.
>>
>> Signed-off-by: Subbaraya Sundeep <sbhatta@marvell.com>
>> Signed-off-by: Sunil Kovvuri Goutham <sgoutham@marvell.com>
>> ---
>>  drivers/net/ethernet/marvell/octeontx2/nic/cn10k_macsec.c | 7 +++++--
>>  drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h  | 1 +
>>  2 files changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_macsec.c
>b/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_macsec.c
>> index b59532c..c5e6d57 100644
>> --- a/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_macsec.c
>> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_macsec.c
>> @@ -426,8 +426,10 @@ static int cn10k_mcs_write_tx_secy(struct otx2_nic
>*pfvf,
>>  	struct mcs_secy_plcy_write_req *req;
>>  	struct mbox *mbox = &pfvf->mbox;
>>  	struct macsec_tx_sc *sw_tx_sc;
>> -	/* Insert SecTag after 12 bytes (DA+SA)*/
>> -	u8 tag_offset = 12;
>> +	/* Insert SecTag after 12 bytes (DA+SA) or 16 bytes
>> +	 * if VLAN tag needs to be sent in clear text.
>> +	 */
>> +	u8 tag_offset = txsc->vlan_dev ? 16 : 12;
>>  	u8 sectag_tci = 0;
>>  	u64 policy;
>>  	u8 cipher;
>
>For networking code, please arrange local variables in reverse xmas tree
>order - longest line to shortest.
>
>I would suggest in this case something like:
>
>        struct mcs_secy_plcy_write_req *req;
>        struct mbox *mbox = &pfvf->mbox;
>        struct macsec_tx_sc *sw_tx_sc;
>        u8 sectag_tci = 0;
>        u8 tag_offset
>        u64 policy;
>        u8 cipher;
>        int ret;
>
>        /* Insert SecTag after 12 bytes (DA+SA)*/
>        tag_offset = txsc->vlan_dev ? 16 : 12;
>
Sure. Will change this and send v2.

>> @@ -1163,6 +1165,7 @@ static int cn10k_mdo_add_secy(struct
>macsec_context *ctx)
>>  	txsc->encoding_sa = secy->tx_sc.encoding_sa;
>>  	txsc->last_validate_frames = secy->validate_frames;
>>  	txsc->last_replay_protect = secy->replay_protect;
>> +	txsc->vlan_dev = is_vlan_dev(ctx->netdev);
>>
>>  	list_add(&txsc->entry, &cfg->txsc_list);
>>
>> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
>b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
>> index 0f2b2a9..b2267c8 100644
>> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
>> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
>> @@ -419,6 +419,7 @@ struct cn10k_mcs_txsc {
>>  	u8 encoding_sa;
>>  	u8 salt[CN10K_MCS_SA_PER_SC][MACSEC_SALT_LEN];
>>  	ssci_t ssci[CN10K_MCS_SA_PER_SC];
>> +	bool vlan_dev; /* macsec running on VLAN ? */
>
>I think it would be good, as a follow-up, to consider adding
>a kdoc for this structure.
>
Okay. We will check on this. 

Thanks,
Sundeep

>>  };
>>
>>  struct cn10k_mcs_rxsc {
>
>--
>pw-bot: cr
diff mbox series

Patch

diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_macsec.c b/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_macsec.c
index b59532c..c5e6d57 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_macsec.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_macsec.c
@@ -426,8 +426,10 @@  static int cn10k_mcs_write_tx_secy(struct otx2_nic *pfvf,
 	struct mcs_secy_plcy_write_req *req;
 	struct mbox *mbox = &pfvf->mbox;
 	struct macsec_tx_sc *sw_tx_sc;
-	/* Insert SecTag after 12 bytes (DA+SA)*/
-	u8 tag_offset = 12;
+	/* Insert SecTag after 12 bytes (DA+SA) or 16 bytes
+	 * if VLAN tag needs to be sent in clear text.
+	 */
+	u8 tag_offset = txsc->vlan_dev ? 16 : 12;
 	u8 sectag_tci = 0;
 	u64 policy;
 	u8 cipher;
@@ -1163,6 +1165,7 @@  static int cn10k_mdo_add_secy(struct macsec_context *ctx)
 	txsc->encoding_sa = secy->tx_sc.encoding_sa;
 	txsc->last_validate_frames = secy->validate_frames;
 	txsc->last_replay_protect = secy->replay_protect;
+	txsc->vlan_dev = is_vlan_dev(ctx->netdev);
 
 	list_add(&txsc->entry, &cfg->txsc_list);
 
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
index 0f2b2a9..b2267c8 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
@@ -419,6 +419,7 @@  struct cn10k_mcs_txsc {
 	u8 encoding_sa;
 	u8 salt[CN10K_MCS_SA_PER_SC][MACSEC_SALT_LEN];
 	ssci_t ssci[CN10K_MCS_SA_PER_SC];
+	bool vlan_dev; /* macsec running on VLAN ? */
 };
 
 struct cn10k_mcs_rxsc {