From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Lebrun Subject: Re: [PATCH net-next RFC 0/5] ipv6: sr: introduce seg6local End.BPF action Date: Tue, 3 Apr 2018 14:40:22 +0100 Message-ID: References: <20180330230326.ol5f2nmucfjdkop4@ast-mbp.dhcp.thefacebook.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Cc: netdev@vger.kernel.org, David Lebrun , Daniel Borkmann To: Mathieu Xhonneux , Alexei Starovoitov Return-path: Received: from mail-wm0-f54.google.com ([74.125.82.54]:54683 "EHLO mail-wm0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750722AbeDCNkY (ORCPT ); Tue, 3 Apr 2018 09:40:24 -0400 Received: by mail-wm0-f54.google.com with SMTP id r191so3083505wmg.4 for ; Tue, 03 Apr 2018 06:40:24 -0700 (PDT) In-Reply-To: Content-Language: en-GB Sender: netdev-owner@vger.kernel.org List-ID: On 04/03/2018 12:16 PM, Mathieu Xhonneux wrote: > >> In patch 2 I was a bit concerned that: >> + struct seg6_bpf_srh_state *srh_state = (struct seg6_bpf_srh_state *) >> + &skb->cb; >> would not collide with other users of skb->cb, but it seems the way >> the hook is placed such usage should always be valid. >> Would be good to add a comment describing the situation. > Yes, it's indeed a little hack, but this should be OK since the IPv6 layer does > not use the cb field. Another solution would be to create a new field in > __sk_buff but it's more cumbersome. > I will add a comment. Good point. The IPv6 layer *does* use the cb field through the IP6CB() macro. It is first filled in ipv6_rcv() for ingress packets and used, among others, in the input path by extension headers processing functions to store EH offsets. Given that input_action_end_bpf is called in the forwarding path and terminates with a call to dst_input(), IP6CB() will be then reset by ipv6_rcv(), and the use of skb->cb here indeed should not collide with other users. > >> Looks like somewhat odd 'End.BPF' name comes from similar names in SRv6 draft. >> Do you plan to disclose such End.BPF action in the draft as well? > This is something I've discussed with David Lebrun (the author of the Segment > Routing implementation). There's no plan to disclose an End.BPF action as-is > in the draft, since eBPF is really specific to Linux, and David doesn't mind not > having a 1:1 mapping between the actions of the draft and the implemented > ones. Writing "End.BPF" instead of just "bpf" is important to indicate that the > action will advance to the next segment by itself, like all other End actions. > One could imagine adding later a T.BPF action (a transit action), whose SID > wouldn't have to be a segment, but that could still e.g. add/edit/delete TLVs. > To clarify, I don't see why we shouldn't support "experimental" features that are not defined in draft-6man-segment-routing-header. However, we could create a separate draft describing the End.BPF feature, but that's perhaps best left for after the ongoing draft's last call. David