linux-kernel - Re: [net-next PATCH v1 09/15] octeontx2-pf: ipsec: Allocate Ingress SA table

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20250507125625.GD3339421@horms.kernel.org>
Date: Wed, 7 May 2025 13:56:25 +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 09/15] octeontx2-pf: ipsec: Allocate Ingress
 SA table

On Fri, May 02, 2025 at 06:49:50PM +0530, Tanmay Jagdale wrote:
> Every NIX LF has the facility to maintain a contiguous SA table that
> is used by NIX RX to find the exact SA context pointer associated with
> a particular flow. Allocate a 128-entry SA table where each entry is of
> 2048 bytes which is enough to hold the complete inbound SA context.
> 
> Add the structure definitions for SA context (cn10k_rx_sa_s) and
> SA bookkeeping information (ctx_inb_ctx_info).
> 
> Also, initialize the inb_sw_ctx_list to track all the SA's and their
> associated NPC rules and hash table related data.
> 
> Signed-off-by: Tanmay Jagdale <tanmay@...vell.com>

...

> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_ipsec.h b/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_ipsec.h

...

> @@ -146,6 +169,76 @@ struct cn10k_tx_sa_s {
>  	u64 hw_ctx[6];		/* W31 - W36 */
>  };
>  
> +struct cn10k_rx_sa_s {
> +	u64 inb_ar_win_sz	: 3; /* W0 */
> +	u64 hard_life_dec	: 1;
> +	u64 soft_life_dec	: 1;
> +	u64 count_glb_octets	: 1;
> +	u64 count_glb_pkts	: 1;
> +	u64 count_mib_bytes	: 1;
> +	u64 count_mib_pkts	: 1;
> +	u64 hw_ctx_off		: 7;
> +	u64 ctx_id		: 16;
> +	u64 orig_pkt_fabs	: 1;
> +	u64 orig_pkt_free	: 1;
> +	u64 pkind		: 6;
> +	u64 rsvd_w0_40		: 1;
> +	u64 eth_ovrwr		: 1;
> +	u64 pkt_output		: 2;
> +	u64 pkt_format		: 1;
> +	u64 defrag_opt		: 2;
> +	u64 x2p_dst		: 1;
> +	u64 ctx_push_size	: 7;
> +	u64 rsvd_w0_55		: 1;
> +	u64 ctx_hdr_size	: 2;
> +	u64 aop_valid		: 1;
> +	u64 rsvd_w0_59		: 1;
> +	u64 ctx_size		: 4;
> +
> +	u64 rsvd_w1_31_0	: 32; /* W1 */
> +	u64 cookie		: 32;
> +
> +	u64 sa_valid		: 1; /* W2 Control Word */
> +	u64 sa_dir		: 1;
> +	u64 rsvd_w2_2_3		: 2;
> +	u64 ipsec_mode		: 1;
> +	u64 ipsec_protocol	: 1;
> +	u64 aes_key_len		: 2;
> +	u64 enc_type		: 3;
> +	u64 life_unit		: 1;
> +	u64 auth_type		: 4;
> +	u64 encap_type		: 2;
> +	u64 et_ovrwr_ddr_en	: 1;
> +	u64 esn_en		: 1;
> +	u64 tport_l4_incr_csum	: 1;
> +	u64 iphdr_verify	: 2;
> +	u64 udp_ports_verify	: 1;
> +	u64 l2_l3_hdr_on_error	: 1;
> +	u64 rsvd_w25_31		: 7;
> +	u64 spi			: 32;

As I understand it, this driver is only intended to run on arm64 systems.
While it is also possible, with COMPILE_TEST test, to compile the driver
on for 64-bit systems.

So, given the first point above, this may be moot. But the above
assumes that the byte order of the host is the same as the device.
Or perhaps more to the point, it has been written for a little-endian
host and the device is expecting the data in that byte order.

But u64 is supposed to represent host byte order.  And, in my understanding
of things, this is the kind of problem that FIELD_PREP and FIELD_GET are
intended to avoid, when combined on endian-specific integer types (in this
case __le64 seems appropriate).

I do hesitate in bringing this up, as the above very likely works on
all systems on which this code is intended to run. But I do so
because it is not correct on all systems for which this code can be
compiled. And thus seems somehow misleading.

> +
> +	u64 w3;			/* W3 */
> +
> +	u8 cipher_key[32];	/* W4 - W7 */
> +	u32 rsvd_w8_0_31;	/* W8 : IV */
> +	u32 iv_gcm_salt;
> +	u64 rsvd_w9;		/* W9 */
> +	u64 rsvd_w10;		/* W10 : UDP Encap */
> +	u32 dest_ipaddr;	/* W11 - Tunnel mode: outer src and dest ipaddr */
> +	u32 src_ipaddr;
> +	u64 rsvd_w12_w30[19];	/* W12 - W30 */
> +
> +	u64 ar_base;		/* W31 */
> +	u64 ar_valid_mask;	/* W32 */
> +	u64 hard_sa_life;	/* W33 */
> +	u64 soft_sa_life;	/* W34 */
> +	u64 mib_octs;		/* W35 */
> +	u64 mib_pkts;		/* W36 */
> +	u64 ar_winbits;		/* W37 */
> +
> +	u64 rsvd_w38_w100[63];
> +};
> +
>  /* CPT instruction parameter-1 */
>  #define CN10K_IPSEC_INST_PARAM1_DIS_L4_CSUM		0x1
>  #define CN10K_IPSEC_INST_PARAM1_DIS_L3_CSUM		0x2