Re: [net-next,v4 5/8] cn10k-ipsec: Add SA add/delete support for outb inline ipsec

[Leon Romanovsky]

On Wed, Jun 12, 2024 at 07:16:19PM +0530, Bharat Bhushan wrote:
> This patch adds support to add and delete Security Association
> (SA) xfrm ops. Hardware maintains SA context in memory allocated
> by software. Each SA context is 128 byte aligned and size of
> each context is multiple of 128-byte. Add support for transport
> and tunnel ipsec mode, ESP protocol, aead aes-gcm-icv16, key size
> 128/192/256-bits with 32bit salt.
> 
> Signed-off-by: Bharat Bhushan <bbhushan2@xxxxxxxxxxx>
> ---
> v3->v4:
>  - Added check for crypto offload (XFRM_DEV_OFFLOAD_CRYPTO)
>    Thanks "Leon Romanovsky" for pointing out
>  
> v2->v3:
>  - Removed memset to zero wherever possible
>   (comment from Kalesh Anakkur Purayil)
>  - Corrected error hanlding when setting SA for inbound
>    (comment from Kalesh Anakkur Purayil)
>  - Move "netdev->xfrmdev_ops = &cn10k_ipsec_xfrmdev_ops;" to this patch
>    This fix build error with W=1
> 
>  .../marvell/octeontx2/nic/cn10k_ipsec.c       | 456 ++++++++++++++++++
>  .../marvell/octeontx2/nic/cn10k_ipsec.h       | 114 +++++
>  2 files changed, 570 insertions(+)
> 
> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_ipsec.c b/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_ipsec.c
> index fc1029c17c00..892bdbde92ee 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_ipsec.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_ipsec.c
> @@ -336,6 +336,12 @@ static int cn10k_outb_cpt_clean(struct otx2_nic *pf)
>  	/* Set inline ipsec disabled for this device */
>  	pf->flags &= ~OTX2_FLAG_INLINE_IPSEC_ENABLED;
>  
> +	if (!bitmap_empty(pf->ipsec.sa_bitmap, CN10K_IPSEC_OUTB_MAX_SA)) {
> +		netdev_err(pf->netdev, "SA installed on this device\n");
> +		mutex_unlock(&pf->ipsec.lock);
> +		return -EBUSY;
> +	}

Sorry for not really reviewing the patches and posting some random
comments, but this addition makes me wonder if it is correct
design/implementation. At the stage of IPsec cleanup, all SAs should be
removed before this call.

BTW, In kernel, this type of IPsec is called "Crypto Offload" and not
"inline ipsec".

Thanks