netdev - Re: [PATCH net-next v2 14/14] octeontx2-pf: ipsec: Add XFRM state and policy hooks for inbound flows

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250620112249.GL194429@horms.kernel.org>
Date: Fri, 20 Jun 2025 12:22:49 +0100
From: Simon Horman <horms@...nel.org>
To: Tanmay Jagdale <tanmay@...vell.com>
Cc: davem@...emloft.net, leon@...nel.org, sgoutham@...vell.com,
	bbhushan2@...vell.com, herbert@...dor.apana.org.au,
	linux-crypto@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [PATCH net-next v2 14/14] octeontx2-pf: ipsec: Add XFRM state
 and policy hooks for inbound flows

On Wed, Jun 18, 2025 at 05:00:08PM +0530, Tanmay Jagdale wrote:
> Add XFRM state hook for inbound flows and configure the following:
>   - Install an NPC rule to classify the 1st pass IPsec packets and
>     direct them to the dedicated RQ
>   - Allocate a free entry from the SA table and populate it with the
>     SA context details based on xfrm state data.
>   - Create a mapping of the SPI value to the SA table index. This is
>     used by NIXRX to calculate the exact SA context  pointer address
>     based on the SPI in the packet.
>   - Prepare the CPT SA context to decrypt buffer in place and the
>     write it the CPT hardware via LMT operation.
>   - When the XFRM state is deleted, clear this SA in CPT hardware.
> 
> Also add XFRM Policy hooks to allow successful offload of inbound
> PACKET_MODE.
> 
> Signed-off-by: Tanmay Jagdale <tanmay@...vell.com>

...

> @@ -1141,6 +1154,137 @@ static int cn10k_outb_write_sa(struct otx2_nic *pf, struct qmem *sa_info)
>  	return ret;
>  }
>  
> +static int cn10k_inb_write_sa(struct otx2_nic *pf,
> +			      struct xfrm_state *x,
> +			      struct cn10k_inb_sw_ctx_info *inb_ctx_info)
> +{
> +	dma_addr_t res_iova, dptr_iova, sa_iova;
> +	struct cn10k_rx_sa_s *sa_dptr, *sa_cptr;
> +	struct cpt_inst_s inst;
> +	u32 sa_size, off;
> +	struct cpt_res_s *res;
> +	u64 reg_val;
> +	int ret;
> +
> +	res = dma_alloc_coherent(pf->dev, sizeof(struct cpt_res_s),
> +				 &res_iova, GFP_ATOMIC);
> +	if (!res)
> +		return -ENOMEM;
> +
> +	sa_cptr = inb_ctx_info->sa_entry;
> +	sa_iova = inb_ctx_info->sa_iova;
> +	sa_size = sizeof(struct cn10k_rx_sa_s);
> +
> +	sa_dptr = dma_alloc_coherent(pf->dev, sa_size, &dptr_iova, GFP_ATOMIC);
> +	if (!sa_dptr) {
> +		dma_free_coherent(pf->dev, sizeof(struct cpt_res_s), res,
> +				  res_iova);
> +		return -ENOMEM;
> +	}
> +
> +	for (off = 0; off < (sa_size / 8); off++)
> +		*((u64 *)sa_dptr + off) = cpu_to_be64(*((u64 *)sa_cptr + off));
> +
> +	memset(&inst, 0, sizeof(struct cpt_inst_s));
> +
> +	res->compcode = 0;
> +	inst.res_addr = res_iova;
> +	inst.dptr = (u64)dptr_iova;
> +	inst.param2 = sa_size >> 3;
> +	inst.dlen = sa_size;
> +	inst.opcode_major = CN10K_IPSEC_MAJOR_OP_WRITE_SA;
> +	inst.opcode_minor = CN10K_IPSEC_MINOR_OP_WRITE_SA;
> +	inst.cptr = sa_iova;
> +	inst.ctx_val = 1;
> +	inst.egrp = CN10K_DEF_CPT_IPSEC_EGRP;
> +
> +	/* Re-use Outbound CPT LF to install Ingress SAs as well because
> +	 * the driver does not own the ingress CPT LF.
> +	 */
> +	pf->ipsec.io_addr = (__force u64)otx2_get_regaddr(pf, CN10K_CPT_LF_NQX(0));
> +	cn10k_cpt_inst_flush(pf, &inst, sizeof(struct cpt_inst_s));
> +	dmb(sy);

Hi Tanmay,

As I understand things the above effectively means that this
driver will only compile for ARM64.

I do understand that the driver is only intended to be used on ARM64.
But it is nice to get compile coverage on other 64bit systems,
in particular x86_64.

And moreover, I think the guiding principle should be for drivers
to be as independent of the host system as possible.

Can we look into handling this a different way?

> +
> +	ret = cn10k_wait_for_cpt_respose(pf, res);
> +	if (ret)
> +		goto out;
> +
> +	/* Trigger CTX flush to write dirty data back to DRAM */
> +	reg_val = FIELD_PREP(GENMASK_ULL(45, 0), sa_iova >> 7);
> +	otx2_write64(pf, CN10K_CPT_LF_CTX_FLUSH, reg_val);
> +
> +out:
> +	dma_free_coherent(pf->dev, sa_size, sa_dptr, dptr_iova);
> +	dma_free_coherent(pf->dev, sizeof(struct cpt_res_s), res, res_iova);
> +	return ret;
> +}

...