On 07/24/2018 07:14 PM, Martin KaFai Lau wrote: > On Tue, Jul 24, 2018 at 04:59:54PM +0000, 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://urldefense.proofpoint.com/v2/url?u=https-3A__tools.ietf.org_html_draft-2Dfilsfils-2Dspring-2Dsrv6-2Dnetwork-2Dprogramming-2D05&d=DwIBAg&c=5VD0RTtNlTh3ycd41b3MUw&r=VQnoQ7LvghIj0gVEaiQSUw&m=c61PGnhPMmCUcL5lpyBsxOmsBU2mU5KFY0-Ioo-pBC4&s=mzShtRc5ofzfknAuqoehbGN1ifA17aKihiVLJVfkuZ8&e= >> >> 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. >> >> Signed-off-by: Mathieu Xhonneux <m.xhonn...@gmail.com> [...] >> + >> static int input_action_end_bpf(struct sk_buff *skb, >> struct seg6_local_lwt *slwt) >> { >> struct seg6_bpf_srh_state *srh_state = >> this_cpu_ptr(&seg6_bpf_srh_states); >> - struct seg6_bpf_srh_state local_srh_state; >> struct ipv6_sr_hdr *srh; >> - int srhoff = 0; >> int ret; >> >> srh = get_and_validate_srh(skb); >> @@ -478,6 +499,7 @@ static int input_action_end_bpf(struct sk_buff *skb, >> * which is also accessed by the bpf_lwt_seg6_* helpers >> */ >> preempt_disable(); >> + srh_state->srh = srh; >> srh_state->hdrlen = srh->hdrlen << 3; >> srh_state->valid = 1; >> >> @@ -486,9 +508,6 @@ static int input_action_end_bpf(struct sk_buff *skb, >> ret = bpf_prog_run_save_cb(slwt->bpf.prog, skb); >> rcu_read_unlock(); >> >> - local_srh_state = *srh_state; >> - preempt_enable(); >> - >> switch (ret) { >> case BPF_OK: >> case BPF_REDIRECT: >> @@ -500,24 +519,17 @@ static int input_action_end_bpf(struct sk_buff *skb, >> goto drop; >> } >> >> - if (unlikely((local_srh_state.hdrlen & 7) != 0)) >> - goto drop; >> - >> - if (ipv6_find_hdr(skb, &srhoff, IPPROTO_ROUTING, NULL, NULL) < 0) >> - goto drop; >> - srh = (struct ipv6_sr_hdr *)(skb->data + srhoff); >> - srh->hdrlen = (u8)(local_srh_state.hdrlen >> 3); >> - >> - if (!local_srh_state.valid && >> - unlikely(!seg6_validate_srh(srh, (srh->hdrlen + 1) << 3))) >> + if (srh_state->srh && !seg6_bpf_has_valid_srh(skb)) >> goto drop; >> >> + preempt_enable(); >> if (ret != BPF_REDIRECT) >> seg6_lookup_nexthop(skb, NULL, 0); >> >> return dst_input(skb); >> >> drop: >> + preempt_enable(); > For this drop case at the beginning of this function: > > srh = get_and_validate_srh(skb); > if (!srh) > goto drop; > > preempt_disable() was not called yet?
Agree, this is buggy.
