Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 13 Oct 2022 08:16:33 +0200
From:   Steffen Klassert <steffen.klassert@secunet.com>
To:     Herbert Xu <herbert@gondor.apana.org.au>
CC:     Christian Langrock <christian.langrock@secunet.com>,
        <davem@davemloft.net>, <edumazet@google.com>, <kuba@kernel.org>,
        <pabeni@redhat.com>, <netdev@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH ipsec v6] xfrm: replay: Fix ESN wrap around for GSO
Message-ID: <20221013061633.GS2950045@gauss3.secunet.de>
References: <6810817b-e6b7-feac-64f8-c83c517ae9a5@secunet.com>
 <Y0aI2bGb24M5vA7B@gondor.apana.org.au>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <Y0aI2bGb24M5vA7B@gondor.apana.org.au>
Precedence: bulk

On Wed, Oct 12, 2022 at 05:28:57PM +0800, Herbert Xu wrote:
> On Fri, Oct 07, 2022 at 04:50:15PM +0200, Christian Langrock wrote:
> > When using GSO it can happen that the wrong seq_hi is used for the last
> > packets before the wrap around. This can lead to double usage of a
> > sequence number. To avoid this, we should serialize this last GSO
> > packet.
> > 
> > Fixes: d7dbefc45cf5 ("xfrm: Add xfrm_replay_overflow functions for offloading")
> > Co-developed-by: Steffen Klassert <steffen.klassert@secunet.com>
> > Signed-off-by: Christian Langrock <christian.langrock@secunet.com>
> > ---
> > Changes in v6:
> >  - move overflow check to offloading path to avoid locking issues
> > 
> > Changes in v5:
> >  - Fix build
> > 
> > Changes in v4:
> >  - move changelog within comment
> >  - add reviewer
> > 
> > Changes in v3:
> > - fix build
> > - remove wrapper function
> > 
> > Changes in v2:
> > - switch to bool as return value
> > - remove switch case in wrapper function
> > ---
> >  net/ipv4/esp4_offload.c |  3 +++
> >  net/ipv6/esp6_offload.c |  3 +++
> >  net/xfrm/xfrm_device.c  | 15 ++++++++++++++-
> >  net/xfrm/xfrm_replay.c  |  2 +-
> >  4 files changed, 21 insertions(+), 2 deletions(-)
> 
> Could you please explain how this code restructure makes it safe
> with respect to multiple users of the same xfrm_state?

That is because with this patch, the sequence number from the xfrm_state
is assigned to the skb and advanced by the number of segments while
holding the state lock, as it was before. The sequence numbers this
patch operates on are exclusive and private to that skb (and its
segments). The next skb will checkout the correct number from the
xfrm_state regardless on which cpu it comes.