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

[Steffen Klassert <steffen.klassert@secunet.com>]

On Wed, Nov 09, 2022 at 02:54:34PM +0200, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@nvidia.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.