Re: [PATCH bpf-next v3] bpf: add End.DT6 action to bpf_lwt_seg6_action helper

2018-07-31 Thread Daniel Borkmann
On 07/26/2018 04:10 AM, Mathieu Xhonneux wrote:
> The seg6local LWT provides the End.DT6 action, which allows to
> decapsulate an outer IPv6 header containing a Segment Routing Header
> (SRH), full specification is available here:
> 
> https://tools.ietf.org/html/draft-filsfils-spring-srv6-network-programming-05
> 
> This patch adds this action now to the seg6local BPF
> interface. Since it is not mandatory that the inner IPv6 header also
> contains a SRH, seg6_bpf_srh_state has been extended with a pointer to
> a possible SRH of the outermost IPv6 header. This helps assessing if the
> validation must be triggered or not, and avoids some calls to
> ipv6_find_hdr.
> 
> v3: s/1/true, s/0/false for boolean values
> v2: - changed true/false -> 1/0
> - preempt_enable no longer called in first conditional block
> 
> Signed-off-by: Mathieu Xhonneux 

Applied to bpf-next, thanks Mathieu!


[PATCH bpf-next v3] bpf: add End.DT6 action to bpf_lwt_seg6_action helper

2018-07-25 Thread Mathieu Xhonneux
The seg6local LWT provides the End.DT6 action, which allows to
decapsulate an outer IPv6 header containing a Segment Routing Header
(SRH), full specification is available here:

https://tools.ietf.org/html/draft-filsfils-spring-srv6-network-programming-05

This patch adds this action now to the seg6local BPF
interface. Since it is not mandatory that the inner IPv6 header also
contains a SRH, seg6_bpf_srh_state has been extended with a pointer to
a possible SRH of the outermost IPv6 header. This helps assessing if the
validation must be triggered or not, and avoids some calls to
ipv6_find_hdr.

v3: s/1/true, s/0/false for boolean values
v2: - changed true/false -> 1/0
- preempt_enable no longer called in first conditional block

Signed-off-by: Mathieu Xhonneux 
---
 include/net/seg6_local.h |  4 ++-
 net/core/filter.c| 89 
 net/ipv6/seg6_local.c| 50 +--
 3 files changed, 95 insertions(+), 48 deletions(-)

diff --git a/include/net/seg6_local.h b/include/net/seg6_local.h
index 661fd5b4d3e0..08359e2d8b35 100644
--- a/include/net/seg6_local.h
+++ b/include/net/seg6_local.h
@@ -21,10 +21,12 @@
 
 extern int seg6_lookup_nexthop(struct sk_buff *skb, struct in6_addr *nhaddr,
   u32 tbl_id);
+extern bool seg6_bpf_has_valid_srh(struct sk_buff *skb);
 
 struct seg6_bpf_srh_state {
-   bool valid;
+   struct ipv6_sr_hdr *srh;
u16 hdrlen;
+   bool valid;
 };
 
 DECLARE_PER_CPU(struct seg6_bpf_srh_state, seg6_bpf_srh_states);
diff --git a/net/core/filter.c b/net/core/filter.c
index 104d560946da..355430cfeb76 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -4542,26 +4542,28 @@ BPF_CALL_4(bpf_lwt_seg6_store_bytes, struct sk_buff *, 
skb, u32, offset,
 {
struct seg6_bpf_srh_state *srh_state =
this_cpu_ptr(_bpf_srh_states);
+   struct ipv6_sr_hdr *srh = srh_state->srh;
void *srh_tlvs, *srh_end, *ptr;
-   struct ipv6_sr_hdr *srh;
int srhoff = 0;
 
-   if (ipv6_find_hdr(skb, , IPPROTO_ROUTING, NULL, NULL) < 0)
+   if (srh == NULL)
return -EINVAL;
 
-   srh = (struct ipv6_sr_hdr *)(skb->data + srhoff);
srh_tlvs = (void *)((char *)srh + ((srh->first_segment + 1) << 4));
srh_end = (void *)((char *)srh + sizeof(*srh) + srh_state->hdrlen);
 
ptr = skb->data + offset;
if (ptr >= srh_tlvs && ptr + len <= srh_end)
-   srh_state->valid = 0;
+   srh_state->valid = false;
else if (ptr < (void *)>flags ||
 ptr + len > (void *)>segments)
return -EFAULT;
 
if (unlikely(bpf_try_make_writable(skb, offset + len)))
return -EFAULT;
+   if (ipv6_find_hdr(skb, , IPPROTO_ROUTING, NULL, NULL) < 0)
+   return -EINVAL;
+   srh_state->srh = (struct ipv6_sr_hdr *)(skb->data + srhoff);
 
memcpy(skb->data + offset, from, len);
return 0;
@@ -4577,52 +4579,79 @@ static const struct bpf_func_proto 
bpf_lwt_seg6_store_bytes_proto = {
.arg4_type  = ARG_CONST_SIZE
 };
 
-BPF_CALL_4(bpf_lwt_seg6_action, struct sk_buff *, skb,
-  u32, action, void *, param, u32, param_len)
+static void bpf_update_srh_state(struct sk_buff *skb)
 {
struct seg6_bpf_srh_state *srh_state =
this_cpu_ptr(_bpf_srh_states);
-   struct ipv6_sr_hdr *srh;
int srhoff = 0;
-   int err;
-
-   if (ipv6_find_hdr(skb, , IPPROTO_ROUTING, NULL, NULL) < 0)
-   return -EINVAL;
-   srh = (struct ipv6_sr_hdr *)(skb->data + srhoff);
-
-   if (!srh_state->valid) {
-   if (unlikely((srh_state->hdrlen & 7) != 0))
-   return -EBADMSG;
-
-   srh->hdrlen = (u8)(srh_state->hdrlen >> 3);
-   if (unlikely(!seg6_validate_srh(srh, (srh->hdrlen + 1) << 3)))
-   return -EBADMSG;
 
-   srh_state->valid = 1;
+   if (ipv6_find_hdr(skb, , IPPROTO_ROUTING, NULL, NULL) < 0) {
+   srh_state->srh = NULL;
+   } else {
+   srh_state->srh = (struct ipv6_sr_hdr *)(skb->data + srhoff);
+   srh_state->hdrlen = srh_state->srh->hdrlen << 3;
+   srh_state->valid = true;
}
+}
+
+BPF_CALL_4(bpf_lwt_seg6_action, struct sk_buff *, skb,
+  u32, action, void *, param, u32, param_len)
+{
+   struct seg6_bpf_srh_state *srh_state =
+   this_cpu_ptr(_bpf_srh_states);
+   int hdroff = 0;
+   int err;
 
switch (action) {
case SEG6_LOCAL_ACTION_END_X:
+   if (!seg6_bpf_has_valid_srh(skb))
+   return -EBADMSG;
if (param_len != sizeof(struct in6_addr))
return -EINVAL;
return seg6_lookup_nexthop(skb, (struct in6_addr *)param, 0);
case SEG6_LOCAL_ACTION_END_T:
+   if