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=-4.1 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no 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 280D0C433E8 for ; Wed, 15 Jul 2020 12:56:44 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 04CF6206D5 for ; Wed, 15 Jul 2020 12:56:44 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="SwpvpRFv" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731325AbgGOM4n (ORCPT ); Wed, 15 Jul 2020 08:56:43 -0400 Received: from us-smtp-delivery-1.mimecast.com ([207.211.31.120]:38774 "EHLO us-smtp-1.mimecast.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1731193AbgGOM4m (ORCPT ); Wed, 15 Jul 2020 08:56:42 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1594817800; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Tyd1Oecq7g1QXX9O1jumuiUNfYulTKddz2EK0Kq40kI=; b=SwpvpRFvIwRIsMvushWBhBISWoIzunz5DnpHKPeYMX2sBawCqQbn4dx1ObiYH0ynMn5Uz6 MihlfF4Rgclp+tP4OPIc4i+9akzFfdKnES1y/8vRtHu8gfgCCF7EyOpF8exyDhVNu+x2rr kU75TbSUos5q7sQZZ8oD9HRCYNLG+N8= Received: from mail-qv1-f69.google.com (mail-qv1-f69.google.com [209.85.219.69]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-79-EMKKXO0QNeSFFrQ62UdeoQ-1; Wed, 15 Jul 2020 08:56:39 -0400 X-MC-Unique: EMKKXO0QNeSFFrQ62UdeoQ-1 Received: by mail-qv1-f69.google.com with SMTP id cv20so1247628qvb.12 for ; Wed, 15 Jul 2020 05:56:39 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:in-reply-to:references:date :message-id:mime-version:content-transfer-encoding; bh=Tyd1Oecq7g1QXX9O1jumuiUNfYulTKddz2EK0Kq40kI=; b=aK1bTL6VrwXpxxhlYxKdwFzb927JhEbHMEI+UZka9qM76jZqhNEqsst81JXcf1IzH0 9HHGvQ0l2oawwet+rMcd8aPu+dgvNndztplCuFDmlSRASCqUEuiRptWJgeFOohRIjkZO N/vB0Jmj1l+i3Zb0663zj0Jg2g+q+eZnTYamtvt3Wng/snffJwDObTH9Tg/DyIGusiRh vTZVs/7Dj5gjM6L4zsgQ9i7ZZurN+ygSJmIrMctZniqO80gDhzDItuLfF4AUNGew53wn 66avl7uZiekY1y8BOduOG3eYrjLJeM7cC9BCkQ7yblNWX5zgFAEmk2vAT6t5HP6gQU5a Nfdw== X-Gm-Message-State: AOAM532QD/g35vfUTN/GX8dO02wTFf+RuKmgGqULv77eQDXUDBhWiTbh khJ8j4pCV4ruuRKZ/8mVHGUvvVPEPe6UxdifluGay/aA4bC4dYARJz92hQHtjEzP/jOndrB7Wbt qQaaLSHX8jjZjgaiZ X-Received: by 2002:a05:620a:50:: with SMTP id t16mr9959751qkt.82.1594817798858; Wed, 15 Jul 2020 05:56:38 -0700 (PDT) X-Google-Smtp-Source: ABdhPJz+A3C8RwmR0b6EoC8M456I6cc7OaR/Q+zLW6AypTd3nNdtFt5RUnmdON6og7oyAeS3BHZRoQ== X-Received: by 2002:a05:620a:50:: with SMTP id t16mr9959728qkt.82.1594817798605; Wed, 15 Jul 2020 05:56:38 -0700 (PDT) Received: from alrua-x1.borgediget.toke.dk ([2a0c:4d80:42:443::2]) by smtp.gmail.com with ESMTPSA id b10sm2070996qkh.124.2020.07.15.05.56.37 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 15 Jul 2020 05:56:37 -0700 (PDT) Received: by alrua-x1.borgediget.toke.dk (Postfix, from userid 1000) id 16E3C1804F0; Wed, 15 Jul 2020 14:56:36 +0200 (CEST) From: Toke =?utf-8?Q?H=C3=B8iland-J=C3=B8rgensen?= To: Alexei Starovoitov Cc: Andrii Nakryiko , bpf , Alexei Starovoitov , Daniel Borkmann , Martin KaFai Lau , Song Liu , Yonghong Song , Andrii Nakryiko , John Fastabend , KP Singh , Networking , Kernel Team Subject: Re: BPF logging infrastructure. Was: [PATCH bpf-next 4/6] tools: add new members to bpf_attr.raw_tracepoint in bpf.h In-Reply-To: <20200714231133.ap5qnalf6moptvfk@ast-mbp.dhcp.thefacebook.com> References: <159467113970.370286.17656404860101110795.stgit@toke.dk> <159467114405.370286.1690821122507970067.stgit@toke.dk> <87r1tegusj.fsf@toke.dk> <87pn8xg6x7.fsf@toke.dk> <87d04xg2p4.fsf@toke.dk> <20200714231133.ap5qnalf6moptvfk@ast-mbp.dhcp.thefacebook.com> X-Clacks-Overhead: GNU Terry Pratchett Date: Wed, 15 Jul 2020 14:56:36 +0200 Message-ID: <874kq9ey2j.fsf@toke.dk> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Alexei Starovoitov writes: > On Wed, Jul 15, 2020 at 12:19:03AM +0200, Toke H=C3=83=C6=92=C3=82=C2=B8i= land-J=C3=83=C6=92=C3=82=C2=B8rgensen wrote: >> Andrii Nakryiko writes: >>=20 >> >> However, assuming it *is* possible, my larger point was that we >> >> shouldn't add just a 'logging struct', but rather a 'common options >> >> struct' which can be extended further as needed. And if it is *not* >> >> possible to add new arguments to a syscall like you're proposing, my >> >> suggestion above would be a different way to achieve basically the sa= me >> >> (at the cost of having to specify the maximum reserved space in advan= ce). >> >> >> > >> > yeah-yeah, I agree, it's less a "logging attr", more of "common attr >> > across all commands". >>=20 >> Right, great. I think we are broadly in agreement with where we want to >> go with this, actually :) > > I really don't like 'common attr across all commands'. > Both of you are talking as libbpf developers who occasionally need to > add printk-s to the kernel. That is not an excuse to bloat api that will = be > useful to two people. What? No, this is about making error messages comprehensible to people who *can't* just go around adding printks. "Guess the source of the EINVAL" is a really bad user experience! > The only reason log_buf sort-of make sense in raw_tp_open is because > btf comparison is moved from prog_load into raw_tp_open. > Miscompare of (prog_fd1, btf_id1) vs (prog_fd2, btf_id2) can be easily so= lved > by libbpf with as nice and as human friendly message libbpf can do. So userspace is supposed to replicate all the checks done by the kernel because we can't be bothered to add proper error messages? Really? > I'm not convinced yet that it's a kernel job to print it nicely. It certa= inly can, > but it's quite a bit different from two existing bpf commands where log_b= uf is used: > PROG_LOAD and BTF_LOAD. In these two cases the kernel verifies the program > and the BTF. raw_tp_open is different, since the kernel needs to compare > that function signatures (prog_fd1, btf_id1) and (prog_fd2, btf_id2) are > exactly the same. The kernel can indicate that with single specific errno= and > libbpf can print human friendly function signatures via btf_dump infra for > humans to see. > So I really don't see why log_buf is such a necessity for raw_tp_open. I'll drop them from raw_tp_open for this series, but I still think they should be added globally (or something like it). Returning a user-friendly error message should be the absolute minimum we do. Just like extack does for netlink. -Toke