| Message ID | 20201123182857.4640-1-andrea.mayer@uniroma2.it |
|---|---|
| Headers | show |
| Series | seg6: add support for SRv6 End.DT4/DT6 behavior | expand |
Before this patch, each SRv6 behavior specifies a set of required attributes that must be provided by the userspace application when such behavior is going to be instantiated. If at least one of the required attributes is not provided, the creation of the behavior fails. The SRv6 behavior framework lacks a way to manage optional attributes. By definition, an optional attribute for a SRv6 behavior consists of an attribute which may or may not be provided by the userspace. Therefore, if an optional attribute is missing (and thus not supplied by the user) the creation of the behavior goes ahead without any issue. This patch explicitly differentiates the required attributes from the optional attributes. In particular, each behavior can declare a set of required attributes and a set of optional ones. The semantic of the required attributes remains *totally* unaffected by this patch. The introduction of the optional attributes does NOT impact on the backward compatibility of the existing SRv6 behaviors. It is essential to note that if an (optional or required) attribute is supplied to a SRv6 behavior which does not expect it, the behavior simply discards such attribute without generating any error or warning. This operating mode remained unchanged both before and after the introduction of the optional attributes extension. The optional attributes are one of the key components used to implement the SRv6 End.DT6 behavior based on the Virtual Routing and Forwarding (VRF) framework. The optional attributes make possible the coexistence of the already existing SRv6 End.DT6 implementation with the new SRv6 End.DT6 VRF-based implementation without breaking any backward compatibility. Further details on the SRv6 End.DT6 behavior (VRF mode) are reported in subsequent patches.
On Mon, 23 Nov 2020 19:28:48 +0100 Andrea Mayer wrote: > - Patch 1 is needed to solve a pre-existing issue with tunneled packets > when a sniffer is attached; > > - Patch 2 improves the management of the seg6local attributes used by the > SRv6 behaviors; > > - Patch 3 adds support for optional attributes in SRv6 behaviors; > > - Patch 4 introduces two callbacks used for customizing the > creation/destruction of a SRv6 behavior; > > - Patch 5 is the core patch that adds support for the SRv6 End.DT4 > behavior; > > - Patch 6 introduces the VRF support for SRv6 End.DT6 behavior; > > - Patch 7 adds the selftest for SRv6 End.DT4 behavior; > > - Patch 8 adds the selftest for SRv6 End.DT6 (VRF mode) behavior; > > - Patch 9 adds the vrftable attribute for End.DT4/DT6 behaviors in iproute2. LGTM! Please address the nit and repost without the iproute2 patch. Mixing the iproute2 patch in has confused patchwork: https://patchwork.kernel.org/project/netdevbpf/list/?series=389667&state=* Note how it thinks that the iproute2 patch is part of the kernel series. This build bot-y thing is pretty new. I'll add a suggestion to our process documentation not to mix patches. > I would like to thank David Ahern for his support during the development of > this patchset. Should I take this to mean that David has review the code off-list?
On 11/24/20 4:49 PM, Jakub Kicinski wrote: > > LGTM! Please address the nit and repost without the iproute2 patch. > Mixing the iproute2 patch in has confused patchwork: > > https://patchwork.kernel.org/project/netdevbpf/list/?series=389667&state=* > > Note how it thinks that the iproute2 patch is part of the kernel > series. This build bot-y thing is pretty new. I'll add a suggestion > to our process documentation not to mix patches. That was me - I suggested doing that. I have done that in the past as has several other people. I don't recall DaveM having a problem, so maybe it is the new patchworks that is not liking it? > >> I would like to thank David Ahern for his support during the development of >> this patchset. > > Should I take this to mean that David has review the code off-list? > reviews and general guidance.
On Tue, 24 Nov 2020 18:24:37 -0700 David Ahern wrote: > On 11/24/20 4:49 PM, Jakub Kicinski wrote: > > > > LGTM! Please address the nit and repost without the iproute2 patch. > > Mixing the iproute2 patch in has confused patchwork: > > > > https://patchwork.kernel.org/project/netdevbpf/list/?series=389667&state=* > > > > Note how it thinks that the iproute2 patch is part of the kernel > > series. This build bot-y thing is pretty new. I'll add a suggestion > > to our process documentation not to mix patches. > > That was me - I suggested doing that. I have done that in the past as > has several other people. I don't recall DaveM having a problem, so > maybe it is the new patchworks that is not liking it? Right, I'm not sure, it's a coin toss whether pw will get the iproute patch first or not (or maybe since 'i' < 'n' we're likely to get the iproute patch first most of the time?) But it's generally not a huge issue for applying the patch. I just like to see the build bot result, to make sure we're not adding W=1 C=1 warnings. > >> I would like to thank David Ahern for his support during the development of > >> this patchset. > > > > Should I take this to mean that David has review the code off-list? > > reviews and general guidance. Great! (I wasn't sure if I should wait for your review tags, hence the poke.)
On 11/24/20 6:58 PM, Jakub Kicinski wrote: > But it's generally not a huge issue for applying the patch. I just like > to see the build bot result, to make sure we're not adding W=1 C=1 > warnings. ah, the build bot part is new. got it.
On Tue, 24 Nov 2020 21:37:18 -0700 David Ahern wrote: > On 11/24/20 6:58 PM, Jakub Kicinski wrote: > > But it's generally not a huge issue for applying the patch. I just like > > to see the build bot result, to make sure we're not adding W=1 C=1 > > warnings. > > ah, the build bot part is new. got it. BTW I was wondering for the longest time how to structure things so that build bot can also build iproute2 in case we want to run tests attached to the series and the tests depend on iproute2 changes... But let's cross that bridge when we get there.
On 11/25/20 9:47 AM, Jakub Kicinski wrote: > On Tue, 24 Nov 2020 21:37:18 -0700 David Ahern wrote: >> On 11/24/20 6:58 PM, Jakub Kicinski wrote: >>> But it's generally not a huge issue for applying the patch. I just like >>> to see the build bot result, to make sure we're not adding W=1 C=1 >>> warnings. >> >> ah, the build bot part is new. got it. > > BTW I was wondering for the longest time how to structure things so > that build bot can also build iproute2 in case we want to run tests > attached to the series and the tests depend on iproute2 changes... > > But let's cross that bridge when we get there. > Why not cross it now? You handled the switch over to new a patchworks with a build bot, so we can take advantage of automation. Seems like the bot needs to detect 'net', 'net-next', 'bpf' and 'bpf-next' as they are all different trees for the kernel patches. iproute2 is just another tree, so it should be able to put those in a different bucket for automated builds - even if it means a 'set' crosses trees.
On Wed, 25 Nov 2020 10:37:20 -0700 David Ahern wrote: > On 11/25/20 9:47 AM, Jakub Kicinski wrote: > > On Tue, 24 Nov 2020 21:37:18 -0700 David Ahern wrote: > >> On 11/24/20 6:58 PM, Jakub Kicinski wrote: > >>> But it's generally not a huge issue for applying the patch. I just like > >>> to see the build bot result, to make sure we're not adding W=1 C=1 > >>> warnings. > >> > >> ah, the build bot part is new. got it. > > > > BTW I was wondering for the longest time how to structure things so > > that build bot can also build iproute2 in case we want to run tests > > attached to the series and the tests depend on iproute2 changes... > > > > But let's cross that bridge when we get there. > > Why not cross it now? You handled the switch over to new a patchworks > with a build bot, so we can take advantage of automation. > > Seems like the bot needs to detect 'net', 'net-next', 'bpf' and > 'bpf-next' as they are all different trees for the kernel patches. > iproute2 is just another tree, so it should be able to put those in a > different bucket for automated builds - even if it means a 'set' crosses > trees. Actually part of the reason is that we use up 32 vCPUs just to do build testing. I don't think we can afford to individually selftest every series. And if we only run the tests ~nightly we can grab all outstanding patches for iproute2 from the ML and we should be good. At least that's my current thinking. I probably won't have time to implement any of this until Dave is back 100% and then some, anyway ;)
Hi Jakub, On Tue, 24 Nov 2020 15:49:04 -0800 Jakub Kicinski <kuba@kernel.org> wrote: > LGTM! Please address the nit and repost without the iproute2 patch. Thanks for the review of the patchset. > Mixing the iproute2 patch in has confused patchwork: > > https://patchwork.kernel.org/project/netdevbpf/list/?series=389667&state=* > > Note how it thinks that the iproute2 patch is part of the kernel > series. This build bot-y thing is pretty new. I'll add a suggestion > to our process documentation not to mix patches. As discussed, I will resubmit the kernel patches and the iproute2 patch in two separate patchsets. Thank you all, Andrea
This patchset provides support for the SRv6 End.DT4 and SRv6 End.DT6 (VRF mode) behavior. The SRv6 End.DT4 is used to implement multi-tenant IPv4 L3 VPN. It decapsulates the received packets and performs IPv4 routing lookup in the routing table of the tenant. The SRv6 End.DT4 Linux implementation leverages a VRF device. The SRv6 End.DT4 is defined in the SRv6 Network Programming [1]. The Linux kernel already offers an implementation of the SRv6 End.DT6 behavior which permits IPv6 L3 VPNs over SRv6 networks. This new implementation of DT6 is based on the same VRF infrastructure already exploited for implementing the SRv6 End.DT4 behavior. The aim of the new SRv6 End.DT6 in VRF mode consists in simplifying the construction of IPv6 L3 VPN services in the multi-tenant environment. Currently, the two SRv6 End.DT6 implementations (legacy and VRF mode) coexist seamlessly and can be chosen according to the context and the user preferences. - Patch 1 is needed to solve a pre-existing issue with tunneled packets when a sniffer is attached; - Patch 2 improves the management of the seg6local attributes used by the SRv6 behaviors; - Patch 3 adds support for optional attributes in SRv6 behaviors; - Patch 4 introduces two callbacks used for customizing the creation/destruction of a SRv6 behavior; - Patch 5 is the core patch that adds support for the SRv6 End.DT4 behavior; - Patch 6 introduces the VRF support for SRv6 End.DT6 behavior; - Patch 7 adds the selftest for SRv6 End.DT4 behavior; - Patch 8 adds the selftest for SRv6 End.DT6 (VRF mode) behavior; - Patch 9 adds the vrftable attribute for End.DT4/DT6 behaviors in iproute2. I would like to thank David Ahern for his support during the development of this patchset. Comments, suggestions and improvements are very welcome! Thanks, Andrea Mayer v3 notes about the build bot: - apparently the ',' (comma) in the subject prefix confused the build bot. Removed the ',' in favor of ' ' (space). Thanks to David Ahern and Konstantin Ryabitsev for shedding light on this fact. Thanks also to Nathan Chancellor for trying to build the patchset v2 by simulating the bot issue. add new patch for iproute2: - [9/9] seg6: add support for vrftable attribute in End.DT4/DT6 behaviors add new patch: - [8/9] selftests: add selftest for the SRv6 End.DT6 (VRF) behavior add new patch: - [6/9] seg6: add VRF support for SRv6 End.DT6 behavior add new patch: - [3/9] seg6: add support for optional attributes in SRv6 behaviors selftests: add selftest for the SRv6 End.DT4 behavior - keep David Ahern's review tag since the code wasn't changed. Thanks to David Ahern for his review. seg6: add support for the SRv6 End.DT4 behavior - remove useless error in seg6_end_dt4_build(); - remove #ifdef/#endif stubs for DT4 when CONFIG_NET_L3_MASTER_DEV is not defined; - fix coding style. Thanks to Jakub Kicinski for his review and for all his suggestions. seg6: add callbacks for customizing the creation/destruction of a behavior - remove typedef(s) slwt_{build/destroy}_state_t; - fix coding style: remove empty lines, trivial comments and rename labels in the seg6_local_build_state() function. Thanks to Jakub Kicinski for his review and for all his suggestions. seg6: improve management of behavior attributes - remove defensive programming approach in destroy_attr_srh(), destroy_attr_bpf() and destroy_attrs(); - change the __destroy_attrs() function signature, renaming the 'end' argument 'parsed_max'. Now, the __destroy_attrs() keeps only the 'parsed_max' and 'slwt' arguments. Thanks to Jakub Kicinski for his review and for all his suggestions. vrf: add mac header for tunneled packets when sniffer is attached - keep David Ahern's review tag since the code wasn't changed. Thanks to Jakub Kicinski for pointing it out and David Ahern for his review. v2 no changes made: resubmitted after false build report. v1 improve comments; add new patch 2/5 titled: seg6: improve management of behavior attributes seg6: add support for the SRv6 End.DT4 behavior - remove the inline keyword in the definition of fib6_config_get_net(). selftests: add selftest for the SRv6 End.DT4 behavior - add check for the vrf sysctl [1] https://tools.ietf.org/html/draft-ietf-spring-srv6-network-programming Andrea Mayer (8): vrf: add mac header for tunneled packets when sniffer is attached seg6: improve management of behavior attributes seg6: add support for optional attributes in SRv6 behaviors seg6: add callbacks for customizing the creation/destruction of a behavior seg6: add support for the SRv6 End.DT4 behavior seg6: add VRF support for SRv6 End.DT6 behavior selftests: add selftest for the SRv6 End.DT4 behavior selftests: add selftest for the SRv6 End.DT6 (VRF) behavior drivers/net/vrf.c | 78 ++- include/uapi/linux/seg6_local.h | 1 + net/ipv6/seg6_local.c | 593 +++++++++++++++++- .../selftests/net/srv6_end_dt4_l3vpn_test.sh | 494 +++++++++++++++ .../selftests/net/srv6_end_dt6_l3vpn_test.sh | 502 +++++++++++++++ 5 files changed, 1649 insertions(+), 19 deletions(-) create mode 100755 tools/testing/selftests/net/srv6_end_dt4_l3vpn_test.sh create mode 100755 tools/testing/selftests/net/srv6_end_dt6_l3vpn_test.sh Paolo Lungaroni (1): seg6: add support for vrftable attribute in End.DT4/DT6 behaviors include/uapi/linux/seg6_local.h | 1 + ip/iproute_lwtunnel.c | 19 ++++++++++++++++--- 2 files changed, 17 insertions(+), 3 deletions(-)