Re: [dpdk-dev] [PATCH v2 1/2] examples/ipsec-secgw: fix SAD selection logic

["Ananyev, Konstantin" <konstantin.ananyev@intel.com>]

Hi Akhil,

> > Ipsec-secgw example application fails to initialize when using default
> > configuration file (ep0.cfg) in library mode (librte_ipsec enabled).
> >
> > The reason is that two of SP rules in ep0.cfg, one for IPv4 and one
> > for IPv6, are using the same SPI number. When SA rules are initialized,
> > their SPI number is checked against SPIs stored in SPD. For library
> > mode, it is not allowed for the same SA to handle both IPv4 and IPv6.
> >
> > Solution is to split SAD into two separate parts - one for IPv4 and one
> > for IPv6. Usage of SAs stays the same. Only change is to pass correct
> > SAD (IPv4 or IPv6) in places where previously combined database was
> > passed.
> 
> Can we have 2 different SAs with same SPI value and with different IPv4 addresses?
> 
> Will the IPSec library be able to handle this case. With Setkey it is possible in linux.
> Now that we have IPSEC library we should be compatible with what linux can do.

For sure, SADB implementation has to be inside librte_ipsec.
In fact Vladimir submitted patches for that:
http://patches.dpdk.org/cover/60910/
I think we already looked at them.
We also plan to integrate it into ipsec-secgw, it would be a separate patch.
We aim for 19.11 right now but might slip to 20.02.

This patch is not about improve/change ipsec-secgw SAD implementation,
see description below.

> 
> So splitting the SADB with IPv4 and IPv6 will just avoid the issue for IPv4 and IPv6 but the
> Issue will still be there.
> I believe this should be fixed in library rather than application maintaining
> Two different databases. Library's intent is to reduce the application overhead for maintaining
> IPSec specific stuff.

Probably we didn't put enough effort to describe the patch goals and methods.
Let me try again:
Right now there is an inconsistency in ipsec-secgw behavior.
In some cases it allows two different IPv4 and IPv6 SP rules to refer to the same SPI (SA),
in other it doesn't.  
So for same config-file ipsec-secgw can either fail or not depending on
 - legacy/library mode
 - selected security action type  

As an example with config file:

sp ipv4 in esp protect 11 pri 2 src 192.168.0.0/16 dst 192.168.0.0/16 sport 0:65535 dport 0:65535
sp ipv6 in esp protect 11 pri 2 src fd12:3456:789a:0031:0000:0000:0000:0092/64 dst fd12:3456:789a:0031:0000:0000:0000:0014/64 sport 0:65535 dport 0:65535
sa out 11 cipher_algo null auth_algo null mode transport
sa in 11 aead_algo aes-128-gcm \
aead_key de:ad:be:ef:de:ad:be:ef:de:ad:be:ef:de:ad:be:ef:de:ad:be:ef \
mode transport port_id 0 type inline-crypto-offload

library mode would fail to start, legacy mode would start but wouldn't be able to work correctly.

That is the issue that Lukasz reported:
https://bugs.dpdk.org/show_bug.cgi?id=239

The reason for that: in some cases we do need to know SA IP type (and SIP/DIP values) at SA creation time.
The way ipsec-secgw obtains that information - search for given SPI value across SPDs (IPv4 and IPv6).
So when it finds entries in both IPv4 and IPv6 it is not clear which one to use and exits with an error.
To avoid such situation that patch does the following:
 - search for given SPI value across both SPDs (IPv4 and IPv6)
 - for each positive result create a new SA.
So if we have same SPI in both IPv4 and IPv6 SPDs instead of one SA that
would be referred by both SPD tables (current situation), 
we will create 2 independent SAs - one for IPv4, second for IPv6.
For each one a separate rte_security/crypto session will be created and programmed.   
 
As side effect of that - we have to split ipsec-secgw SADB into two,
as right now it is just a raw array indexed by (SPI%N) value.     

> 
> >
> > Split of SA entries is done at initialization stage. Most of given SA
> > entries are checked against SPD. If matching entry is in IPv4 SPD, SA
> > rule is added to IPv4 SAD (respectively for IPv6). Different splitting
> > method is used only when SA entry is for tunnel in inbound direction.
> > In that case if IPv4 tunnel should be used, SA entry is added to IPv4
> > SAD (respectively for IPv6). Reasoning is that inner IP version can
> > be different than outer IP version for tunneled traffic.
> >
> > Bugzilla ID: 239
> > Fixes: 5a032a71c6d3 ("examples/ipsec-secgw: make app to use IPsec library")
> >
> > Reported-by: Lukasz Bartosik <lbartosik@marvell.com>
> > Signed-off-by: Mariusz Drost <mariuszx.drost@intel.com>
> > ---
> >  examples/ipsec-secgw/ipsec-secgw.c |  48 ++--
> >  examples/ipsec-secgw/ipsec.c       |   5 +-
> >  examples/ipsec-secgw/ipsec.h       |  21 +-
> >  examples/ipsec-secgw/sa.c          | 396 ++++++++++++++++++++---------
> >  4 files changed, 312 insertions(+), 158 deletions(-)
> >