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=-5.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 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 06B17C433B4 for ; Mon, 17 May 2021 07:40:09 +0000 (UTC) Received: from lists.ozlabs.org (lists.ozlabs.org [112.213.38.117]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 55ED9611C2 for ; Mon, 17 May 2021 07:40:08 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 55ED9611C2 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.intel.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=openbmc-bounces+openbmc=archiver.kernel.org@lists.ozlabs.org Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4Fk9yZ682Lz303x for ; Mon, 17 May 2021 17:40:06 +1000 (AEST) Authentication-Results: lists.ozlabs.org; spf=none (no SPF record) smtp.mailfrom=linux.intel.com (client-ip=134.134.136.126; helo=mga18.intel.com; envelope-from=adrian.ambrozewicz@linux.intel.com; receiver=) X-Greylist: delayed 64 seconds by postgrey-1.36 at boromir; Mon, 17 May 2021 17:39:40 AEST Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4Fk9y41ZYtz2yYK for ; Mon, 17 May 2021 17:39:40 +1000 (AEST) IronPort-SDR: qBJkXGS53nfBAHPmFmJ2IEo9qAVwdgn9cjMdim5T/u4Bl4ipsoxeNCtZGRAEvIF/Hw2Xqeigkl 7YSJbHKtDb2A== X-IronPort-AV: E=McAfee;i="6200,9189,9986"; a="187811058" X-IronPort-AV: E=Sophos;i="5.82,306,1613462400"; d="scan'208";a="187811058" Received: from orsmga008.jf.intel.com ([10.7.209.65]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 17 May 2021 00:38:31 -0700 IronPort-SDR: nGok/5LND7jOXUB8B/EMGW25ocyOImjPPGCtpLPip6VAW6EGzq5wi+cntyBqPfPyP7zn664Tts a2Nvvx8ZRrhw== X-IronPort-AV: E=Sophos;i="5.82,306,1613462400"; d="scan'208";a="438788019" Received: from aambroze-mobl.ger.corp.intel.com (HELO [10.213.1.152]) ([10.213.1.152]) by orsmga008-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 17 May 2021 00:38:29 -0700 Subject: Re: Mapping standard D-Bus sensors to ProcessorMetrics (and other specific schemas) From: "Ambrozewicz, Adrian" To: Ed Tanous , Patrick Williams References: <3d5f8ede-3506-afac-d5bd-4bc7f3331cbc@linux.intel.com>

Message-ID: <5252d5e3-02b8-1dba-5ca6-c703e2e9c87c@linux.intel.com> Date: Mon, 17 May 2021 09:38:27 +0200 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.10.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-BeenThere: openbmc@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Development list for OpenBMC List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "openbmc@lists.ozlabs.org" Errors-To: openbmc-bounces+openbmc=archiver.kernel.org@lists.ozlabs.org Sender: "openbmc" Guys, Part of the problem can be solved with existing solutions. At least If I understood you correctly. There are still some questions or opens to cover. I would appreciate if you found time to respond. Regards, Adrian W dniu 5/12/2021 o 13:17, Ambrozewicz, Adrian pisze: > W dniu 5/11/2021 o 18:26, Ed Tanous pisze: >> On Tue, May 11, 2021 at 7:53 AM Patrick Williams >> wrote: >>> >>> On Tue, Apr 27, 2021 at 01:52:51PM +0200, Ambrozewicz, Adrian wrote: >>>> W dniu 4/9/2021 o 14:05, Patrick Williams pisze: >>>>> On Wed, Apr 07, 2021 at 02:24:55PM +0200, Ambrozewicz, Adrian wrote: >>>>> >>>>> I suspect this would be the more difficult direction to go down. >>>>> There >>>>> is already enough code that looks for sensors at specific paths that >>>>> you'd have to track down and fix up. Also, there has been some >>>>> concern >>>>> by some maintainers in other cases about having information in the >>>>> paths >>>>> have meaning and prefering to reduce the reliance on that. >>>>> >>>> >>>> Please see message from Ed, as he's supposedly proposing to follow that >>>> path. I don't have strong opinions on one or the other approach. >>> >>> I suspect you are not signing up to change all the existing code. I'll >>> look at Ed's reply though. >>> > > Crisis averted - paths and names dropped out from scope :) > >>>> I've read the design, however one thing is not clear for me. My current >>>> understanding was that for each association there would need to exist >>>> some D-bus object at given path somewhere. Would i need my CPU >>>> inventory >>>> service to also expose separate objects for each core for my >>>> association >>>> to be 'legal', or could we represent some virtual hierarchy with no >>>> actual D-Bus object in the system? >>> >>> Yes. You would need an inventory object for each entity you want to >>> attach sensors or metrics to. This doesn't seem like it should really >>> be an issue. Other people have been working on adding CPU Cores already >>> and there is the xyz.openbmc_project.Inventory.Item.CpuCore defined. >>> > > Thanks for pointing that out. It seems like logical path to follow. Do > you have some pointers to some reviews or discussion? CpuCore as of now > is empty. > >>>>>> I've done some PoC implementation of ProcessorMetrics, which >>>>>> relies on >>>>>> new D-Bus interface with 'Mapping' property (eg. >>>>>> 'TemperatureCelsius' or >>>>>> 'CoreMetrics/12/UnhaltedCycles'). ProcessorMetrics node >>>>>> implementation >>>>>> queries D-Bus for all sensors associated with given CPU and populates >>>>>> properties if proper mapping was found. >>>>> >>>>> I'm not really grasping what the contents of this mapping property >>>>> are. >>>>> Generally we want to stay away from free-form strings having >>>>> programatic >>>>> uses. Maybe if you can define these mappings as enumerations? >>>>> >>>>> What is the additional information you need besides the assocation >>>>> from >>>>> a sensor to its inventory item? >>>> >>>> In given example I would like my sensor to be source of information for >>>> property defined by ProcessorMetrics schema. In the example I've used >>>> property associated with given Core, thus CoreMetrics/12/UnhaltedCycles >>>> maps directly to ProcessorMetrics sub-property. Enumerations could be >>>> not enough as we have multiple informations to represent: >>>> - association with given processor (done by ProcessorMetrics) >>>> - association with given core (could that be handled by your proposed >>>> design?) >>>> - linking to given property >>>> >>>> Would the enumeration be used for the last element, while leaving >>>> hierarchy problem to Associations? >>> >>> "UnhaltedCycles" is not a sensor, just to be clear. IPMI might have >>> called these kinds of things sensors but we do not. Sensors for us >>> measure physical properties. This is just a property (or maybe a >>> "metric") but it doesn't belong in the sensors namespace or modeled with >>> a Sensor.Value. > > Up to this point we've established, that sensors/metrics would be linked > to given item by association. That leaves figuring out how to 'glue' > together Redfish property with given D-Bus entity. > >> >> This somewhat brings up a good point, what is a "sensor" on dbus? I >> would've assumed that these would be well represented as sensors, as >> they do measure physical properties. I hadn't assumed that they had >> this limitation because we do have the >> /xyz/openbmc_project/utilization namespace defined already. If we're >> going down the path of "must be physical" it would seem like >> utilization should be moved out of the sensors interface? Or am I >> taking your statement too literally? >> > > Agreed. I suppose we should forget about 'old ways' and previous meaning > of IPMI sensor. BTW Redfish specifies such properties as 'metrics'. > > What are the limitations of Sensor.Value interface when it comes to > representing values in ProcessorMetrics and similar schemas? > > ProcessorMetrics uses such units for its metrics: > - bytes > - % (already available as 'utilization') > - MHz > - Cel (altready available as 'temperature') > - count (number of events/occurrences) > > I suppose that we could just extend namespaces and units of Sensor.Value > to cover them and call it a day. We would retain compatibility with > existing sensors code, TelemetryService etc. (I agree with below comment > by Ed). I am aware, that when more schemas and metric types comes this > list will grow, but do we have other alternative? > > What are your thoughts? If Sensor.Value is not the way, then how would > we define the next interface? > >> Not reusing sensor seems like it would lead to a lot of code >> duplication, as every API would now need to understand everything >> about every "publishes real time telemetry" type interface, and every >> time we add a new one, we'd (probably) have to update the code to add >> the new type. That doesn't really seem maintainable to me for every >> type of telemetry we might want; If a sensor isn't the right place to >> put it, how would we solve the "I want to publish all telemetry >> values" type use cases? Maybe namespace the interface itself, so we >> can use the arg0namespace feature in match expressions? I'm thinking >> out loud at this point... >> >>> I don't know why the "linking to a given property" would be a dbus >>> representation. Metric service should know which properties from dbus >>> map to some metric entity, right? For a one-user piece of information, >>> I don't see a good reason to put this on dbus. >> >> I think the issue here is how do you know that a specific value >> relates to say, the processor utilization, or the ram utilization, or >> the smart statistics? > > Yes, this is the gap we still need to address. Perhaps idea with with an > well defined enum is not a bad one? > > Taking an example of ProcessorMetrics\CoreMetrics[]\UnhaltedCycles > We would have an D-Bus sensor with given interfaces: > > xyz.openbmc_project.Association.Definitions > .Associations {"cpucore", "all_sensors", "/xyz.../core/5"} > > xyz.openbmc_project.Sensor.Value > .Unit "Count" # new unit > .Value 123123 > > xyz.openbmc_project.CpuCore.Metrics # New imagined interface with enum > .Type "UnhaltedCycles" > >> >>> >>> -- >>> Patrick Williams > > Thanks a lot for your input, it seems like we're going in good direction. >