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

[Tanmay Jagdale <tanmay@marvell.com>]

Hi Simon,

On 2025-06-20 at 16:52:49, Simon Horman (horms@kernel.org) wrote:
> 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@marvell.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?
ACK, I will use dma_wmb() here so that it is generic.

With Regards,
Tanmay