From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-f66.google.com (mail-wr1-f66.google.com [209.85.221.66]) by dpdk.org (Postfix) with ESMTP id B20282B92 for ; Wed, 29 Aug 2018 10:23:28 +0200 (CEST) Received: by mail-wr1-f66.google.com with SMTP id a108-v6so3901480wrc.13 for ; Wed, 29 Aug 2018 01:23:28 -0700 (PDT) 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:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=1FijhbMW1G1bPkbLkCSQsjF1Di2LqYx7w/TKZw9Ps/I=; b=L+KScV0QdyPvpsyrgC07zQt1M5LcHSVOSyhvne709osIIszBtCOgPo080cHTlbc27R 79eluP6PvAWBA/C/4GYen+kjnxlGNtct2SKHFTYqbS+0g/aT8OtMAv5vuvbRUuL9M9IA C9VlufbGdOP2lhjEydfH9I8sT94cwOfdtUp4ie1tXl3DUMTTcKVYm6QiQXJRAWVpbGkD QYctcYt95c/6IqKKvl50uPB3q0h+FSvf9OFr4jJuPMGvDffG5Y6gLbMSbWYoB/b0M1bj PDptoZEmAVUYamAdtaQRM1nqmM7JSmSejIkWgSJtaQy25AyEHAWWwJQI6PXhEc5/x055 SRzA== 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:content-transfer-encoding :in-reply-to:user-agent; bh=1FijhbMW1G1bPkbLkCSQsjF1Di2LqYx7w/TKZw9Ps/I=; b=I7ob93Fy8/WbrvpEiK6vsKUMyZQewsPr7UrlwOMB401Rb2b3/NrBPqSLk3yDgEauNg DadMeO8OiG7zTiOJoxGvscY1Sp5OTgjPiYrdzWNlyzmeQ/l8oyPA5FTDfNE2bF3lg6Wu bhdCEZRYIr2mBGL+h3UCRPGfnAvjYtH0hS39Si+1oEWk5Il0r3fQtt09jXfJbaTED9N8 jXn4G5j7NE3BYs8J4Eyu38NTl4uoW+YRkD4+VsXa7pM1lzLSfpLxUrZzx90d32ZzbShn 1LE11l6tBi77I9Mdrg0JM2afb9BoTBpRcZu+WCWczXWsq1ND89iygwTXIKkxOsBRW4kZ qfpA== X-Gm-Message-State: APzg51AjkOg40o9Ff0PG5iXMX47gzMiNrCwpCaxHIEI/EuVjIzODrJsR /dUISTjE32+zuVNyfgIL4+HFKA== X-Google-Smtp-Source: ANB0VdZKfNDDuy/z1zRJywlDSVtKErkYS7BkfU7WDa3fZaUy//Se8mpAHetd2IutZb3/yZ9RDvGwhg== X-Received: by 2002:adf:ce90:: with SMTP id r16-v6mr3451464wrn.112.1535531008321; Wed, 29 Aug 2018 01:23:28 -0700 (PDT) Received: from bidouze.vm.6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id 124-v6sm6850438wmk.20.2018.08.29.01.23.27 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 29 Aug 2018 01:23:27 -0700 (PDT) Date: Wed, 29 Aug 2018 10:23:11 +0200 From: =?iso-8859-1?Q?Ga=EBtan?= Rivet To: "Van Haaren, Harry" Cc: "Power, Ciara" , "Archbold, Brian" , "Kenny, Emma" , "dev@dpdk.org" Message-ID: <20180829082311.6cknwcvua2gsazb3@bidouze.vm.6wind.com> References: <1535026093-101872-1-git-send-email-ciara.power@intel.com> <1535026093-101872-2-git-send-email-ciara.power@intel.com> <20180828114633.vky27x53tu7eyag2@bidouze.vm.6wind.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [dpdk-dev] [PATCH 01/11] telemetry: initial telemetry infrastructure 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: Wed, 29 Aug 2018 08:23:28 -0000 On Tue, Aug 28, 2018 at 04:54:33PM +0000, Van Haaren, Harry wrote: > Hi Gaetan, > > > -----Original Message----- > > From: Gaëtan Rivet [mailto:gaetan.rivet@6wind.com] > > Sent: Tuesday, August 28, 2018 12:47 PM > > To: Power, Ciara > > Cc: Van Haaren, Harry ; Archbold, Brian > > ; Kenny, Emma ; dev@dpdk.org > > Subject: Re: [dpdk-dev] [PATCH 01/11] telemetry: initial telemetry > > infrastructure > > > > Hi Ciara, > > > > On Thu, Aug 23, 2018 at 01:08:03PM +0100, Ciara Power wrote: > > > This patch adds the infrastructure and initial code for the > > > telemetry library. > > > > > > A virtual device is used for telemetry, which is included in > > > this patch. To use telemetry, a command-line argument must be > > > used - "--vdev=telemetry". > > > > > > > Why use a virtual device? > > > > It seems that you are only using the device semantic as a way to carry > > around a tag telling the DPDK framework to init your library once it has > > finished its initialization. > > > > I guess you wanted to avoid having to add the call to rte_telemetry_init > > to all applications. In the absence of a proper EAL option framework, > > you can workaround by adding a --telemetry EAL parameter, setting a flag > > on, and checking this flag from librte_telemetry, within a routine > > declared with RTE_INIT_PRIO. > > I suppose that an EAL --flag could work too, it would mean that EAL would > depend on this library. The --vdev trick keeps the library standalone. > > I don't have a strong opinion either way. :) > This was done already for specific EAL configuration items such as vfio intr_mode or PCI uio configuration. Of course this is ugly, but the --telemetry parameter can exist without compiling the lib. You can add a warning if the TELEMETRY Mconfig item is not set to mitigate. The main issue is that you need to add getters because you cannot declare an external *struct internal_config* reference. I agree this is awkward, and this is exactly the reason we need a way for libraries to register options in the EAL, but this is not yet done. The virtual device solution however is a crutch used to emulate this absent framework. This will complicate developping the proper solution and its adoption once done. I would not be clear then to the dev that they can translate the telemetry shim parameter to the new framework, without having to rework the whole infrastructure of the lib (and this is without talking about reworking the build system to remove the telemetry driver). Even having to add a new driver subsection only for telemetry is awkward. So we might certainly wait for second or third opinions, but I am firmly convinced it would be easier in order to maintain the project (both from EAL and systems standpoint and library standpoint) without the vdev trick. > > > I only see afterward the selftest being triggered via kvargs. I haven't > > yet looked at the testing done, but if it is only unit test, the "test" > > app would be better suited. If it is integration testing to verify the > > behavior of the library with other PMDs, you probably need specific > > context, thus selftest being insufficient on its own and useless for > > other users. > > Correct, self tests are triggered by kvargs. This same model is used > in eg: eventdev PMDs to run selftests, where the tests are pretty complex > and specific to the device under test. > > Again, I don't have a strong opinion but I don't see any issue with it > being included in the vdev / telemetry library. We could write a shim > test that the "official" test binary runs the telemetry tests if that is > your concern? > > Okay, I have no strong opinion about this (actually I prefer having the test code close to the code-under-test), but eventdev can spawn device objects to drive the test and provide configuration. It would be more complicated using the same logic with a pure library, without the vdev. > > > Control threads are used to get CPU cycles for telemetry, which > > > are configured in this patch also. > > > > > > Signed-off-by: Ciara Power > > > Signed-off-by: Brian Archbold > > > > Regards, > > -- > > Gaëtan Rivet > > 6WIND > > Thanks for review, and there's a lightning talk at Userspace so please > do provide input there too :) -Harry -- Gaëtan Rivet 6WIND