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
| ||
|
Date: Mon, 7 Nov 2022 09:46:46 +0000 From: Yinjun Zhang <yinjun.zhang@...igine.com> To: Leon Romanovsky <leon@...nel.org> CC: David Miller <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Steffen Klassert <steffen.klassert@...unet.com>, Herbert Xu <herbert@...dor.apana.org.au>, Chengtian Liu <chengtian.liu@...igine.com>, HuanHuan Wang <huanhuan.wang@...igine.com>, Louis Peens <louis.peens@...igine.com>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>, oss-drivers <oss-drivers@...igine.com>, Simon Horman <simon.horman@...igine.com> Subject: RE: [PATCH net-next v3 3/3] nfp: implement xfrm callbacks and expose ipsec offload feature to upper layer On Mon, 7 Nov 2022 08:14:12 +0200, Leon Romanovsky wrote: <...> > > + struct sa_ctrl_word { > > + uint32_t hash :4; /* From nfp_ipsec_sa_hash_type */ > > + uint32_t cimode :4; /* From nfp_ipsec_sa_cipher_mode */ > > + uint32_t cipher :4; /* From nfp_ipsec_sa_cipher */ > > + uint32_t mode :2; /* From nfp_ipsec_sa_mode */ > > + uint32_t proto :2; /* From nfp_ipsec_sa_prot */ > > + uint32_t dir :1; /* SA direction */ > > + uint32_t ena_arw:1; /* Anti-Replay Window */ > > + uint32_t ext_seq:1; /* 64-bit Sequence Num */ > > + uint32_t ext_arw:1; /* 64b Anti-Replay Window */ > > + uint32_t spare2 :9; /* Must be set to 0 */ > > + uint32_t encap_dsbl:1; /* Encap/Decap disable */ > > + uint32_t gen_seq:1; /* Firmware Generate Seq */ > > + uint32_t spare8 :1; /* Must be set to 0 */ > > + } ctrl_word; > > + u32 spi; /* SPI Value */ > > + uint32_t pmtu_limit :16; /* PMTU Limit */ > > + uint32_t spare3 :1; > > + uint32_t frag_check :1; /* Stateful fragment checking flag */ > > + uint32_t bypass_DSCP:1; /* Bypass DSCP Flag */ > > + uint32_t df_ctrl :2; /* DF Control bits */ > > + uint32_t ipv6 :1; /* Outbound IPv6 addr format */ > > + uint32_t udp_enable :1; /* Add/Remove UDP header for NAT */ > > + uint32_t tfc_enable :1; /* Traffic Flow Confidentiality */ > > + uint32_t spare4 :8; > > + u32 soft_lifetime_byte_count; > > + u32 hard_lifetime_byte_count; > > These fields are not relevant for IPsec crypto offload. I would be more > than happy to see only used fields here. They are not used currently in kernel indeed. However the HW is not designed for crypto-offloading only, not for kernel only, some extensive features are supported. So they're reserved here. <...> > > + > > + /* General */ > > + switch (x->props.mode) { > > + case XFRM_MODE_TUNNEL: > > + cfg->ctrl_word.mode = NFP_IPSEC_PROTMODE_TUNNEL; > > + break; > > + case XFRM_MODE_TRANSPORT: > > + cfg->ctrl_word.mode = NFP_IPSEC_PROTMODE_TRANSPORT; > > + break; > > Why is it important for IPsec crypto? The HW logic must be the same for > all modes. There are no differences between transport and tunnel. As I mentioned above, it's differentiated in HW to support more features. > > > + default: > > + nn_err(nn, "Unsupported mode for xfrm offload\n"); > > There are no other modes. > <...> > > > + err = xa_alloc(&nn->xa_ipsec, &saidx, x, > > + XA_LIMIT(0, NFP_NET_IPSEC_MAX_SA_CNT - 1), > GFP_KERNEL); > > Create XArray with XA_FLAGS_ALLOC1, it will cause to xarray skip 0. > See DEFINE_XARRAY_ALLOC1() for more info. Actually 0 is a valid SA id in HW/driver, we don't want to skip 0. > > > > + if (err < 0) { > > + nn_err(nn, "Unable to get sa_data number for IPsec\n"); > > + return err; > > + } > > + > > + /* Allocate saidx and commit the SA */ > > + err = nfp_ipsec_cfg_cmd_issue(nn, NFP_IPSEC_CFG_MSSG_ADD_SA, > saidx, &msg); > > + if (err) { > > + xa_erase(&nn->xa_ipsec, saidx); > > + nn_err(nn, "Failed to issue IPsec command err ret=%d\n", > err); > > + return err; > > + } > > + > > + /* 0 is invalid offload_handle for kernel */ > > + x->xso.offload_handle = saidx + 1; > > If you create XArray as I said above, you won't need to add +1. >
Powered by blists - more mailing lists