[PATCH ipsec] xfrm: fix non-GRO codepath for IPsec hardware offloading

[PATCH ipsec] xfrm: fix non-GRO codepath for IPsec hardware offloading

[Raed Salem]

In xfrm_input() when called with IPsec hardware offload done and without GRO, encap_type == 0, we end up skipping esp_input_tail as crypto_done is set only within GRO code path, fix by move out crypto_done assignment from the GRO code path and change code accordingly

Fixes: d77e38e612a0 ("xfrm: Add an IPsec hardware offloading API")
Signed-off-by: Raed Salem <raeds@mellanox.com>
Reviewed-by: Yossi Kuperman< yossiku@mellanox.com>
Reviewed-by: Boris Pismenny <borisp@mellanox.com>
---
 net/xfrm/xfrm_input.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/net/xfrm/xfrm_input.c b/net/xfrm/xfrm_input.c index d563590..6de8f3c 100644
--- a/net/xfrm/xfrm_input.c
+++ b/net/xfrm/xfrm_input.c
@@ -234,10 +234,10 @@ int xfrm_input(struct sk_buff *skb, int nexthdr, __be32 spi, int encap_type)
 	int decaps = 0;
 	int async = 0;
 	bool xfrm_gro = false;
-	bool crypto_done = false;
 	struct xfrm_offload *xo = xfrm_offload(skb);
+	bool crypto_done = (xo && (xo->flags & CRYPTO_DONE));
 
