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=-7.2 required=3.0 tests=DKIMWL_WL_HIGH,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS 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 4F633C43381 for ; Tue, 19 Feb 2019 23:53:54 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 02B8721738 for ; Tue, 19 Feb 2019 23:53:54 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=fb.com header.i=@fb.com header.b="Y36ZTtXe"; dkim=pass (1024-bit key) header.d=fb.onmicrosoft.com header.i=@fb.onmicrosoft.com header.b="Ih81X7OQ" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729194AbfBSXxw (ORCPT ); Tue, 19 Feb 2019 18:53:52 -0500 Received: from mx0a-00082601.pphosted.com ([67.231.145.42]:37212 "EHLO mx0a-00082601.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726405AbfBSXxw (ORCPT ); Tue, 19 Feb 2019 18:53:52 -0500 Received: from pps.filterd (m0109333.ppops.net [127.0.0.1]) by mx0a-00082601.pphosted.com (8.16.0.27/8.16.0.27) with SMTP id x1JNqFAv004582; Tue, 19 Feb 2019 15:53:31 -0800 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=fb.com; h=from : to : cc : subject : date : message-id : references : in-reply-to : content-type : content-id : content-transfer-encoding : mime-version; s=facebook; bh=7H89YHOvUNxLtfLdsHmbxhtf1/Id/I3nQK9yXwFWqDk=; b=Y36ZTtXesRnO+WeuyKPnnTjUwBgQP/NNQrcfrK2ZwCCKw4uQaBkloZs2Gz3u86HaOP8I An7XyZKimvCae+ukvBMJquxCG94QLWBnjAaUyN/Zsp85FMZgP6MQuXFCsPgtaU/doppY ff4SW5QfjD+Ikuggn16d4YgiJpdbeGCftr0= Received: from maileast.thefacebook.com ([199.201.65.23]) by mx0a-00082601.pphosted.com with ESMTP id 2qrtsw07cq-3 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-SHA384 bits=256 verify=NOT); Tue, 19 Feb 2019 15:53:30 -0800 Received: from frc-mbx06.TheFacebook.com (2620:10d:c0a1:f82::30) by frc-hub03.TheFacebook.com (2620:10d:c021:18::173) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384) id 15.1.1531.3; Tue, 19 Feb 2019 15:53:26 -0800 Received: from frc-hub06.TheFacebook.com (2620:10d:c021:18::176) by frc-mbx06.TheFacebook.com (2620:10d:c0a1:f82::30) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384) id 15.1.1531.3; Tue, 19 Feb 2019 15:53:26 -0800 Received: from NAM03-BY2-obe.outbound.protection.outlook.com (192.168.183.28) by o365-in.thefacebook.com (192.168.177.76) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384) id 15.1.1531.3 via Frontend Transport; Tue, 19 Feb 2019 15:53:26 -0800 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=fb.onmicrosoft.com; s=selector1-fb-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=7H89YHOvUNxLtfLdsHmbxhtf1/Id/I3nQK9yXwFWqDk=; b=Ih81X7OQdaEhMiIXHdIjDQ7X85gbvIPKWn5/M5Lx1FKhu4cWuzDoU6vPiEAIMZdsYCfulDvcX/bgwLlBB0D+0YgkC/8sotgToYpKzM5a5VBOa7v5BtmqvX4imKl47tpjReeiwf9h+i7MAciYj3lXed8lunTP6bOk5j1Y1mlb3C8= Received: from BYAPR15MB2311.namprd15.prod.outlook.com (52.135.197.145) by BYAPR15MB2198.namprd15.prod.outlook.com (52.135.196.145) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1643.14; Tue, 19 Feb 2019 23:53:05 +0000 Received: from BYAPR15MB2311.namprd15.prod.outlook.com ([fe80::e538:f6a2:82db:3d9c]) by BYAPR15MB2311.namprd15.prod.outlook.com ([fe80::e538:f6a2:82db:3d9c%3]) with mapi id 15.20.1622.018; Tue, 19 Feb 2019 23:53:05 +0000 From: Lawrence Brakmo To: Daniel Borkmann , netdev CC: Martin Lau , Alexei Starovoitov , "Daniel Borkmann --cc=Kernel Team" <"daniel@iogearbox.netKernel-team"@fb.com> Subject: Re: [PATCH bpf-next 3/9] bpf: add bpf helper bpf_skb_set_ecn Thread-Topic: [PATCH bpf-next 3/9] bpf: add bpf helper bpf_skb_set_ecn Thread-Index: AQHUyBV0VSnF0B9yGkSBG3wc4ueD4qXm8kUAgABT9IA= Date: Tue, 19 Feb 2019 23:53:04 +0000 Message-ID: References: <20190219053832.2086706-1-brakmo@fb.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: user-agent: Microsoft-MacOutlook/10.14.0.181208 x-originating-ip: [2620:10d:c090:180::1:18d4] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: ad520627-5838-4ef7-5fcd-08d696c56413 x-microsoft-antispam: BCL:0;PCL:0;RULEID:(2390118)(7020095)(4652040)(8989299)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(5600110)(711020)(4605104)(2017052603328)(7153060)(7193020);SRVR:BYAPR15MB2198; x-ms-traffictypediagnostic: BYAPR15MB2198: x-microsoft-exchange-diagnostics: 1;BYAPR15MB2198;20:1UvS5BckVESGi8J5f+JjrV5tWIAFZmt6k76sOzU+yMokOQRYh3IebOZhBQEYALAQOvrSoCyqP2dTSUq/yu2xy6PIkcZqA6gnCuNbwGCC+2Jxdqdlgl+QqD2frJC1WH/4RVlaRLMZ6Yoc7ttEQK/ptZZfC50eb3Pu3n695Z70/OY= x-microsoft-antispam-prvs: x-forefront-prvs: 09538D3531 x-forefront-antispam-report: SFV:NSPM;SFS:(10019020)(396003)(39860400002)(366004)(136003)(376002)(346002)(199004)(189003)(2906002)(97736004)(102836004)(486006)(76176011)(83716004)(8936002)(4326008)(58126008)(14454004)(99286004)(2616005)(446003)(53936002)(316002)(25786009)(33656002)(6506007)(478600001)(476003)(5660300002)(81156014)(81166006)(6116002)(36756003)(53546011)(11346002)(7736002)(8676002)(71200400001)(71190400001)(305945005)(6436002)(46003)(5024004)(68736007)(256004)(14444005)(229853002)(6486002)(105586002)(82746002)(54906003)(110136005)(106356001)(6246003)(186003)(86362001)(6512007);DIR:OUT;SFP:1102;SCL:1;SRVR:BYAPR15MB2198;H:BYAPR15MB2311.namprd15.prod.outlook.com;FPR:;SPF:None;LANG:en;PTR:InfoNoRecords;A:1;MX:1; received-spf: None (protection.outlook.com: fb.com does not designate permitted sender hosts) x-ms-exchange-senderadcheck: 1 x-microsoft-antispam-message-info: RKLwh3KtvoUWNILFmMTIENgI6vubnAf3h/3wGQND+zKxCLGtvkRx3b5rYV0YPiBKZI/GGT0tsGeblou10Nw5i44KosKDWkArhtFQhDeqCPNcET1J24lkANBE5+gGELMUw3wF1Of738O98nyB1qHJqg71KH0gA7SW4/uYht2/RCjOiA0uUzwGLdovuDk4Ku9YIw6cC3QXEJHkKPkmxhJ4xgTz4V4OWW2e+2yu9XpMQKP0MNhbX/TT/xm1dsn4G5fyBarR02xBNhZMiT7YKx8qLVEwdnKdQJWRtMCuJwotYPLgi+dzuW51z5U9eNWgzWrunthyYh0dq57uc6OQ4p48smyh8TDU1e/YtUeMSX2ANNotV9kZmw1EzwecyBS1sBCGCnd052rSbHel3X2hqmoMzBnofjTg1VV18kNTuuLvZg0= Content-Type: text/plain; charset="utf-7" Content-ID: Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-CrossTenant-Network-Message-Id: ad520627-5838-4ef7-5fcd-08d696c56413 X-MS-Exchange-CrossTenant-originalarrivaltime: 19 Feb 2019 23:53:04.9227 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 8ae927fe-1255-47a7-a2af-5f3a069daaa2 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR15MB2198 X-OriginatorOrg: fb.com X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2019-02-19_16:,, signatures=0 X-Proofpoint-Spam-Reason: safe X-FB-Internal: Safe Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On 2/19/19, 2:52 AM, +ACI-netdev-owner+AEA-vger.kernel.org on behalf of Dan= iel Borkmann+ACI- +ADw-netdev-owner+AEA-vger.kernel.org on behalf of daniel= +AEA-iogearbox.net+AD4- wrote: On 02/19/2019 06:38 AM, brakmo wrote: +AD4- This patch adds a new bpf helper BPF+AF8-FUNC+AF8-skb+AF8-set+AF8= -ecn +AD4- +ACI-int bpf+AF8-skb+AF8-set+AF8-Ecn(struct sk+AF8-buff +ACo-skb)= +ACI-. It is added to +AD4- BPF+AF8-PROG+AF8-TYPE+AF8-CGROUP+AF8-SKB typed bpf+AF8-prog which= currently can +AD4- be attached to the ingress and egress path. This type of +AD4- bpf+AF8-prog cannot modify the skb directly. +AD4-=20 +AD4- This helper is used to set the ECN bits (2) of the IPv6 or IPv4 +AD4- header in skb. It can be used by a bpf+AF8-prog to manage egress +AD4- network bandwdith limit per cgroupv2 by inducing an ECN +AD4- response in the TCP sender (when the packet is ECN enabled). +AD4- This works best when using DCTCP. +AD4-=20 +AD4- Signed-off-by: Lawrence Brakmo +ADw-brakmo+AEA-fb.com+AD4- +AD4- --- +AD4- include/uapi/linux/bpf.h +AHw- 10 +-+-+-+-+-+-+-+-+-- +AD4- net/core/filter.c +AHw- 29 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-= +-+-+-+-+-+-+-+-+-+-+-+-+-+- +AD4- 2 files changed, 38 insertions(+-), 1 deletion(-) +AD4-=20 +AD4- diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h +AD4- index 9e9f4f1a0370..5daf404511f7 100644 +AD4- --- a/include/uapi/linux/bpf.h +AD4- +-+-+- b/include/uapi/linux/bpf.h +AD4- +AEAAQA- -2365,6 +-2365,13 +AEAAQA- union bpf+AF8-attr +AHs- +AD4- +ACo- Make a tcp+AF8-sock enter CWR state. +AD4- +ACo- Return +AD4- +ACo- 0 +AD4- +- +ACo- +AD4- +- +ACo- int bpf+AF8-skb+AF8-set+AF8-ecn(struct sk+AF8-buf +ACo-s= kb, int val) =20 Nit: BPF+AF8-CALL+AF8-2() has u32 val Thanks+ACE- =20 +AD4- +- +ACo- Description +AD4- +- +ACo- Sets ECN bits (2) of IP header. Works with IPv6 and IPv= 4. +AD4- +- +ACo- val should be one of 0, 1, 2, 3. +AD4- +- +ACo- Return +AD4- +- +ACo- -EINVAL on error (e.g. val +AD4- 3), 0 otherwise. +AD4- +ACo-/ +AD4- +ACM-define +AF8AXw-BPF+AF8-FUNC+AF8-MAPPER(FN) +AFw- +AD4- FN(unspec), +AFw- +AD4- +AEAAQA- -2464,7 +-2471,8 +AEAAQA- union bpf+AF8-attr +AHs- +AD4- FN(spin+AF8-unlock), +AFw- +AD4- FN(sk+AF8-fullsock), +AFw- +AD4- FN(tcp+AF8-sock), +AFw- +AD4- - FN(tcp+AF8-enter+AF8-cwr), +AD4- +- FN(tcp+AF8-enter+AF8-cwr), +AFw- +AD4- +- FN(skb+AF8-set+AF8-ecn), +AD4- =20 +AD4- /+ACo- integer value in 'imm' field of BPF+AF8-CALL instruction = selects which helper +AD4- +ACo- function eBPF program intends to call +AD4- diff --git a/net/core/filter.c b/net/core/filter.c +AD4- index f51c4a781844..275acfb2117d 100644 +AD4- --- a/net/core/filter.c +AD4- +-+-+- b/net/core/filter.c +AD4- +AEAAQA- -5438,6 +-5438,33 +AEAAQA- static const struct bpf+AF8-f= unc+AF8-proto bpf+AF8-tcp+AF8-enter+AF8-cwr+AF8-proto +AD0- +AHs- +AD4- .ret+AF8-type +AD0- RET+AF8-INTEGER, +AD4- .arg1+AF8-type +AD0- ARG+AF8-PTR+AF8-TO+AF8-TCP+AF8-SOCK, +AD4- +AH0AOw- +AD4- +- +AD4- +-BPF+AF8-CALL+AF8-2(bpf+AF8-skb+AF8-set+AF8-ecn, struct sk+AF8-b= uff +ACo-, skb, u32, val) +AD4- +-+AHs- +AD4- +- struct ipv6hdr +ACo-ip6h +AD0- ipv6+AF8-hdr(skb)+ADs- +AD4- +- +AD4- +- if ((val +ACY- +AH4-0x3) +ACEAPQ- 0) =20 Nit: INET+AF8-ECN+AF8-MASK Thanks+ACE- =20 +AD4- +- return -EINVAL+ADs- +AD4- +- +AD4- +- if (ip6h-+AD4-version +AD0APQ- 6) +AHs- +AD4- +- ip6h-+AD4-flow+AF8-lbl+AFs-0+AF0- +AD0- (ip6h-+AD4-flow+AF8-l= bl+AFs-0+AF0- +ACY- +AH4-0x30) +AHw- (val +ADwAPA- 4)+ADs- +AD4- +- return 0+ADs- +AD4- +- +AH0- else if (ip6h-+AD4-version +AD0APQ- 4) +AHs- +AD4- +- struct iphdr +ACo-ip4h +AD0- (struct iphdr +ACo-)ip6h+ADs- +AD4- +- +AD4- +- ip4h-+AD4-tos +AD0- (ip4h-+AD4-tos +ACY- +AH4-0x3) +AHw- val+= ADs- +AD4- +- return 0+ADs- +AD4- +- +AH0- =20 Couldn't this be done as native BPF code via direct packet access inste= ad? Afaik, skb-+AD4-data should most likely points to network header for th= e hooks and skb-+AD4-protocol should be one of ETH+AF8-P+AF8-IP+AHs-,V6+AH0-, n= o? Cgroup skb bpf programs do not have write access to packet data. I original= ly was doing what you propose, by adding write support and changing the ecn= value in the bpf program, but Alexei felt that could create problems. Henc= e this approach. =20 Aside from this, don't we also have cloned skbs here (in particular fro= m TCP side)? =20 Looking at cg+AF8-skb+AF8-verifier+AF8-ops ... it seems there also a bu= g in the current code, namely that if we have a direct packet write, we don't make the s= kb writable+ADs- at that point skb-+AD4-data is not private. The cg+AF8-sk= b+AF8-is+AF8-valid+AF8-access() allows to fetch PTR+AF8-TO+AF8-PACKET+AHs-,+AF8-END+AH0-, so we need a = fix like the below for -bpf: =20 diff --git a/net/core/filter.c b/net/core/filter.c index f7d0004fc160..34fe6da0a236 100644 --- a/net/core/filter.c +-+-+- b/net/core/filter.c +AEAAQA- -5796,6 +-5796,12 +AEAAQA- static bool sk+AF8-filter+AF8-is+AF= 8-valid+AF8-access(int off, int size, return bpf+AF8-skb+AF8-is+AF8-valid+AF8-access(off, size, type,= prog, info)+ADs- +AH0- =20 +-static int cg+AF8-skb+AF8-prologue(struct bpf+AF8-insn +ACo-insn+AF8-= buf, bool direct+AF8-write, +- const struct bpf+AF8-prog +ACo-prog) +-+AHs- +- return bpf+AF8-unclone+AF8-prologue(insn+AF8-buf, direct+AF8-w= rite, prog, 0)+ADs- +-+AH0- +- static bool cg+AF8-skb+AF8-is+AF8-valid+AF8-access(int off, int size, enum bpf+AF8-access+AF8-type type, const struct bpf+AF8-prog +ACo-prog, +AEAAQA- -7595,6 +-7601,7 +AEAAQA- const struct bpf+AF8-verifier+AF8-op= s cg+AF8-skb+AF8-verifier+AF8-ops +AD0- +AHs- .get+AF8-func+AF8-proto +AD0- cg+AF8-skb+AF8-func+AF8-p= roto, .is+AF8-valid+AF8-access +AD0- cg+AF8-skb+AF8-is+AF8-val= id+AF8-access, .convert+AF8-ctx+AF8-access +AD0- bpf+AF8-convert+AF8-ctx+A= F8-access, +- .gen+AF8-prologue +AD0- cg+AF8-skb+AF8-prologue, +AH0AOw- =20 const struct bpf+AF8-prog+AF8-ops cg+AF8-skb+AF8-prog+AF8-ops +AD0- +A= Hs- =20 +AD4- +- return -EINVAL+ADs- +AD4- +-+AH0- +AD4- +- +AD4- +-static const struct bpf+AF8-func+AF8-proto bpf+AF8-skb+AF8-set+= AF8-ecn+AF8-proto +AD0- +AHs- +AD4- +- .func +AD0- bpf+AF8-skb+AF8-set+AF8-ecn, +AD4- +- .gpl+AF8-only +AD0- false, +AD4- +- .ret+AF8-type +AD0- RET+AF8-INTEGER, +AD4- +- .arg1+AF8-type +AD0- ARG+AF8-PTR+AF8-TO+AF8-CTX, +AD4- +- .arg2+AF8-type +AD0- ARG+AF8-ANYTHING, +AD4- +-+AH0AOw- +AD4- +ACM-endif /+ACo- CONFIG+AF8-INET +ACo-/ +AD4- =20 +AD4- bool bpf+AF8-helper+AF8-changes+AF8-pkt+AF8-data(void +ACo-func) +AD4- +AEAAQA- -5599,6 +-5626,8 +AEAAQA- cg+AF8-skb+AF8-func+AF8-proto(= enum bpf+AF8-func+AF8-id func+AF8-id, const struct bpf+AF8-prog +ACo-prog) +AD4- return +ACY-bpf+AF8-tcp+AF8-sock+AF8-proto+ADs- +AD4- case BPF+AF8-FUNC+AF8-tcp+AF8-enter+AF8-cwr: +AD4- return +ACY-bpf+AF8-tcp+AF8-enter+AF8-cwr+AF8-proto+ADs- +AD4- +- case BPF+AF8-FUNC+AF8-skb+AF8-set+AF8-ecn: +AD4- +- return +ACY-bpf+AF8-skb+AF8-set+AF8-ecn+AF8-proto+ADs- +AD4- +ACM-endif +AD4- default: +AD4- return sk+AF8-filter+AF8-func+AF8-proto(func+AF8-id, prog)+ADs= - +AD4-=20 =20 =20