From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-f178.google.com (mail-wr0-f178.google.com [209.85.128.178]) by dpdk.org (Postfix) with ESMTP id B4FA18DA2 for ; Mon, 6 Feb 2017 17:18:34 +0100 (CET) Received: by mail-wr0-f178.google.com with SMTP id 89so24322414wrr.2 for ; Mon, 06 Feb 2017 08:18:34 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=0xW6PdRTylYciL+s6NmoKNRm04zcT1nV7kHKt2wFaV0=; b=gVqjGRmtMitpLgwQJm+qusuIl1Z5v34Imd1QGZA5VBjE4ymwS6M2Z7BlA01Vo5FCAW ZD9q28mJ2Sy4EexLTePPigFO1dvs8YOoz1eZ6Nm03GYSCylo1R5nCs17caWVXpxrQ7Fu UeJAgRvprrbpho00fMGkNRm3ffNcaBTEkVVgR7ns2vuMq0r2ojSrLFm6hknhb0/WZ4Qu c3Ax4d+GK8buW7kAo0e4D8gDOnlYCfG+OB2sTjWhI5PMYDg7K75nUzqJZVudWU/m9oeM b+HOiCTGTW2bBnszoqyPkFVoGA30YMNreBYihnlRKKnI3DTm0jrrHyUTeFW7+LWK+an5 CkuA== 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:in-reply-to :references:mime-version:content-transfer-encoding; bh=0xW6PdRTylYciL+s6NmoKNRm04zcT1nV7kHKt2wFaV0=; b=dWVZgsqyKyvney/0LYbTB6nx8O937TWKh6CTuqkdQmbaES3yj6D6kCjMfZaQldDBcu 6yEhSSXnHZzNa3yyorALwLkdqElT8LumazGzXAp5roazR1Ggr4eSdbPqcQctEz+u8jAB 5iJkZHBdVJ+HJVg/A/mNq7pOVdWHtLPr8ZClxlWvjxWtJrubYBgyQA3BvJlYSNJRNqdM FMzDPfZhDZE+K0hQxyoMZnjg1ZG+k1kD2SpHuw9Gbd0glDOdCrZQg/Y5qQHkK8jV19+y LSFS7suR3RC3+nsfsXGbFvqAVfZMr7u4jIzBaY6zl8jZi7LuUYZAY9/Ak83cX2NbwKqR lB+Q== X-Gm-Message-State: AIkVDXJjhPHlUkUdUAFMMu71ZqG440IKGUsrTj47ScBTrvqciP/W636cgY7hoG7crNzgvgOh X-Received: by 10.223.171.12 with SMTP id q12mr9703024wrc.74.1486397914053; Mon, 06 Feb 2017 08:18:34 -0800 (PST) Received: from platinum (2a01cb0c03c651000226b0fffeed02fc.ipv6.abo.wanadoo.fr. [2a01:cb0c:3c6:5100:226:b0ff:feed:2fc]) by smtp.gmail.com with ESMTPSA id i189sm13808420wmg.7.2017.02.06.08.18.33 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 06 Feb 2017 08:18:33 -0800 (PST) Date: Mon, 6 Feb 2017 17:18:31 +0100 From: Olivier Matz To: "Wiles, Keith" Cc: "Richardson, Bruce" , "dev@dpdk.org" , "david.marchand@6wind.com" Message-ID: <20170206171831.6c8996e7@platinum> In-Reply-To: References: <1486387756-16865-1-git-send-email-olivier.matz@6wind.com> <20170206134903.GA256940@bricha3-MOBL3.ger.corp.intel.com> <20170206151034.1ae936bc@platinum> <20170206162749.46bdcf22@platinum> X-Mailer: Claws Mail 3.14.1 (GTK+ 2.24.31; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [dpdk-dev] [RFC 0/8] eal: dynamic logs X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 06 Feb 2017 16:18:34 -0000 On Mon, 6 Feb 2017 15:55:20 +0000, "Wiles, Keith" wrote: > > On Feb 6, 2017, at 9:27 AM, Olivier Matz > > wrote: > >=20 > > On Mon, 6 Feb 2017 15:01:30 +0000, "Wiles, Keith" > > wrote: =20 > >>> On Feb 6, 2017, at 8:10 AM, Olivier Matz > >>> wrote: > >>>=20 > >>> Hi Bruce, > >>>=20 > >>> On Mon, 6 Feb 2017 13:49:03 +0000, Bruce Richardson > >>> wrote: =20 > >>>> On Mon, Feb 06, 2017 at 02:29:08PM +0100, Olivier Matz wrote: =20 > >>>>> The objective of this RFC patchset is to introduce a framework > >>>>> to support dynamic log types in EAL. It also provides one > >>>>> example of use (in i40e). > >>>>>=20 > >>>>> Features: > >>>>> - log types are identified by a string > >>>>> - at registration, a uniq identifier is associated to a log type > >>>>> - each log type can have its level changed dynamically > >>>>> - extend command line parameters to set the log level of a > >>>>> specific type, or logs matching a regular expression > >>>>> - keep compat with other legacy types (eal, malloc, ring, user*, > >>>>> etc... keep their hardcoded log type value) > >>>>>=20 > >>>>> At the end, when, we can expect that all non-dataplane logs are > >>>>> moved to be dynamic, so we can enable/disable them at runtime, > >>>>> without recompiling. Many debug options can probably be removed > >>>>> from configuration: > >>>>> $ git grep DEBUG config/common_base | wc -l > >>>>> 89 > >>>>>=20 > >>>>> Comments are welcome! > >>>>> =20 > >>>> Initial scan through the patches this looks pretty good. However, > >>>> rather than continuing with our own logging system, have you > >>>> investigated what other tracing and logging systems might be out > >>>> there that we could possibly re-use? Could LTTng be a good fit > >>>> for DPDK, perhaps? =20 > >>>=20 > >>> I did not investigate much about existing logging system. I agree > >>> that could be a good alternative too. > >>>=20 > >>> On the other hand, I'm not really confident in adding a dependency > >>> to an external lib for a core component like eal. What if we > >>> deicide tomorrow to port dpdk to windows or any baremetal env? > >>>=20 > >>> Also, as far as I remember, the components that depend on external > >>> libraries are disabled today because we don't know how to properly > >>> manage the dependency (ex: pmd-pcap, vhost-numa, pmd-mlx, =E2=80=A6).= =20 > >>=20 > >> In a previous project I worked in we did not use log levels per say > >> for debugging code. We did have a couple general logging for misc > >> type logging. > >>=20 > >> When debugging you normally only need a couple files or functions > >> that need to produce logging output. In that case we defined > >> logging using the file and function as the key to determine if the > >> dynamic log messages are output. We use a string in the format of > >> "filename:function=E2=80=9D we allowed for a wildcard to enable all > >> functions in a file or you can select a single function. > >>=20 > >> Using this type of logging for debugging a system is the most > >> useful if you just want general logging then we define something > >> similar to what we have today. =20 > >=20 > > I think the "filename:function" is not adequate if you are not the > > developer of that code. Example: you have a problem with a PMD, you > > just enable all logs for this PMD (or even all PMDs), you can check > > it and if you don't find the problem, you can send them on the ML > > for help. I think you don't care where the code is located. =20 >=20 > I do not understand your concern the design allows you to enable a > single file, which means all functions within a file =E2=80=9Cfilename:*". I think the user doesn't want to care about the file name of the C source. > In > the case of the all PMDs it not the best way to debug as you get a > lot of output that may not be even related to the problem you are > trying to solve. The design does allow you to enable one or more PMDs > if say you are debugging say two PMDs. The output would be more > readable and less cluttered with output that is not germane to the > problem. >=20 > If I was debugging the TAP driver I would like to just enable > =E2=80=9Crte_eth_tap_pmd.c:*=E2=80=9D or maybe we can define a something = registered > other then file name e.g. rte_log_register(=E2=80=9Ctap_pmd=E2=80=9D); = =E2=80=9Ctap_pmd:*=E2=80=9D or > =E2=80=9Ctap_pmd:pmd_rx_burst=E2=80=9D or =E2=80=9Ctap_pmd:rte_tap_pmd_pr= obe=E2=80=9D. We could for > the PMDs just use the PMD name we define at registration. That's very similar to what is proposed in the patch... See: + i40e_logtype_init =3D rte_log_register("pmd.i40e.init"); + i40e_logtype_driver =3D rte_log_register("pmd.i40e.driver"); >=20 > Maybe the register option brings us closer to the same goal, but add > the function or selecting a specific set functions. The design does > require a more active lookup at run time for dynamic debugging and we > would have to make sure if enabled it does not effect performance. We > used a hash table to locate the enabled debug log output. Again, I don't really see the added value to be able enable logs on a per function basis. For users, it brings more complexity. =20 > The design allowed us to use the command line or CLI to > enable/disable logging output. Same with this proposal.