From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.5 required=3.0 tests=DKIMWL_WL_MED,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_PASS,URIBL_BLOCKED,USER_AGENT_MUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A1A9FC43381 for ; Mon, 25 Feb 2019 23:14:43 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 5B7C020C01 for ; Mon, 25 Feb 2019 23:14:43 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=fomichev-me.20150623.gappssmtp.com header.i=@fomichev-me.20150623.gappssmtp.com header.b="uItaMy0F" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728879AbfBYXOm (ORCPT ); Mon, 25 Feb 2019 18:14:42 -0500 Received: from mail-pf1-f194.google.com ([209.85.210.194]:41049 "EHLO mail-pf1-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727421AbfBYXOl (ORCPT ); Mon, 25 Feb 2019 18:14:41 -0500 Received: by mail-pf1-f194.google.com with SMTP id d25so5210590pfn.8 for ; Mon, 25 Feb 2019 15:14:41 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=fomichev-me.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=EUTS04dMII99kQ4sbRguzRuQ4z54uTqcpOBdT5yToHA=; b=uItaMy0FRuiqoEpRg/XEzIC6dx0qcy8iUNxGam6MGGiL9YdAXk6wj62kX2VJc0pGHP 0AmNsOh38BV/2PKTcrVZG43yUzsE/3mYcLx6tdQwS2i4MJzJHeAHY/xJ8FxLsW+809Lm WrmH72OTaZ+iypDBBBkwMv9qJndqkkzw1VVEEAOzqozU45I2LedgCwykbwhpjW1XFMYS 1uyaGS6eUgl+ENqnqIsx/+7iGlVZYM9ebKnmLhRaW0e8PK3Z5yT2MkAEzgK7l+Jkv6Lr 6fo75DfeMzK0XpxdBQEQVTa7pSSz02c4+olzjljZ/BA3IlAKUxHjI/I8HJVHENkwVGO5 l7xw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=EUTS04dMII99kQ4sbRguzRuQ4z54uTqcpOBdT5yToHA=; b=CF9CvzMKRSOav7QhNs2BXSKiOSMTfXpScePVudKSG3hde5yimXo5bTXnrrdbqAE9k0 Gz3xSVX1jaANiqAYzaTQmy2+VH1Asgv15Voi+I8RO32s1mWc19tYIhupFQLxjzjvOvm8 dAZ7AN7W7mphmGbcRYLD3GeX4fzyvjXeRRmJEOQuLK3eI8LE2ZLyczJ+5btg7gxb80x/ xZOesKz958ohLIdq6nGz0+TgqdfXqk8TSDPTugOj1OdCADS2T0OWs5q/pN1paQ6fbncH oyAiwicuZWy2pDysWLvdmrsYC7CZUO4F0OE9JKa7/yzTDuzH8Scq8bt3x9pcaHJgLAoo OKUg== X-Gm-Message-State: AHQUAuZr29aDHEDiCtDpWXBGwbQY6CU1lAILMlB0MANO0x8PIeQZMjgc 9ITp77ATXBV3HP9k+QB5h5sSww== X-Google-Smtp-Source: AHgI3Ib7MeXdf1uOXLFNUQrJXCwggMUJZFw9vzbiOTdEDuvmkbGt6edjKlgwvSifEjqH00EoRMSHtw== X-Received: by 2002:a62:fc10:: with SMTP id e16mr22995849pfh.83.1551136480292; Mon, 25 Feb 2019 15:14:40 -0800 (PST) Received: from localhost ([2601:646:8f00:18d9:d0fa:7a4b:764f:de48]) by smtp.gmail.com with ESMTPSA id e24sm15023737pfi.153.2019.02.25.15.14.39 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 25 Feb 2019 15:14:39 -0800 (PST) Date: Mon, 25 Feb 2019 15:14:38 -0800 From: Stanislav Fomichev To: brakmo Cc: netdev , Martin Lau , Alexei Starovoitov , Daniel Borkmann , Eric Dumazet , Kernel Team Subject: Re: [PATCH v2 bpf-next 2/9] bpf: Add bpf helper bpf_tcp_enter_cwr Message-ID: <20190225231438.GC32115@mini-arch> References: <20190223010703.678070-1-brakmo@fb.com> <20190223010703.678070-3-brakmo@fb.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190223010703.678070-3-brakmo@fb.com> User-Agent: Mutt/1.11.3 (2019-02-01) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On 02/22, brakmo wrote: > From: Martin KaFai Lau > > This patch adds a new bpf helper BPF_FUNC_tcp_enter_cwr > "int bpf_tcp_enter_cwr(struct bpf_tcp_sock *tp)". > It is added to BPF_PROG_TYPE_CGROUP_SKB which can be attached > to the egress path where the bpf prog is called by > ip_finish_output() or ip6_finish_output(). The verifier > ensures that the parameter must be a tcp_sock. > > This helper makes a tcp_sock enter CWR state. It can be used > by a bpf_prog to manage egress network bandwidth limit per > cgroupv2. A later patch will have a sample program to > show how it can be used to limit bandwidth usage per cgroupv2. > > To ensure it is only called from BPF_CGROUP_INET_EGRESS, the > attr->expected_attach_type must be specified as BPF_CGROUP_INET_EGRESS > during load time if the prog uses this new helper. > The newly added prog->enforce_expected_attach_type bit will also be set > if this new helper is used. This bit is for backward compatibility reason > because currently prog->expected_attach_type has been ignored in > BPF_PROG_TYPE_CGROUP_SKB. During attach time, > prog->expected_attach_type is only enforced if the > prog->enforce_expected_attach_type bit is set. > i.e. prog->expected_attach_type is only enforced if this new helper > is used by the prog. > > Signed-off-by: Lawrence Brakmo > Signed-off-by: Martin KaFai Lau > --- > include/linux/bpf.h | 1 + > include/linux/filter.h | 3 ++- > include/uapi/linux/bpf.h | 9 ++++++++- > kernel/bpf/syscall.c | 12 ++++++++++++ > kernel/bpf/verifier.c | 4 ++++ > net/core/filter.c | 25 +++++++++++++++++++++++++ > 6 files changed, 52 insertions(+), 2 deletions(-) > > diff --git a/include/linux/bpf.h b/include/linux/bpf.h > index d5ba2fc01af3..2d54ba7cf9dd 100644 > --- a/include/linux/bpf.h > +++ b/include/linux/bpf.h > @@ -195,6 +195,7 @@ enum bpf_arg_type { > ARG_PTR_TO_SOCKET, /* pointer to bpf_sock */ > ARG_PTR_TO_SPIN_LOCK, /* pointer to bpf_spin_lock */ > ARG_PTR_TO_SOCK_COMMON, /* pointer to sock_common */ > + ARG_PTR_TO_TCP_SOCK, /* pointer to tcp_sock */ > }; > > /* type of values returned from helper functions */ > diff --git a/include/linux/filter.h b/include/linux/filter.h > index f32b3eca5a04..c6e878bdc5a6 100644 > --- a/include/linux/filter.h > +++ b/include/linux/filter.h > @@ -510,7 +510,8 @@ struct bpf_prog { > blinded:1, /* Was blinded */ > is_func:1, /* program is a bpf function */ > kprobe_override:1, /* Do we override a kprobe? */ > - has_callchain_buf:1; /* callchain buffer allocated? */ > + has_callchain_buf:1, /* callchain buffer allocated? */ > + enforce_expected_attach_type:1; /* Enforce expected_attach_type checking at attach time */ > enum bpf_prog_type type; /* Type of BPF program */ > enum bpf_attach_type expected_attach_type; /* For some prog types */ > u32 len; /* Number of filter blocks */ > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h > index bcdd2474eee7..95b5058fa945 100644 > --- a/include/uapi/linux/bpf.h > +++ b/include/uapi/linux/bpf.h > @@ -2359,6 +2359,12 @@ union bpf_attr { > * Return > * A **struct bpf_tcp_sock** pointer on success, or NULL in > * case of failure. > + * > + * int bpf_tcp_enter_cwr(struct bpf_tcp_sock *tp) > + * Description > + * Make a tcp_sock enter CWR state. > + * Return > + * 0 on success, or a negative error in case of failure. > */ > #define __BPF_FUNC_MAPPER(FN) \ > FN(unspec), \ > @@ -2457,7 +2463,8 @@ union bpf_attr { > FN(spin_lock), \ > FN(spin_unlock), \ > FN(sk_fullsock), \ > - FN(tcp_sock), > + FN(tcp_sock), \ > + FN(tcp_enter_cwr), > > /* integer value in 'imm' field of BPF_CALL instruction selects which helper > * function eBPF program intends to call > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c > index ec7c552af76b..9a478f2875cd 100644 > --- a/kernel/bpf/syscall.c > +++ b/kernel/bpf/syscall.c > @@ -1482,6 +1482,14 @@ bpf_prog_load_check_attach_type(enum bpf_prog_type prog_type, > default: > return -EINVAL; > } > + case BPF_PROG_TYPE_CGROUP_SKB: > + switch (expected_attach_type) { > + case BPF_CGROUP_INET_INGRESS: > + case BPF_CGROUP_INET_EGRESS: > + return 0; > + default: > + return -EINVAL; > + } > default: > return 0; > } > @@ -1725,6 +1733,10 @@ static int bpf_prog_attach_check_attach_type(const struct bpf_prog *prog, > case BPF_PROG_TYPE_CGROUP_SOCK: > case BPF_PROG_TYPE_CGROUP_SOCK_ADDR: > return attach_type == prog->expected_attach_type ? 0 : -EINVAL; > + case BPF_PROG_TYPE_CGROUP_SKB: > + return prog->enforce_expected_attach_type && > + prog->expected_attach_type != attach_type ? > + -EINVAL : 0; > default: > return 0; > } > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > index 1b9496c41383..95fb385c6f3c 100644 > --- a/kernel/bpf/verifier.c > +++ b/kernel/bpf/verifier.c > @@ -2424,6 +2424,10 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 regno, > return -EFAULT; > } > meta->ptr_id = reg->id; > + } else if (arg_type == ARG_PTR_TO_TCP_SOCK) { > + expected_type = PTR_TO_TCP_SOCK; > + if (type != expected_type) > + goto err_type; > } else if (arg_type == ARG_PTR_TO_SPIN_LOCK) { > if (meta->func_id == BPF_FUNC_spin_lock) { > if (process_spin_lock(env, regno, true)) > diff --git a/net/core/filter.c b/net/core/filter.c > index 97916eedfe69..ca57ef25279c 100644 > --- a/net/core/filter.c > +++ b/net/core/filter.c > @@ -5426,6 +5426,24 @@ static const struct bpf_func_proto bpf_tcp_sock_proto = { > .arg1_type = ARG_PTR_TO_SOCK_COMMON, > }; > > +BPF_CALL_1(bpf_tcp_enter_cwr, struct tcp_sock *, tp) > +{ > + struct sock *sk = (struct sock *)tp; > + > + if (sk->sk_state == TCP_ESTABLISHED) { > + tcp_enter_cwr(sk); > + return 0; > + } > + > + return -EINVAL; > +} > + > +static const struct bpf_func_proto bpf_tcp_enter_cwr_proto = { > + .func = bpf_tcp_enter_cwr, > + .gpl_only = false, > + .ret_type = RET_INTEGER, > + .arg1_type = ARG_PTR_TO_TCP_SOCK, > +}; > #endif /* CONFIG_INET */ > > bool bpf_helper_changes_pkt_data(void *func) > @@ -5585,6 +5603,13 @@ cg_skb_func_proto(enum bpf_func_id func_id, struct bpf_prog *prog) > #ifdef CONFIG_INET > case BPF_FUNC_tcp_sock: > return &bpf_tcp_sock_proto; [...] > + case BPF_FUNC_tcp_enter_cwr: > + if (prog->expected_attach_type == BPF_CGROUP_INET_EGRESS) { > + prog->enforce_expected_attach_type = 1; > + return &bpf_tcp_enter_cwr_proto; Instead of this back and forth with enforce_expected_attach_type, can we just do here: if (prog->expected_attach_type == BPF_CGROUP_INET_EGRESS) return &bpf_tcp_enter_cwr_proto; else return null; Wouldn't it have the same effect? > + } else { > + return NULL; > + } > #endif > default: > return sk_filter_func_proto(func_id, prog); > -- > 2.17.1 >