[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Y3to7FYBwfkBSZYA@unreal>
Date: Mon, 21 Nov 2022 14:02:52 +0200
From: Leon Romanovsky <leon@...nel.org>
To: Steffen Klassert <steffen.klassert@...unet.com>
Cc: "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 Mon, Nov 21, 2022 at 01:34:30PM +0200, Leon Romanovsky wrote:
> On Mon, Nov 21, 2022 at 12:25:21PM +0100, Steffen Klassert wrote:
> > On Mon, Nov 21, 2022 at 01:15:36PM +0200, Leon Romanovsky wrote:
> > > On Mon, Nov 21, 2022 at 12:09:26PM +0100, Steffen Klassert wrote:
> > > > On Mon, Nov 21, 2022 at 12:27:01PM +0200, Leon Romanovsky wrote:
> > > > > On Mon, Nov 21, 2022 at 10:44:04AM +0100, Steffen Klassert wrote:
> > > > > > On Sun, Nov 20, 2022 at 09:17:02PM +0200, Leon Romanovsky wrote:
> > > > > > > On Fri, Nov 18, 2022 at 11:49:07AM +0100, Steffen Klassert wrote:
> > > > > > > > On Thu, Nov 17, 2022 at 02:51:33PM +0200, Leon Romanovsky wrote:
> > > > > > > > > On Thu, Nov 17, 2022 at 01:12:43PM +0100, Steffen Klassert wrote:
> > > > > > > > > > On Wed, Nov 09, 2022 at 02:54:34PM +0200, Leon Romanovsky wrote:
> > > > > > > > > > > From: Leon Romanovsky <leonro@...dia.com>
> > > > > > > > >
> > > > > > > > > > So this raises the question how to handle acquires with this packet
> > > > > > > > > > offload.
> > > > > > > > >
> > > > > > > > > We handle acquires as SW policies and don't offload them.
> > > > > > > >
> > > > > > > > We trigger acquires with states, not policies. The thing is,
> > > > > > > > we might match a HW policy but create a SW acquire state.
> > > > > > > > This will not match anymore as soon as the lookup is
> > > > > > > > implemented correctly.
> > > > > > >
> > > > > > > For now, all such packets will be dropped as we have offlaoded
> > > > > > > policy but not SA.
> > > > > >
> > > > > > I think you missed my point. If the HW policy does not match
> > > > > > the SW acquire state, then each packet will geneate a new
> > > > > > acquire. So you need to make sure that policy and acquire
> > > > > > state will match to send the acquire just once to userspace.
> > > > >
> > > > > I think that I'm still missing the point.
> > > > >
> > > > > We require both policy and SA to be offloaded. It means that once
> > > > > we hit HW policy, we must hit SA too (at least this is how mlx5 part
> > > > > is implemented).
> > > >
> > > > Let's assume a packet hits a HW policy. Then this HW policy must match
> > > > a HW state. In case there is no matching HW state, we generate an acquire
> > > > and insert a larval state. Currently, larval states are never marked as HW.
> > >
> > > And this is there our views are different. If HW (in RX) sees policy but
> > > doesn't have state, this packet will be dropped in HW. It won't get to
> > > stack and no acquire request will be issues.
> >
> > This makes no sense. Acquires are always generated at TX, never at RX.
>
> Sorry, my bad. But why can't we drop all packets that don't have HW
> state? Why do we need to add larval?
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;
+ }
} else {
+ if (pol->xdo.type == XFRM_DEV_OFFLOAD_PACKET)
+ error = -EINVAL;
+
*err = acquire_in_progress ? -EAGAIN : error;
}
rcu_read_unlock();
(END)
>
> >
> > On RX, the state lookup happens first, the policy must match to the
> > decapsulated packet.
> >
Powered by blists - more mailing lists