Re: [net-next,v2,4/5] seg6: add support for the SRv6 End.DT4 behavior

From: Andrea Mayer <andrea.mayer@uniroma2.it>
To: Jakub Kicinski <kuba@kernel.org>
Cc: David Ahern <dsahern@gmail.com>,
	Stefano Salsano <stefano.salsano@uniroma2.it>,
	"David S. Miller" <davem@davemloft.net>,
	David Ahern <dsahern@kernel.org>,
	Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>,
	Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>,
	Shuah Khan <shuah@kernel.org>,
	Shrijeet Mukherjee <shrijeet@gmail.com>,
	Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andrii@kernel.org>,
	Martin KaFai Lau <kafai@fb.com>, Song Liu <songliubraving@fb.com>,
	Yonghong Song <yhs@fb.com>,
	John Fastabend <john.fastabend@gmail.com>,
	KP Singh <kpsingh@chromium.org>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-kselftest@vger.kernel.org,
	Paolo Lungaroni <paolo.lungaroni@cnit.it>,
	Ahmed Abdelsalam <ahabdels.dev@gmail.com>,
	Andrea Mayer <andrea.mayer@uniroma2.it>
Subject: Re: [net-next,v2,4/5] seg6: add support for the SRv6 End.DT4 behavior
Date: Sat, 14 Nov 2020 03:29:48 +0100	[thread overview]
Message-ID: <20201114032948.895abbf4d9a12758fc702ce6@uniroma2.it> (raw)
In-Reply-To: <20201113180126.33bc1045@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>

Hi Jakub,

On Fri, 13 Nov 2020 18:01:26 -0800
Jakub Kicinski <kuba@kernel.org> wrote:

> > UAPI solution 2
> > 
> > we turn "table" into an optional parameter and we add the "vrftable" optional
> > parameter. DT4 can only be used with the "vrftable" (hence it is a required
> > parameter for DT4).
> > DT6 can be used with "vrftable" (new vrf mode) or with "table" (legacy mode)
> > (hence it is an optional parameter for DT6).
> > 
> > UAPI solution 2 examples:
> > 
> > ip -6 route add 2001:db8::1/128 encap seg6local action End.DT4 vrftable 100 dev eth0
> > ip -6 route add 2001:db8::1/128 encap seg6local action End.DT6 vrftable 100 dev eth0
> > ip -6 route add 2001:db8::1/128 encap seg6local action End.DT6 table 100 dev eth0
> > 
> > IMO solution 2 is nicer from UAPI POV because we always have only one 
> > parameter, maybe solution 1 is slightly easier to implement, all in all 
> > we prefer solution 2 but we can go for 1 if you prefer.
> 
> Agreed, 2 looks better to me as well. But let's not conflate uABI with
> iproute2's command line. I'm more concerned about the kernel ABI.

Sorry I was a little imprecise here. I reported only the user command perspective.
From the kernel point of view in solution 2 the vrftable will be a new
[SEG6_LOCAL_VRFTABLE] optional parameter.

> BTW you prefer to operate on tables (and therefore require
> net.vrf.strict_mode=1) because that's closer to the spirit of the RFC,
> correct? As I said from the implementation perspective passing any VRF
> ifindex down from user space to the kernel should be fine?

Yes, I definitely prefer to operate on tables (and so on the table ID) due to
the spirit of the RFC. We have discussed in depth this design choice with
David Ahern when implementing the DT4 patch and we are confident that operating
with VRF strict mode is a sound approach also for DT6. 

Thanks
Andrea,