From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tushar Dave Subject: Re: [RFC v2 PATCH 2/4] ebpf: Add sg_filter_run and sg helper Date: Fri, 29 Jun 2018 17:24:59 -0700 Message-ID: <6db49f0a-8b38-9e34-c392-0b5cdbb75742@oracle.com> References: <1529431217-5264-1-git-send-email-tushar.n.dave@oracle.com> <1529431217-5264-3-git-send-email-tushar.n.dave@oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit To: Daniel Borkmann , ast@kernel.org, davem@davemloft.net, jakub.kicinski@netronome.com, quentin.monnet@netronome.com, jiong.wang@netronome.com, guro@fb.com, sandipan@linux.vnet.ibm.com, john.fastabend@gmail.com, kafai@fb.com, rdna@fb.com, brakmo@fb.com, netdev@vger.kernel.org, acme@redhat.com, sowmini.varadhan@oracle.com Return-path: Received: from userp2120.oracle.com ([156.151.31.85]:54830 "EHLO userp2120.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753127AbeF3AkB (ORCPT ); Fri, 29 Jun 2018 20:40:01 -0400 In-Reply-To: Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 06/29/2018 01:18 AM, Daniel Borkmann wrote: > On 06/19/2018 08:00 PM, Tushar Dave wrote: >> When sg_filter_run() is invoked it runs the attached eBPF >> SOCKET_SG_FILTER program which deals with struct scatterlist. >> >> In addition, this patch also adds bpf_sg_next helper function that >> allows users to retrieve the next sg element from sg list. >> >> Signed-off-by: Tushar Dave >> Acked-by: Sowmini Varadhan >> --- >> include/linux/filter.h | 2 + >> include/uapi/linux/bpf.h | 10 ++++- >> net/core/filter.c | 72 +++++++++++++++++++++++++++++++ >> tools/include/uapi/linux/bpf.h | 10 ++++- >> tools/testing/selftests/bpf/bpf_helpers.h | 3 ++ >> 5 files changed, 95 insertions(+), 2 deletions(-) >> >> diff --git a/include/linux/filter.h b/include/linux/filter.h >> index 71618b1..d176402 100644 >> --- a/include/linux/filter.h >> +++ b/include/linux/filter.h >> @@ -1072,4 +1072,6 @@ struct bpf_sock_ops_kern { >> */ >> }; >> >> +int sg_filter_run(struct sock *sk, struct scatterlist *sg); >> + >> #endif /* __LINUX_FILTER_H__ */ >> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h >> index ef0a7b6..036432b 100644 >> --- a/include/uapi/linux/bpf.h >> +++ b/include/uapi/linux/bpf.h >> @@ -2076,6 +2076,13 @@ struct bpf_stack_build_id { >> * Return >> * A 64-bit integer containing the current cgroup id based >> * on the cgroup within which the current task is running. >> + * >> + * int bpf_sg_next(struct bpf_scatterlist *sg) >> + * Description >> + * This helper allows user to retrieve next sg element from >> + * sg list. >> + * Return >> + * Returns 0 on success, or a negative error in case of failure. >> */ >> #define __BPF_FUNC_MAPPER(FN) \ >> FN(unspec), \ >> @@ -2158,7 +2165,8 @@ struct bpf_stack_build_id { >> FN(rc_repeat), \ >> FN(rc_keydown), \ >> FN(skb_cgroup_id), \ >> - FN(get_current_cgroup_id), >> + FN(get_current_cgroup_id), \ >> + FN(sg_next), >> >> /* integer value in 'imm' field of BPF_CALL instruction selects which helper >> * function eBPF program intends to call >> diff --git a/net/core/filter.c b/net/core/filter.c >> index 8f67942..702ff5b 100644 >> --- a/net/core/filter.c >> +++ b/net/core/filter.c >> @@ -121,6 +121,53 @@ int sk_filter_trim_cap(struct sock *sk, struct sk_buff *skb, unsigned int cap) >> } >> EXPORT_SYMBOL(sk_filter_trim_cap); >> >> +int sg_filter_run(struct sock *sk, struct scatterlist *sg) >> +{ >> + struct sk_filter *filter; >> + int err; >> + >> + rcu_read_lock(); >> + filter = rcu_dereference(sk->sk_filter); >> + if (filter) { >> + struct bpf_scatterlist bpfsg; >> + int num_sg; >> + >> + if (!sg) { >> + err = -EINVAL; >> + goto out; >> + } >> + >> + num_sg = sg_nents(sg); >> + if (num_sg <= 0) { >> + err = -EINVAL; >> + goto out; >> + } >> + >> + /* We store a reference to the sg list so it can later used by >> + * eBPF helpers to retrieve the next sg element. >> + */ >> + bpfsg.num_sg = num_sg; >> + bpfsg.cur_sg = 0; >> + bpfsg.sg = sg; >> + >> + /* For the first sg element, we store the pkt access pointers >> + * into start and end so eBPF program can have pkt access using >> + * data and data_end. The pkt access for subsequent element of >> + * sg list is possible when eBPF program invokes bpf_sg_next >> + * which takes care of setting start and end to the correct sg >> + * element. >> + */ >> + bpfsg.start = sg_virt(sg); >> + bpfsg.end = bpfsg.start + sg->length; >> + BPF_PROG_RUN(filter->prog, &bpfsg); >> + } >> +out: >> + rcu_read_unlock(); >> + >> + return err; >> +} >> +EXPORT_SYMBOL(sg_filter_run); >> + >> BPF_CALL_1(bpf_skb_get_pay_offset, struct sk_buff *, skb) >> { >> return skb_get_poff(skb); >> @@ -3753,6 +3800,29 @@ static unsigned long bpf_xdp_copy(void *dst_buff, const void *src_buff, >> .arg1_type = ARG_PTR_TO_CTX, >> }; >> >> +BPF_CALL_1(bpf_sg_next, struct bpf_scatterlist *, bpfsg) >> +{ >> + struct scatterlist *sg = bpfsg->sg; >> + int cur_sg = bpfsg->cur_sg; >> + >> + cur_sg++; >> + if (cur_sg >= bpfsg->num_sg) >> + return -ENODATA; >> + >> + bpfsg->cur_sg = cur_sg; >> + bpfsg->start = sg_virt(&sg[cur_sg]); >> + bpfsg->end = bpfsg->start + sg[cur_sg].length; >> + >> + return 0; >> +} >> + >> +static const struct bpf_func_proto bpf_sg_next_proto = { >> + .func = bpf_sg_next, >> + .gpl_only = false, >> + .ret_type = RET_INTEGER, >> + .arg1_type = ARG_PTR_TO_CTX, >> +}; > > Should be added to bpf_helper_changes_pkt_data() in order to enforce a reload > of all pkt pointers. Otherwise this is buggy in the sense that someone could only > reload pkt_end pointer in the prog while old pkt_start still points to previous > sg entry, so you would be able to access out of bounds. Sure thing. Will do so. Thank you. -Tushar > > Thanks, > Daniel >