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 6/8] xfrm: speed-up lookup of HW policies
Date: Wed, 23 Nov 2022 14:53:10 +0200 [thread overview]
Message-ID: <Y34Xtqa+F79DCf6S@unreal> (raw)
In-Reply-To: <Y33pk/3rUxFqbH2h@unreal>
On Wed, Nov 23, 2022 at 11:36:19AM +0200, Leon Romanovsky wrote:
> On Wed, Nov 23, 2022 at 09:37:20AM +0100, Steffen Klassert wrote:
> > On Tue, Nov 22, 2022 at 03:57:43PM +0200, Leon Romanovsky wrote:
> > > On Tue, Nov 22, 2022 at 02:10:02PM +0100, Steffen Klassert wrote:
> > > > On Mon, Nov 21, 2022 at 03:01:42PM +0200, Leon Romanovsky wrote:
> > > > > 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:
> > > > > >
> > > > > > 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.
> > > >
> > > > This does not change the fact that the lookup returned the wrong state.
> > >
> > > Steffen, but this is exactly why we added this check - to catch wrong
> > > states and configurations.
> >
> > No, you have to adjust the lookup so that this can't happen.
> > This is not a missconfiguration, The lookup found the wrong
> > SA, this is a difference.
> >
> > Use the offload type and dev as a lookup key and don't consider
> > SAs that don't match this in the lookup.
> >
> > This is really not too hard to do. The thing that could be a bit
> > more difficult is that the lookup should be only adjusted when
> > we really have HW policies installed. Otherwise this affects
> > even systems that don't use this kind of offload.
>
> Thanks for an explanation, trying it now.
Something like that?
The code is untested yet.
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index 5076f9d7a752..5819023c32ba 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -1115,6 +1115,19 @@ xfrm_state_find(const xfrm_address_t *daddr, const xfrm_address_t *saddr,
rcu_read_lock();
h = xfrm_dst_hash(net, daddr, saddr, tmpl->reqid, encap_family);
hlist_for_each_entry_rcu(x, net->xfrm.state_bydst + h, bydst) {
+ if (IS_ENABLED(CONFIG_XFRM_OFFLOAD) &&
+ pol->xdo.type == XFRM_DEV_OFFLOAD_PACKET) {
+ if (x->xso.type != XFRM_DEV_OFFLOAD_PACKET)
+ /* HW states are in the head of list, there is no need
+ * to iterate further.
+ */
+ break;
+
+ /* Packet offload: both policy and SA should have same device */
+ if (pol->xdo.dev != x->xso.dev)
+ continue;
+ }
+
if (x->props.family == encap_family &&
x->props.reqid == tmpl->reqid &&
(mark & x->mark.m) == x->mark.v &&
@@ -1132,6 +1145,19 @@ xfrm_state_find(const xfrm_address_t *daddr, const xfrm_address_t *saddr,
h_wildcard = xfrm_dst_hash(net, daddr, &saddr_wildcard, tmpl->reqid, encap_family);
hlist_for_each_entry_rcu(x, net->xfrm.state_bydst + h_wildcard, bydst) {
+ if (IS_ENABLED(CONFIG_XFRM_OFFLOAD) &&
+ pol->xdo.type == XFRM_DEV_OFFLOAD_PACKET) {
+ if (x->xso.type != XFRM_DEV_OFFLOAD_PACKET)
+ /* HW states are in the head of list, there is no need
+ * to iterate further.
+ */
+ break;
+
+ /* Packet offload: both policy and SA should have same device */
+ if (pol->xdo.dev != x->xso.dev)
+ continue;
+ }
+
if (x->props.family == encap_family &&
x->props.reqid == tmpl->reqid &&
(mark & x->mark.m) == x->mark.v &&
@@ -1185,6 +1211,17 @@ xfrm_state_find(const xfrm_address_t *daddr, const xfrm_address_t *saddr,
goto out;
}
+ if (pol->xdo.type == XFRM_DEV_OFFLOAD_PACKET) {
+ memcpy(&x->xso, &pol->xdo, sizeof(x->xso));
+ error = pol->xdo.dev->xfrmdev_ops->xdo_dev_state_add(x);
+ if (error) {
+ x->km.state = XFRM_STATE_DEAD;
+ to_put = x;
+ x = NULL;
+ goto out;
+ }
+ }
+
if (km_query(x, tmpl, pol) == 0) {
spin_lock_bh(&net->xfrm.xfrm_state_lock);
x->km.state = XFRM_STATE_ACQ;
next prev parent reply other threads:[~2022-11-23 13:10 UTC|newest]
Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-09 12:54 [PATCH xfrm-next v7 0/8] Extend XFRM core to allow packet offload configuration Leon Romanovsky
2022-11-09 12:54 ` [PATCH xfrm-next v7 1/8] xfrm: add new packet offload flag Leon Romanovsky
2022-11-09 12:54 ` [PATCH xfrm-next v7 2/8] xfrm: allow state packet offload mode Leon Romanovsky
2022-11-09 12:54 ` [PATCH xfrm-next v7 3/8] xfrm: add an interface to offload policy Leon Romanovsky
2022-11-09 12:54 ` [PATCH xfrm-next v7 4/8] xfrm: add TX datapath support for IPsec packet offload mode Leon Romanovsky
2022-11-17 11:59 ` Steffen Klassert
2022-11-17 12:32 ` Leon Romanovsky
2022-11-18 10:23 ` Steffen Klassert
2022-11-21 11:10 ` Leon Romanovsky
2022-11-09 12:54 ` [PATCH xfrm-next v7 5/8] xfrm: add RX datapath protection " Leon Romanovsky
2022-11-09 12:54 ` [PATCH xfrm-next v7 6/8] xfrm: speed-up lookup of HW policies Leon Romanovsky
2022-11-17 12:12 ` Steffen Klassert
2022-11-17 12:51 ` Leon Romanovsky
2022-11-18 10:49 ` Steffen Klassert
2022-11-20 19:17 ` Leon Romanovsky
2022-11-21 9:44 ` Steffen Klassert
2022-11-21 10:27 ` Leon Romanovsky
2022-11-21 11:09 ` Steffen Klassert
2022-11-21 11:15 ` Leon Romanovsky
2022-11-21 11:25 ` Steffen Klassert
2022-11-21 11:34 ` Leon Romanovsky
2022-11-21 12:02 ` Leon Romanovsky
2022-11-21 12:43 ` Steffen Klassert
2022-11-21 13:01 ` Leon Romanovsky
2022-11-22 13:10 ` Steffen Klassert
2022-11-22 13:57 ` Leon Romanovsky
2022-11-23 8:37 ` Steffen Klassert
2022-11-23 9:36 ` Leon Romanovsky
2022-11-23 12:53 ` Leon Romanovsky [this message]
2022-11-24 11:07 ` Steffen Klassert
2022-11-25 6:23 ` Leon Romanovsky
2022-11-21 12:10 ` Steffen Klassert
2022-11-21 13:21 ` Leon Romanovsky
2022-11-22 4:29 ` Herbert Xu
2022-11-22 6:27 ` Leon Romanovsky
2022-11-22 13:00 ` Steffen Klassert
2022-11-22 13:54 ` Leon Romanovsky
2022-11-23 8:23 ` Steffen Klassert
2022-11-23 10:25 ` Leon Romanovsky
2022-11-09 12:54 ` [PATCH xfrm-next v7 7/8] xfrm: add support to HW update soft and hard limits Leon Romanovsky
2022-11-17 12:13 ` Steffen Klassert
2022-11-17 12:32 ` Leon Romanovsky
2022-11-09 12:54 ` [PATCH xfrm-next v7 8/8] xfrm: document IPsec packet offload mode Leon Romanovsky
2022-11-17 12:15 ` Steffen Klassert
2022-11-17 12:33 ` Leon Romanovsky
2022-11-15 18:09 ` [PATCH xfrm-next v7 0/8] Extend XFRM core to allow packet offload configuration Leon Romanovsky
2022-11-15 18:30 ` Steffen Klassert
2022-11-15 19:00 ` Leon Romanovsky
2022-11-16 23:07 ` Saeed Mahameed
2022-11-17 12:20 ` Steffen Klassert
2022-11-17 12:24 ` Leon Romanovsky
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Y34Xtqa+F79DCf6S@unreal \
--to=leon@kernel.org \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=herbert@gondor.apana.org.au \
--cc=kuba@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=steffen.klassert@secunet.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).