| 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
| ||
|
Message-ID: <20250507163050.GH3339421@horms.kernel.org>
Date: Wed, 7 May 2025 17:30:50 +0100
From: Simon Horman <horms@...nel.org>
To: Tanmay Jagdale <tanmay@...vell.com>
Cc: bbrezillon@...nel.org, arno@...isbad.org, schalla@...vell.com,
herbert@...dor.apana.org.au, davem@...emloft.net,
sgoutham@...vell.com, lcherian@...vell.com, gakula@...vell.com,
jerinj@...vell.com, hkelam@...vell.com, sbhatta@...vell.com,
andrew+netdev@...n.ch, edumazet@...gle.com, kuba@...nel.org,
pabeni@...hat.com, bbhushan2@...vell.com, bhelgaas@...gle.com,
pstanner@...hat.com, gregkh@...uxfoundation.org,
peterz@...radead.org, linux@...blig.org,
krzysztof.kozlowski@...aro.org, giovanni.cabiddu@...el.com,
linux-crypto@...r.kernel.org, linux-kernel@...r.kernel.org,
netdev@...r.kernel.org, rkannoth@...vell.com, sumang@...vell.com,
gcherian@...vell.com
Subject: Re: [net-next PATCH v1 14/15] octeontx2-pf: ipsec: Process CPT
metapackets
On Fri, May 02, 2025 at 06:49:55PM +0530, Tanmay Jagdale wrote:
> CPT hardware forwards decrypted IPsec packets to NIX via the X2P bus
> as metapackets which are of 256 bytes in length. Each metapacket
> contains CPT_PARSE_HDR_S and initial bytes of the decrypted packet
> that helps NIX RX in classifying and submitting to CPU. Additionally,
> CPT also sets BIT(11) of the channel number to indicate that it's a
> 2nd pass packet from CPT.
>
> Since the metapackets are not complete packets, they don't have to go
> through L3/L4 layer length and checksum verification so these are
> disabled via the NIX_LF_INLINE_RQ_CFG mailbox during IPsec initialization.
>
> The CPT_PARSE_HDR_S contains a WQE pointer to the complete decrypted
> packet. Add code in the rx NAPI handler to parse the header and extract
> WQE pointer. Later, use this WQE pointer to construct the skb, set the
> XFRM packet mode flags to indicate successful decryption before submitting
> it to the network stack.
>
> Signed-off-by: Tanmay Jagdale <tanmay@...vell.com>
> ---
> .../marvell/octeontx2/nic/cn10k_ipsec.c | 61 +++++++++++++++++++
> .../marvell/octeontx2/nic/cn10k_ipsec.h | 47 ++++++++++++++
> .../marvell/octeontx2/nic/otx2_struct.h | 16 +++++
> .../marvell/octeontx2/nic/otx2_txrx.c | 25 +++++++-
> 4 files changed, 147 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_ipsec.c b/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_ipsec.c
> index 91c8f13b6e48..bebf5cdedee4 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_ipsec.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_ipsec.c
> @@ -346,6 +346,67 @@ static int cn10k_outb_cpt_init(struct net_device *netdev)
> return ret;
> }
>
> +struct nix_wqe_rx_s *cn10k_ipsec_process_cpt_metapkt(struct otx2_nic *pfvf,
> + struct nix_rx_sg_s *sg,
> + struct sk_buff *skb,
> + int qidx)
> +{
> + struct nix_wqe_rx_s *wqe = NULL;
> + u64 *seg_addr = &sg->seg_addr;
> + struct cpt_parse_hdr_s *cptp;
> + struct xfrm_offload *xo;
> + struct otx2_pool *pool;
> + struct xfrm_state *xs;
> + struct sec_path *sp;
> + u64 *va_ptr;
> + void *va;
> + int i;
> +
> + /* CPT_PARSE_HDR_S is present in the beginning of the buffer */
> + va = phys_to_virt(otx2_iova_to_phys(pfvf->iommu_domain, *seg_addr));
> +
> + /* Convert CPT_PARSE_HDR_S from BE to LE */
> + va_ptr = (u64 *)va;
phys_to_virt returns a void *. And there is no need to explicitly cast
another pointer type to or from a void *.
So probably this can simply be:
va_ptr = phys_to_virt(...);
> + for (i = 0; i < (sizeof(struct cpt_parse_hdr_s) / sizeof(u64)); i++)
> + va_ptr[i] = be64_to_cpu(va_ptr[i]);
Please don't use the same variable to hold both big endian and
host byte order values. Because tooling can no longer provide
information about endian mismatches.
Flagged by Sparse.
Also, isn't only the long word that exactly comprises the
wqe_ptr field of cpt_parse_hdr_s used? If so, perhaps
only that portion needs to be converted to host byte order?
I'd explore describing the members of struct cpt_parse_hdr_s as __be64.
And use FIELD_PREP and FIELD_GET to deal with parts of each __be64.
I think that would lead to a simpler implementation.
> +
> + cptp = (struct cpt_parse_hdr_s *)va;
> +
> + /* Convert the wqe_ptr from CPT_PARSE_HDR_S to a CPU usable pointer */
> + wqe = (struct nix_wqe_rx_s *)phys_to_virt(otx2_iova_to_phys(pfvf->iommu_domain,
> + cptp->wqe_ptr));
There is probably no need to cast from void * here either.
wqe = phys_to_virt(otx2_iova_to_phys(pfvf->iommu_domain,
cptp->wqe_ptr));
> +
> + /* Get the XFRM state pointer stored in SA context */
> + va_ptr = pfvf->ipsec.inb_sa->base +
> + (cptp->cookie * pfvf->ipsec.sa_tbl_entry_sz) + 1024;
> + xs = (struct xfrm_state *)*va_ptr;
Maybe this can be more succinctly written as follows?
xs = pfvf->ipsec.inb_sa->base +
(cptp->cookie * pfvf->ipsec.sa_tbl_entry_sz) + 1024;
> +
> + /* Set XFRM offload status and flags for successful decryption */
> + sp = secpath_set(skb);
> + if (!sp) {
> + netdev_err(pfvf->netdev, "Failed to secpath_set\n");
> + wqe = NULL;
> + goto err_out;
> + }
> +
> + rcu_read_lock();
> + xfrm_state_hold(xs);
> + rcu_read_unlock();
> +
> + sp->xvec[sp->len++] = xs;
> + sp->olen++;
> +
> + xo = xfrm_offload(skb);
> + xo->flags = CRYPTO_DONE;
> + xo->status = CRYPTO_SUCCESS;
> +
> +err_out:
> + /* Free the metapacket memory here since it's not needed anymore */
> + pool = &pfvf->qset.pool[qidx];
> + otx2_free_bufs(pfvf, pool, *seg_addr - OTX2_HEAD_ROOM, pfvf->rbsize);
> + return wqe;
> +}
> +
> static int cn10k_inb_alloc_mcam_entry(struct otx2_nic *pfvf,
> struct cn10k_inb_sw_ctx_info *inb_ctx_info)
> {
> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_ipsec.h b/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_ipsec.h
> index aad5ebea64ef..68046e377486 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_ipsec.h
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_ipsec.h
> @@ -8,6 +8,7 @@
> #define CN10K_IPSEC_H
>
> #include <linux/types.h>
> +#include "otx2_struct.h"
>
> DECLARE_STATIC_KEY_FALSE(cn10k_ipsec_sa_enabled);
>
> @@ -302,6 +303,41 @@ struct cpt_sg_s {
> u64 rsvd_63_50 : 14;
> };
>
> +/* CPT Parse Header Structure for Inbound packets */
> +struct cpt_parse_hdr_s {
> + /* Word 0 */
> + u64 cookie : 32;
> + u64 match_id : 16;
> + u64 err_sum : 1;
> + u64 reas_sts : 4;
> + u64 reserved_53 : 1;
> + u64 et_owr : 1;
> + u64 pkt_fmt : 1;
> + u64 pad_len : 3;
> + u64 num_frags : 3;
> + u64 pkt_out : 2;
> +
> + /* Word 1 */
> + u64 wqe_ptr;
> +
> + /* Word 2 */
> + u64 frag_age : 16;
> + u64 res_32_16 : 16;
> + u64 pf_func : 16;
> + u64 il3_off : 8;
> + u64 fi_pad : 3;
> + u64 fi_offset : 5;
> +
> + /* Word 3 */
> + u64 hw_ccode : 8;
> + u64 uc_ccode : 8;
> + u64 res3_32_16 : 16;
> + u64 spi : 32;
> +
> + /* Word 4 */
> + u64 misc;
> +};
> +
> /* CPT LF_INPROG Register */
> #define CPT_LF_INPROG_INFLIGHT GENMASK_ULL(8, 0)
> #define CPT_LF_INPROG_GRB_CNT GENMASK_ULL(39, 32)
...
> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c
...
> @@ -355,8 +359,25 @@ static void otx2_rcv_pkt_handler(struct otx2_nic *pfvf,
> if (unlikely(!skb))
> return;
>
> - start = (void *)sg;
> - end = start + ((cqe->parse.desc_sizem1 + 1) * 16);
> + if (parse->chan & 0x800) {
> + orig_pkt_wqe = cn10k_ipsec_process_cpt_metapkt(pfvf, sg, skb, cq->cq_idx);
> + if (!orig_pkt_wqe) {
> + netdev_err(pfvf->netdev, "Invalid WQE in CPT metapacket\n");
> + napi_free_frags(napi);
> + cq->pool_ptrs++;
> + return;
> + }
> + /* Switch *sg to the orig_pkt_wqe's *sg which has the actual
> + * complete decrypted packet by CPT.
> + */
> + sg = &orig_pkt_wqe->sg;
> + start = (void *)sg;
I don't think this cast is necessary, start is a void *.
Likewise below.
> + end = start + ((orig_pkt_wqe->parse.desc_sizem1 + 1) * 16);
> + } else {
> + start = (void *)sg;
> + end = start + ((cqe->parse.desc_sizem1 + 1) * 16);
> + }
The (size + 1) * 16 calculation seems to be repeated.
Perhaps a helper function is appropriate.
> +
> while (start < end) {
> sg = (struct nix_rx_sg_s *)start;
> seg_addr = &sg->seg_addr;
> --
> 2.43.0
>
>
Powered by blists - more mailing lists