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
