Re: [PATCH v7] xfrm: add SA information to the offloaded packet when if_id is set

[Leon Romanovsky <leon@kernel.org>]

On Mon, Dec 09, 2024 at 08:28:12PM +0000, Feng Wang wrote:
> In packet offload mode, append Security Association (SA) information
> to each packet, replicating the crypto offload implementation. This
> SA info helps HW offload match packets to their correct security
> policies. The XFRM interface ID is included, which is used in setups
> with multiple XFRM interfaces where source/destination addresses alone
> can't pinpoint the right policy.
> 
> The XFRM_XMIT flag is set to enable packet to be returned immediately
> from the validate_xmit_xfrm function, thus aligning with the existing
> code path for packet offload mode.
> 
> Enable packet offload mode on netdevsim and add code to check the XFRM
> interface ID.
> 
> Signed-off-by: wangfe <wangfe@google.com>
> ---

<...>

> @@ -728,7 +730,27 @@ int xfrm_output(struct sock *sk, struct sk_buff *skb)
>  			kfree_skb(skb);
>  			return -EHOSTUNREACH;
>  		}
> +		if (x->if_id) {
> +			sp = secpath_set(skb);
> +			if (!sp) {
> +				XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTERROR);
> +				kfree_skb(skb);
> +				return -ENOMEM;
> +			}
> +
> +			sp->olen++;
> +			sp->xvec[sp->len++] = x;
> +			xfrm_state_hold(x);
>  
> +			xo = xfrm_offload(skb);
> +			if (!xo) {
> +				secpath_reset(skb);
> +				XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTERROR);
> +				kfree_skb(skb);
> +				return -EINVAL;
> +			}
> +			xo->flags |= XFRM_XMIT;
> +		}

Steffen,

I would like to ask from you to delay this patch till this "if_id"
support is implemented and tested on real upstreamed device.

I have no confidence that the solution proposed above is the right thing
to do as it doesn't solve the claim "This SA info helps HW offload match
packets to their correct security". HW is going to perform lookup anyway
on the source and destination, so it is unclear how will it "help".

Thanks

>  		return xfrm_output_resume(sk, skb, 0);
>  	}
>  
> -- 
> 2.47.0.338.g60cca15819-goog
> 
>