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

[Leon Romanovsky <leon@kernel.org>]

On Mon, Nov 21, 2022 at 01:43:49PM +0100, Steffen Klassert wrote:
> On Mon, Nov 21, 2022 at 02:02:52PM +0200, Leon Romanovsky wrote:
> > 
> > I think that something like this will do the trick.
> > 
> > diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
> > index 5076f9d7a752..d1c9ef857755 100644
> > --- a/net/xfrm/xfrm_state.c
> > +++ b/net/xfrm/xfrm_state.c
> > @@ -1090,6 +1090,28 @@ static void xfrm_state_look_at(struct xfrm_policy *pol, struct xfrm_state *x,
> >         }
> >  }
> > 
> > +static bool xfrm_state_and_policy_mixed(struct xfrm_state *x,
> > +                                       struct xfrm_policy *p)
> > +{
> > +       /* Packet offload: both policy and SA should be offloaded */
> > +       if (p->xdo.type == XFRM_DEV_OFFLOAD_PACKET &&
> > +           x->xso.type != XFRM_DEV_OFFLOAD_PACKET)
> > +               return true;
> > +
> > +       if (p->xdo.type != XFRM_DEV_OFFLOAD_PACKET &&
> > +           x->xso.type == XFRM_DEV_OFFLOAD_PACKET)
> > +               return true;
> > +
> > +       if (p->xdo.type != XFRM_DEV_OFFLOAD_PACKET)
> > +               return false;
> > +
> > +       /* Packet offload: both policy and SA should have same device */
> > +       if (p->xdo.dev != x->xso.dev)
> > +               return true;
> > +
> > +       return false;
> > +}
> > +
> >  struct xfrm_state *
> >  xfrm_state_find(const xfrm_address_t *daddr, const xfrm_address_t *saddr,
> >                 const struct flowi *fl, struct xfrm_tmpl *tmpl,
> > @@ -1147,7 +1169,8 @@ xfrm_state_find(const xfrm_address_t *daddr, const xfrm_address_t *saddr,
> > 
> >  found:
> >         x = best;
> > -       if (!x && !error && !acquire_in_progress) {
> > +       if (!x && !error && !acquire_in_progress &&
> > +           pol->xdo.type != XFRM_DEV_OFFLOAD_PACKET) {
> >                 if (tmpl->id.spi &&
> >                     (x0 = __xfrm_state_lookup(net, mark, daddr, tmpl->id.spi,
> >                                               tmpl->id.proto, encap_family)) != NULL) {
> > @@ -1228,7 +1251,14 @@ xfrm_state_find(const xfrm_address_t *daddr, const xfrm_address_t *saddr,
> >                         *err = -EAGAIN;
> >                         x = NULL;
> >                 }
> > +               if (x && xfrm_state_and_policy_mixed(x, pol)) {
> > +                       *err = -EINVAL;
> > +                       x = NULL;
> 
> If policy and state do not match here, this means the lookup
> returned the wrong state. The correct state might still sit
> in the database. At this point, you should either have found
> a matching state, or no state at all.

I check for "x" because of "x = NULL" above.

Thanks