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.3 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=unavailable 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 9E9BAC54FCE for ; Tue, 24 Mar 2020 23:59:42 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 774322072E for ; Tue, 24 Mar 2020 23:59:42 +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="HYYku5Dn" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727054AbgCXX7l (ORCPT ); Tue, 24 Mar 2020 19:59:41 -0400 Received: from mail-pl1-f195.google.com ([209.85.214.195]:37256 "EHLO mail-pl1-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727189AbgCXX7l (ORCPT ); Tue, 24 Mar 2020 19:59:41 -0400 Received: by mail-pl1-f195.google.com with SMTP id x1so79381plm.4 for ; Tue, 24 Mar 2020 16:59:40 -0700 (PDT) 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=zoADOvMNHptTBoGslSX+n4kBaQJztbrsX0ScjuUx7E8=; b=HYYku5DnqLNyWu+DQPSge/YBY2qiiiwOR8KCu4jcI5US4tWBxJVW1AVqwOXP7PIpjM AtJq0D74L34QFtD1VjGI23bOUY8u/+nvBw322GHskCX9jZMj9yd3zzWxL0V/k3gQXHVr vi9U760LnSpTgvCL9ce+CcRnBledwtCWyPIzR5QLEDAF87I5aKqmQhuWJVDdqVgU+JRd 7rL/ANopYYn16kQELIDGmig5ZjKQ+/VteUxD2/mb3lxWRryP+dYDjMhtyGk8YWsW/FDi 6ixglC21z6yTX1qqEjD17M/GoQTL+7zjwKK86T8evBRUCDqCBonRRBfgkNxtD/fkYBEm RJAQ== 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=zoADOvMNHptTBoGslSX+n4kBaQJztbrsX0ScjuUx7E8=; b=X1WV8WrQevkRJHICJOY5RKOAxKp9NKLiFYIeaEkvgRf70IuzKFOQJMGfbfDwxYUaN8 piBKjjqgCiPW8/AIBGvVD0liJ1JlFCy4z2V9T9Lewk/la0IL4YOrQ3C3iIu8WowUu/Qk FUprwJZGxKx4PVj/WhQMSMVZ8q4smRwYRtmDWzP6yAdMfEaGY03zAo6z12hcAW5cfBEm zvHN1mXyUq2/Nq1hHLofqqH6isoOnh+HjnJEbcIiHmhVvxcWNSs5+YZUkNRa9ro3n9bI HeVZGNwvqhAT2uA1vUcrG9dmDyqn26ynehxwFqqx483oLqwKCjns7lWLd16VB19fLXSQ 8ioA== X-Gm-Message-State: ANhLgQ28kKGIFUGKjjFKDn0v/G1qRf+W2vPvxbChJoEJ3DW/SLRLKSr/ BkBe7wlCWX3FejQU8TSVBEQ4XQ== X-Google-Smtp-Source: ADFU+vuHvE3AAHXxKlE2OrBSeeCGbEy52NmWD53AYbht0Ycw+lsw1lDQlutLAuEesg+IiJeTYQnSiA== X-Received: by 2002:a17:90a:33c1:: with SMTP id n59mr490312pjb.4.1585094380054; Tue, 24 Mar 2020 16:59:40 -0700 (PDT) Received: from localhost ([2601:646:8f00:18d9:d0fa:7a4b:764f:de48]) by smtp.gmail.com with ESMTPSA id f3sm14949656pgg.46.2020.03.24.16.59.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 24 Mar 2020 16:59:39 -0700 (PDT) Date: Tue, 24 Mar 2020 16:59:38 -0700 From: Stanislav Fomichev To: Andrii Nakryiko Cc: Stanislav Fomichev , Networking , bpf , "David S. Miller" , Alexei Starovoitov , Daniel Borkmann Subject: Re: [PATCH bpf-next] libbpf: don't allocate 16M for log buffer by default Message-ID: <20200324235938.GA2805006@mini-arch.hsd1.ca.comcast.net> References: <20200324233120.66314-1-sdf@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.12.2 (2019-09-21) Sender: bpf-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org On 03/24, Andrii Nakryiko wrote: > On Tue, Mar 24, 2020 at 4:31 PM Stanislav Fomichev wrote: > > > > For each prog/btf load we allocate and free 16 megs of verifier buffer. > > On production systems it doesn't really make sense because the > > programs/btf have gone through extensive testing and (mostly) guaranteed > > to successfully load. > > > > Let's switch to a much smaller buffer by default (128 bytes, sys_bpf > > doesn't accept smaller log buffer) and resize it if the kernel returns > > ENOSPC. On the first ENOSPC error we resize the buffer to BPF_LOG_BUF_SIZE > > and then, on each subsequent ENOSPC, we keep doubling the buffer. > > > > Signed-off-by: Stanislav Fomichev > > --- > > tools/lib/bpf/btf.c | 10 +++++++++- > > tools/lib/bpf/libbpf.c | 10 ++++++++-- > > tools/lib/bpf/libbpf_internal.h | 2 ++ > > 3 files changed, 19 insertions(+), 3 deletions(-) > > > > diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c > > index 3d1c25fc97ae..53c7efc3b347 100644 > > --- a/tools/lib/bpf/btf.c > > +++ b/tools/lib/bpf/btf.c > > @@ -657,13 +657,14 @@ int btf__finalize_data(struct bpf_object *obj, struct btf *btf) > > > > int btf__load(struct btf *btf) > > { > > - __u32 log_buf_size = BPF_LOG_BUF_SIZE; > > + __u32 log_buf_size = BPF_MIN_LOG_BUF_SIZE; > > char *log_buf = NULL; > > int err = 0; > > > > if (btf->fd >= 0) > > return -EEXIST; > > > > +retry_load: > > log_buf = malloc(log_buf_size); > > if (!log_buf) > > return -ENOMEM; > > I'd argue that on first try we shouldn't allocate log_buf at all, then > start allocating it using reasonable starting size (see below). Agreed, makes sense. > > @@ -673,6 +674,13 @@ int btf__load(struct btf *btf) > > btf->fd = bpf_load_btf(btf->data, btf->data_size, > > log_buf, log_buf_size, false); > > if (btf->fd < 0) { > > + if (errno == ENOSPC) { > > + log_buf_size = max((__u32)BPF_LOG_BUF_SIZE, > > + log_buf_size << 1); > > + free(log_buf); > > + goto retry_load; > > + } > > + > > err = -errno; > > pr_warn("Error loading BTF: %s(%d)\n", strerror(errno), errno); > > if (*log_buf) > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c > > index 085e41f9b68e..793c81b35ccc 100644 > > --- a/tools/lib/bpf/libbpf.c > > +++ b/tools/lib/bpf/libbpf.c > > @@ -4855,7 +4855,7 @@ load_program(struct bpf_program *prog, struct bpf_insn *insns, int insns_cnt, > > { > > struct bpf_load_program_attr load_attr; > > char *cp, errmsg[STRERR_BUFSIZE]; > > - int log_buf_size = BPF_LOG_BUF_SIZE; > > + size_t log_buf_size = BPF_MIN_LOG_BUF_SIZE; > > char *log_buf; > > int btf_fd, ret; > > > > @@ -4911,7 +4911,13 @@ load_program(struct bpf_program *prog, struct bpf_insn *insns, int insns_cnt, > > } > > > > if (errno == ENOSPC) { > > same, doing if (!log_buf || errno == ENOSPC) should handle this > without any other major changes? Yeah, I don't see why it shouldn't. Let me try to see if I hit something in the selftests with that approach. > > - log_buf_size <<= 1; > > + if (errno == ENOSPC) { > > + log_buf_size = max((size_t)BPF_LOG_BUF_SIZE, > > + log_buf_size << 1); > > + free(log_buf); > > + goto retry_load; > > + } > > + > > free(log_buf); > > goto retry_load; > > } > > diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h > > index 8c3afbd97747..2720f3366798 100644 > > --- a/tools/lib/bpf/libbpf_internal.h > > +++ b/tools/lib/bpf/libbpf_internal.h > > @@ -23,6 +23,8 @@ > > #define BTF_PARAM_ENC(name, type) (name), (type) > > #define BTF_VAR_SECINFO_ENC(type, offset, size) (type), (offset), (size) > > > > +#define BPF_MIN_LOG_BUF_SIZE 128 > > This seems way too low, if there is some error it almost certainly > will be too short, probably for few iterations, just causing waste. > Let's make it something a bit more reasonable, like 32KB or something? In this case, maybe start with the existing 16M BPF_LOG_BUF_SIZE? My goal here is optimize for the successful case. If there is an error the size shouldn't matter that much.