Message ID | 20201123182857.4640-1-andrea.mayer@uniroma2.it |
---|---|
Headers | show |
Series | seg6: add support for SRv6 End.DT4/DT6 behavior | expand |
On Mon, 23 Nov 2020 19:28:53 +0100 Andrea Mayer wrote: > +static int cmp_nla_vrftable(struct seg6_local_lwt *a, struct seg6_local_lwt *b) > +{ > + struct seg6_end_dt_info *info_a = seg6_possible_end_dt_info(a); > + struct seg6_end_dt_info *info_b = seg6_possible_end_dt_info(b); > + > + if (IS_ERR(info_a) || IS_ERR(info_b)) > + return 1; Isn't this impossible? I thought cmp() can only be called on fully created lwtunnels and if !CONFIG_NET_L3_MASTER_DEV the tunnel won't be created? > + if (info_a->vrf_table != info_b->vrf_table) > + return 1; > + > + return 0;
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 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 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