Openbmc archive at lore.kernel.org
 help / color / Atom feed
* Using bios-settings-mgr for setting hypervisor network attributes
@ 2020-09-16 14:47 manoj kiran
  2020-09-16 16:26 ` Ed Tanous
  2020-09-16 17:20 ` Patrick Williams
  0 siblings, 2 replies; 22+ messages in thread
From: manoj kiran @ 2020-09-16 14:47 UTC (permalink / raw)
  To: ed, james.feist; +Cc: openbmc


[-- Attachment #1: Type: text/plain, Size: 377 bytes --]

Hi Ed & James,

Till now IBM was using phosphor-settings infrastructure as back-end and uses Ethernet Schema for Hypervisor computer system(hypervisor_ethernet.hpp) for setting the IP address of hypervisor. And now we are planning to leverage the capabilities of bios-settings-mgr(backend) as well to set the hypervisor attributes.
do you see any concerns here ?
Thanks,
Manoj

[-- Attachment #2: Type: text/html, Size: 712 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Using bios-settings-mgr for setting hypervisor network attributes
  2020-09-16 14:47 Using bios-settings-mgr for setting hypervisor network attributes manoj kiran
@ 2020-09-16 16:26 ` Ed Tanous
  2020-09-16 16:33   ` James Feist
  2020-09-16 17:20 ` Patrick Williams
  1 sibling, 1 reply; 22+ messages in thread
From: Ed Tanous @ 2020-09-16 16:26 UTC (permalink / raw)
  To: manoj kiran; +Cc: james.feist, openbmc


[-- Attachment #1: Type: text/plain, Size: 695 bytes --]

On Wed, Sep 16, 2020 at 7:47 AM manoj kiran <manojkiran.eda@gmail.com>
wrote:

> Hi Ed & James,
>
> Till now IBM was using phosphor-settings infrastructure as back-end and
> uses Ethernet Schema for Hypervisor computer
> system(hypervisor_ethernet.hpp) for setting the IP address of hypervisor.
> And now we are planning to leverage the capabilities of
> bios-settings-mgr(backend) as well to set the hypervisor attributes.
>
> do you see any concerns here ?
>
> Thanks,
> Manoj
> [image: Sent from Mailspring]


None that I can think of (although I don't know much about
bios-settings-mgr).  Holding all the host data in one daemon seems like a
better way to do it than putting it in settings.

[-- Attachment #2: Type: text/html, Size: 1291 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Using bios-settings-mgr for setting hypervisor network attributes
  2020-09-16 16:26 ` Ed Tanous
@ 2020-09-16 16:33   ` James Feist
  0 siblings, 0 replies; 22+ messages in thread
From: James Feist @ 2020-09-16 16:33 UTC (permalink / raw)
  To: Ed Tanous, manoj kiran; +Cc: openbmc

On 9/16/2020 9:26 AM, Ed Tanous wrote:
> 
> 
> On Wed, Sep 16, 2020 at 7:47 AM manoj kiran <manojkiran.eda@gmail.com 
> <mailto:manojkiran.eda@gmail.com>> wrote:
> 
>     Hi Ed & James,
> 
>     Till now IBM was using phosphor-settings infrastructure as back-end
>     and uses Ethernet Schema for Hypervisor computer
>     system(hypervisor_ethernet.hpp) for setting the IP address of
>     hypervisor. And now we are planning to leverage the capabilities of
>     bios-settings-mgr(backend) as well to set the hypervisor attributes.
> 
>     do you see any concerns here ?
> 
>     Thanks,
>     Manoj
>     Sent from Mailspring
> 
> 
> None that I can think of (although I don't know much about 
> bios-settings-mgr).  Holding all the host data in one daemon seems like 
> a better way to do it than putting it in settings.

Agree, as long as we use the mapper to look up interfaces, should be 
transparent.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Using bios-settings-mgr for setting hypervisor network attributes
  2020-09-16 14:47 Using bios-settings-mgr for setting hypervisor network attributes manoj kiran
  2020-09-16 16:26 ` Ed Tanous
@ 2020-09-16 17:20 ` Patrick Williams
  2020-09-16 17:44   ` Ed Tanous
  1 sibling, 1 reply; 22+ messages in thread
From: Patrick Williams @ 2020-09-16 17:20 UTC (permalink / raw)
  To: manoj kiran; +Cc: ed, james.feist, openbmc


[-- Attachment #1: Type: text/plain, Size: 2280 bytes --]

On Wed, Sep 16, 2020 at 08:17:01PM +0530, manoj kiran wrote:
> Hi Ed & James,
> 
> Till now IBM was using phosphor-settings infrastructure as back-end and uses Ethernet Schema for Hypervisor computer system(hypervisor_ethernet.hpp) for setting the IP address of hypervisor. And now we are planning to leverage the capabilities of bios-settings-mgr(backend) as well to set the hypervisor attributes.
> do you see any concerns here ?
> Thanks,
> Manoj

These end up being two quite different implementations from a dbus
perspective, which could have implications to Redfish and webui users.

With 'settings' there is no generic settings interfacess on dbus; every
setting is required to have some modeled interface.  This is great when
you are exposing some hypervisor setting that the BMC also has for
itself, such as network.  We have a single dbus interface for all
network end-points and it doesn't matter if it is for the BMC or the
Hypervisor.

With 'bios-settings-mgr' there are only generic free-form settings
values, which presently can be either int64 or string[1].  This means
there is no overlap with any similar settings we have on the BMC and
there is no programatic way to ensure the data is of the right type and
named with the right key.  This approach is better when you have large
numbers of attributes for concepts which the BMC doesn't have itself.

My understanding was that the 'bios-settings-mgr' was typically going to be
used for uploading a large blob of configuration values and the external
interfaces would have fairly minimal code related to individual
settings.  My concern with using 'bios-settings-mgr' in general is that
it will end up being very tight coupling between external interfaces
(Redfish / webui) and BIOS implementations.  When you use 'settings',
you can implement much more generic external interface code and likely
limit the coupling, if any, to the PLDM provider.

Net is, if you're expecting to be able to modify hypervisor values
through Redfish or WebUI, I think the best approach is to use
'settings'.

1. https://github.com/openbmc/phosphor-dbus-interfaces/blob/77a742627edde54aec625d7c1a200d9f4832f0ba/xyz/openbmc_project/BIOSConfig/Manager.interface.yaml#L44

-- 
Patrick Williams

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Using bios-settings-mgr for setting hypervisor network attributes
  2020-09-16 17:20 ` Patrick Williams
@ 2020-09-16 17:44   ` Ed Tanous
  2020-09-17  7:40     ` Ratan Gupta
  0 siblings, 1 reply; 22+ messages in thread
From: Ed Tanous @ 2020-09-16 17:44 UTC (permalink / raw)
  To: Patrick Williams; +Cc: manoj kiran, james.feist, openbmc

On Wed, Sep 16, 2020 at 10:20 AM Patrick Williams <patrick@stwcx.xyz> wrote:
>
> On Wed, Sep 16, 2020 at 08:17:01PM +0530, manoj kiran wrote:
> > Hi Ed & James,
> >
> > Till now IBM was using phosphor-settings infrastructure as back-end and uses Ethernet Schema for Hypervisor computer system(hypervisor_ethernet.hpp) for setting the IP address of hypervisor. And now we are planning to leverage the capabilities of bios-settings-mgr(backend) as well to set the hypervisor attributes.
> > do you see any concerns here ?
> > Thanks,
> > Manoj
>
> These end up being two quite different implementations from a dbus
> perspective, which could have implications to Redfish and webui users.
>
> With 'settings' there is no generic settings interfacess on dbus; every
> setting is required to have some modeled interface.  This is great when
> you are exposing some hypervisor setting that the BMC also has for
> itself, such as network.  We have a single dbus interface for all
> network end-points and it doesn't matter if it is for the BMC or the
> Hypervisor.
>
> With 'bios-settings-mgr' there are only generic free-form settings
> values, which presently can be either int64 or string[1].

If this is correct, then I withdraw my support.  I had assumed
bios-settings-mgr would host several objects that contain an
EthernetInterface [1] api, similar to what phosphor-networkd does, and
whose endpoints require no new code in most of the endpoints.  If
we're talking about moving all this to a simple key-value store,
running on yet another representation of what a network interface
looks like, that's going in the wrong direction in terms of fidelity
and complexity.

With that said, if I'm mistaken, let me know.

>  This means
> there is no overlap with any similar settings we have on the BMC and
> there is no programatic way to ensure the data is of the right type and
> named with the right key.  This approach is better when you have large
> numbers of attributes for concepts which the BMC doesn't have itself.
>
> My understanding was that the 'bios-settings-mgr' was typically going to be
> used for uploading a large blob of configuration values and the external
> interfaces would have fairly minimal code related to individual
> settings.  My concern with using 'bios-settings-mgr' in general is that
> it will end up being very tight coupling between external interfaces
> (Redfish / webui) and BIOS implementations.  When you use 'settings',
> you can implement much more generic external interface code and likely
> limit the coupling, if any, to the PLDM provider.

I think we have one benefit here in that there's going to be several
implementations of the bios-settings-mgr for the various bios
implementations that will keep us more "honest" about our APIs.  It's
not a satisfying answer, I realize, but I think it's the best we can
do at the moment.

>
> Net is, if you're expecting to be able to modify hypervisor values
> through Redfish or WebUI, I think the best approach is to use
> 'settings'.

The problem with the "settings" approach becomes error handling.
Settingsd has no context of a transaction, or a backend on the other
side, so when and if things fail, they fail silently, or possibly with
a log.  In the case of hosting this API in the BIOS daemon, it can
actually do the "commit" of the parameters to BIOS as part of the dbus
transaction, so once the return code is received from the method call,
the user can know that the values were "latched", and can knowingly
move on.  If they weren't latched, the client can know if it makes
sense to retry, or do some other procedure.
This also has nice properties for the BMC, as it never has to "own"
storage of the data, nor does it have to implement all the validation
routines, as it can rely on the actual data owner to do so.

>
> 1. https://github.com/openbmc/phosphor-dbus-interfaces/blob/77a742627edde54aec625d7c1a200d9f4832f0ba/xyz/openbmc_project/BIOSConfig/Manager.interface.yaml#L44
>
> --
> Patrick Williams

1. https://github.com/openbmc/phosphor-dbus-interfaces/blob/master/xyz/openbmc_project/Network/EthernetInterface.interface.yaml

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Using bios-settings-mgr for setting hypervisor network attributes
  2020-09-16 17:44   ` Ed Tanous
@ 2020-09-17  7:40     ` Ratan Gupta
  2020-09-17 12:21       ` Deepak Kodihalli
                         ` (2 more replies)
  0 siblings, 3 replies; 22+ messages in thread
From: Ratan Gupta @ 2020-09-17  7:40 UTC (permalink / raw)
  To: openbmc


[-- Attachment #1: Type: text/plain, Size: 5107 bytes --]

Hi Pattrick, Ed,


We need to address the below two concerns with the existing settings infra.

  * Pending v/s configured value: Currently settings have single Dbus
    Object, Some properties which is for host firmware we need to have
    two placeholders one for Pending values and one for Configured
    values. Bios settings have this concept.
      o Should we add two Dbus objects in settings infra?
  * Dynamic Dbus objects: Currently settings infrastructure is only for
    static objects, Objects which gets added on runtime, settings infra
    doesn't support that.
      o Eg: IP address on ethernet interface is dynamic in nature, An
        ethernet interface can have multiple IP address on it.
        considering if SLAAC is enabled(ipV6).
      o Seems this problem is common for both(settings v/s bios-settings)

Regards
Ratan Gupta


On 9/16/20 11:14 PM, Ed Tanous wrote:
> On Wed, Sep 16, 2020 at 10:20 AM Patrick Williams <patrick@stwcx.xyz> wrote:
>> On Wed, Sep 16, 2020 at 08:17:01PM +0530, manoj kiran wrote:
>>> Hi Ed & James,
>>>
>>> Till now IBM was using phosphor-settings infrastructure as back-end and uses Ethernet Schema for Hypervisor computer system(hypervisor_ethernet.hpp) for setting the IP address of hypervisor. And now we are planning to leverage the capabilities of bios-settings-mgr(backend) as well to set the hypervisor attributes.
>>> do you see any concerns here ?
>>> Thanks,
>>> Manoj
>> These end up being two quite different implementations from a dbus
>> perspective, which could have implications to Redfish and webui users.
>>
>> With 'settings' there is no generic settings interfacess on dbus; every
>> setting is required to have some modeled interface.  This is great when
>> you are exposing some hypervisor setting that the BMC also has for
>> itself, such as network.  We have a single dbus interface for all
>> network end-points and it doesn't matter if it is for the BMC or the
>> Hypervisor.
>>
>> With 'bios-settings-mgr' there are only generic free-form settings
>> values, which presently can be either int64 or string[1].
> If this is correct, then I withdraw my support.  I had assumed
> bios-settings-mgr would host several objects that contain an
> EthernetInterface [1] api, similar to what phosphor-networkd does, and
> whose endpoints require no new code in most of the endpoints.  If
> we're talking about moving all this to a simple key-value store,
> running on yet another representation of what a network interface
> looks like, that's going in the wrong direction in terms of fidelity
> and complexity.
>
> With that said, if I'm mistaken, let me know.
>
>>   This means
>> there is no overlap with any similar settings we have on the BMC and
>> there is no programatic way to ensure the data is of the right type and
>> named with the right key.  This approach is better when you have large
>> numbers of attributes for concepts which the BMC doesn't have itself.
>>
>> My understanding was that the 'bios-settings-mgr' was typically going to be
>> used for uploading a large blob of configuration values and the external
>> interfaces would have fairly minimal code related to individual
>> settings.  My concern with using 'bios-settings-mgr' in general is that
>> it will end up being very tight coupling between external interfaces
>> (Redfish / webui) and BIOS implementations.  When you use 'settings',
>> you can implement much more generic external interface code and likely
>> limit the coupling, if any, to the PLDM provider.
> I think we have one benefit here in that there's going to be several
> implementations of the bios-settings-mgr for the various bios
> implementations that will keep us more "honest" about our APIs.  It's
> not a satisfying answer, I realize, but I think it's the best we can
> do at the moment.
>
>> Net is, if you're expecting to be able to modify hypervisor values
>> through Redfish or WebUI, I think the best approach is to use
>> 'settings'.
> The problem with the "settings" approach becomes error handling.
> Settingsd has no context of a transaction, or a backend on the other
> side, so when and if things fail, they fail silently, or possibly with
> a log.  In the case of hosting this API in the BIOS daemon, it can
> actually do the "commit" of the parameters to BIOS as part of the dbus
> transaction, so once the return code is received from the method call,
> the user can know that the values were "latched", and can knowingly
> move on.  If they weren't latched, the client can know if it makes
> sense to retry, or do some other procedure.
> This also has nice properties for the BMC, as it never has to "own"
> storage of the data, nor does it have to implement all the validation
> routines, as it can rely on the actual data owner to do so.
>
>> 1. https://github.com/openbmc/phosphor-dbus-interfaces/blob/77a742627edde54aec625d7c1a200d9f4832f0ba/xyz/openbmc_project/BIOSConfig/Manager.interface.yaml#L44
>>
>> --
>> Patrick Williams
> 1. https://github.com/openbmc/phosphor-dbus-interfaces/blob/master/xyz/openbmc_project/Network/EthernetInterface.interface.yaml

[-- Attachment #2: Type: text/html, Size: 6938 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Using bios-settings-mgr for setting hypervisor network attributes
  2020-09-17  7:40     ` Ratan Gupta
@ 2020-09-17 12:21       ` Deepak Kodihalli
  2020-09-17 14:20       ` Thomaiyar, Richard Marian
  2020-09-17 15:36       ` Patrick Williams
  2 siblings, 0 replies; 22+ messages in thread
From: Deepak Kodihalli @ 2020-09-17 12:21 UTC (permalink / raw)
  To: Ratan Gupta, openbmc

On 17/09/20 1:10 pm, Ratan Gupta wrote:
> Hi Pattrick, Ed, We need to address the below two concerns with the 
> existing...
> This Message Is From an External Sender
> This message came from outside your organization.
> 
> Hi Pattrick, Ed,
> 
> 
> We need to address the below two concerns with the existing settings infra.
> 
>   * Pending v/s configured value: Currently settings have single Dbus
>     Object, Some properties which is for host firmware we need to have
>     two placeholders one for Pending values and one for Configured
>     values. Bios settings have this concept.
>       o Should we add two Dbus objects in settings infra?
>   * Dynamic Dbus objects: Currently settings infrastructure is only for
>     static objects, Objects which gets added on runtime, settings infra
>     doesn't support that.
>       o Eg: IP address on ethernet interface is dynamic in nature, An
>         ethernet interface can have multiple IP address on it.
>         considering if SLAAC is enabled(ipV6).
>       o Seems this problem is common for both(settings v/s bios-settings)

+1. The settings daemon doesn't offer all the functionality expected to 
work with BIOS attributes. For that reason, if there are a class of 
attributes which are updated out of band via non-BIOS Redfish schema, 
but these are sent up to the host as updated BIOS attributes, the 
bios-settings-mgr app seems to be a better fit.

Thanks,
Deepak

> Regards
> Ratan Gupta
> 
> 
> On 9/16/20 11:14 PM, Ed Tanous wrote:
>> On Wed, Sep 16, 2020 at 10:20 AM Patrick Williams<patrick@stwcx.xyz>  wrote:
>>> On Wed, Sep 16, 2020 at 08:17:01PM +0530, manoj kiran wrote:
>>>> Hi Ed & James,
>>>>
>>>> Till now IBM was using phosphor-settings infrastructure as back-end and uses Ethernet Schema for Hypervisor computer system(hypervisor_ethernet.hpp) for setting the IP address of hypervisor. And now we are planning to leverage the capabilities of bios-settings-mgr(backend) as well to set the hypervisor attributes.
>>>> do you see any concerns here ?
>>>> Thanks,
>>>> Manoj
>>> These end up being two quite different implementations from a dbus
>>> perspective, which could have implications to Redfish and webui users.
>>>
>>> With 'settings' there is no generic settings interfacess on dbus; every
>>> setting is required to have some modeled interface.  This is great when
>>> you are exposing some hypervisor setting that the BMC also has for
>>> itself, such as network.  We have a single dbus interface for all
>>> network end-points and it doesn't matter if it is for the BMC or the
>>> Hypervisor.
>>>
>>> With 'bios-settings-mgr' there are only generic free-form settings
>>> values, which presently can be either int64 or string[1].
>> If this is correct, then I withdraw my support.  I had assumed
>> bios-settings-mgr would host several objects that contain an
>> EthernetInterface [1] api, similar to what phosphor-networkd does, and
>> whose endpoints require no new code in most of the endpoints.  If
>> we're talking about moving all this to a simple key-value store,
>> running on yet another representation of what a network interface
>> looks like, that's going in the wrong direction in terms of fidelity
>> and complexity.
>>
>> With that said, if I'm mistaken, let me know.
>>
>>>   This means
>>> there is no overlap with any similar settings we have on the BMC and
>>> there is no programatic way to ensure the data is of the right type and
>>> named with the right key.  This approach is better when you have large
>>> numbers of attributes for concepts which the BMC doesn't have itself.
>>>
>>> My understanding was that the 'bios-settings-mgr' was typically going to be
>>> used for uploading a large blob of configuration values and the external
>>> interfaces would have fairly minimal code related to individual
>>> settings.  My concern with using 'bios-settings-mgr' in general is that
>>> it will end up being very tight coupling between external interfaces
>>> (Redfish / webui) and BIOS implementations.  When you use 'settings',
>>> you can implement much more generic external interface code and likely
>>> limit the coupling, if any, to the PLDM provider.
>> I think we have one benefit here in that there's going to be several
>> implementations of the bios-settings-mgr for the various bios
>> implementations that will keep us more "honest" about our APIs.  It's
>> not a satisfying answer, I realize, but I think it's the best we can
>> do at the moment.
>>
>>> Net is, if you're expecting to be able to modify hypervisor values
>>> through Redfish or WebUI, I think the best approach is to use
>>> 'settings'.
>> The problem with the "settings" approach becomes error handling.
>> Settingsd has no context of a transaction, or a backend on the other
>> side, so when and if things fail, they fail silently, or possibly with
>> a log.  In the case of hosting this API in the BIOS daemon, it can
>> actually do the "commit" of the parameters to BIOS as part of the dbus
>> transaction, so once the return code is received from the method call,
>> the user can know that the values were "latched", and can knowingly
>> move on.  If they weren't latched, the client can know if it makes
>> sense to retry, or do some other procedure.
>> This also has nice properties for the BMC, as it never has to "own"
>> storage of the data, nor does it have to implement all the validation
>> routines, as it can rely on the actual data owner to do so.
>>
>>> 1.https://github.com/openbmc/phosphor-dbus-interfaces/blob/77a742627edde54aec625d7c1a200d9f4832f0ba/xyz/openbmc_project/BIOSConfig/Manager.interface.yaml#L44
>>>
>>> --
>>> Patrick Williams
>> 1.https://github.com/openbmc/phosphor-dbus-interfaces/blob/master/xyz/openbmc_project/Network/EthernetInterface.interface.yaml

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Using bios-settings-mgr for setting hypervisor network attributes
  2020-09-17  7:40     ` Ratan Gupta
  2020-09-17 12:21       ` Deepak Kodihalli
@ 2020-09-17 14:20       ` Thomaiyar, Richard Marian
  2020-09-17 15:36       ` Patrick Williams
  2 siblings, 0 replies; 22+ messages in thread
From: Thomaiyar, Richard Marian @ 2020-09-17 14:20 UTC (permalink / raw)
  To: Ratan Gupta, openbmc, Sekar, Suryakanth


[-- Attachment #1: Type: text/plain, Size: 6696 bytes --]

Ratan / Manoj,

Hypervisor (VM) Ethernet Interfaces is not a BIOS / Host firmware 
settings right?. Is there any model, where the BIOS Settings of Host 
Network interface like IPV4 / IPV6 is passed to the OS level (If yes, 
through what mechanism, proprietary ?). We have BIOS Network settings, 
but mostly that will be used in terms of PXE boot etc., but this will 
not be passed to the Host OS/ Hypervisor (which has to manage this on 
it's own). Let me know if i am missing anything here. So not sure, why 
Hypervisor / OS Ethernet interface must be passed to BIOS Settings 
instead of directly communicating to the Hypervisor / OS level software 
directly.

For BIOS settings --> Pending v/s configured value, Remote BIOS 
configuration design doc 
<https://github.com/openbmc/docs/blob/master/designs/remote-bios-configuration.md>, 
already handles the same using PendingAttributes. This is based on the 
AttributeRegistry and as per the design it don't advertise every single 
setting in D-Bus, instead it will be collection (dynamic in nature).

For Hypervisor / System Ethernet Interfaces I agree with James F, As 
long as bmcweb does the mapper query and / or association to determine 
the Service and Object path of the daemon, which will handle the 
ComputerSystem (Host) EthernetInterfaces it should be fine as the 
mechanism of forwarding the data to the OS will be different based on 
implementation.

Regards,

Richard

On 9/17/2020 1:10 PM, Ratan Gupta wrote:
> Hi Pattrick, Ed,
>
>
> We need to address the below two concerns with the existing settings 
> infra.
>
>   * Pending v/s configured value: Currently settings have single Dbus
>     Object, Some properties which is for host firmware we need to have
>     two placeholders one for Pending values and one for Configured
>     values. Bios settings have this concept.
>       o Should we add two Dbus objects in settings infra?
>   * Dynamic Dbus objects: Currently settings infrastructure is only
>     for static objects, Objects which gets added on runtime, settings
>     infra doesn't support that.
>       o Eg: IP address on ethernet interface is dynamic in nature, An
>         ethernet interface can have multiple IP address on it.
>         considering if SLAAC is enabled(ipV6).
>       o Seems this problem is common for both(settings v/s bios-settings)
>
> Regards
> Ratan Gupta
>
>
> On 9/16/20 11:14 PM, Ed Tanous wrote:
>> On Wed, Sep 16, 2020 at 10:20 AM Patrick Williams<patrick@stwcx.xyz>  wrote:
>>> On Wed, Sep 16, 2020 at 08:17:01PM +0530, manoj kiran wrote:
>>>> Hi Ed & James,
>>>>
>>>> Till now IBM was using phosphor-settings infrastructure as back-end and uses Ethernet Schema for Hypervisor computer system(hypervisor_ethernet.hpp) for setting the IP address of hypervisor. And now we are planning to leverage the capabilities of bios-settings-mgr(backend) as well to set the hypervisor attributes.
>>>> do you see any concerns here ?
>>>> Thanks,
>>>> Manoj
>>> These end up being two quite different implementations from a dbus
>>> perspective, which could have implications to Redfish and webui users.
>>>
>>> With 'settings' there is no generic settings interfacess on dbus; every
>>> setting is required to have some modeled interface.  This is great when
>>> you are exposing some hypervisor setting that the BMC also has for
>>> itself, such as network.  We have a single dbus interface for all
>>> network end-points and it doesn't matter if it is for the BMC or the
>>> Hypervisor.
>>>
>>> With 'bios-settings-mgr' there are only generic free-form settings
>>> values, which presently can be either int64 or string[1].
>> If this is correct, then I withdraw my support.  I had assumed
>> bios-settings-mgr would host several objects that contain an
>> EthernetInterface [1] api, similar to what phosphor-networkd does, and
>> whose endpoints require no new code in most of the endpoints.  If
>> we're talking about moving all this to a simple key-value store,
>> running on yet another representation of what a network interface
>> looks like, that's going in the wrong direction in terms of fidelity
>> and complexity.
>>
>> With that said, if I'm mistaken, let me know.
>>
>>>   This means
>>> there is no overlap with any similar settings we have on the BMC and
>>> there is no programatic way to ensure the data is of the right type and
>>> named with the right key.  This approach is better when you have large
>>> numbers of attributes for concepts which the BMC doesn't have itself.
>>>
>>> My understanding was that the 'bios-settings-mgr' was typically going to be
>>> used for uploading a large blob of configuration values and the external
>>> interfaces would have fairly minimal code related to individual
>>> settings.  My concern with using 'bios-settings-mgr' in general is that
>>> it will end up being very tight coupling between external interfaces
>>> (Redfish / webui) and BIOS implementations.  When you use 'settings',
>>> you can implement much more generic external interface code and likely
>>> limit the coupling, if any, to the PLDM provider.
>> I think we have one benefit here in that there's going to be several
>> implementations of the bios-settings-mgr for the various bios
>> implementations that will keep us more "honest" about our APIs.  It's
>> not a satisfying answer, I realize, but I think it's the best we can
>> do at the moment.
>>
>>> Net is, if you're expecting to be able to modify hypervisor values
>>> through Redfish or WebUI, I think the best approach is to use
>>> 'settings'.
>> The problem with the "settings" approach becomes error handling.
>> Settingsd has no context of a transaction, or a backend on the other
>> side, so when and if things fail, they fail silently, or possibly with
>> a log.  In the case of hosting this API in the BIOS daemon, it can
>> actually do the "commit" of the parameters to BIOS as part of the dbus
>> transaction, so once the return code is received from the method call,
>> the user can know that the values were "latched", and can knowingly
>> move on.  If they weren't latched, the client can know if it makes
>> sense to retry, or do some other procedure.
>> This also has nice properties for the BMC, as it never has to "own"
>> storage of the data, nor does it have to implement all the validation
>> routines, as it can rely on the actual data owner to do so.
>>
>>> 1.https://github.com/openbmc/phosphor-dbus-interfaces/blob/77a742627edde54aec625d7c1a200d9f4832f0ba/xyz/openbmc_project/BIOSConfig/Manager.interface.yaml#L44
>>>
>>> --
>>> Patrick Williams
>> 1.https://github.com/openbmc/phosphor-dbus-interfaces/blob/master/xyz/openbmc_project/Network/EthernetInterface.interface.yaml

[-- Attachment #2: Type: text/html, Size: 9086 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Using bios-settings-mgr for setting hypervisor network attributes
  2020-09-17  7:40     ` Ratan Gupta
  2020-09-17 12:21       ` Deepak Kodihalli
  2020-09-17 14:20       ` Thomaiyar, Richard Marian
@ 2020-09-17 15:36       ` Patrick Williams
  2020-09-19  5:41         ` Ratan Gupta
  2 siblings, 1 reply; 22+ messages in thread
From: Patrick Williams @ 2020-09-17 15:36 UTC (permalink / raw)
  To: Ratan Gupta; +Cc: openbmc


[-- Attachment #1: Type: text/plain, Size: 1898 bytes --]

On Thu, Sep 17, 2020 at 01:10:06PM +0530, Ratan Gupta wrote:
> We need to address the below two concerns with the existing settings infra.

Both of these seem like missing features based on our now greater
understanding of the problem domain from where we were 3-4 years ago
when phosphor-settings-manager was originally written, right?  That
doesn't seem like a good reason to entirely throw out the approach.

>   * Pending v/s configured value: Currently settings have single Dbus
>     Object, Some properties which is for host firmware we need to have
>     two placeholders one for Pending values and one for Configured
>     values. Bios settings have this concept.
>       o Should we add two Dbus objects in settings infra?

This was going to be my suggestion, yes.  You could have two sets of the
objects: current and pending.  'current' objects may not be written by
dbus-clients.  These are the same terms used by the BIOSConfig proposal.

What I am not seeing in BIOSConfig and is equally applicable here is
_when_ pending is applied to current.  You will need some interface that
IPMI / PLDM can call to apply those settings?  Or, do you monitor host
state signals automatically?

>   * Dynamic Dbus objects: Currently settings infrastructure is only for
>     static objects, Objects which gets added on runtime, settings infra
>     doesn't support that.
>       o Eg: IP address on ethernet interface is dynamic in nature, An
>         ethernet interface can have multiple IP address on it.
>         considering if SLAAC is enabled(ipV6).
>       o Seems this problem is common for both(settings v/s bios-settings)

I assume these would be requested for creation by IPMI / PLDM?  We could
use a similar model to xyz.openbmc_project.Inventory.Manager where
objects are requested for creation dynamically through a method.

-- 
Patrick Williams

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Using bios-settings-mgr for setting hypervisor network attributes
  2020-09-17 15:36       ` Patrick Williams
@ 2020-09-19  5:41         ` Ratan Gupta
  2020-09-22  9:09           ` Ratan Gupta
  0 siblings, 1 reply; 22+ messages in thread
From: Ratan Gupta @ 2020-09-19  5:41 UTC (permalink / raw)
  To: openbmc


On 9/17/20 9:06 PM, Patrick Williams wrote:
> On Thu, Sep 17, 2020 at 01:10:06PM +0530, Ratan Gupta wrote:
>> We need to address the below two concerns with the existing settings infra.
> Both of these seem like missing features based on our now greater
> understanding of the problem domain from where we were 3-4 years ago
> when phosphor-settings-manager was originally written, right?  That
> doesn't seem like a good reason to entirely throw out the approach.
>
>>    * Pending v/s configured value: Currently settings have single Dbus
>>      Object, Some properties which is for host firmware we need to have
>>      two placeholders one for Pending values and one for Configured
>>      values. Bios settings have this concept.
>>        o Should we add two Dbus objects in settings infra?
> This was going to be my suggestion, yes.  You could have two sets of the
> objects: current and pending.  'current' objects may not be written by
> dbus-clients.  These are the same terms used by the BIOSConfig proposal.
Thanks Patrick, seems reasonable to have two D-bus objects.
>
> What I am not seeing in BIOSConfig and is equally applicable here is
> _when_ pending is applied to current.  You will need some interface that
> IPMI / PLDM can call to apply those settings?  Or, do you monitor host
> state signals automatically?
>
>>    * Dynamic Dbus objects: Currently settings infrastructure is only for
>>      static objects, Objects which gets added on runtime, settings infra
>>      doesn't support that.
>>        o Eg: IP address on ethernet interface is dynamic in nature, An
>>          ethernet interface can have multiple IP address on it.
>>          considering if SLAAC is enabled(ipV6).
>>        o Seems this problem is common for both(settings v/s bios-settings)
> I assume these would be requested for creation by IPMI / PLDM?  We could
> use a similar model to xyz.openbmc_project.Inventory.Manager where
> objects are requested for creation dynamically through a method.
We don't have this requirement now but in near future it is going to
be there, we can improve the settings infra to support this.
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Using bios-settings-mgr for setting hypervisor network attributes
  2020-09-19  5:41         ` Ratan Gupta
@ 2020-09-22  9:09           ` Ratan Gupta
  2020-09-22 12:08             ` Deepak Kodihalli
  2020-09-23 19:24             ` Patrick Williams
  0 siblings, 2 replies; 22+ messages in thread
From: Ratan Gupta @ 2020-09-22  9:09 UTC (permalink / raw)
  To: openbmc, Deepak Kodihalli, Patrick Williams, Ed Tanous


[-- Attachment #1: Type: text/plain, Size: 3478 bytes --]

Hi All,

Adding one more problem here with settings infra and with some proposed 
solutions.

Problem Domain:

       - With multi property update from redfish , webserver updates the 
settings object
       - PLDM on bmc listens on the property update of settings object 
and notifies to Hypervisor
       - As there can be multiple properties in single PATCH operation, 
PLDM on bmc sends
         multiple Notifications to Hypervisor
       - Specifically in case of network config,  single property update 
on phyp may lead to network inconsistency.

How can we solve this?

  * Proposal 1: Add one more property in the settings Dbus object itself
    which tells that it is ready to be read, PLDM on the BMC watching on
    that property and read the whole network configuration and notifies
    Hypervisor.

  * Proposal 2: Hypervisor runs the timer if the bios attr belongs to
    network configuration and once the timer expires,it reads the bios
    attr related to network and applies it.

  * Proposal 3: Add one more bios attribute in the bios table which
    tells that Bios configuration can be read and applied by the
    Hypervisor for the network configuration.


   Looking for suggestion what could be the best way here?

Ratan

On 9/19/20 11:11 AM, Ratan Gupta wrote:
>
> On 9/17/20 9:06 PM, Patrick Williams wrote:
>> On Thu, Sep 17, 2020 at 01:10:06PM +0530, Ratan Gupta wrote:
>>> We need to address the below two concerns with the existing settings 
>>> infra.
>> Both of these seem like missing features based on our now greater
>> understanding of the problem domain from where we were 3-4 years ago
>> when phosphor-settings-manager was originally written, right? That
>> doesn't seem like a good reason to entirely throw out the approach.
>>
>>>    * Pending v/s configured value: Currently settings have single Dbus
>>>      Object, Some properties which is for host firmware we need to have
>>>      two placeholders one for Pending values and one for Configured
>>>      values. Bios settings have this concept.
>>>        o Should we add two Dbus objects in settings infra?
>> This was going to be my suggestion, yes.  You could have two sets of the
>> objects: current and pending.  'current' objects may not be written by
>> dbus-clients.  These are the same terms used by the BIOSConfig proposal.
> Thanks Patrick, seems reasonable to have two D-bus objects.
>>
>> What I am not seeing in BIOSConfig and is equally applicable here is
>> _when_ pending is applied to current.  You will need some interface that
>> IPMI / PLDM can call to apply those settings?  Or, do you monitor host
>> state signals automatically?
>>
>>>    * Dynamic Dbus objects: Currently settings infrastructure is only 
>>> for
>>>      static objects, Objects which gets added on runtime, settings 
>>> infra
>>>      doesn't support that.
>>>        o Eg: IP address on ethernet interface is dynamic in nature, An
>>>          ethernet interface can have multiple IP address on it.
>>>          considering if SLAAC is enabled(ipV6).
>>>        o Seems this problem is common for both(settings v/s 
>>> bios-settings)
>> I assume these would be requested for creation by IPMI / PLDM? We could
>> use a similar model to xyz.openbmc_project.Inventory.Manager where
>> objects are requested for creation dynamically through a method.
> We don't have this requirement now but in near future it is going to
> be there, we can improve the settings infra to support this.
>>

[-- Attachment #2: Type: text/html, Size: 5188 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Using bios-settings-mgr for setting hypervisor network attributes
  2020-09-22  9:09           ` Ratan Gupta
@ 2020-09-22 12:08             ` Deepak Kodihalli
  2020-09-23 19:24             ` Patrick Williams
  1 sibling, 0 replies; 22+ messages in thread
From: Deepak Kodihalli @ 2020-09-22 12:08 UTC (permalink / raw)
  To: Ratan Gupta, Patrick Williams, Ed Tanous; +Cc: openbmc

On 22/09/20 2:39 pm, Ratan Gupta wrote:
> Hi All,
> 
> Adding one more problem here with settings infra and with some proposed 
> solutions.

Hi Ratan,

Thanks for bringing this problem out!

> Problem Domain:
> 
>        - With multi property update from redfish , webserver updates the 
> settings object
>        - PLDM on bmc listens on the property update of settings object 
> and notifies to Hypervisor

To add to this, from a PLDM perspective, we plan to send up the 
hypervisor network config properties to the host as BIOS attributes. 
There isn't a PLDM network config schema. The PLDM daemon talks to the 
new bios-settings-mgr app to find BIOS properties that have been updated 
out of band. The Redfish schema we plan to use here is EthernetInterface 
though, and not the Redfish BIOS schema. All this is causing a need for 
some conversion layers.

My initial thought was bmcweb can update the BIOS backend store that's 
implemented by the bios-settings-mgr, but it looks like Patrick and Ed 
have concerns with that approach. I think I agree their reasoning, but 
at the same time I don't think there should be special code in the PLDM 
daemon (timers/special knowledge about a set of BIOS attributes/special 
BIOS attribute which indicates other BIOS attributes have been 
updated/etc) for this, and this should be processed like any other BIOS 
attribute that the PLDM daemon deals with. This implies these attributes 
should also make it to the store that bios-settings-mgr owns (that 
likely means an additional D-Bus hop). So, another option (proposal 4) 
could be an intermediate app that accumulates (for eg by means of 
watching the 'Enabled' property that's in the Object.Enable interface; 
the hypervisor settings object would have to additionally implement this 
interface) the hypervisor network config property updates, and then 
provides these as key-value pairs to the bios-settings-mgr app.

>        - As there can be multiple properties in single PATCH operation, 
> PLDM on bmc sends
>          multiple Notifications to Hypervisor
>        - Specifically in case of network config,  single property update 
> on phyp may lead to network inconsistency.
> 
> How can we solve this?
> 
>   * Proposal 1: Add one more property in the settings Dbus object itself
>     which tells that it is ready to be read, PLDM on the BMC watching on
>     that property and read the whole network configuration and notifies
>     Hypervisor.
> 
>   * Proposal 2: Hypervisor runs the timer if the bios attr belongs to
>     network configuration and once the timer expires,it reads the bios
>     attr related to network and applies it.
> 
>   * Proposal 3: Add one more bios attribute in the bios table which
>     tells that Bios configuration can be read and applied by the
>     Hypervisor for the network configuration.
> 
> 
>    Looking for suggestion what could be the best way here?
> 
> Ratan
> 
> On 9/19/20 11:11 AM, Ratan Gupta wrote:
>>
>> On 9/17/20 9:06 PM, Patrick Williams wrote:
>>> On Thu, Sep 17, 2020 at 01:10:06PM +0530, Ratan Gupta wrote:
>>>> We need to address the below two concerns with the existing settings 
>>>> infra.
>>> Both of these seem like missing features based on our now greater
>>> understanding of the problem domain from where we were 3-4 years ago
>>> when phosphor-settings-manager was originally written, right? That
>>> doesn't seem like a good reason to entirely throw out the approach.
>>>
>>>>    * Pending v/s configured value: Currently settings have single Dbus
>>>>      Object, Some properties which is for host firmware we need to have
>>>>      two placeholders one for Pending values and one for Configured
>>>>      values. Bios settings have this concept.
>>>>        o Should we add two Dbus objects in settings infra?
>>> This was going to be my suggestion, yes.  You could have two sets of the
>>> objects: current and pending.  'current' objects may not be written by
>>> dbus-clients.  These are the same terms used by the BIOSConfig proposal.
>> Thanks Patrick, seems reasonable to have two D-bus objects.
>>>
>>> What I am not seeing in BIOSConfig and is equally applicable here is
>>> _when_ pending is applied to current.  You will need some interface that
>>> IPMI / PLDM can call to apply those settings?  Or, do you monitor host
>>> state signals automatically?
>>>
>>>>    * Dynamic Dbus objects: Currently settings infrastructure is only 
>>>> for
>>>>      static objects, Objects which gets added on runtime, settings 
>>>> infra
>>>>      doesn't support that.
>>>>        o Eg: IP address on ethernet interface is dynamic in nature, An
>>>>          ethernet interface can have multiple IP address on it.
>>>>          considering if SLAAC is enabled(ipV6).
>>>>        o Seems this problem is common for both(settings v/s 
>>>> bios-settings)
>>> I assume these would be requested for creation by IPMI / PLDM? We could
>>> use a similar model to xyz.openbmc_project.Inventory.Manager where
>>> objects are requested for creation dynamically through a method.
>> We don't have this requirement now but in near future it is going to
>> be there, we can improve the settings infra to support this.
>>>


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Using bios-settings-mgr for setting hypervisor network attributes
  2020-09-22  9:09           ` Ratan Gupta
  2020-09-22 12:08             ` Deepak Kodihalli
@ 2020-09-23 19:24             ` Patrick Williams
  2020-09-23 20:51               ` Ed Tanous
  2020-09-24  7:30               ` Ratan Gupta
  1 sibling, 2 replies; 22+ messages in thread
From: Patrick Williams @ 2020-09-23 19:24 UTC (permalink / raw)
  To: Ratan Gupta; +Cc: openbmc, Ed Tanous


[-- Attachment #1: Type: text/plain, Size: 3624 bytes --]

On Tue, Sep 22, 2020 at 02:39:04PM +0530, Ratan Gupta wrote:
> Hi All,
> 
> Adding one more problem here with settings infra and with some proposed 
> solutions.
> 
> Problem Domain:
> 
>        - With multi property update from redfish , webserver updates the 
> settings object
>        - PLDM on bmc listens on the property update of settings object 
> and notifies to Hypervisor
>        - As there can be multiple properties in single PATCH operation, 
> PLDM on bmc sends
>          multiple Notifications to Hypervisor
>        - Specifically in case of network config,  single property update 
> on phyp may lead to network inconsistency.

The original bios config seemed to only apply settings at specific times
(ie. when the BIOS restarts) but your problem seems to indicate that
you're immediately sending settings up to the host whenever they change?

> How can we solve this?
> 
>   * Proposal 1: Add one more property in the settings Dbus object itself
>     which tells that it is ready to be read, PLDM on the BMC watching on
>     that property and read the whole network configuration and notifies
>     Hypervisor.
> 
>   * Proposal 2: Hypervisor runs the timer if the bios attr belongs to
>     network configuration and once the timer expires,it reads the bios
>     attr related to network and applies it.
> 
>   * Proposal 3: Add one more bios attribute in the bios table which
>     tells that Bios configuration can be read and applied by the
>     Hypervisor for the network configuration.

It is unfortunate that org.freedesktop.DBus.Properties doesn't have a
way to set multiple properties as the analogous operation to 'GetAll'.

In the case of networking, how do we handle this for the BMC settings?
Don't we have a similar situation where multiple properties are changed
via some interface and could leave the network in an unusual state?  I'm
thinking IPMI does this.

When all of our DBus objects were serial we likely never had this issue
because the request to read the properties (to send to the hypervisor)
would come behind the signal and subsequent property updates.  Now that
we're moving towards more ASIO we likely will see this kind of issue
more often.  I don't like it but we could certainly proposal a
'SetMultiple' extension to org.freedesktop or create our own interface.

Proposal #2 isn't great because, well, how long do you wait?  In the
case of hypervisor updates, delaying something on the order of a second
is probably sufficient for Redfish/PLDM, but that doesn't really
generally solve the problem.

We could define an interface to implement something like Proposal #1,
but we would need a new interface and not a property we tack onto
existing interfaces.  We'd probably need to revisit a lot of our
interface definitions and see which ones typicallly have multi-property
updates and does an intermediate state leave us in a bad situation.

Specifically for BIOS/Hypervisor settings, I mentioned before that it
isn't clear to me what the proposal is for applying Pending to Current.
Again, this isn't general, but we could define an interface specific for
BIOS/Hypervisor settings which has a way to indicate 'Pending
transaction is complete' (set by entities like Redfish) and 'Pending
values applied to Current' (set by entities like PLDM).  For the current
settings-style values though, this requires external interfaces to
somehow know that the setting is associated with the Host in order to do
the application, since BMC-owned properties won't have or need this.

-- 
Patrick Williams

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Using bios-settings-mgr for setting hypervisor network attributes
  2020-09-23 19:24             ` Patrick Williams
@ 2020-09-23 20:51               ` Ed Tanous
  2020-09-23 21:26                 ` Patrick Williams
  2020-09-24  7:30               ` Ratan Gupta
  1 sibling, 1 reply; 22+ messages in thread
From: Ed Tanous @ 2020-09-23 20:51 UTC (permalink / raw)
  To: Patrick Williams; +Cc: OpenBMC Maillist, Ratan Gupta

On Wed, Sep 23, 2020 at 12:24 PM Patrick Williams <patrick@stwcx.xyz> wrote:
>
> On Tue, Sep 22, 2020 at 02:39:04PM +0530, Ratan Gupta wrote:
> > Hi All,
> >
> > Adding one more problem here with settings infra and with some proposed
> > solutions.
> >
> > Problem Domain:
> >
> >        - With multi property update from redfish , webserver updates the
> > settings object
> >        - PLDM on bmc listens on the property update of settings object
> > and notifies to Hypervisor
> >        - As there can be multiple properties in single PATCH operation,
> > PLDM on bmc sends
> >          multiple Notifications to Hypervisor
> >        - Specifically in case of network config,  single property update
> > on phyp may lead to network inconsistency.
>
> The original bios config seemed to only apply settings at specific times
> (ie. when the BIOS restarts) but your problem seems to indicate that
> you're immediately sending settings up to the host whenever they change?

I have a very similar use case that I will need to build out in the
next year.  Yes, if the host is on, we'd like them to be pushed
immediately, ideally with error codes returned to dbus if the
operation fails.

>
> > How can we solve this?
> >
> >   * Proposal 1: Add one more property in the settings Dbus object itself
> >     which tells that it is ready to be read, PLDM on the BMC watching on
> >     that property and read the whole network configuration and notifies
> >     Hypervisor.
> >
> >   * Proposal 2: Hypervisor runs the timer if the bios attr belongs to
> >     network configuration and once the timer expires,it reads the bios
> >     attr related to network and applies it.
> >
> >   * Proposal 3: Add one more bios attribute in the bios table which
> >     tells that Bios configuration can be read and applied by the
> >     Hypervisor for the network configuration.
>
> It is unfortunate that org.freedesktop.DBus.Properties doesn't have a
> way to set multiple properties as the analogous operation to 'GetAll'.

It was proposed we (OpenBMC) add one while back.  I think it muddies
the water of what it means to be a method call, and what it means to
be a property, especially for the use case that it was being proposed
to cover.

>
> In the case of networking, how do we handle this for the BMC settings?
> Don't we have a similar situation where multiple properties are changed
> via some interface and could leave the network in an unusual state?  I'm
> thinking IPMI does this.

I think today the behavior is that the network is left in an unusual
state until it's not.  If the config is invalid, and can't be
written(because of depdent properties) we just don't push it down
until it's valid.

>
> When all of our DBus objects were serial we likely never had this issue
> because the request to read the properties (to send to the hypervisor)
> would come behind the signal and subsequent property updates.  Now that
> we're moving towards more ASIO we likely will see this kind of issue
> more often.  I don't like it but we could certainly proposal a
> 'SetMultiple' extension to org.freedesktop or create our own interface.

If you have properties that need to be set in lockstep with one
another to be valid, I suspect that indicates that properties are not
the right tool.  Redfish hits this a lot, where each resource is
expected that any property is modifiable independently, and certain
implementations need an atomic "unit" of update.  bmcweb doesn't want
to have to cache properties that are collectively invalid right now,
but might become valid in the future, so there's an impasse.  Who
keeps the state while it's invalid?  Thus Far, that falls to the
dbus-daemons to store.

In terms of this issue, most (all?) ASIO clients are single threaded,
so I think you have the same dependencies.

>
> Proposal #2 isn't great because, well, how long do you wait?  In the
> case of hypervisor updates, delaying something on the order of a second
> is probably sufficient for Redfish/PLDM, but that doesn't really
> generally solve the problem.

This tends to be what I've recommended in the past (assuming the
"update" is computationally expensive).  If the update is
computationally "cheap", just go ahead and do it on every transaction.
Ideally PLDM would be modeled such that transactional changes are
"cheap", and don't require a full payload update on every property
change.

>
> We could define an interface to implement something like Proposal #1,
> but we would need a new interface and not a property we tack onto
> existing interfaces.  We'd probably need to revisit a lot of our
> interface definitions and see which ones typicallly have multi-property
> updates and does an intermediate state leave us in a bad situation.
>
> Specifically for BIOS/Hypervisor settings, I mentioned before that it
> isn't clear to me what the proposal is for applying Pending to Current.
> Again, this isn't general, but we could define an interface specific for
> BIOS/Hypervisor settings which has a way to indicate 'Pending
> transaction is complete' (set by entities like Redfish) and 'Pending
> values applied to Current' (set by entities like PLDM).  For the current
> settings-style values though, this requires external interfaces to
> somehow know that the setting is associated with the Host in order to do
> the application, since BMC-owned properties won't have or need this.

Dumb question: Does anyone actually need to know the "current" value?
Redfish certainly would need to return  the "pending" value in all
cases, as it's required so the restful API emulates ACID-like
compliance to the user.  Could we just have an optional interface that
indicates "values might not be loaded yet" and simplify the dbus API a
little?

>
> --
> Patrick Williams

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Using bios-settings-mgr for setting hypervisor network attributes
  2020-09-23 20:51               ` Ed Tanous
@ 2020-09-23 21:26                 ` Patrick Williams
  2020-09-24 13:08                   ` Deepak Kodihalli
  2020-09-24 15:36                   ` Ed Tanous
  0 siblings, 2 replies; 22+ messages in thread
From: Patrick Williams @ 2020-09-23 21:26 UTC (permalink / raw)
  To: Ed Tanous; +Cc: OpenBMC Maillist, Ratan Gupta


[-- Attachment #1: Type: text/plain, Size: 4276 bytes --]

On Wed, Sep 23, 2020 at 01:51:33PM -0700, Ed Tanous wrote:
> On Wed, Sep 23, 2020 at 12:24 PM Patrick Williams <patrick@stwcx.xyz> wrote:
> >
> > On Tue, Sep 22, 2020 at 02:39:04PM +0530, Ratan Gupta wrote:
> >
> > It is unfortunate that org.freedesktop.DBus.Properties doesn't have a
> > way to set multiple properties as the analogous operation to 'GetAll'.
> 
> It was proposed we (OpenBMC) add one while back.  I think it muddies
> the water of what it means to be a method call, and what it means to
> be a property, especially for the use case that it was being proposed
> to cover.

I'm not sure why it would be considered mudding the water.  All property
Get/Set/GetAll operations really are just a method call under the covers
anyhow to org.freedesktop.DBus.Properties.  I do think that ideally we'd
get the method added directly to that interface because then the DBus
bindings will support it natively.

I forgot the mention this again, but another way to solve it is similar
to xyz.openbmc_project.Inventory.Manager where you take a fully (or
partially) formed object as a method parameter and the process which
hosts Inventory.Manager hosts the object.  Settings could be done the
same way.  The issue is, again, having other processes know when to use
this new method and when to just update properties.

> > When all of our DBus objects were serial we likely never had this issue
> > because the request to read the properties (to send to the hypervisor)
> > would come behind the signal and subsequent property updates.  Now that
> > we're moving towards more ASIO we likely will see this kind of issue
> > more often.  I don't like it but we could certainly proposal a
> > 'SetMultiple' extension to org.freedesktop or create our own interface.
> 
> If you have properties that need to be set in lockstep with one
> another to be valid, I suspect that indicates that properties are not
> the right tool.  Redfish hits this a lot, where each resource is
> expected that any property is modifiable independently, and certain
> implementations need an atomic "unit" of update.  bmcweb doesn't want
> to have to cache properties that are collectively invalid right now,
> but might become valid in the future, so there's an impasse.  Who
> keeps the state while it's invalid?  Thus Far, that falls to the
> dbus-daemons to store.

Agreed.  This has also been a general statement  we've made in reviews
for new interfaces.  "If you need to update multiple properties, use
a method; if you are just updating a single property, update the property."

> > We could define an interface to implement something like Proposal #1,
> > but we would need a new interface and not a property we tack onto
> > existing interfaces.  We'd probably need to revisit a lot of our
> > interface definitions and see which ones typicallly have multi-property
> > updates and does an intermediate state leave us in a bad situation.
> >
> > Specifically for BIOS/Hypervisor settings, I mentioned before that it
> > isn't clear to me what the proposal is for applying Pending to Current.
> > Again, this isn't general, but we could define an interface specific for
> > BIOS/Hypervisor settings which has a way to indicate 'Pending
> > transaction is complete' (set by entities like Redfish) and 'Pending
> > values applied to Current' (set by entities like PLDM).  For the current
> > settings-style values though, this requires external interfaces to
> > somehow know that the setting is associated with the Host in order to do
> > the application, since BMC-owned properties won't have or need this.
> 
> Dumb question: Does anyone actually need to know the "current" value?
> Redfish certainly would need to return  the "pending" value in all
> cases, as it's required so the restful API emulates ACID-like
> compliance to the user.  Could we just have an optional interface that
> indicates "values might not be loaded yet" and simplify the dbus API a
> little?

I think this is generally for humans in the case of BIOS settings.
   - "What is the setting my system is currently running with?"
   - "What will happen next time I reboot?"

I don't know how this is modeled in Redfish.

-- 
Patrick Williams

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Using bios-settings-mgr for setting hypervisor network attributes
  2020-09-23 19:24             ` Patrick Williams
  2020-09-23 20:51               ` Ed Tanous
@ 2020-09-24  7:30               ` Ratan Gupta
  1 sibling, 0 replies; 22+ messages in thread
From: Ratan Gupta @ 2020-09-24  7:30 UTC (permalink / raw)
  To: Patrick Williams; +Cc: openbmc, Ed Tanous

On 9/24/20 12:54 AM, Patrick Williams wrote:
> On Tue, Sep 22, 2020 at 02:39:04PM +0530, Ratan Gupta wrote:
>> Hi All,
>>
>> Adding one more problem here with settings infra and with some proposed
>> solutions.
>>
>> Problem Domain:
>>
>>         - With multi property update from redfish , webserver updates the
>> settings object
>>         - PLDM on bmc listens on the property update of settings object
>> and notifies to Hypervisor
>>         - As there can be multiple properties in single PATCH operation,
>> PLDM on bmc sends
>>           multiple Notifications to Hypervisor
>>         - Specifically in case of network config,  single property update
>> on phyp may lead to network inconsistency.
> The original bios config seemed to only apply settings at specific times
> (ie. when the BIOS restarts) but your problem seems to indicate that
> you're immediately sending settings up to the host whenever they change?
Yes, you are correct we are sending immediately to the host.
>
>> How can we solve this?
>>
>>    * Proposal 1: Add one more property in the settings Dbus object itself
>>      which tells that it is ready to be read, PLDM on the BMC watching on
>>      that property and read the whole network configuration and notifies
>>      Hypervisor.
>>
>>    * Proposal 2: Hypervisor runs the timer if the bios attr belongs to
>>      network configuration and once the timer expires,it reads the bios
>>      attr related to network and applies it.
>>
>>    * Proposal 3: Add one more bios attribute in the bios table which
>>      tells that Bios configuration can be read and applied by the
>>      Hypervisor for the network configuration.
> It is unfortunate that org.freedesktop.DBus.Properties doesn't have a
> way to set multiple properties as the analogous operation to 'GetAll'.
>
> In the case of networking, how do we handle this for the BMC settings?
> Don't we have a similar situation where multiple properties are changed
> via some interface and could leave the network in an unusual state?  I'm
> thinking IPMI does this.

I hope you are asking for BMC network settings where we wait for a 
certain amount of time and then apply the settings,same with IPMI.

> When all of our DBus objects were serial we likely never had this issue
> because the request to read the properties (to send to the hypervisor)
> would come behind the signal and subsequent property updates.  Now that
> we're moving towards more ASIO we likely will see this kind of issue
> more often.  I don't like it but we could certainly proposal a
> 'SetMultiple' extension to org.freedesktop or create our own interface.
>
> Proposal #2 isn't great because, well, how long do you wait?  In the
> case of hypervisor updates, delaying something on the order of a second
> is probably sufficient for Redfish/PLDM, but that doesn't really
> generally solve the problem.
>
> We could define an interface to implement something like Proposal #1,
> but we would need a new interface and not a property we tack onto
> existing interfaces.  We'd probably need to revisit a lot of our
> interface definitions and see which ones typicallly have multi-property
> updates and does an intermediate state leave us in a bad situation.
My point was to add a property through new Dbus Interface, sorry for the 
confusion.
>
> Specifically for BIOS/Hypervisor settings, I mentioned before that it
> isn't clear to me what the proposal is for applying Pending to Current.
> Again, this isn't general, but we could define an interface specific for
> BIOS/Hypervisor settings which has a way to indicate 'Pending
> transaction is complete' (set by entities like Redfish) and 'Pending
> values applied to Current' (set by entities like PLDM).  For the current
> settings-style values though, this requires external interfaces to
> somehow know that the setting is associated with the Host in order to do
> the application, since BMC-owned properties won't have or need this.
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Using bios-settings-mgr for setting hypervisor network attributes
  2020-09-23 21:26                 ` Patrick Williams
@ 2020-09-24 13:08                   ` Deepak Kodihalli
  2020-09-24 15:36                   ` Ed Tanous
  1 sibling, 0 replies; 22+ messages in thread
From: Deepak Kodihalli @ 2020-09-24 13:08 UTC (permalink / raw)
  To: Patrick Williams, Ed Tanous; +Cc: OpenBMC Maillist, Ratan Gupta

On 24/09/20 2:56 am, Patrick Williams wrote:
> On Wed, Sep 23, 2020 at 01:51:33PM -0700, Ed Tanous wrote:
>> On Wed, Sep 23, 2020 at 12:24 PM Patrick Williams <patrick@stwcx.xyz> wrote:
>>>
>>> On Tue, Sep 22, 2020 at 02:39:04PM +0530, Ratan Gupta wrote:
>>>
>>> It is unfortunate that org.freedesktop.DBus.Properties doesn't have a
>>> way to set multiple properties as the analogous operation to 'GetAll'.
>>
>> It was proposed we (OpenBMC) add one while back.  I think it muddies
>> the water of what it means to be a method call, and what it means to
>> be a property, especially for the use case that it was being proposed
>> to cover.
> 
> I'm not sure why it would be considered mudding the water.  All property
> Get/Set/GetAll operations really are just a method call under the covers
> anyhow to org.freedesktop.DBus.Properties.  I do think that ideally we'd
> get the method added directly to that interface because then the DBus
> bindings will support it natively.

I had proposed 
https://gerrit.openbmc-project.xyz/#/c/openbmc/phosphor-dbus-interfaces/+/12861/ 
a while back but there were concerns expressed in the review.

> I forgot the mention this again, but another way to solve it is similar
> to xyz.openbmc_project.Inventory.Manager where you take a fully (or
> partially) formed object as a method parameter and the process which
> hosts Inventory.Manager hosts the object.  Settings could be done the
> same way.  The issue is, again, having other processes know when to use
> this new method and when to just update properties.
> 
>>> When all of our DBus objects were serial we likely never had this issue
>>> because the request to read the properties (to send to the hypervisor)
>>> would come behind the signal and subsequent property updates.  Now that
>>> we're moving towards more ASIO we likely will see this kind of issue
>>> more often.  I don't like it but we could certainly proposal a
>>> 'SetMultiple' extension to org.freedesktop or create our own interface.
>>
>> If you have properties that need to be set in lockstep with one
>> another to be valid, I suspect that indicates that properties are not
>> the right tool.  Redfish hits this a lot, where each resource is
>> expected that any property is modifiable independently, and certain
>> implementations need an atomic "unit" of update.  bmcweb doesn't want
>> to have to cache properties that are collectively invalid right now,
>> but might become valid in the future, so there's an impasse.  Who
>> keeps the state while it's invalid?  Thus Far, that falls to the
>> dbus-daemons to store.
> 
> Agreed.  This has also been a general statement  we've made in reviews
> for new interfaces.  "If you need to update multiple properties, use
> a method; if you are just updating a single property, update the property."

Can we add a method to 
https://github.com/openbmc/phosphor-dbus-interfaces/blob/master/xyz/openbmc_project/Network/EthernetInterface.interface.yaml 
to update multiple properties? Also, how does one ensure these updates 
via the method result in a single PropertiesChanged event on D-Bus. Is 
that implicit D-Bus behavior, or does the binding need to enable this?

>>> We could define an interface to implement something like Proposal #1,
>>> but we would need a new interface and not a property we tack onto
>>> existing interfaces.  We'd probably need to revisit a lot of our
>>> interface definitions and see which ones typicallly have multi-property
>>> updates and does an intermediate state leave us in a bad situation.
>>>
>>> Specifically for BIOS/Hypervisor settings, I mentioned before that it
>>> isn't clear to me what the proposal is for applying Pending to Current.
>>> Again, this isn't general, but we could define an interface specific for
>>> BIOS/Hypervisor settings which has a way to indicate 'Pending
>>> transaction is complete' (set by entities like Redfish) and 'Pending
>>> values applied to Current' (set by entities like PLDM).  For the current
>>> settings-style values though, this requires external interfaces to
>>> somehow know that the setting is associated with the Host in order to do
>>> the application, since BMC-owned properties won't have or need this.
>>
>> Dumb question: Does anyone actually need to know the "current" value?
>> Redfish certainly would need to return  the "pending" value in all
>> cases, as it's required so the restful API emulates ACID-like
>> compliance to the user.  Could we just have an optional interface that
>> indicates "values might not be loaded yet" and simplify the dbus API a
>> little?
> 
> I think this is generally for humans in the case of BIOS settings.
>     - "What is the setting my system is currently running with?"
>     - "What will happen next time I reboot?"
> 
> I don't know how this is modeled in Redfish.

I believe there is a Bios.Attributes for currently applied BIOS settings 
and Bios.Settings for what will be applied post a system reset.



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Using bios-settings-mgr for setting hypervisor network attributes
  2020-09-23 21:26                 ` Patrick Williams
  2020-09-24 13:08                   ` Deepak Kodihalli
@ 2020-09-24 15:36                   ` Ed Tanous
  2020-09-30 15:05                     ` Ratan Gupta
  1 sibling, 1 reply; 22+ messages in thread
From: Ed Tanous @ 2020-09-24 15:36 UTC (permalink / raw)
  To: Patrick Williams; +Cc: OpenBMC Maillist, Ratan Gupta

On Wed, Sep 23, 2020 at 2:26 PM Patrick Williams <patrick@stwcx.xyz> wrote:
>
> On Wed, Sep 23, 2020 at 01:51:33PM -0700, Ed Tanous wrote:
> > On Wed, Sep 23, 2020 at 12:24 PM Patrick Williams <patrick@stwcx.xyz> wrote:
> > >
> > > On Tue, Sep 22, 2020 at 02:39:04PM +0530, Ratan Gupta wrote:
> > >
> > > It is unfortunate that org.freedesktop.DBus.Properties doesn't have a
> > > way to set multiple properties as the analogous operation to 'GetAll'.
> >
> > It was proposed we (OpenBMC) add one while back.  I think it muddies
> > the water of what it means to be a method call, and what it means to
> > be a property, especially for the use case that it was being proposed
> > to cover.
>
> I'm not sure why it would be considered mudding the water.  All property
> Get/Set/GetAll operations really are just a method call under the covers
> anyhow to org.freedesktop.DBus.Properties.  I do think that ideally we'd
> get the method added directly to that interface because then the DBus
> bindings will support it natively.

Mudding the water of when to use a property, versus when to use a
method call (yes, properties are method calls underneath).  If there's
a method call, the dependency between the parameters is documented in
the interface, with a SetProperties method call, it isn't, and you
have to rely on just knowing, or it being implementation defined.  In
those cases, I'd much rather the itnerface make the jump straight to a
method call, and skip properties entirely.

>
> I forgot the mention this again, but another way to solve it is similar
> to xyz.openbmc_project.Inventory.Manager where you take a fully (or
> partially) formed object as a method parameter and the process which
> hosts Inventory.Manager hosts the object.  Settings could be done the
> same way.  The issue is, again, having other processes know when to use
> this new method and when to just update properties.

This tends to be the pattern we use.  My usual take on it when I see a
new interface is, if the create method exists, use it.

>
> > > When all of our DBus objects were serial we likely never had this issue
> > > because the request to read the properties (to send to the hypervisor)
> > > would come behind the signal and subsequent property updates.  Now that
> > > we're moving towards more ASIO we likely will see this kind of issue
> > > more often.  I don't like it but we could certainly proposal a
> > > 'SetMultiple' extension to org.freedesktop or create our own interface.
> >
> > If you have properties that need to be set in lockstep with one
> > another to be valid, I suspect that indicates that properties are not
> > the right tool.  Redfish hits this a lot, where each resource is
> > expected that any property is modifiable independently, and certain
> > implementations need an atomic "unit" of update.  bmcweb doesn't want
> > to have to cache properties that are collectively invalid right now,
> > but might become valid in the future, so there's an impasse.  Who
> > keeps the state while it's invalid?  Thus Far, that falls to the
> > dbus-daemons to store.
>
> Agreed.  This has also been a general statement  we've made in reviews
> for new interfaces.  "If you need to update multiple properties, use
> a method; if you are just updating a single property, update the property."

+1

>
> > > We could define an interface to implement something like Proposal #1,
> > > but we would need a new interface and not a property we tack onto
> > > existing interfaces.  We'd probably need to revisit a lot of our
> > > interface definitions and see which ones typicallly have multi-property
> > > updates and does an intermediate state leave us in a bad situation.
> > >
> > > Specifically for BIOS/Hypervisor settings, I mentioned before that it
> > > isn't clear to me what the proposal is for applying Pending to Current.
> > > Again, this isn't general, but we could define an interface specific for
> > > BIOS/Hypervisor settings which has a way to indicate 'Pending
> > > transaction is complete' (set by entities like Redfish) and 'Pending
> > > values applied to Current' (set by entities like PLDM).  For the current
> > > settings-style values though, this requires external interfaces to
> > > somehow know that the setting is associated with the Host in order to do
> > > the application, since BMC-owned properties won't have or need this.
> >
> > Dumb question: Does anyone actually need to know the "current" value?
> > Redfish certainly would need to return  the "pending" value in all
> > cases, as it's required so the restful API emulates ACID-like
> > compliance to the user.  Could we just have an optional interface that
> > indicates "values might not be loaded yet" and simplify the dbus API a
> > little?
>
> I think this is generally for humans in the case of BIOS settings.
>    - "What is the setting my system is currently running with?"
>    - "What will happen next time I reboot?"

I wonder if we could make a logging API for humans to use, and keep
the "present" things off dbus.  It seems like it would simplify the
implementation quite a bit. <thinking out loud a little>

>
> I don't know how this is modeled in Redfish.
>
> --
> Patrick Williams

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Using bios-settings-mgr for setting hypervisor network attributes
  2020-09-24 15:36                   ` Ed Tanous
@ 2020-09-30 15:05                     ` Ratan Gupta
  2020-09-30 15:56                       ` Ed Tanous
  0 siblings, 1 reply; 22+ messages in thread
From: Ratan Gupta @ 2020-09-30 15:05 UTC (permalink / raw)
  To: openbmc, Ed Tanous, Patrick Williams

Thanks all for providing the suggestions

Currently Redfish Ethernet interface is not having the concept of 
pending and configured values,That means if we use the EthernetInterface 
schema, User can only see the configured values, There is no way through 
which user can see the pending value, We need to come up with some REST 
API to show the pending values.

To solve this problem, Redfish has bios schema whch has the pending 
attributes as well as the configured attributes

How about using the Redfish Bios schema for front end and Bios-settings 
manager as backend to make the things simpler?

Ratan

On 9/24/20 9:06 PM, Ed Tanous wrote:
> On Wed, Sep 23, 2020 at 2:26 PM Patrick Williams <patrick@stwcx.xyz> wrote:
>> On Wed, Sep 23, 2020 at 01:51:33PM -0700, Ed Tanous wrote:
>>> On Wed, Sep 23, 2020 at 12:24 PM Patrick Williams <patrick@stwcx.xyz> wrote:
>>>> On Tue, Sep 22, 2020 at 02:39:04PM +0530, Ratan Gupta wrote:
>>>>
>>>> It is unfortunate that org.freedesktop.DBus.Properties doesn't have a
>>>> way to set multiple properties as the analogous operation to 'GetAll'.
>>> It was proposed we (OpenBMC) add one while back.  I think it muddies
>>> the water of what it means to be a method call, and what it means to
>>> be a property, especially for the use case that it was being proposed
>>> to cover.
>> I'm not sure why it would be considered mudding the water.  All property
>> Get/Set/GetAll operations really are just a method call under the covers
>> anyhow to org.freedesktop.DBus.Properties.  I do think that ideally we'd
>> get the method added directly to that interface because then the DBus
>> bindings will support it natively.
> Mudding the water of when to use a property, versus when to use a
> method call (yes, properties are method calls underneath).  If there's
> a method call, the dependency between the parameters is documented in
> the interface, with a SetProperties method call, it isn't, and you
> have to rely on just knowing, or it being implementation defined.  In
> those cases, I'd much rather the itnerface make the jump straight to a
> method call, and skip properties entirely.
>
>> I forgot the mention this again, but another way to solve it is similar
>> to xyz.openbmc_project.Inventory.Manager where you take a fully (or
>> partially) formed object as a method parameter and the process which
>> hosts Inventory.Manager hosts the object.  Settings could be done the
>> same way.  The issue is, again, having other processes know when to use
>> this new method and when to just update properties.
> This tends to be the pattern we use.  My usual take on it when I see a
> new interface is, if the create method exists, use it.
>
>>>> When all of our DBus objects were serial we likely never had this issue
>>>> because the request to read the properties (to send to the hypervisor)
>>>> would come behind the signal and subsequent property updates.  Now that
>>>> we're moving towards more ASIO we likely will see this kind of issue
>>>> more often.  I don't like it but we could certainly proposal a
>>>> 'SetMultiple' extension to org.freedesktop or create our own interface.
>>> If you have properties that need to be set in lockstep with one
>>> another to be valid, I suspect that indicates that properties are not
>>> the right tool.  Redfish hits this a lot, where each resource is
>>> expected that any property is modifiable independently, and certain
>>> implementations need an atomic "unit" of update.  bmcweb doesn't want
>>> to have to cache properties that are collectively invalid right now,
>>> but might become valid in the future, so there's an impasse.  Who
>>> keeps the state while it's invalid?  Thus Far, that falls to the
>>> dbus-daemons to store.
>> Agreed.  This has also been a general statement  we've made in reviews
>> for new interfaces.  "If you need to update multiple properties, use
>> a method; if you are just updating a single property, update the property."
> +1
>
>>>> We could define an interface to implement something like Proposal #1,
>>>> but we would need a new interface and not a property we tack onto
>>>> existing interfaces.  We'd probably need to revisit a lot of our
>>>> interface definitions and see which ones typicallly have multi-property
>>>> updates and does an intermediate state leave us in a bad situation.
>>>>
>>>> Specifically for BIOS/Hypervisor settings, I mentioned before that it
>>>> isn't clear to me what the proposal is for applying Pending to Current.
>>>> Again, this isn't general, but we could define an interface specific for
>>>> BIOS/Hypervisor settings which has a way to indicate 'Pending
>>>> transaction is complete' (set by entities like Redfish) and 'Pending
>>>> values applied to Current' (set by entities like PLDM).  For the current
>>>> settings-style values though, this requires external interfaces to
>>>> somehow know that the setting is associated with the Host in order to do
>>>> the application, since BMC-owned properties won't have or need this.
>>> Dumb question: Does anyone actually need to know the "current" value?
>>> Redfish certainly would need to return  the "pending" value in all
>>> cases, as it's required so the restful API emulates ACID-like
>>> compliance to the user.  Could we just have an optional interface that
>>> indicates "values might not be loaded yet" and simplify the dbus API a
>>> little?
>> I think this is generally for humans in the case of BIOS settings.
>>     - "What is the setting my system is currently running with?"
>>     - "What will happen next time I reboot?"
> I wonder if we could make a logging API for humans to use, and keep
> the "present" things off dbus.  It seems like it would simplify the
> implementation quite a bit. <thinking out loud a little>
>
>> I don't know how this is modeled in Redfish.
>>
>> --
>> Patrick Williams

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Using bios-settings-mgr for setting hypervisor network attributes
  2020-09-30 15:05                     ` Ratan Gupta
@ 2020-09-30 15:56                       ` Ed Tanous
  2020-10-01 11:17                         ` Ratan Gupta
  0 siblings, 1 reply; 22+ messages in thread
From: Ed Tanous @ 2020-09-30 15:56 UTC (permalink / raw)
  To: Ratan Gupta; +Cc: OpenBMC Maillist

On Wed, Sep 30, 2020 at 8:05 AM Ratan Gupta <ratagupt@linux.vnet.ibm.com> wrote:
>
> Thanks all for providing the suggestions
>
> Currently Redfish Ethernet interface is not having the concept of
> pending and configured values,That means if we use the EthernetInterface
> schema, User can only see the configured values, There is no way through
> which user can see the pending value, We need to come up with some REST
> API to show the pending values.
>
> To solve this problem, Redfish has bios schema whch has the pending
> attributes as well as the configured attributes

Did not realize that about the Redfish schema.  Sounds like we need both then.

>
> How about using the Redfish Bios schema for front end and Bios-settings
> manager as backend to make the things simpler?

I'm not quite following.  Are you saying put the pending settings in
the webserver?

>
> Ratan
>
> On 9/24/20 9:06 PM, Ed Tanous wrote:
> > On Wed, Sep 23, 2020 at 2:26 PM Patrick Williams <patrick@stwcx.xyz> wrote:
> >> On Wed, Sep 23, 2020 at 01:51:33PM -0700, Ed Tanous wrote:
> >>> On Wed, Sep 23, 2020 at 12:24 PM Patrick Williams <patrick@stwcx.xyz> wrote:
> >>>> On Tue, Sep 22, 2020 at 02:39:04PM +0530, Ratan Gupta wrote:
> >>>>
> >>>> It is unfortunate that org.freedesktop.DBus.Properties doesn't have a
> >>>> way to set multiple properties as the analogous operation to 'GetAll'.
> >>> It was proposed we (OpenBMC) add one while back.  I think it muddies
> >>> the water of what it means to be a method call, and what it means to
> >>> be a property, especially for the use case that it was being proposed
> >>> to cover.
> >> I'm not sure why it would be considered mudding the water.  All property
> >> Get/Set/GetAll operations really are just a method call under the covers
> >> anyhow to org.freedesktop.DBus.Properties.  I do think that ideally we'd
> >> get the method added directly to that interface because then the DBus
> >> bindings will support it natively.
> > Mudding the water of when to use a property, versus when to use a
> > method call (yes, properties are method calls underneath).  If there's
> > a method call, the dependency between the parameters is documented in
> > the interface, with a SetProperties method call, it isn't, and you
> > have to rely on just knowing, or it being implementation defined.  In
> > those cases, I'd much rather the itnerface make the jump straight to a
> > method call, and skip properties entirely.
> >
> >> I forgot the mention this again, but another way to solve it is similar
> >> to xyz.openbmc_project.Inventory.Manager where you take a fully (or
> >> partially) formed object as a method parameter and the process which
> >> hosts Inventory.Manager hosts the object.  Settings could be done the
> >> same way.  The issue is, again, having other processes know when to use
> >> this new method and when to just update properties.
> > This tends to be the pattern we use.  My usual take on it when I see a
> > new interface is, if the create method exists, use it.
> >
> >>>> When all of our DBus objects were serial we likely never had this issue
> >>>> because the request to read the properties (to send to the hypervisor)
> >>>> would come behind the signal and subsequent property updates.  Now that
> >>>> we're moving towards more ASIO we likely will see this kind of issue
> >>>> more often.  I don't like it but we could certainly proposal a
> >>>> 'SetMultiple' extension to org.freedesktop or create our own interface.
> >>> If you have properties that need to be set in lockstep with one
> >>> another to be valid, I suspect that indicates that properties are not
> >>> the right tool.  Redfish hits this a lot, where each resource is
> >>> expected that any property is modifiable independently, and certain
> >>> implementations need an atomic "unit" of update.  bmcweb doesn't want
> >>> to have to cache properties that are collectively invalid right now,
> >>> but might become valid in the future, so there's an impasse.  Who
> >>> keeps the state while it's invalid?  Thus Far, that falls to the
> >>> dbus-daemons to store.
> >> Agreed.  This has also been a general statement  we've made in reviews
> >> for new interfaces.  "If you need to update multiple properties, use
> >> a method; if you are just updating a single property, update the property."
> > +1
> >
> >>>> We could define an interface to implement something like Proposal #1,
> >>>> but we would need a new interface and not a property we tack onto
> >>>> existing interfaces.  We'd probably need to revisit a lot of our
> >>>> interface definitions and see which ones typicallly have multi-property
> >>>> updates and does an intermediate state leave us in a bad situation.
> >>>>
> >>>> Specifically for BIOS/Hypervisor settings, I mentioned before that it
> >>>> isn't clear to me what the proposal is for applying Pending to Current.
> >>>> Again, this isn't general, but we could define an interface specific for
> >>>> BIOS/Hypervisor settings which has a way to indicate 'Pending
> >>>> transaction is complete' (set by entities like Redfish) and 'Pending
> >>>> values applied to Current' (set by entities like PLDM).  For the current
> >>>> settings-style values though, this requires external interfaces to
> >>>> somehow know that the setting is associated with the Host in order to do
> >>>> the application, since BMC-owned properties won't have or need this.
> >>> Dumb question: Does anyone actually need to know the "current" value?
> >>> Redfish certainly would need to return  the "pending" value in all
> >>> cases, as it's required so the restful API emulates ACID-like
> >>> compliance to the user.  Could we just have an optional interface that
> >>> indicates "values might not be loaded yet" and simplify the dbus API a
> >>> little?
> >> I think this is generally for humans in the case of BIOS settings.
> >>     - "What is the setting my system is currently running with?"
> >>     - "What will happen next time I reboot?"
> > I wonder if we could make a logging API for humans to use, and keep
> > the "present" things off dbus.  It seems like it would simplify the
> > implementation quite a bit. <thinking out loud a little>
> >
> >> I don't know how this is modeled in Redfish.
> >>
> >> --
> >> Patrick Williams

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Using bios-settings-mgr for setting hypervisor network attributes
  2020-09-30 15:56                       ` Ed Tanous
@ 2020-10-01 11:17                         ` Ratan Gupta
  2020-10-16 11:40                           ` manoj kiran
  0 siblings, 1 reply; 22+ messages in thread
From: Ratan Gupta @ 2020-10-01 11:17 UTC (permalink / raw)
  To: Ed Tanous; +Cc: OpenBMC Maillist


[-- Attachment #1: Type: text/plain, Size: 7234 bytes --]

On 9/30/20 9:26 PM, Ed Tanous wrote:
> On Wed, Sep 30, 2020 at 8:05 AM Ratan Gupta <ratagupt@linux.vnet.ibm.com> wrote:
>> Thanks all for providing the suggestions
>>
>> Currently Redfish Ethernet interface is not having the concept of
>> pending and configured values,That means if we use the EthernetInterface
>> schema, User can only see the configured values, There is no way through
>> which user can see the pending value, We need to come up with some REST
>> API to show the pending values.
>>
>> To solve this problem, Redfish has bios schema whch has the pending
>> attributes as well as the configured attributes
> Did not realize that about the Redfish schema.  Sounds like we need both then.

https://redfish.dmtf.org/schemas/v1/Bios.v1_1_1.json

The Bios schema contains properties related to the BIOS attribute 
registry. The attribute registry describes the system-specific BIOS 
attributes and actions for changing to BIOS settings. Changes to the 
BIOS typically require a system reset before they take effect. It is 
likely that a client finds the `@Redfish.Settings` term in this 
resource, and if it is found, the client makes requests to change BIOS 
settings by modifying the resource identified by the `@Redfish.Settings` 
term."

>
>> How about using the Redfish Bios schema for front end and Bios-settings
>> manager as backend to make the things simpler?
> I'm not quite following.  Are you saying put the pending settings in
> the webserver?

No, I was mentioning that instead of using the EthernetInterface schema 
, Can we use theBios schema for the network configuration and this bios 
schema is backed up with bios-settings manager D-bus Repo.

https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/29670

https://gerrit.openbmc-project.xyz/c/openbmc/bios-settings-mgr/+/35563

>> Ratan
>>
>> On 9/24/20 9:06 PM, Ed Tanous wrote:
>>> On Wed, Sep 23, 2020 at 2:26 PM Patrick Williams <patrick@stwcx.xyz> wrote:
>>>> On Wed, Sep 23, 2020 at 01:51:33PM -0700, Ed Tanous wrote:
>>>>> On Wed, Sep 23, 2020 at 12:24 PM Patrick Williams <patrick@stwcx.xyz> wrote:
>>>>>> On Tue, Sep 22, 2020 at 02:39:04PM +0530, Ratan Gupta wrote:
>>>>>>
>>>>>> It is unfortunate that org.freedesktop.DBus.Properties doesn't have a
>>>>>> way to set multiple properties as the analogous operation to 'GetAll'.
>>>>> It was proposed we (OpenBMC) add one while back.  I think it muddies
>>>>> the water of what it means to be a method call, and what it means to
>>>>> be a property, especially for the use case that it was being proposed
>>>>> to cover.
>>>> I'm not sure why it would be considered mudding the water.  All property
>>>> Get/Set/GetAll operations really are just a method call under the covers
>>>> anyhow to org.freedesktop.DBus.Properties.  I do think that ideally we'd
>>>> get the method added directly to that interface because then the DBus
>>>> bindings will support it natively.
>>> Mudding the water of when to use a property, versus when to use a
>>> method call (yes, properties are method calls underneath).  If there's
>>> a method call, the dependency between the parameters is documented in
>>> the interface, with a SetProperties method call, it isn't, and you
>>> have to rely on just knowing, or it being implementation defined.  In
>>> those cases, I'd much rather the itnerface make the jump straight to a
>>> method call, and skip properties entirely.
>>>
>>>> I forgot the mention this again, but another way to solve it is similar
>>>> to xyz.openbmc_project.Inventory.Manager where you take a fully (or
>>>> partially) formed object as a method parameter and the process which
>>>> hosts Inventory.Manager hosts the object.  Settings could be done the
>>>> same way.  The issue is, again, having other processes know when to use
>>>> this new method and when to just update properties.
>>> This tends to be the pattern we use.  My usual take on it when I see a
>>> new interface is, if the create method exists, use it.
>>>
>>>>>> When all of our DBus objects were serial we likely never had this issue
>>>>>> because the request to read the properties (to send to the hypervisor)
>>>>>> would come behind the signal and subsequent property updates.  Now that
>>>>>> we're moving towards more ASIO we likely will see this kind of issue
>>>>>> more often.  I don't like it but we could certainly proposal a
>>>>>> 'SetMultiple' extension to org.freedesktop or create our own interface.
>>>>> If you have properties that need to be set in lockstep with one
>>>>> another to be valid, I suspect that indicates that properties are not
>>>>> the right tool.  Redfish hits this a lot, where each resource is
>>>>> expected that any property is modifiable independently, and certain
>>>>> implementations need an atomic "unit" of update.  bmcweb doesn't want
>>>>> to have to cache properties that are collectively invalid right now,
>>>>> but might become valid in the future, so there's an impasse.  Who
>>>>> keeps the state while it's invalid?  Thus Far, that falls to the
>>>>> dbus-daemons to store.
>>>> Agreed.  This has also been a general statement  we've made in reviews
>>>> for new interfaces.  "If you need to update multiple properties, use
>>>> a method; if you are just updating a single property, update the property."
>>> +1
>>>
>>>>>> We could define an interface to implement something like Proposal #1,
>>>>>> but we would need a new interface and not a property we tack onto
>>>>>> existing interfaces.  We'd probably need to revisit a lot of our
>>>>>> interface definitions and see which ones typicallly have multi-property
>>>>>> updates and does an intermediate state leave us in a bad situation.
>>>>>>
>>>>>> Specifically for BIOS/Hypervisor settings, I mentioned before that it
>>>>>> isn't clear to me what the proposal is for applying Pending to Current.
>>>>>> Again, this isn't general, but we could define an interface specific for
>>>>>> BIOS/Hypervisor settings which has a way to indicate 'Pending
>>>>>> transaction is complete' (set by entities like Redfish) and 'Pending
>>>>>> values applied to Current' (set by entities like PLDM).  For the current
>>>>>> settings-style values though, this requires external interfaces to
>>>>>> somehow know that the setting is associated with the Host in order to do
>>>>>> the application, since BMC-owned properties won't have or need this.
>>>>> Dumb question: Does anyone actually need to know the "current" value?
>>>>> Redfish certainly would need to return  the "pending" value in all
>>>>> cases, as it's required so the restful API emulates ACID-like
>>>>> compliance to the user.  Could we just have an optional interface that
>>>>> indicates "values might not be loaded yet" and simplify the dbus API a
>>>>> little?
>>>> I think this is generally for humans in the case of BIOS settings.
>>>>      - "What is the setting my system is currently running with?"
>>>>      - "What will happen next time I reboot?"
>>> I wonder if we could make a logging API for humans to use, and keep
>>> the "present" things off dbus.  It seems like it would simplify the
>>> implementation quite a bit. <thinking out loud a little>
>>>
>>>> I don't know how this is modeled in Redfish.
>>>>
>>>> --
>>>> Patrick Williams

[-- Attachment #2: Type: text/html, Size: 10433 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Using bios-settings-mgr for setting hypervisor network attributes
  2020-10-01 11:17                         ` Ratan Gupta
@ 2020-10-16 11:40                           ` manoj kiran
  0 siblings, 0 replies; 22+ messages in thread
From: manoj kiran @ 2020-10-16 11:40 UTC (permalink / raw)
  To: Ratan Gupta, Ed Tanous; +Cc: OpenBMC Maillist

Hi Ed/Ratan, 

Just bumping this thread again to see if we can get to a conclusion on
this problem.

Thanks,
Manoj

On Oct 1 2020, at 4:47 pm, Ratan Gupta <ratagupt@linux.vnet.ibm.com> wrote:

> On 9/30/20 9:26 PM, Ed Tanous wrote:
> 
>> On Wed, Sep 30, 2020 at 8:05 AM Ratan Gupta
>> <ratagupt@linux.vnet.ibm.com> wrote:
>> 
>> 
>>> Thanks all for providing the suggestions
>>> 
>>> Currently Redfish Ethernet interface is not having the concept of
>>> pending and configured values,That means if we use the EthernetInterface
>>> schema, User can only see the configured values, There is no way through
>>> which user can see the pending value, We need to come up with some REST
>>> API to show the pending values.
>>> 
>>> To solve this problem, Redfish has bios schema whch has the pending
>>> attributes as well as the configured attributes
>>> 
>> Did not realize that about the Redfish schema.  Sounds like we need
>> both then.
> https://redfish.dmtf.org/schemas/v1/Bios.v1_1_1.json
> 
> The Bios schema contains properties related to the BIOS attribute
> registry. The attribute registry describes the system-specific BIOS
> attributes and actions for changing to BIOS settings. Changes to the
> BIOS typically require a system reset before they take effect. It is
> likely that a client finds the `@Redfish.Settings` term in this
> resource, and if it is found, the client makes requests to change BIOS
> settings by modifying the resource identified by the
> `@Redfish.Settings` term."
> 
> 
>> 
>> 
>>> How about using the Redfish Bios schema for front end and Bios-settings
>>> manager as backend to make the things simpler?
>>> 
>> I'm not quite following.  Are you saying put the pending settings in
>> the webserver?
>> 
> No, I was mentioning that instead of using the EthernetInterface
> schema , Can we use theBios schema for the network configuration and
> this bios schema is backed up with bios-settings manager D-bus Repo.
> 
> https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/29670
> 
> https://gerrit.openbmc-project.xyz/c/openbmc/bios-settings-mgr/+/35563
> 
> 
>> 
>> 
>>> Ratan
>>> 
>>> On 9/24/20 9:06 PM, Ed Tanous wrote:
>>> 
>>> 
>>>> On Wed, Sep 23, 2020 at 2:26 PM Patrick Williams
>>>> <patrick@stwcx.xyz> wrote:
>>>> 
>>>> 
>>>>> On Wed, Sep 23, 2020 at 01:51:33PM -0700, Ed Tanous wrote:
>>>>> 
>>>>> 
>>>>>> On Wed, Sep 23, 2020 at 12:24 PM Patrick Williams
>>>>>> <patrick@stwcx.xyz> wrote:
>>>>>> 
>>>>>> 
>>>>>>> On Tue, Sep 22, 2020 at 02:39:04PM +0530, Ratan Gupta wrote:
>>>>>>> 
>>>>>>> It is unfortunate that org.freedesktop.DBus.Properties doesn't
>>>>>>> have a
>>>>>>> way to set multiple properties as the analogous operation to 'GetAll'.
>>>>>>> 
>>>>>> It was proposed we (OpenBMC) add one while back.  I think it muddies
>>>>>> the water of what it means to be a method call, and what it means to
>>>>>> be a property, especially for the use case that it was being proposed
>>>>>> to cover.
>>>>>> 
>>>>> I'm not sure why it would be considered mudding the water.  All property
>>>>> Get/Set/GetAll operations really are just a method call under the covers
>>>>> anyhow to org.freedesktop.DBus.Properties.  I do think that
>>>>> ideally we'd
>>>>> get the method added directly to that interface because then the DBus
>>>>> bindings will support it natively.
>>>>> 
>>>> Mudding the water of when to use a property, versus when to use a
>>>> method call (yes, properties are method calls underneath).  If there's
>>>> a method call, the dependency between the parameters is documented in
>>>> the interface, with a SetProperties method call, it isn't, and you
>>>> have to rely on just knowing, or it being implementation defined.  In
>>>> those cases, I'd much rather the itnerface make the jump straight
>>>> to a
>>>> method call, and skip properties entirely.
>>>> 
>>>> 
>>>> 
>>>>> I forgot the mention this again, but another way to solve it is similar
>>>>> to xyz.openbmc_project.Inventory.Manager where you take a fully (or
>>>>> partially) formed object as a method parameter and the process which
>>>>> hosts Inventory.Manager hosts the object.  Settings could be done the
>>>>> same way.  The issue is, again, having other processes know when
>>>>> to use
>>>>> this new method and when to just update properties.
>>>>> 
>>>> This tends to be the pattern we use.  My usual take on it when I
>>>> see a
>>>> new interface is, if the create method exists, use it.
>>>> 
>>>> 
>>>> 
>>>>> 
>>>>>> 
>>>>>>> When all of our DBus objects were serial we likely never had
>>>>>>> this issue
>>>>>>> because the request to read the properties (to send to the hypervisor)
>>>>>>> would come behind the signal and subsequent property updates. 
>>>>>>> Now that
>>>>>>> we're moving towards more ASIO we likely will see this kind of issue
>>>>>>> more often.  I don't like it but we could certainly proposal a
>>>>>>> 'SetMultiple' extension to org.freedesktop or create our own interface.
>>>>>>> 
>>>>>> If you have properties that need to be set in lockstep with one
>>>>>> another to be valid, I suspect that indicates that properties are not
>>>>>> the right tool.  Redfish hits this a lot, where each resource is
>>>>>> expected that any property is modifiable independently, and certain
>>>>>> implementations need an atomic "unit" of update.  bmcweb doesn't want
>>>>>> to have to cache properties that are collectively invalid right now,
>>>>>> but might become valid in the future, so there's an impasse.  Who
>>>>>> keeps the state while it's invalid?  Thus Far, that falls to the
>>>>>> dbus-daemons to store.
>>>>>> 
>>>>> Agreed.  This has also been a general statement  we've made in reviews
>>>>> for new interfaces.  "If you need to update multiple properties, use
>>>>> a method; if you are just updating a single property, update the property."
>>>>> 
>>>> +1
>>>> 
>>>> 
>>>> 
>>>>> 
>>>>>> 
>>>>>>> We could define an interface to implement something like
>>>>>>> Proposal #1,
>>>>>>> but we would need a new interface and not a property we tack onto
>>>>>>> existing interfaces.  We'd probably need to revisit a lot of our
>>>>>>> interface definitions and see which ones typicallly have multi-property
>>>>>>> updates and does an intermediate state leave us in a bad situation.
>>>>>>> 
>>>>>>> Specifically for BIOS/Hypervisor settings, I mentioned before
>>>>>>> that it
>>>>>>> isn't clear to me what the proposal is for applying Pending to Current.
>>>>>>> Again, this isn't general, but we could define an interface
>>>>>>> specific for
>>>>>>> BIOS/Hypervisor settings which has a way to indicate 'Pending
>>>>>>> transaction is complete' (set by entities like Redfish) and 'Pending
>>>>>>> values applied to Current' (set by entities like PLDM).  For the current
>>>>>>> settings-style values though, this requires external interfaces to
>>>>>>> somehow know that the setting is associated with the Host in
>>>>>>> order to do
>>>>>>> the application, since BMC-owned properties won't have or need this.
>>>>>>> 
>>>>>> Dumb question: Does anyone actually need to know the "current" value?
>>>>>> Redfish certainly would need to return  the "pending" value in all
>>>>>> cases, as it's required so the restful API emulates ACID-like
>>>>>> compliance to the user.  Could we just have an optional interface that
>>>>>> indicates "values might not be loaded yet" and simplify the dbus
>>>>>> API a
>>>>>> little?
>>>>>> 
>>>>> I think this is generally for humans in the case of BIOS settings.
>>>>>    - "What is the setting my system is currently running with?"
>>>>>    - "What will happen next time I reboot?"
>>>>> 
>>>> I wonder if we could make a logging API for humans to use, and keep
>>>> the "present" things off dbus.  It seems like it would simplify the
>>>> implementation quite a bit. <thinking out loud a little>
>>>> 
>>>> 
>>>> 
>>>>> I don't know how this is modeled in Redfish.
>>>>> 
>>>>> --
>>>>> Patrick Williams
>>>>> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, back to index

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-16 14:47 Using bios-settings-mgr for setting hypervisor network attributes manoj kiran
2020-09-16 16:26 ` Ed Tanous
2020-09-16 16:33   ` James Feist
2020-09-16 17:20 ` Patrick Williams
2020-09-16 17:44   ` Ed Tanous
2020-09-17  7:40     ` Ratan Gupta
2020-09-17 12:21       ` Deepak Kodihalli
2020-09-17 14:20       ` Thomaiyar, Richard Marian
2020-09-17 15:36       ` Patrick Williams
2020-09-19  5:41         ` Ratan Gupta
2020-09-22  9:09           ` Ratan Gupta
2020-09-22 12:08             ` Deepak Kodihalli
2020-09-23 19:24             ` Patrick Williams
2020-09-23 20:51               ` Ed Tanous
2020-09-23 21:26                 ` Patrick Williams
2020-09-24 13:08                   ` Deepak Kodihalli
2020-09-24 15:36                   ` Ed Tanous
2020-09-30 15:05                     ` Ratan Gupta
2020-09-30 15:56                       ` Ed Tanous
2020-10-01 11:17                         ` Ratan Gupta
2020-10-16 11:40                           ` manoj kiran
2020-09-24  7:30               ` Ratan Gupta

Openbmc archive at lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/openbmc/0 openbmc/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 openbmc openbmc/ https://lore.kernel.org/openbmc \
		openbmc@lists.ozlabs.org openbmc@ozlabs.org
	public-inbox-index openbmc

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.ozlabs.lists.openbmc


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git