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.6 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_PASS,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 C772DC169C4 for ; Thu, 31 Jan 2019 20:47:15 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 981F3218AC for ; Thu, 31 Jan 2019 20:47:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1548967635; bh=C0zJmwB3dYmbfbYKMm+ugYe/9rSgmIaOb2t5ZpcPLOc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:List-ID:From; b=zKi/MCshQFfF8X5Iat3qPtNdeGQRxPGO6Jm/RAsdnCxRkwOEVLQ9mAoglh27FkNdZ 8Z/67+eA4R4Sk2q0v57Kyfddv1t2e5pi7AtuTMZmhrDL/p8dqxEqNLXbAgwlgUBvmc 2RpJfObzincyvnIBOXnMrkCv7sQTJG0TFmE4gXqA= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727813AbfAaUrO (ORCPT ); Thu, 31 Jan 2019 15:47:14 -0500 Received: from mail.kernel.org ([198.145.29.99]:48932 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725883AbfAaUrO (ORCPT ); Thu, 31 Jan 2019 15:47:14 -0500 Received: from quaco.ghostprotocols.net (unknown [94.113.247.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 9146620881; Thu, 31 Jan 2019 20:47:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1548967632; bh=C0zJmwB3dYmbfbYKMm+ugYe/9rSgmIaOb2t5ZpcPLOc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=fFPWMg1x6Zrn20b1HHSBV8Wn9cZ4kZWz2o7/SYMQkH1syIS43/aXGC1AXarNDV0CD Nr/rElQdJ1VW0IQarDlrKpj+X4abf+J9AWAZY3tYCzIgWc6UmUGdSyTMSbbAWY1iTW tVj2NXzlrGtxwJA+b+g5Jr1IsPpOCQSvPIsJ6awU= Received: by quaco.ghostprotocols.net (Postfix, from userid 1000) id 2DDFE4034F; Thu, 31 Jan 2019 21:47:10 +0100 (CET) Date: Thu, 31 Jan 2019 21:47:10 +0100 From: Arnaldo CArvalho de Melo To: Yonghong Song Cc: Alexei Starovoitov , Magnus Karlsson , "bjorn.topel@intel.com" , "ast@kernel.org" , "daniel@iogearbox.net" , "netdev@vger.kernel.org" , "jakub.kicinski@netronome.com" , "bjorn.topel@gmail.com" , "qi.z.zhang@intel.com" , "brouer@redhat.com" Subject: Re: [PATCH bpf-next v3 1/3] libbpf: move pr_*() functions to common header file Message-ID: <20190131204710.GA4519@kernel.org> References: <1548774737-16579-1-git-send-email-magnus.karlsson@intel.com> <1548774737-16579-2-git-send-email-magnus.karlsson@intel.com> <20190131185206.6rvqxlvutpedwnqc@ast-mbp.dhcp.thefacebook.com> <44beb7c1-9299-1c43-bd1b-38f13e87cf80@fb.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <44beb7c1-9299-1c43-bd1b-38f13e87cf80@fb.com> X-Url: http://acmel.wordpress.com User-Agent: Mutt/1.10.1 (2018-07-13) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Em Thu, Jan 31, 2019 at 07:12:33PM +0000, Yonghong Song escreveu: > > > On 1/31/19 10:52 AM, Alexei Starovoitov wrote: > > On Tue, Jan 29, 2019 at 04:12:15PM +0100, Magnus Karlsson wrote: > >> Move the pr_*() functions in libbpf.c to a common header file called > >> libbpf_internal.h. This so that the later libbpf AF_XDP helper library > >> code in xsk.c can use these printing functions too. > >> > >> Signed-off-by: Magnus Karlsson > >> --- > >> tools/lib/bpf/libbpf.c | 30 +----------------------------- > >> tools/lib/bpf/libbpf_internal.h | 41 +++++++++++++++++++++++++++++++++++++++++ > >> 2 files changed, 42 insertions(+), 29 deletions(-) > >> create mode 100644 tools/lib/bpf/libbpf_internal.h > >> > >> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c > >> index 2ccde17..1d7fe26 100644 > >> --- a/tools/lib/bpf/libbpf.c > >> +++ b/tools/lib/bpf/libbpf.c > >> @@ -39,6 +39,7 @@ > >> #include > >> > >> #include "libbpf.h" > >> +#include "libbpf_internal.h" > >> #include "bpf.h" > >> #include "btf.h" > >> #include "str_error.h" > >> @@ -51,34 +52,6 @@ > >> #define BPF_FS_MAGIC 0xcafe4a11 > >> #endif > >> > >> -#define __printf(a, b) __attribute__((format(printf, a, b))) > >> - > >> -__printf(1, 2) > >> -static int __base_pr(const char *format, ...) > >> -{ > >> - va_list args; > >> - int err; > >> - > >> - va_start(args, format); > >> - err = vfprintf(stderr, format, args); > >> - va_end(args); > >> - return err; > >> -} > >> - > >> -static __printf(1, 2) libbpf_print_fn_t __pr_warning = __base_pr; > >> -static __printf(1, 2) libbpf_print_fn_t __pr_info = __base_pr; > >> -static __printf(1, 2) libbpf_print_fn_t __pr_debug; > >> - > >> -#define __pr(func, fmt, ...) \ > >> -do { \ > >> - if ((func)) \ > >> - (func)("libbpf: " fmt, ##__VA_ARGS__); \ > >> -} while (0) > >> - > >> -#define pr_warning(fmt, ...) __pr(__pr_warning, fmt, ##__VA_ARGS__) > >> -#define pr_info(fmt, ...) __pr(__pr_info, fmt, ##__VA_ARGS__) > >> -#define pr_debug(fmt, ...) __pr(__pr_debug, fmt, ##__VA_ARGS__) > > > > since these funcs are about to be used more widely > > let's clean this api while we still can. > > How about we convert it to single pr_log callback function > > with verbosity flag instead of three callbacks ? Probably. I just wanted to keep, in the source code, the pr_{debug,warn,info} APIs, to reduce the learning curve for people used to program in the kernel and in tools/perf/. > Another possible change related to the API function > libbpf_set_print > > Currently the function takes three parameters, > > LIBBPF_API void libbpf_set_print(libbpf_print_fn_t warn, > libbpf_print_fn_t info, > libbpf_print_fn_t debug); > > So it currently supports three level of debugging output. > Is it possible in the future more debugging output level > may be supported? if this is the case, maybe we could > change the API function libbpf_set_print to something like > the below before the library version bumps into 1.0.0? > > LIBBPF_API void libbpf_set_print(libbpf_print_fn_t dprint); > and > typedef int (*libbpf_print_fn_t)(enum libbpf_debug_level level, > const char *, ...) > __attribute__((format(printf, 1, 2))); > enum libbpf_debug_level { > LIBBPF_WARN, > LIBBPF_INFO, > LIBBPF_DEBUG, > }; > > Basically, the user provided callback function must have > the first parameters as the level. > > Any comments? Arnaldo? I think it is ok, as long as we can override the pr_log thing to allow for using with TUI, stdio, GUI. - Arnaldo