Re: [PATCH xfrm-next v7 4/8] xfrm: add TX datapath support for IPsec packet offload mode

From: Leon Romanovsky <leon@kernel.org>
To: Steffen Klassert <steffen.klassert@secunet.com>
Cc: "David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Herbert Xu <herbert@gondor.apana.org.au>,
	Jakub Kicinski <kuba@kernel.org>,
	netdev@vger.kernel.org
Subject: Re: [PATCH xfrm-next v7 4/8] xfrm: add TX datapath support for IPsec packet offload mode
Date: Thu, 17 Nov 2022 14:32:10 +0200	[thread overview]
Message-ID: <Y3YpyplG969qtYO3@unreal> (raw)
In-Reply-To: <20221117115939.GI704954@gauss3.secunet.de>

On Thu, Nov 17, 2022 at 12:59:39PM +0100, Steffen Klassert wrote:
> On Wed, Nov 09, 2022 at 02:54:32PM +0200, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@nvidia.com>
> 
> > @@ -2708,6 +2710,23 @@ static struct dst_entry *xfrm_bundle_create(struct xfrm_policy *policy,
> >  	if (!dev)
> >  		goto free_dst;
> >  
> > +	dst1 = &xdst0->u.dst;
> > +	/* Packet offload: both policy and SA should be offloaded */
> > +	if ((policy->xdo.type == XFRM_DEV_OFFLOAD_PACKET &&
> > +	     dst1->xfrm->xso.type != XFRM_DEV_OFFLOAD_PACKET) ||
> > +	    (policy->xdo.type != XFRM_DEV_OFFLOAD_PACKET &&
> > +	     dst1->xfrm->xso.type == XFRM_DEV_OFFLOAD_PACKET)) {
> > +		err = -EINVAL;
> > +		goto free_dst;
> > +	}
> > +
> > +	/* Packet offload: both policy and SA should have same device */
> > +	if (policy->xdo.type == XFRM_DEV_OFFLOAD_PACKET &&
> > +	    policy->xdo.dev != dst1->xfrm->xso.dev) {
> > +		err = -EINVAL;
> > +		goto free_dst;
> > +	}
> > +
> 
> This is the wrong place for these checks. Things went already wrong
> in the lookup if policy and state do not match here.

Where do you think we should put such checks?

We need to make sure that both policy and SA are offloaded when handle
packet, It prevents various corner cases where we will mix SW and HW
paths.

xfrm_bundle_create() is called when we perform XFRM lookup to create dst_entry.

Thanks