-	if (encap_type < 0) {
+	if (encap_type < 0 || crypto_done) {
 		x = xfrm_input_state(skb);
 
 		if (unlikely(x->km.state != XFRM_STATE_VALID)) { @@ -262,8 +262,7 @@ int xfrm_input(struct sk_buff *skb, int nexthdr, __be32 spi, int encap_type)
 		encap_type = 0;
 		seq = XFRM_SPI_SKB_CB(skb)->seq;
 
-		if (xo && (xo->flags & CRYPTO_DONE)) {
-			crypto_done = true;
+		if (crypto_done) {
 			family = XFRM_SPI_SKB_CB(skb)->family;
 
 			if (!(xo->status & CRYPTO_SUCCESS)) {

Re: [PATCH ipsec] xfrm: fix non-GRO codepath for IPsec hardware offloading

[Steffen Klassert]

On Thu, Dec 27, 2018 at 01:32:14PM +0000, Raed Salem wrote:
> In xfrm_input() when called with IPsec hardware offload done and without GRO, encap_type == 0, we end up skipping esp_input_tail as crypto_done is set only within GRO code path, fix by move out crypto_done assignment from the GRO code path and change code accordingly

We currently don't support IPsec hardware offload without GRO enabled.
This is because the IPsec hardware offload does not decapsulate
the packet. So the reverse policy check is done on the outer
header instead of the inner header for tunnel mode. This means
that the reverse policy check will fail for almost all tunnel
mode configurations. The packet must be decapsulated before we
do the policy check, and that's not the case without GRO.

How did you test this?

RE: [PATCH ipsec] xfrm: fix non-GRO codepath for IPsec hardware offloading

[Raed Salem]

> -----Original Message-----
> From: Steffen Klassert [mailto:steffen.klassert@secunet.com]
> Sent: Friday, January 04, 2019 8:34 AM
> To: Raed Salem <raeds@mellanox.com>
> Cc: Boris Pismenny <borisp@mellanox.com>; Yossi Kuperman
> <yossiku@mellanox.com>; netdev@vger.kernel.org;
> herbert@gondor.apana.org.au; davem@davemloft.net
> Subject: Re: [PATCH ipsec] xfrm: fix non-GRO codepath for IPsec hardware
> offloading
> 
> On Thu, Dec 27, 2018 at 01:32:14PM +0000, Raed Salem wrote:
> > In xfrm_input() when called with IPsec hardware offload done and
> > without GRO, encap_type == 0, we end up skipping esp_input_tail as
> > crypto_done is set only within GRO code path, fix by move out
> > crypto_done assignment from the GRO code path and change code
> > accordingly
> 
> We currently don't support IPsec hardware offload without GRO enabled.
> This is because the IPsec hardware offload does not decapsulate the packet.
> So the reverse policy check is done on the outer header instead of the inner
> header for tunnel mode. This means that the reverse policy check will fail for
> almost all tunnel mode configurations. The packet must be decapsulated
> before we do the policy check, and that's not the case without GRO.
> 
> How did you test this?
Used the iproute to configure IPsec hardware offload in transport mode with gro off,
Running traffic using ping

Re: [PATCH ipsec] xfrm: fix non-GRO codepath for IPsec hardware offloading

[Steffen Klassert]

On Fri, Jan 04, 2019 at 10:43:45AM +0000, Raed Salem wrote:
> 
> 
> > -----Original Message-----
> > From: Steffen Klassert [mailto:steffen.klassert@secunet.com]
> > Sent: Friday, January 04, 2019 8:34 AM
> > To: Raed Salem <raeds@mellanox.com>
> > Cc: Boris Pismenny <borisp@mellanox.com>; Yossi Kuperman
> > <yossiku@mellanox.com>; netdev@vger.kernel.org;
> > herbert@gondor.apana.org.au; davem@davemloft.net
> > Subject: Re: [PATCH ipsec] xfrm: fix non-GRO codepath for IPsec hardware
> > offloading
> > 
> > On Thu, Dec 27, 2018 at 01:32:14PM +0000, Raed Salem wrote:
> > > In xfrm_input() when called with IPsec hardware offload done and
> > > without GRO, encap_type == 0, we end up skipping esp_input_tail as
> > > crypto_done is set only within GRO code path, fix by move out
> > > crypto_done assignment from the GRO code path and change code
> > > accordingly
> > 
> > We currently don't support IPsec hardware offload without GRO enabled.
> > This is because the IPsec hardware offload does not decapsulate the packet.
> > So the reverse policy check is done on the outer header instead of the inner
> > header for tunnel mode. This means that the reverse policy check will fail for
> > almost all tunnel mode configurations. The packet must be decapsulated
> > before we do the policy check, and that's not the case without GRO.
> > 
> > How did you test this?
> Used the iproute to configure IPsec hardware offload in transport mode with gro off,
> Running traffic using ping

How does your SA and policy database look like?

RE: [PATCH ipsec] xfrm: fix non-GRO codepath for IPsec hardware offloading

[Raed Salem]

> -----Original Message-----
> From: Steffen Klassert [mailto:steffen.klassert@secunet.com]
> Sent: Friday, January 04, 2019 12:46 PM
> > > -----Original Message-----
> > > From: Steffen Klassert [mailto:steffen.klassert@secunet.com]
> > > Sent: Friday, January 04, 2019 8:34 AM
> > > To: Raed Salem <raeds@mellanox.com>
> > > Cc: Boris Pismenny <borisp@mellanox.com>; Yossi Kuperman
> > > <yossiku@mellanox.com>; netdev@vger.kernel.org;
> > > herbert@gondor.apana.org.au; davem@davemloft.net
> > > Subject: Re: [PATCH ipsec] xfrm: fix non-GRO codepath for IPsec
> > > hardware offloading
> > >
> > > On Thu, Dec 27, 2018 at 01:32:14PM +0000, Raed Salem wrote:
> > > > In xfrm_input() when called with IPsec hardware offload done and
> > > > without GRO, encap_type == 0, we end up skipping esp_input_tail as
> > > > crypto_done is set only within GRO code path, fix by move out
> > > > crypto_done assignment from the GRO code path and change code
> > > > accordingly
> > >
> > > We currently don't support IPsec hardware offload without GRO enabled.
> > > This is because the IPsec hardware offload does not decapsulate the
> packet.
> > > So the reverse policy check is done on the outer header instead of
> > > the inner header for tunnel mode. This means that the reverse policy
> > > check will fail for almost all tunnel mode configurations. The
> > > packet must be decapsulated before we do the policy check, and that's
> not the case without GRO.
> > >
> > > How did you test this?
> > Used the iproute to configure IPsec hardware offload in transport mode
> > with gro off, Running traffic using ping
> 
> How does your SA and policy database look like?
Ip x s s:
src 1.2.3.4 dst 1.2.3.5
        proto esp spi 0x31fe38e7 reqid 838744295 mode transport
        replay-window 0
        aead rfc4106(gcm(aes)) 0x665aaa6855d6a0bcec53686c3c1cf6e68b96d8f7 128
        anti-replay context: seq 0x0, oseq 0x0, bitmap 0x00000000
        crypto offload parameters: dev enp4s0 dir out
        sel src 0.0.0.0/0 dst 0.0.0.0/0
src 1.2.3.5 dst 1.2.3.4
        proto esp spi 0x1e9d4491 reqid 513623185 mode transport
        replay-window 0
        aead rfc4106(gcm(aes)) 0xb06d1980838185be356bfe9026c9735942369047 128
        anti-replay context: seq 0x0, oseq 0x0, bitmap 0x00000000
        crypto offload parameters: dev enp4s0 dir in
        sel src 0.0.0.0/0 dst 0.0.0.0/0

ip x p s:
src 1.2.3.5/32 dst 1.2.3.4/32
        dir in priority 0 ptype main
        tmpl src 0.0.0.0 dst 0.0.0.0
                proto esp reqid 513623185 mode transport
src 1.2.3.4/32 dst 1.2.3.5/32
        dir out priority 0 ptype main
        tmpl src 0.0.0.0 dst 0.0.0.0
                proto esp reqid 838744295 mode transport

Re: [PATCH ipsec] xfrm: fix non-GRO codepath for IPsec hardware offloading

[Steffen Klassert]

On Fri, Jan 04, 2019 at 10:49:55AM +0000, Raed Salem wrote:
> > -----Original Message-----
> > From: Steffen Klassert [mailto:steffen.klassert@secunet.com]
> > > >
> > > > We currently don't support IPsec hardware offload without GRO enabled.
> > > > This is because the IPsec hardware offload does not decapsulate the
> > packet.
> > > > So the reverse policy check is done on the outer header instead of
> > > > the inner header for tunnel mode. This means that the reverse policy
> > > > check will fail for almost all tunnel mode configurations. The
> > > > packet must be decapsulated before we do the policy check, and that's
> > not the case without GRO.
> > > >
> > > > How did you test this?
> > > Used the iproute to configure IPsec hardware offload in transport mode
> > > with gro off, Running traffic using ping
> > 
> > How does your SA and policy database look like?
> Ip x s s:
> src 1.2.3.4 dst 1.2.3.5
>         proto esp spi 0x31fe38e7 reqid 838744295 mode transport
>         replay-window 0
>         aead rfc4106(gcm(aes)) 0x665aaa6855d6a0bcec53686c3c1cf6e68b96d8f7 128
>         anti-replay context: seq 0x0, oseq 0x0, bitmap 0x00000000
>         crypto offload parameters: dev enp4s0 dir out
>         sel src 0.0.0.0/0 dst 0.0.0.0/0
> src 1.2.3.5 dst 1.2.3.4
>         proto esp spi 0x1e9d4491 reqid 513623185 mode transport
>         replay-window 0
>         aead rfc4106(gcm(aes)) 0xb06d1980838185be356bfe9026c9735942369047 128
>         anti-replay context: seq 0x0, oseq 0x0, bitmap 0x00000000
>         crypto offload parameters: dev enp4s0 dir in
>         sel src 0.0.0.0/0 dst 0.0.0.0/0
> 
> ip x p s:
> src 1.2.3.5/32 dst 1.2.3.4/32
>         dir in priority 0 ptype main
>         tmpl src 0.0.0.0 dst 0.0.0.0
>                 proto esp reqid 513623185 mode transport
> src 1.2.3.4/32 dst 1.2.3.5/32
>         dir out priority 0 ptype main
>         tmpl src 0.0.0.0 dst 0.0.0.0
>                 proto esp reqid 838744295 mode transport

Thanks!

I guess this works because of transport mode, here we don't have
different inner and outer IP headers. Can you please test this with
some tunnel mode configurations?

RE: [PATCH ipsec] xfrm: fix non-GRO codepath for IPsec hardware offloading

[Raed Salem]

> -----Original Message-----
> From: Steffen Klassert [mailto:steffen.klassert@secunet.com]
> Sent: Friday, January 04, 2019 12:55 PM> 
> On Fri, Jan 04, 2019 at 10:49:55AM +0000, Raed Salem wrote:
> > > -----Original Message-----
> > > From: Steffen Klassert [mailto:steffen.klassert@secunet.com]
> > > > >
> > > > > We currently don't support IPsec hardware offload without GRO
> enabled.
> > > > > This is because the IPsec hardware offload does not decapsulate
> > > > > the
> > > packet.
> > > > > So the reverse policy check is done on the outer header instead
> > > > > of the inner header for tunnel mode. This means that the reverse
> > > > > policy check will fail for almost all tunnel mode
> > > > > configurations. The packet must be decapsulated before we do the
> > > > > policy check, and that's
> > > not the case without GRO.
> > > > >
> > > > > How did you test this?
> > > > Used the iproute to configure IPsec hardware offload in transport
> > > > mode with gro off, Running traffic using ping
> > >
> > > How does your SA and policy database look like?
> > Ip x s s:
> > src 1.2.3.4 dst 1.2.3.5
> >         proto esp spi 0x31fe38e7 reqid 838744295 mode transport
> >         replay-window 0
> >         aead rfc4106(gcm(aes))
> 0x665aaa6855d6a0bcec53686c3c1cf6e68b96d8f7 128
> >         anti-replay context: seq 0x0, oseq 0x0, bitmap 0x00000000
> >         crypto offload parameters: dev enp4s0 dir out
> >         sel src 0.0.0.0/0 dst 0.0.0.0/0 src 1.2.3.5 dst 1.2.3.4
> >         proto esp spi 0x1e9d4491 reqid 513623185 mode transport
> >         replay-window 0
> >         aead rfc4106(gcm(aes))
> 0xb06d1980838185be356bfe9026c9735942369047 128
> >         anti-replay context: seq 0x0, oseq 0x0, bitmap 0x00000000
> >         crypto offload parameters: dev enp4s0 dir in
> >         sel src 0.0.0.0/0 dst 0.0.0.0/0
> >
> > ip x p s:
> > src 1.2.3.5/32 dst 1.2.3.4/32
> >         dir in priority 0 ptype main
> >         tmpl src 0.0.0.0 dst 0.0.0.0
> >                 proto esp reqid 513623185 mode transport src
> > 1.2.3.4/32 dst 1.2.3.5/32
> >         dir out priority 0 ptype main
> >         tmpl src 0.0.0.0 dst 0.0.0.0
> >                 proto esp reqid 838744295 mode transport
> 
> Thanks!
> 
> I guess this works because of transport mode, here we don't have different
> inner and outer IP headers. Can you please test this with some tunnel mode
> configurations?
Sure,
Works with the following SA and policy DB:
ip x s s:
src 1.2.3.4 dst 1.2.3.5
        proto esp spi 0x56c8245e reqid 1455957086 mode tunnel
        replay-window 0
        aead rfc4106(gcm(aes)) 0xae41773f260e9fbda4cd9f117d8d5fb42d4a4a68 128
        anti-replay context: seq 0x0, oseq 0x0, bitmap 0x00000000
        crypto offload parameters: dev p6p1 dir in
        sel src 1.2.3.4/32 dst 1.2.3.5/32
src 1.2.3.5 dst 1.2.3.4
        proto esp spi 0xd94ba134 reqid 3645612340 mode tunnel
        replay-window 0
        aead rfc4106(gcm(aes)) 0x55a5136bec858fcb6944b8596fc73aa0ca553bf7 128
        anti-replay context: seq 0x0, oseq 0xd, bitmap 0x00000000
        crypto offload parameters: dev p6p1 dir out
        sel src 1.2.3.5/32 dst 1.2.3.4/32
ip x p s:
src 1.2.3.4/32 dst 1.2.3.5/32
        dir fwd priority 0 ptype main
        tmpl src 1.2.3.4 dst 1.2.3.5
                proto esp reqid 1455957086 mode tunnel
src 1.2.3.4/32 dst 1.2.3.5/32
        dir in priority 0 ptype main
        tmpl src 1.2.3.4 dst 1.2.3.5
                proto esp reqid 1455957086 mode tunnel
src 1.2.3.5/32 dst 1.2.3.4/32
        dir out priority 0 ptype main
        tmpl src 1.2.3.5 dst 1.2.3.4
                proto esp reqid 3645612340 mode tunnel

Re: [PATCH ipsec] xfrm: fix non-GRO codepath for IPsec hardware offloading

[Steffen Klassert]

On Fri, Jan 04, 2019 at 11:17:33AM +0000, Raed Salem wrote:
> > 
> > I guess this works because of transport mode, here we don't have different
> > inner and outer IP headers. Can you please test this with some tunnel mode
> > configurations?
> Sure,
> Works with the following SA and policy DB:
> ip x s s:
> src 1.2.3.4 dst 1.2.3.5
>         proto esp spi 0x56c8245e reqid 1455957086 mode tunnel
>         replay-window 0
>         aead rfc4106(gcm(aes)) 0xae41773f260e9fbda4cd9f117d8d5fb42d4a4a68 128
>         anti-replay context: seq 0x0, oseq 0x0, bitmap 0x00000000
>         crypto offload parameters: dev p6p1 dir in
>         sel src 1.2.3.4/32 dst 1.2.3.5/32
> src 1.2.3.5 dst 1.2.3.4
>         proto esp spi 0xd94ba134 reqid 3645612340 mode tunnel
>         replay-window 0
>         aead rfc4106(gcm(aes)) 0x55a5136bec858fcb6944b8596fc73aa0ca553bf7 128
>         anti-replay context: seq 0x0, oseq 0xd, bitmap 0x00000000
>         crypto offload parameters: dev p6p1 dir out
>         sel src 1.2.3.5/32 dst 1.2.3.4/32
> ip x p s:
> src 1.2.3.4/32 dst 1.2.3.5/32
>         dir fwd priority 0 ptype main
>         tmpl src 1.2.3.4 dst 1.2.3.5
>                 proto esp reqid 1455957086 mode tunnel
> src 1.2.3.4/32 dst 1.2.3.5/32
>         dir in priority 0 ptype main
>         tmpl src 1.2.3.4 dst 1.2.3.5
>                 proto esp reqid 1455957086 mode tunnel
> src 1.2.3.5/32 dst 1.2.3.4/32
>         dir out priority 0 ptype main
>         tmpl src 1.2.3.5 dst 1.2.3.4
>                 proto esp reqid 3645612340 mode tunnel

Here are the src and dst addresses are the same for inner
and outer headers. You need to test this with a bit more
complex configuration.

RE: [PATCH ipsec] xfrm: fix non-GRO codepath for IPsec hardware offloading

[Raed Salem]

> -----Original Message-----
> From: Steffen Klassert [mailto:steffen.klassert@secunet.com]
> Sent: Friday, January 04, 2019 1:22 PM
> 
> On Fri, Jan 04, 2019 at 11:17:33AM +0000, Raed Salem wrote:
> > >
> > > I guess this works because of transport mode, here we don't have
> > > different inner and outer IP headers. Can you please test this with
> > > some tunnel mode configurations?
> > Sure,
> > Works with the following SA and policy DB:
> > ip x s s:
> > src 1.2.3.4 dst 1.2.3.5
> >         proto esp spi 0x56c8245e reqid 1455957086 mode tunnel
> >         replay-window 0
> >         aead rfc4106(gcm(aes))
> 0xae41773f260e9fbda4cd9f117d8d5fb42d4a4a68 128
> >         anti-replay context: seq 0x0, oseq 0x0, bitmap 0x00000000
> >         crypto offload parameters: dev p6p1 dir in
> >         sel src 1.2.3.4/32 dst 1.2.3.5/32 src 1.2.3.5 dst 1.2.3.4
> >         proto esp spi 0xd94ba134 reqid 3645612340 mode tunnel
> >         replay-window 0
> >         aead rfc4106(gcm(aes)) 0x55a5136bec858fcb6944b8596fc73aa0ca553bf7
> 128
> >         anti-replay context: seq 0x0, oseq 0xd, bitmap 0x00000000
> >         crypto offload parameters: dev p6p1 dir out
> >         sel src 1.2.3.5/32 dst 1.2.3.4/32 ip x p s:
> > src 1.2.3.4/32 dst 1.2.3.5/32
> >         dir fwd priority 0 ptype main
> >         tmpl src 1.2.3.4 dst 1.2.3.5
> >                 proto esp reqid 1455957086 mode tunnel src 1.2.3.4/32
> > dst 1.2.3.5/32
> >         dir in priority 0 ptype main
> >         tmpl src 1.2.3.4 dst 1.2.3.5
> >                 proto esp reqid 1455957086 mode tunnel src 1.2.3.5/32
> > dst 1.2.3.4/32
> >         dir out priority 0 ptype main
> >         tmpl src 1.2.3.5 dst 1.2.3.4
> >                 proto esp reqid 3645612340 mode tunnel
> 
> Here are the src and dst addresses are the same for inner and outer headers.
> You need to test this with a bit more complex configuration.
Ok will try and report once I have setups for test , thanks

RE: [PATCH ipsec] xfrm: fix non-GRO codepath for IPsec hardware offloading

[Raed Salem]

> -----Original Message-----
> From: Steffen Klassert [mailto:steffen.klassert@secunet.com]
> Sent: Friday, January 04, 2019 1:22 PM
> To: Raed Salem <raeds@mellanox.com>
> Cc: Boris Pismenny <borisp@mellanox.com>; Yossi Kuperman
> <yossiku@mellanox.com>; netdev@vger.kernel.org;
> herbert@gondor.apana.org.au; davem@davemloft.net
> Subject: Re: [PATCH ipsec] xfrm: fix non-GRO codepath for IPsec hardware
> offloading
> 
> On Fri, Jan 04, 2019 at 11:17:33AM +0000, Raed Salem wrote:
> > >
> > > I guess this works because of transport mode, here we don't have
> > > different inner and outer IP headers. Can you please test this with
> > > some tunnel mode configurations?
> > Sure,
> > Works with the following SA and policy DB:
> > ip x s s:
> > src 1.2.3.4 dst 1.2.3.5
> >         proto esp spi 0x56c8245e reqid 1455957086 mode tunnel
> >         replay-window 0
> >         aead rfc4106(gcm(aes))
> 0xae41773f260e9fbda4cd9f117d8d5fb42d4a4a68 128
> >         anti-replay context: seq 0x0, oseq 0x0, bitmap 0x00000000
> >         crypto offload parameters: dev p6p1 dir in
> >         sel src 1.2.3.4/32 dst 1.2.3.5/32 src 1.2.3.5 dst 1.2.3.4
> >         proto esp spi 0xd94ba134 reqid 3645612340 mode tunnel
> >         replay-window 0
> >         aead rfc4106(gcm(aes)) 0x55a5136bec858fcb6944b8596fc73aa0ca553bf7
> 128
> >         anti-replay context: seq 0x0, oseq 0xd, bitmap 0x00000000
> >         crypto offload parameters: dev p6p1 dir out
> >         sel src 1.2.3.5/32 dst 1.2.3.4/32 ip x p s:
> > src 1.2.3.4/32 dst 1.2.3.5/32
> >         dir fwd priority 0 ptype main
> >         tmpl src 1.2.3.4 dst 1.2.3.5
> >                 proto esp reqid 1455957086 mode tunnel src 1.2.3.4/32
> > dst 1.2.3.5/32
> >         dir in priority 0 ptype main
> >         tmpl src 1.2.3.4 dst 1.2.3.5
> >                 proto esp reqid 1455957086 mode tunnel src 1.2.3.5/32
> > dst 1.2.3.4/32
> >         dir out priority 0 ptype main
> >         tmpl src 1.2.3.5 dst 1.2.3.4
> >                 proto esp reqid 3645612340 mode tunnel
> 
> Here are the src and dst addresses are the same for inner and outer headers.
> You need to test this with a bit more complex configuration.

tested based on kernel v4.20-rc7 with the patch it works when configured with hw offload and without gro:
topology used:
Server A (192.168.8.2) <--> GW C <---> GW D <--> Server B (192.168.9.4):
Server A (vm) connected to gateway C (host) via linux bridge (bripsec - 192.168.8.1)
Gateway C connected to gateway D back to back with ipsec hw offload capable NICs (interface names [ip] respectfully p6p1 [192.168.7.2] /enp4s0 [192.168.7.9])
Server B (vm) connected to gateway D (host) via linux bridge (bripsec - 192.168.9.1)

test flow:
1- configure ipsec tunnel with hw offload on and appropriate ip route at gateways C+D
2- Server A send ping traffic to server B using the tunnel

Server A config:
ip route:
192.168.7.0/24 via 192.168.8.1 dev ens6
192.168.8.0/24 dev ens6 proto kernel scope link src 192.168.8.2
192.168.9.0/24 via 192.168.8.1 dev ens6

GW C:
ip route:
192.168.7.0/24 dev p6p1 proto kernel scope link src 192.168.7.2
192.168.8.0/24 dev bripsec proto kernel scope link src 192.168.8.1
192.168.9.0/24 via 192.168.7.9 dev p6p1
ip x s s:
src 192.168.7.9 dst 192.168.7.2
        proto esp spi 0x052dada4 reqid 86879652 mode tunnel
        replay-window 0
        aead rfc4106(gcm(aes)) 0xa04f6b814ce4b8738ced2adc2e1abea3152822281df5338560fe9f50cf473cfa16747cd9 128
        anti-replay context: seq 0x0, oseq 0x0, bitmap 0x00000000
        crypto offload parameters: dev p6p1 dir in
        sel src 192.168.9.2/24 dst 192.168.8.2/24
src 192.168.7.2 dst 192.168.7.9
        proto esp spi 0x2b4f9d61 reqid 726637921 mode tunnel
        replay-window 0
        aead rfc4106(gcm(aes)) 0x4f580ac2e9b959c195c212451498edfc54204dcf152dac387baaf3aa6be7677952954644 128
        anti-replay context: seq 0x0, oseq 0x2d, bitmap 0x00000000
        crypto offload parameters: dev p6p1 dir out
        sel src 192.168.8.2/24 dst 192.168.9.2/24
ip x p s:
src 192.168.9.2/24 dst 192.168.8.2/24
        dir fwd priority 0 ptype main
        tmpl src 192.168.7.9 dst 192.168.7.2
                proto esp reqid 86879652 mode tunnel
src 192.168.9.2/24 dst 192.168.8.2/24
        dir in priority 0 ptype main
        tmpl src 192.168.7.9 dst 192.168.7.2
                proto esp reqid 86879652 mode tunnel
src 192.168.8.2/24 dst 192.168.9.2/24
        dir out priority 0 ptype main
        tmpl src 192.168.7.2 dst 192.168.7.9
                proto esp reqid 726637921 mode tunnel
				
GW D:
ip route:
192.168.7.0/24 dev enp4s0 proto kernel scope link src 192.168.7.9
192.168.8.0/24 via 192.168.7.2 dev enp4s0
192.168.9.0/24 dev bripsec proto kernel scope link src 192.168.9.1
ip x s s:
src 192.168.7.9 dst 192.168.7.2
        proto esp spi 0x052dada4 reqid 86879652 mode tunnel
        replay-window 0
        aead rfc4106(gcm(aes)) 0xa04f6b814ce4b8738ced2adc2e1abea3152822281df5338560fe9f50cf473cfa16747cd9 128
        anti-replay context: seq 0x0, oseq 0x2d, bitmap 0x00000000
        crypto offload parameters: dev enp4s0 dir out
        sel src 192.168.9.2/24 dst 192.168.8.2/24
src 192.168.7.2 dst 192.168.7.9
        proto esp spi 0x2b4f9d61 reqid 726637921 mode tunnel
        replay-window 0
        aead rfc4106(gcm(aes)) 0x4f580ac2e9b959c195c212451498edfc54204dcf152dac387baaf3aa6be7677952954644 128
        anti-replay context: seq 0x0, oseq 0x0, bitmap 0x00000000
        crypto offload parameters: dev enp4s0 dir in
        sel src 192.168.8.2/24 dst 192.168.9.2/24
ip x p s:
src 192.168.8.2/24 dst 192.168.9.2/24
        dir fwd priority 0 ptype main
        tmpl src 192.168.7.2 dst 192.168.7.9
                proto esp reqid 726637921 mode tunnel
src 192.168.8.2/24 dst 192.168.9.2/24
        dir in priority 0 ptype main
        tmpl src 192.168.7.2 dst 192.168.7.9
                proto esp reqid 726637921 mode tunnel
src 192.168.9.2/24 dst 192.168.8.2/24
        dir out priority 0 ptype main
        tmpl src 192.168.7.9 dst 192.168.7.2
                proto esp reqid 86879652 mode tunnel

Server B:
Ip route:
192.168.7.0/24 via 192.168.9.1 dev ens6
192.168.8.0/24 via 192.168.9.1 dev ens6
192.168.9.0/24 dev ens6 proto kernel scope link src 192.168.9.2

test:
at Server A:  ping -I ens6 192.168.9.2

Results: without the patch it does not work without GRO, with the patch it works

Re: [PATCH ipsec] xfrm: fix non-GRO codepath for IPsec hardware offloading

[Steffen Klassert]

On Mon, Jan 07, 2019 at 09:53:31AM +0000, Raed Salem wrote:
> 
> tested based on kernel v4.20-rc7 with the patch it works when configured with hw offload and without gro:
> topology used:
> Server A (192.168.8.2) <--> GW C <---> GW D <--> Server B (192.168.9.4):
> Server A (vm) connected to gateway C (host) via linux bridge (bripsec - 192.168.8.1)
> Gateway C connected to gateway D back to back with ipsec hw offload capable NICs (interface names [ip] respectfully p6p1 [192.168.7.2] /enp4s0 [192.168.7.9])
> Server B (vm) connected to gateway D (host) via linux bridge (bripsec - 192.168.9.1)
> 
> test flow:
> 1- configure ipsec tunnel with hw offload on and appropriate ip route at gateways C+D
> 2- Server A send ping traffic to server B using the tunnel

Thanks a lot for doing this tests!

Unfortunately it seems that we have problem
with our policy lookups. The problem is not
related to your patch, but I can't apply
the pach as is because of that (see below).

> 
> Server A config:
> ip route:
> 192.168.7.0/24 via 192.168.8.1 dev ens6
> 192.168.8.0/24 dev ens6 proto kernel scope link src 192.168.8.2
> 192.168.9.0/24 via 192.168.8.1 dev ens6
> 
> GW C:
> ip route:
> 192.168.7.0/24 dev p6p1 proto kernel scope link src 192.168.7.2
> 192.168.8.0/24 dev bripsec proto kernel scope link src 192.168.8.1
> 192.168.9.0/24 via 192.168.7.9 dev p6p1
> ip x s s:
> src 192.168.7.9 dst 192.168.7.2
>         proto esp spi 0x052dada4 reqid 86879652 mode tunnel
>         replay-window 0
>         aead rfc4106(gcm(aes)) 0xa04f6b814ce4b8738ced2adc2e1abea3152822281df5338560fe9f50cf473cfa16747cd9 128
>         anti-replay context: seq 0x0, oseq 0x0, bitmap 0x00000000
>         crypto offload parameters: dev p6p1 dir in
>         sel src 192.168.9.2/24 dst 192.168.8.2/24
> src 192.168.7.2 dst 192.168.7.9
>         proto esp spi 0x2b4f9d61 reqid 726637921 mode tunnel
>         replay-window 0
>         aead rfc4106(gcm(aes)) 0x4f580ac2e9b959c195c212451498edfc54204dcf152dac387baaf3aa6be7677952954644 128
>         anti-replay context: seq 0x0, oseq 0x2d, bitmap 0x00000000
>         crypto offload parameters: dev p6p1 dir out
>         sel src 192.168.8.2/24 dst 192.168.9.2/24
> ip x p s:
> src 192.168.9.2/24 dst 192.168.8.2/24
>         dir fwd priority 0 ptype main
>         tmpl src 192.168.7.9 dst 192.168.7.2
>                 proto esp reqid 86879652 mode tunnel
> src 192.168.9.2/24 dst 192.168.8.2/24
>         dir in priority 0 ptype main
>         tmpl src 192.168.7.9 dst 192.168.7.2
>                 proto esp reqid 86879652 mode tunnel
> src 192.168.8.2/24 dst 192.168.9.2/24
>         dir out priority 0 ptype main
>         tmpl src 192.168.7.2 dst 192.168.7.9
>                 proto esp reqid 726637921 mode tunnel
> 				
> GW D:
> ip route:
> 192.168.7.0/24 dev enp4s0 proto kernel scope link src 192.168.7.9
> 192.168.8.0/24 via 192.168.7.2 dev enp4s0
> 192.168.9.0/24 dev bripsec proto kernel scope link src 192.168.9.1
> ip x s s:
> src 192.168.7.9 dst 192.168.7.2
>         proto esp spi 0x052dada4 reqid 86879652 mode tunnel
>         replay-window 0
>         aead rfc4106(gcm(aes)) 0xa04f6b814ce4b8738ced2adc2e1abea3152822281df5338560fe9f50cf473cfa16747cd9 128
>         anti-replay context: seq 0x0, oseq 0x2d, bitmap 0x00000000
>         crypto offload parameters: dev enp4s0 dir out
>         sel src 192.168.9.2/24 dst 192.168.8.2/24
> src 192.168.7.2 dst 192.168.7.9
>         proto esp spi 0x2b4f9d61 reqid 726637921 mode tunnel
>         replay-window 0
>         aead rfc4106(gcm(aes)) 0x4f580ac2e9b959c195c212451498edfc54204dcf152dac387baaf3aa6be7677952954644 128
>         anti-replay context: seq 0x0, oseq 0x0, bitmap 0x00000000
>         crypto offload parameters: dev enp4s0 dir in
>         sel src 192.168.8.2/24 dst 192.168.9.2/24

With HW IPsec offload, the packet is decrypted but not
decapsulated. So for the SA above, the outer packet
header should have src 192.168.7.2 and dst 192.168.7.9.
This does not match the selector for that SA, so the
packet must be discarded. Unfortunately this does not
happen and it took me quite some time to find out
why this is the case. The problem is that we set
the no_policy flag for IPsec protocols. If this flag
is set, we skip the inbound policy check (including the
SA selector check). The patch that introduced this
behaviour predates our git history and I think it
was ok until we started to support tunnel in tunnel
scenarios (and now HW offload).

I'm thinking about removing the no_policy flag
from the IPsec protocols to actually do the inbound
policy check for these protocols too, but have to make
sure that this has no side effects first.

For HW offload, we should either refuse to do it
if GRO is disabled, or to do the inbound policy
check against the inner headers (they are valid
in this case).

RE: [PATCH ipsec] xfrm: fix non-GRO codepath for IPsec hardware offloading

[Raed Salem]

> -----Original Message-----
> From: Steffen Klassert [mailto:steffen.klassert@secunet.com]
> Sent: Monday, January 14, 2019 10:07 AM
> To: Raed Salem <raeds@mellanox.com>
> Cc: Boris Pismenny <borisp@mellanox.com>; Yossi Kuperman
> <yossiku@mellanox.com>; netdev@vger.kernel.org;
> herbert@gondor.apana.org.au; davem@davemloft.net
> Subject: Re: [PATCH ipsec] xfrm: fix non-GRO codepath for IPsec hardware
> offloading
> 
> On Mon, Jan 07, 2019 at 09:53:31AM +0000, Raed Salem wrote:
> >
> > tested based on kernel v4.20-rc7 with the patch it works when configured
> with hw offload and without gro:
> > topology used:
> > Server A (192.168.8.2) <--> GW C <---> GW D <--> Server B (192.168.9.4):
> > Server A (vm) connected to gateway C (host) via linux bridge (bripsec
> > - 192.168.8.1) Gateway C connected to gateway D back to back with
> > ipsec hw offload capable NICs (interface names [ip] respectfully p6p1
> > [192.168.7.2] /enp4s0 [192.168.7.9]) Server B (vm) connected to
> > gateway D (host) via linux bridge (bripsec - 192.168.9.1)
> >
> > test flow:
> > 1- configure ipsec tunnel with hw offload on and appropriate ip route
> > at gateways C+D
> > 2- Server A send ping traffic to server B using the tunnel
> 
> Thanks a lot for doing this tests!
> 
> Unfortunately it seems that we have problem with our policy lookups. The
> problem is not related to your patch, but I can't apply the pach as is because
> of that (see below).
> 
> >
> > Server A config:
> > ip route:
> > 192.168.7.0/24 via 192.168.8.1 dev ens6
> > 192.168.8.0/24 dev ens6 proto kernel scope link src 192.168.8.2
> > 192.168.9.0/24 via 192.168.8.1 dev ens6
> >
> > GW C:
> > ip route:
> > 192.168.7.0/24 dev p6p1 proto kernel scope link src 192.168.7.2
> > 192.168.8.0/24 dev bripsec proto kernel scope link src 192.168.8.1
> > 192.168.9.0/24 via 192.168.7.9 dev p6p1 ip x s s:
> > src 192.168.7.9 dst 192.168.7.2
> >         proto esp spi 0x052dada4 reqid 86879652 mode tunnel
> >         replay-window 0
> >         aead rfc4106(gcm(aes))
> 0xa04f6b814ce4b8738ced2adc2e1abea3152822281df5338560fe9f50cf473cfa16
> 747cd9 128
> >         anti-replay context: seq 0x0, oseq 0x0, bitmap 0x00000000
> >         crypto offload parameters: dev p6p1 dir in
> >         sel src 192.168.9.2/24 dst 192.168.8.2/24 src 192.168.7.2 dst
> > 192.168.7.9
> >         proto esp spi 0x2b4f9d61 reqid 726637921 mode tunnel
> >         replay-window 0
> >         aead rfc4106(gcm(aes))
> 0x4f580ac2e9b959c195c212451498edfc54204dcf152dac387baaf3aa6be767795
> 2954644 128
> >         anti-replay context: seq 0x0, oseq 0x2d, bitmap 0x00000000
> >         crypto offload parameters: dev p6p1 dir out
> >         sel src 192.168.8.2/24 dst 192.168.9.2/24 ip x p s:
> > src 192.168.9.2/24 dst 192.168.8.2/24
> >         dir fwd priority 0 ptype main
> >         tmpl src 192.168.7.9 dst 192.168.7.2
> >                 proto esp reqid 86879652 mode tunnel src
> > 192.168.9.2/24 dst 192.168.8.2/24
> >         dir in priority 0 ptype main
> >         tmpl src 192.168.7.9 dst 192.168.7.2
> >                 proto esp reqid 86879652 mode tunnel src
> > 192.168.8.2/24 dst 192.168.9.2/24
> >         dir out priority 0 ptype main
> >         tmpl src 192.168.7.2 dst 192.168.7.9
> >                 proto esp reqid 726637921 mode tunnel
> >
> > GW D:
> > ip route:
> > 192.168.7.0/24 dev enp4s0 proto kernel scope link src 192.168.7.9
> > 192.168.8.0/24 via 192.168.7.2 dev enp4s0
> > 192.168.9.0/24 dev bripsec proto kernel scope link src 192.168.9.1 ip
> > x s s:
> > src 192.168.7.9 dst 192.168.7.2
> >         proto esp spi 0x052dada4 reqid 86879652 mode tunnel
> >         replay-window 0
> >         aead rfc4106(gcm(aes))
> 0xa04f6b814ce4b8738ced2adc2e1abea3152822281df5338560fe9f50cf473cfa16
> 747cd9 128
> >         anti-replay context: seq 0x0, oseq 0x2d, bitmap 0x00000000
> >         crypto offload parameters: dev enp4s0 dir out
> >         sel src 192.168.9.2/24 dst 192.168.8.2/24 src 192.168.7.2 dst
> > 192.168.7.9
> >         proto esp spi 0x2b4f9d61 reqid 726637921 mode tunnel
> >         replay-window 0
> >         aead rfc4106(gcm(aes))
> 0x4f580ac2e9b959c195c212451498edfc54204dcf152dac387baaf3aa6be767795
> 2954644 128
> >         anti-replay context: seq 0x0, oseq 0x0, bitmap 0x00000000
> >         crypto offload parameters: dev enp4s0 dir in
> >         sel src 192.168.8.2/24 dst 192.168.9.2/24
> 
> With HW IPsec offload, the packet is decrypted but not decapsulated. So for
> the SA above, the outer packet header should have src 192.168.7.2 and dst
> 192.168.7.9.
> This does not match the selector for that SA, so the packet must be
> discarded. Unfortunately this does not happen and it took me quite some
> time to find out why this is the case. The problem is that we set the no_policy
> flag for IPsec protocols. If this flag is set, we skip the inbound policy check
> (including the SA selector check). The patch that introduced this behaviour
> predates our git history and I think it was ok until we started to support
> tunnel in tunnel scenarios (and now HW offload).
> 
> I'm thinking about removing the no_policy flag from the IPsec protocols to
> actually do the inbound policy check for these protocols too, but have to
> make sure that this has no side effects first.
> 
> For HW offload, we should either refuse to do it if GRO is disabled, or to do
> the inbound policy check against the inner headers (they are valid in this
> case).
 thanks a lot for your valued comments,
once the decision is made about the no_policy flag with IPsec including the desired behavior
when HW offload present will may need to tweak the patch or discard it altogether,
is this change is expected to be implemented anytime soon ?

Re: [PATCH ipsec] xfrm: fix non-GRO codepath for IPsec hardware offloading

[Steffen Klassert]

On Mon, Jan 14, 2019 at 08:47:37AM +0000, Raed Salem wrote:
> > -----Original Message-----
> > From: Steffen Klassert [mailto:steffen.klassert@secunet.com]
> > 
> > I'm thinking about removing the no_policy flag from the IPsec protocols to
> > actually do the inbound policy check for these protocols too, but have to
> > make sure that this has no side effects first.
> > 
> > For HW offload, we should either refuse to do it if GRO is disabled, or to do
> > the inbound policy check against the inner headers (they are valid in this
> > case).
>  thanks a lot for your valued comments,
> once the decision is made about the no_policy flag with IPsec including the desired behavior
> when HW offload present will may need to tweak the patch or discard it altogether,
> is this change is expected to be implemented anytime soon ?

I do it as soon as I know that I don't introduce some
other bug with this. It is on my todo list, should
not take for too long.