Message-ID: <20180105071631.lmwu7zmt47r2f32i@gauss3.secunet.de>
Date:   Fri, 5 Jan 2018 08:16:32 +0100
From:   Steffen Klassert <steffen.klassert@...unet.com>
To:     Lorenzo Colitti <lorenzo@...gle.com>
CC:     <netdev@...r.kernel.org>,
        Subash Abhinov Kasiviswanathan <subashab@...eaurora.org>,
        Nathan Harold <nharold@...gle.com>,
        David Miller <davem@...emloft.net>
Subject: Re: [PATCH ipsec-next 0/7]: Support multiple VTIs with the same
 src+dst pair

On Fri, Jan 05, 2018 at 01:41:46AM +0900, Lorenzo Colitti wrote:
> On Wed, Jan 3, 2018 at 9:10 PM, Steffen Klassert
> <steffen.klassert@...unet.com> wrote:
> > The fact that you need new keyed VTIs looks a bit like a workaround
> > of the design limitations the VTI interfaces have. Unfortunately
> > this is not the only limitation of VTI and I think we don't get what
> > we really want by changing VTI without breaking existing userspace.
> 
> Actually, I added the flag mostly to ensure that there would be no
> changes in behaviour at all - so, for example, to return EEXIST if
> someone tried to create two VTIs on the same IP address pair without
> the flag. But perhaps that's not important. It's unlikely that anyone
> would be trying to do this, since it has always returned an error.
> 
> If it's indeed not important, then I think it may be possible to fix
> the limitations that stop there from being two VTIs with the same IP
> address pair without introducing a new flag or userspace-visible
> changes. (I don't think it's too far off what this patch series does
> today.) If existing setups that only have one VTI per IP address pair
> continue to work as before, but setups where there is more than one
> VTI per IP address pair now work in some way, would that be
> acceptable?

Well, it would be acceptable to support this. The reason why I don't
want it is that we already had problems with such extensions in the
past. The VTI interfaces were developed for one special usecase,
then extended to try to support more. The result was that some user
configurations did not work anymore. And still it has design
limitations that we can't work around.

Also we changed generic tunnel lookup code to flag that we use GRE
keys for something that is not a GRE key but a VTI interface marker.
Your patches need to extend the generic tunnel lookup code again
for this very special usecase. I just don't want to mess around
too much with this.

> 
> > The problem is that VTI interfaces are IP tunnels, and this is
> > not the thing we need. The tunnel is already implemented in the
> > generic xfrm code. All we need is some interface we can route
> > through. In particular we need something that can work with
> > transport mode too.
> 
> Well, I'm not sure. Personally I think VTIs are a pretty natural fit
> for tunnel mode IPsec. For example, they provide an easy way to assign
> an IP address to an IPsec tunnel which is then used for packets
> originated on that tunnel.

The IPsec tunnel endpoint addresses are already defined by the SA,
so there is no need to define them again at the interface. All we
need is some marker (maybe a new lookup key) to assign the SAs
to a certain VTI interface.

> That doesn't really make much sense in
> transport mode, because in transport mode the IP addresses used are
> the ones of the physical interfaces that send the packets.

Right, and this is one of the limitations we can't overcome with
a VTI. So we need to find a new solution for transport mode anyway.

> 
> > I showed already some ideas on creating xfrm interfaces at the
> > IPsec workshop in Seoul and my plan is to discuss this at the
> > upcomming IPsec workshop, so that we get something everybody is
> > happy with. In particular I want to have feedback from the
> > userspace IPsec IKE developers before we change/create something.
> 
> I did look at the code in the ipsec-next-xfrmi tree for a while -
> wrote some tests for it, etc.

To give people on the list a chance to follow what we are talking about,
here is the link to the code:

https://git.kernel.org/pub/scm/linux/kernel/git/klassert/linux-stk.git/log/?h=ipsec-next-xfrmi

> The main reason I didn't pursue it is
> that, as written, it couldn't support our use cases. The main reasons
> were:
> 
> 1. It needs to be bound to a specific underlying interface. It looks
> like that interface must have a 6-byte hardware address (and thus
> can't be a cellular interface), but I'm not 100% sure. By contrast,
> the VTI supports an optional underlying link index, and doesn't pose
> any requirements on hardware addresses. If it's possible to make the
> underlying interface optional, by storing the underlying ifindex
> instead of the dev (like tunnels do) then that might work.

This should be possible.

> 2. It cannot use the output mark to influence routing of the
> transformed packets, because it uses the output mark/mask for its own
> purposes. Unfortunately, influencing routing of the transformed
> packets was the reason we proposed XFRMA_OUTPUT_MARK in the first
> place, so this is a showstopper :-(. Do you recall why you used the
> output mark for this, as opposed to the SA mark? If it's possible to
> use the SA mark instead, that might work.

Well, using the output mark was an easy way to get a lookup working
for the first version. I already noticed that it was not a so good
idea. Maybe we need some new lookup key for IPsec lookups...

> 
> If you're willing to evolve the xfrmi design in response to our
> feedback, I can try to make the xfrmi code fit our use cases and send
> patches over the next couple of weeks.

Yes, sure :-)

The current code is just a discussion base, it was likely that
it changes.

> But I don't think we can wait
> until the discussion at the ipsec workshop to discover whether xfrmi
> might be a feasible solution for us. By then we'll either have had to
> do something out of tree (likely the keyed VTI patches, or something
> like them) or postponed this work to a future release.

I don't want to rush with this, I want to have feedback from as many
potential users as possible to be sure to end up with the right thing.
I really want to avoid to have yet another inerface that is almost what
we need, like VTI is.

Powered by blists - more mailing lists