Enumerate policies on start #2
Conversation
Some NM requests return additional information along with the non-success Completion Code. E.g. 0xC2 (get NM policy) returns the next valid policy ID for faster enumeration. Signed-off-by: Paul Fertser <fercerpav@gmail.com>
To ensure consistency between the set of policies used by NM and the set presented via D-Bus fetch all the existing policies on daemon startup and create the corresponding objects on the bus. Signed-off-by: Paul Fertser <fercerpav@gmail.com>
|
Hello @wdembinski , |
|
Hey Paul |
|
Hi Wojciech, |
|
Hi @paulfertser |
|
Hi Joanna,
On Mon, Dec 20, 2021 at 02:18:27AM -0800, Joanna Jatkiewicz wrote:
I prepared a patch on downstream, but I changed a bit implementation. I removed
class storedPolicies and I have moved this functionality to the new method in
the Domian class. Instead of iterator I used a simple loop to find an existing
policies. Is that OK with you?
I obviously can't review or test your new code without seeing it. That
said, I wonder what flaws have you found in my code, does it have any
layering violations or is your implementation ended up being
considerably shorter and concise? I thought providing iterators for
anything "enumerable" is the most common idiomatic C++ (and that's
what OpenBMC community aims for), so would be really interesting to
learn why you considered it unsuitable in this particular case. I also
thought this class I created might come useful later for some other
related purpose but I'm not familiar enough with NodeManager to judge.
Thank you for working on this!
…--
Be free, use free (http://www.gnu.org/philosophy/free-sw.html) software!
***@***.***
|
|
@paulfertser Your code is correct :) I think that my solution is more concise. I agree with your suggest about iterator, but in this case I think that using iterator is more difficult to understand code. Our aims is check policy id from 1 to 255.
|
|
@paulfertser Your code lists node-manager policies only on startup. If anything changes in node-manager (policy removed, policy updated), DBus information will be outdated. We see 2 options here: i) implement periodical refresh, ii) implement on-demand refresh. What do you think would be better? Have you considered such scenario? Do you need a list of those policies only during startup? |
|
Hello Wojciech,
On Tue, Dec 21, 2021 at 03:06:29AM -0800, Wojciech Dembinski wrote:
Your code lists node-manager policies only on startup. If anything
changes in node-manager (policy removed, policy updated),
When working on this I assumed that only node-manager-proxy can be
changing the policies, as I am not aware of any other mechanisms to do
that.
DBus information will be outdated. We see 2 options here: i)
implement periodical refresh, ii) implement on-demand refresh. What
do you think would be better? Have you considered such scenario?
i) looks like a no-go to me as it's providing non-deterministic
results; ii) is also questionable as it's unclear what and when should
be requesting the refresh, and it's still racy.
Do you need a list of those policies only during startup?
Initially I thought that bringing them in sync once on startup is
sufficient. But since you're asking I guess you have some other
scenario in mind, what is it?
BTW, I still have no idea what that OEM policy is doing there, why
it's present in Lewisburg PCH, whether it should be exposed anyhow or
not, and how it's supposed to work on other platforms.
To get it fully clear, we just needed a way for BMC to reliably limit
power consumption of a server (and provide an API to control the
value), and asking NM to do that looks like a solution. I do not have
any real insight or experience with it, so the questions you raised
need to be answered by someone more familiar with the topic.
|
|
Node Manager on PCH can be also accessed via IPMI. Refer to https://www.intel.com/content/dam/www/public/us/en/documents/technical-specifications/intel-power-node-manager-v3-spec.pdf. This is version 3.0 of Node Manager, you need to check what version you are using on your platform. Maybe that approach would be better for you? You can also access IPMI from BMC. |
|
On Tue, Dec 21, 2021 at 04:19:06AM -0800, Wojciech Dembinski wrote:
Node Manager on PCH can be also accessed via IPMI
...
You can also access IPMI from BMC.
Now I am confused. Isn't that /exactly/ what node-manager-proxy (the
software in this repository) is doing? Sending IPMI commands over IPMB
channel from BMC to NM running on ME inside PCH?
|
|
That's right but we only provide a limited set of functionality required for other components within OpenBMC. To access all features of Node Manager in your case, you need to use IPMI. "When working on this I assumed that only node-manager-proxy can be |
|
On Tue, Dec 21, 2021 at 06:01:20AM -0800, Wojciech Dembinski wrote:
That's a wrong assumption. It can be also accessed with IPMI interface - so, if
you are sure that nothing on your platform accesses it that way, that's ok.
Otherwise, once data is collected during startup, it can be outdated later -
because of any application with access to IPMI which communicates with Node
Manager.
I thought any sane usecase not covered by node-manager-proxy means
only one thing: node-manager-proxy needs to be extended to cover
it. Sending commands via IPMI directly behind its back sounds like a
horrible idea, why would anyone do that and why would anyone care
about such a usecase; are you aware of anybody doing that?
|
|
Hi @paulfertser, |
|
Hello Joanna,
On Thu, Feb 03, 2022 at 05:58:16AM -0800, Joanna Jatkiewicz wrote:
To answer your question, yes, in some cases node-manager-proxy isn't
used - direct communicate with ME is made via IPMI.
Is this a hypothetical situation or are you aware of some real life
limitations which call for using IPMI directly rather than via
node-manager-proxy? What are they? Are they related to the policy
reading code I wrote?
We discussed about your pull request, but we don't have enough resources to
fully extended node-manager-proxy now.
What do you mean by fully? Is my "github pull request" missing
something important? Please tell me what you have found, I do not want
buggy code to be running on our servers.
|
|
It's been more than a month since your last reply, did this fall through the cracks? |
Continuation of the accidentally-closed github pull request #1.