From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
by inbox.dpdk.org (Postfix) with ESMTP id B780EA0561;
Tue, 21 Apr 2020 02:32:55 +0200 (CEST)
Received: from [92.243.14.124] (localhost [127.0.0.1])
by dpdk.org (Postfix) with ESMTP id 934331C299;
Tue, 21 Apr 2020 02:32:55 +0200 (CEST)
Received: from mga02.intel.com (mga02.intel.com [134.134.136.20])
by dpdk.org (Postfix) with ESMTP id 8FB9A1C243
for <dev@dpdk.org>; Tue, 21 Apr 2020 02:32:53 +0200 (CEST)
IronPort-SDR: uJm9kwLIZAZBGFY4MMtATo28MEdiTKAXLVoox8TBa6AlW01SLdfsHj1jwUH0TqejeuWOX9DWbI
bXhlUp1iyYWA==
X-Amp-Result: SKIPPED(no attachment in message)
X-Amp-File-Uploaded: False
Received: from fmsmga006.fm.intel.com ([10.253.24.20])
by orsmga101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;
20 Apr 2020 17:32:52 -0700
IronPort-SDR: e2qjhSjNIzZj9P+hwKQdrogDJVcujWUw7vxmTsV4+m8QosVJtQx5PvDbqZxeJODw1MFzO+m73/
ELWYTKkp01TQ==
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.72,408,1580803200"; d="scan'208";a="456570684"
Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203])
by fmsmga006.fm.intel.com with ESMTP; 20 Apr 2020 17:32:52 -0700
Received: from FMSEDG001.ED.cps.intel.com (10.1.192.133) by
FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS)
id 14.3.439.0; Mon, 20 Apr 2020 17:32:52 -0700
Received: from NAM10-BN7-obe.outbound.protection.outlook.com (104.47.70.104)
by edgegateway.intel.com (192.55.55.68) with Microsoft SMTP Server (TLS) id
14.3.439.0; Mon, 20 Apr 2020 17:32:50 -0700
ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none;
b=XanZ5Hi8Uu7k8qwD63ii2WP2him4Np7isNknHcpZW3+VeRI/kPxcBUXFmMkg3358yrGrt42oY2OE5R7ePDtpwUQtCo7J8g5FY9hE22BvOaVkRQ1BMsS0J2Yz+P6ShpPuBAePDd/99/w210flYd1It7Ei6pvBNip9e2RV2+uuw+qDvQL9Z48lZmuHUDamPznjaiIAnqNLpK6k3v6VqPHUM9nX6Wh8QURZ8ylTcVQsykpot2/StDyVs1zeCPP5B0e7uVh2jMK+KXhEsv1R7nV64zD0IhjrvKRaiDRq7oivC9z1gJcKLXhFW8J4lh1DPot6t7W5gvo8bPMJqyLoyEExYw==
ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com;
s=arcselector9901;
h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck;
bh=DE2gnktHA9+xq4IY7GxVQPmdDatAkvH0Shyps0+X5Qc=;
b=OAyzEaOtgYOaTE0s0VjowQibnm6QX79CrbPeXNe8m8nfhV8Zqh6ycXBKEFCKp8ns6l5yJbzrajaUUYiLbBoKvYiMy9Hj33wySYK6Zp8sx3KmU3gBZnHb5qWa4CjXg/M+oTQwjlPTTKbg0OR8cfvPHCw5OT4+7VIu4QzaUWwypvj5RwDx3GQ2LZ3OTlOkBEb7hxd9QkWEzO5bf7fT1fgdXE/KT8G3VgRBC1PK3+3GIfINXpPfJgLsSUXZ7oW+G7Srh+rYmLSGqsQNWqfpU+ufb2eeBzX07u4zRI4ethmgMfarLyeUsMxtpRxOZ5F9HVRFbOlhDmvOmr+13vyL+M3azQ==
ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass
smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com;
dkim=pass header.d=intel.com; arc=none
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel.onmicrosoft.com;
s=selector2-intel-onmicrosoft-com;
h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck;
bh=DE2gnktHA9+xq4IY7GxVQPmdDatAkvH0Shyps0+X5Qc=;
b=MXZgfp8faHGOTF+SG0h5SOYVrKSNo66sDFzO0CkQMJe6KzMt7oWq2krOxtNVjDgjktg/XF6qAUdXe+1xAgc3RuXxSN+/f5qaLVfKveY+ZKNdn7qvMLN0x2NP9+aK6a/YcDvm2LbIlZ5qMwQhH34sNUcA4eDjdIxl0A/KO4GCUp4=
Received: from BYAPR11MB3301.namprd11.prod.outlook.com (2603:10b6:a03:7f::26)
by BYAPR11MB3352.namprd11.prod.outlook.com (2603:10b6:a03:1d::26)
with Microsoft SMTP Server (version=TLS1_2,
cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2921.25; Tue, 21 Apr
2020 00:32:30 +0000
Received: from BYAPR11MB3301.namprd11.prod.outlook.com
([fe80::f8cb:58cd:e958:fff4]) by BYAPR11MB3301.namprd11.prod.outlook.com
([fe80::f8cb:58cd:e958:fff4%6]) with mapi id 15.20.2921.027; Tue, 21 Apr 2020
00:32:30 +0000
From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
To: "Richardson, Bruce" <bruce.richardson@intel.com>, Thomas Monjalon
<thomas@monjalon.net>
CC: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com>, "Dumitrescu,
Cristian" <cristian.dumitrescu@intel.com>, Igor Russkikh
<igor.russkikh@aquantia.com>, Pavel Belous <pavel.belous@aquantia.com>, "Lu,
Wenzhuo" <wenzhuo.lu@intel.com>, Marcin Wojtas <mw@semihalf.com>, "Michal
Krawczyk" <mk@semihalf.com>, Guy Tzalik <gtzalik@amazon.com>, "Evgeny
Schemeilin" <evgenys@amazon.com>, Igor Chauskin <igorch@amazon.com>, "John
Daley" <johndale@cisco.com>, Hyong Youb Kim <hyonkim@cisco.com>, "Zhang, Qi
Z" <qi.z.zhang@intel.com>, "Wang, Xiao W" <xiao.w.wang@intel.com>, "Ziyang
Xuan" <xuanziyang2@huawei.com>, Xiaoyun Wang <cloud.wangxiaoyun@huawei.com>,
Guoyang Zhou <zhouguoyang@huawei.com>, "Wei Hu (Xavier)"
<xavier.huwei@huawei.com>, "Min Hu (Connor)" <humin29@huawei.com>, "Yisen
Zhuang" <yisen.zhuang@huawei.com>, "Xing, Beilei" <beilei.xing@intel.com>,
"Wu, Jingjing" <jingjing.wu@intel.com>, "Yang, Qiming"
<qiming.yang@intel.com>, Rasesh Mody <rmody@marvell.com>, Shahed Shaikh
<shshaikh@marvell.com>, "Singh, Jasvinder" <jasvinder.singh@intel.com>,
Maxime Coquelin <maxime.coquelin@redhat.com>, "Wang, Zhihong"
<zhihong.wang@intel.com>, "Ye, Xiaolong" <xiaolong.ye@intel.com>, Yong Wang
<yongwang@vmware.com>, "Yigit, Ferruh" <ferruh.yigit@intel.com>, "Andrew
Rybchenko" <arybchenko@solarflare.com>, Olivier Matz
<olivier.matz@6wind.com>, "dev@dpdk.org" <dev@dpdk.org>
Thread-Topic: [dpdk-dev] [PATCH v1 03/17] ethdev: replace library debug flag
with global one
Thread-Index: AQHWFQNglnRcCbrjHUKnbi8uf4bdj6iBu2+AgABDdSCAABUSAIAABhaAgAApZACAAALOAIAAAm+AgAABfwCAABcBAIAATYdg
Date: Tue, 21 Apr 2020 00:32:29 +0000
Message-ID: <BYAPR11MB3301B3A7C00E95C8287CB4479AD50@BYAPR11MB3301.namprd11.prod.outlook.com>
References: <20200417215739.23180-1-l.wojciechow@partner.samsung.com>
<7682483.vcMjziH8VY@thomas>
<20200420173015.GC1719@bricha3-MOBL.ger.corp.intel.com>
<3171532.h4nFI6E9mP@thomas>
<20200420185756.GA1725@bricha3-MOBL.ger.corp.intel.com>
In-Reply-To: <20200420185756.GA1725@bricha3-MOBL.ger.corp.intel.com>
Accept-Language: en-GB, en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
dlp-product: dlpe-windows
dlp-reaction: no-action
dlp-version: 11.2.0.6
authentication-results: spf=none (sender IP is )
smtp.mailfrom=konstantin.ananyev@intel.com;
x-originating-ip: [192.198.151.166]
x-ms-publictraffictype: Email
x-ms-office365-filtering-correlation-id: 3e158208-fdcc-4199-caa7-08d7e58b79d9
x-ms-traffictypediagnostic: BYAPR11MB3352:
x-ld-processed: 46c98d88-e344-4ed4-8496-4ed7712e255d,ExtAddr
x-ms-exchange-transport-forked: True
x-microsoft-antispam-prvs: <BYAPR11MB3352F6122411073A0651CE5E9AD50@BYAPR11MB3352.namprd11.prod.outlook.com>
x-ms-oob-tlc-oobclassifiers: OLM:10000;
x-forefront-prvs: 038002787A
x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:;
IPV:NLI; SFV:NSPM; H:BYAPR11MB3301.namprd11.prod.outlook.com; PTR:; CAT:NONE;
SFTY:;
SFS:(10019020)(396003)(366004)(136003)(346002)(376002)(39860400002)(8936002)(4326008)(6506007)(26005)(110136005)(7416002)(316002)(54906003)(81156014)(66476007)(8676002)(66946007)(64756008)(66556008)(66446008)(71200400001)(2906002)(186003)(7696005)(5660300002)(55016002)(9686003)(76116006)(478600001)(86362001)(33656002)(52536014)(21314003);
DIR:OUT; SFP:1102;
x-ms-exchange-senderadcheck: 1
x-microsoft-antispam: BCL:0;
x-microsoft-antispam-message-info: LA7grxgBNoiXJHa8xu1OmjX4Q5DNpA6MCD2v/DCM6YCkTD2zqDAe9CJoxzTjk6sxWoO0NWu1xPzv1AuLHnjop4IbpEfP0BaM6r81QoS/SO0EDAQtqYah7fOloTJvUGiZbI1brzFzw/X8FnWgzTErZxjnMze/owRcPozHZhdXEel1uZm8eg2cdYX7uhSxyo0f2MDlH0Z4DS57QnerWk9VR9xDGfDEHdMNn+Oyf0KMdBjxaOyXI8QcoomDRiGPgwwwzKUowZDY8VfZDYP+jOJQ1702p7Wwxe8Ol7n1tT9GBW5RUnVYN1kNqyvy77a7WJP2tSOorurKfOth9rFeV7YePc55oKS3Gkpnm7Y9yITpNAVv7OyQDe0/btNQpHefkfa+IB6QSmfKO8ngDdLF4AV6T/+5p+/VYF56tmhYplsQ71yThVEq8eZD78BsW95WIRlgKUK1LpXNnFn/2LPJWoOYKNxkOuG80J5JiNeINbJHiaiG4NVIvb0ydwctCFAKrEEp
x-ms-exchange-antispam-messagedata: w+llFmkZl+3o3c5i43EHoTONtndK7Xv1sBH4ldeeLMu7ae3Uq84SeVi7d+8qU6fpCnEg0g6NYpTKSfOcggWGCToWWaMxw879zfb/F8zD6WBHaFFsmMfsKICKawJLTqIJU98vHxOqcN2r74pdL0xwoA==
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 3e158208-fdcc-4199-caa7-08d7e58b79d9
X-MS-Exchange-CrossTenant-originalarrivaltime: 21 Apr 2020 00:32:29.8928 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 7g20Ak/LB0y+R8CLoQUq9XvRyRJTKmuLC14bwU7vp+bi5plNRnkC5UMkVB+9P13UpxbrjVgZZOJ2Tt3jU82TVeP1COphYolOWFl/T2SZI24=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR11MB3352
X-OriginatorOrg: intel.com
Subject: Re: [dpdk-dev] [PATCH v1 03/17] ethdev: replace library debug flag
with global one
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>
> > > > > > >> I am agree with Cristian concern here: that patch removes ab=
ility to
> > > > > > >> enable/disable debug on particular library/PMD. If the purp=
ose is to
> > > > > > >> minimize number of config compile options, I wonder can't it=
be done
> > > > > > >> in a slightly different way: 1. introduce gloabal RTE_DEBUG =
2. keep
> > > > > > >> actual .[c,h] files intact 3. In actual librte_xxx/meson.bui=
ld file
> > > > > > >> check if RTE_DEBUG is enabled, If yes, then enable particula=
r debug
> > > > > > >> flag for these libs. Something like: If dpdk_conf.get('RTE_=
DEBUG') =3D=3D
> > > > > > >> true dpdk_conf.set('RTE_LIBRTE_XXX_DEBUG ', 1)
> > > > > > >>
> > > > > > >> defines that are used by multiple libs, probably can be set =
in upper
> > > > > > >> layer meson.build.
> > > > > > >>
> > > > > > >> That way will have global 'debug' flag, but users will still=
have an
> > > > > > >> ability to enable/disable debug flags on a per lib basis via
> > > > > > >> CFLAGS=3D"-D..."
> > > > > > >>
> > > > > > >> Konstantin
> > > > > > >>
> > > > > > > That seems a reasonable idea to me.
> > > > > > >
> > > > > > > However, in this case, we don't need the RTE_DEBUG flag at al=
l, we can
> > > > > > > either:
> > > > > > >
> > > > > > > * allow each component meson.build file define its own flags =
after
> > > > > > > checking get_option('debug') * have lib/meson.build and
> > > > > > > drivers/meson.build automatically define a specific define fo=
r each
> > > > > > > library or driver to standardize the naming. [This would sav=
e anyone
> > > > > > > working on it from having to lookup what the define was, sinc=
e it's
> > > > > > > always e.g. RTE_DEBUG_ + library-base-name, e.g. RTE_DEBUG_L=
PM,
> > > > > > > RTE_DEBUG_SCHED etc]
> > > > > > >
> > > > > > > Theoretically we can also do both, have the standard ones def=
ined and
> > > > > > > then allow a component to provide extra flags itself if so de=
sired.
> > > > > > >
> > > > > > > /Bruce
> > > > > > OK, so let's summarize how the patches should be redo: * usage =
of global
> > > > > > "debug" flag for meson build stays * we standardize names of de=
bug flags
> > > > > > in the components to RTE_DEBUG_ + components name * debug flag =
enables al
> > > > > > the RTE_DEBUG_... flags
> > > > > >
> > > > > > This allow to easily use both: * the debug flag - to enable all=
debugs *
> > > > > > or define manually RTE_DEBUG+component name, just for debug fro=
m a single
> > > > > > component
> > > > > >
> > > > > > I like Bruce's idea of adding it to the lib/meson.build and
> > > > > > drivers/meson.build. This way they will be added to dpdk_conf m=
eson
> > > > > > object and written then later to rte_build_config.h before comp=
ilation
> > > > > > stage. All the other modules will be able to use these flags.
> > > > > >
> > > > > Sounds good to me (obviously!), but I'd like other feedback to en=
sure
> > > > > others are ok with this before you spend too much effort implemen=
ting it.
> > > > >
> > > > > For the drivers, the flag probably needs to include the category =
as well as
> > > > > the name, e.g. RTE_DEBUG_NET_IXGBE, RTE_DEBUG_RAW_IOAT, to avoid =
possible
> > > > > name confusion. Those flags can then be checked inside individual
> > > > > meson.build files to enable other debug flags if necessary e.g. i=
n ixgbe,
> > > > > you could theoretically do:
> > > > >
> > > > > if dpdk_conf.has('RTE_DEBUG_NET_IXGBE')
> > > > > cflags +=3D '-DRTE_LIBRTE_IXGBE_DEBUG_RX'
> > > > > cflags +=3D '-DRTE_LIBRTE_IXGBE_DEBUG_TX'
> > > > > ...
> > > > > endif
> > > > >
> > > > > to enable more fine-grained control if so desired, and to maintai=
n
> > > > > compatibility with existing defines, again if so desired.
> > > >
> > > > Nak the nak from Cristian.
> > > >
> > > > We don't need all these flags.
> > > > If the user choose to compile DPDK for debug, every debug facilitie=
s
> > > > should be enabled. Then at runtime it is possible to enable/disable
> > > > the interesting logs.
> > > > If you need to disable something which is not a log,
> > > > you can align with the log level thanks to the function rte_log_can=
_log.
For many libs these flags mean much more than just logging.
Let say RTE_LIBRTE_ETHDEV_DEBUG changes behaviour of tx_prepare() for many
drivers - extra validation performed.
RTE_LIBRTE_MBUF_DEBUG makes __rte_mbuf_sanity_check() a call to real
rte_mbuf_sanity_check() instead of just NOP.
Which means performance would be greatly affected.
RTE_LIBRTE_MEMPOOL_DEBUG changes format of the mempool object header
and enforce extra checking, stats collection.
etc.
Probably that's ok for some cases to enable all that extra validation we ha=
ve at once.
But I suppose in many cases people just interested to enable debug on one
(ok might be two/three) particular libraries, not the whole system.
Right now there is such ability, we are going to remove it without
providing adequate replacement. =20
Approach with rte_log_can_log() seems workable,
but right now these patches don't implement it.=20
Konstantin
> > > >
> > > > Please let's stop complicating things for the contributors and the =
users.
> > > > Note: I am strong on this position.
> > > >
> > > Note, this means that you need to ensure all debug printouts from lib=
s and
> > > drivers are using the RTE_LOG macros so can be runtime controlled. I =
think
> > > that may be some distance from reality right now.
> >
> > Perfect! Let's expose those nasty logs which are not (yet) controllable=
.
> > And next step is to block any patch in those drivers or libs,
> > until it is fixed. Dynamic logging should have been complete for long.
> >
> I can live with that, I suppose. Do we have any idea of the magnitude of
> the work required here?
>=20
> > > Even if we do want all debug enabled from one flag, I'm still not 100=
%
> > > convinced that the existing debug flag is the way to do, since it onl=
y adds
> > > debug info to binary without affecting code generation.
> >
> > OK, we want to keep this flag for gdb symbols only?
> > And add a new flag for debugging facilities which hurt the runtime perf=
ormance?
> >
> I think that would be wise, yes. We can call the option "rte_debug" or
> something instead.
>=20
> /Bruce