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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 11 Oct 2022 11:41:47 +0300 From: Leon Romanovsky <leon@...nel.org> To: Yinjun Zhang <yinjun.zhang@...igine.com> Cc: Cc: chengtian.liu@...igine.com, ; Simon Horman <simon.horman@...igine.com>, David Miller <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org, oss-drivers@...igine.com, Huanhuan Wang <huanhuan.wang@...igine.com> Subject: Re: [PATCH net-next v2 2/3] nfp: add framework to support ipsec offloading On Mon, Oct 10, 2022 at 03:05:12PM +0800, Yinjun Zhang wrote: > Thanks for your comments and sorry for the late reply. > > On Thu, Sep 29, 2022 at 11:10:12AM +0300, Leon Romanovsky wrote: > > On Tue, Sep 27, 2022 at 12:27:06PM +0200, Simon Horman wrote: > > > +struct nfp_net_ipsec_data { > > > + struct nfp_net_ipsec_sa_data sa_entries[NFP_NET_IPSEC_MAX_SA_CNT]; > > > + unsigned int sa_free_stack[NFP_NET_IPSEC_MAX_SA_CNT]; > > > + unsigned int sa_free_cnt; > > > > I don't see in this patch what are you doing with this free_stack array, > > but whole nfp_net_ipsec_data is more than 32Kb of arrays. > > > > `sa_free_stack` is used to maintain the used/available sa entries, which > is initialized in `nfp_net_ipsec_init`. > Yes, it's indeed a big array, and we're going to use pointer instead of array > here. Why do you want to use array and not Xarray? > > > > +bool nfp_net_ipsec_tx_prep(struct nfp_net_dp *dp, struct sk_buff *skb, > > > + struct nfp_ipsec_offload *offload_info) > > > +{ > > > + struct xfrm_offload *xo = xfrm_offload(skb); > > > + struct xfrm_state *x; > > > + > > > + if (!xo) > > > + return false; > > > > How is it possible in offload path? > > Why do all drivers check sec_path length and not xo? > > > > `tx_prep` is called in the tx datapath, we use `xo` to check if the > packet needs offload-encrypto or not. You didn't answer on any of my questions above. How is it possible in offload path? Why do all drivers check sec_path length and not xo? > > > > +int nfp_net_ipsec_rx(struct nfp_meta_parsed *meta, struct sk_buff *skb) > > > +{ > > > + struct nfp_net_ipsec_sa_data *sa_data; > > > + struct net_device *netdev = skb->dev; > > > + struct nfp_net_ipsec_data *ipd; > > > + struct xfrm_offload *xo; > > > + struct nfp_net_dp *dp; > > > + struct xfrm_state *x; > > > + struct sec_path *sp; > > > + struct nfp_net *nn; > > > + int saidx; > > > + > > > + nn = netdev_priv(netdev); > > > + ipd = nn->ipsec_data; > > > + dp = &nn->dp; > > > + > > > + if (meta->ipsec_saidx == 0) > > > + return 0; /* No offload took place */ > > > + > > > + saidx = meta->ipsec_saidx - 1; > > > + if (saidx > NFP_NET_IPSEC_MAX_SA_CNT || saidx < 0) { > > > + nn_dp_warn(dp, "Invalid SAIDX from NIC %d\n", saidx); > > > > No prints in data path that can be triggered from the network, please. > > > > It's a ratelimit print, and it means severe error happens, probably > unrecoverable, when running into this path. The main part of the sentence is "... can be triggered from the network ..." Thanks
Powered by blists - more mailing lists