netdev - Re: [PATCH xfrm-next v7 6/8] xfrm: speed-up lookup of HW policies

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20221117121243.GJ704954@gauss3.secunet.de>
Date:   Thu, 17 Nov 2022 13:12:43 +0100
From:   Steffen Klassert <steffen.klassert@...unet.com>
To:     Leon Romanovsky <leon@...nel.org>
CC:     Leon Romanovsky <leonro@...dia.com>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        Jakub Kicinski <kuba@...nel.org>, <netdev@...r.kernel.org>
Subject: Re: [PATCH xfrm-next v7 6/8] xfrm: speed-up lookup of HW policies

On Wed, Nov 09, 2022 at 02:54:34PM +0200, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@...dia.com>
> 
> @@ -1166,16 +1187,24 @@ xfrm_state_find(const xfrm_address_t *daddr, const xfrm_address_t *saddr,
>  			spin_lock_bh(&net->xfrm.xfrm_state_lock);
>  			x->km.state = XFRM_STATE_ACQ;
>  			list_add(&x->km.all, &net->xfrm.state_all);
> -			hlist_add_head_rcu(&x->bydst, net->xfrm.state_bydst + h);
> +			XFRM_STATE_INSERT(bydst, &x->bydst,
> +					  net->xfrm.state_bydst + h,
> +					  x->xso.type);
>  			h = xfrm_src_hash(net, daddr, saddr, encap_family);
> -			hlist_add_head_rcu(&x->bysrc, net->xfrm.state_bysrc + h);
> +			XFRM_STATE_INSERT(bysrc, &x->bysrc,
> +					  net->xfrm.state_bysrc + h,
> +					  x->xso.type);
>  			if (x->id.spi) {
>  				h = xfrm_spi_hash(net, &x->id.daddr, x->id.spi, x->id.proto, encap_family);
> -				hlist_add_head_rcu(&x->byspi, net->xfrm.state_byspi + h);
> +				XFRM_STATE_INSERT(byspi, &x->byspi,
> +						  net->xfrm.state_byspi + h,
> +						  x->xso.type);
>  			}
>  			if (x->km.seq) {
>  				h = xfrm_seq_hash(net, x->km.seq);
> -				hlist_add_head_rcu(&x->byseq, net->xfrm.state_byseq + h);
> +				XFRM_STATE_INSERT(byseq, &x->byseq,
> +						  net->xfrm.state_byseq + h,
> +						  x->xso.type);
>  			}

This does not work. A larval state will never have a x->xso.type set.
So this raises the question how to handle acquires with this packet
offload. You could place the type and offload device to the template,
but we also have to make sure not to mess too much with the non
offloaded codepath.

This is yet another corner case where the concept of doing policy and
state lookup in software for a HW offload does not work so well. I
fear this is not the last corner case that comes up once you put this
into a real network.