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

[Simon Horman <simon.horman@corigine.com>]

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