Document on : Time Manager in OpenBMC --> Need your review.

vishwa vishwa at linux.vnet.ibm.com
Tue Sep 6 19:01:43 AEST 2016


hello Rick,

Sorry on the delay. Please find my comments.


On 08/25/2016 10:49 PM, Rick Altherr wrote:
>
>
> On Thu, Aug 25, 2016 at 4:59 AM, vishwa <vishwa at linux.vnet.ibm.com 
> <mailto:vishwa at linux.vnet.ibm.com>> wrote:
>
>     Rick,
>
>     Thanks for the thoughts. I have put my response in-line. Please
>     let me know if you have any feedback on this.
>
>
>     On 08/23/2016 05:14 AM, Rick Altherr wrote:
>>     In the future, please just send the text inline instead of as an
>>     attachment.
>>
>>     I don't understand the setting hierarchy.  To me,
>>     /org/openbmc/settings/Host/host0 implies I am modifying something
>>     related to the host CPU, not the BMC.  TimeMode only applies to
>>     the BMC so having it under host0 feels weird.
>>
>     Agree. Doing this in 'host0' is not appropriate. However, the
>     current 'settingsd' framework does not allow multiple levels since
>     the need until now was only for 'host'. Now that we have a
>     requirement to get something new, I have suggested a change to
>     have another entry called 'bmc' and that work will be taken in
>     future sprints. So when that change is made, I will make a
>     corresponding change to clock as well.
>>     1. I like the clear separation of NTP vs manual.
>>
>>     2.1. "Both" seems like what we have today which doesn't really
>>     work at all.  I suggest omitting it.  "Split" feels like it
>>     should work for all cases.  If TimeMode is NTP, an offset is
>>     recorded.  If TimeMode is Manual, the BMC time is set.
>>
>     We had one of the customers ask for "Both" and that is why we are
>     putting this in there.
>
>
> What are they expecting in terms of behavior?  "Both" has the host and 
> BMC fighting for ownership of the BMC system time which has quirky 
> behavior.
One of the OpenPower customers had specifically requested for this. I 
will dig into what the expectation from them was and let you know.
>
>     For the *split* mode, whether it is NTP / Manual, it still demands
>     an update to offset.
>
>
> In "Split" with Manual, the BMC time is meant to be set from the 
> host.  In that case, the offset can be assumed to be zero and the BMC 
> clock set to the provided time.
In my view, Only when the time mode is set to "HOST", the BMC time is 
meant to be set from the host. In the case of split, BMC can choose to 
maintain its own time. The host time requests are handled as offsets 
into BMC's time.
>
>
>>     APIs:
>>     1. What time zone does SetTime assume?  If UTC, make sure to add
>>     tests for a valid leap second and leap year.
>
>     It does not handle Time Zones. Its a mere seconds since epoch.
>     Okay.. I will add the tests.
>
>
> Seconds since epoch != UTC.  Specifically for a leap second event, a 
> single POSIX time point is used for both 23:59:60 and the following 
> 00:00:00.  If the API is supposed to be UTC, specifically state that 
> in the API. If it's supposed to be a POSIX time, have them provide a 
> 64-bit integer.
>
>
>>     2. I really dislike APIs that change behavior.  Provide separate
>>     GetBmcTime and GetHostTime APIs if you must.
>     I like this idea. Implemented it.
>>     3. SetNTP is a very limiting name.  if this really changes
>>     TimeMode, call it SetTimeMode. That way we can support things
>>     like 1588, GPS, etc later.
>
>     I have changed the way NTP setting gets set completely. There will
>     not be an exposed API to change NTP setting. The changes that are
>     done to 'time_mode' will be picked up automatically by the daemon
>     and then appropriate services are started / stopped.
>>
>>     Changes to NetworkManager:
>>     - I don't see the point of UseNTP for SetDHCP.  Configuring an
>>     NTP address is different from using NTP as a time source.  It's
>>     up to the DHCP server to provide NTP options. Whether the BMC
>>     uses them is controlled by TimeMode.
>     The only reason I wanted this in there is if someone wants to take
>     a call to put NTP choice in there while changing the BMC
>     networking to DHCP, they can do it right then and there than
>     having to make another call.
>
>
> I'm hearing that you are optimizing for a case that we don't know is 
> important to optimize for.  BMC networking settings are conceptually 
> unrelated to time settings. Combining them in the API adds a lot more 
> complexity as all the places the NTP settings can be modified needs to 
> be considered.
I kind of agree with your thoughts. In my patch-set, I have removed 
these options from SetAddress4 and SetDHCP.
>
>
>     In addition to this, there are 2 parts to having NTP as TimeMode.
>     (1) We can let timesyncd choose NTP settings that have been given
>     by DHCP
>     (2) Let timesyncd choose NTP settings that were manually entered
>     using "SetNtpAddress"
>
>
> Both of those cases can be handled with a single SetNtpAddress API.
#1 is actually a setting that the user can manipulate and the time 
daemon reacts to that change and hence the need of 2 API.
>
>
>>     - Add a SetNtpServer API instead of adding to SetAddress4.  NTP
>>     is entirely separate from IPv4 address configuration.
>     I do have SetNtpServer and the reason I also chose to make it a
>     part of SetAddress4 was the exact same reason I mentioned for the
>     UseNTP= for DHCP.
>>
>>     On Thu, Aug 18, 2016 at 4:08 AM, vishwa
>>     <vishwa at linux.vnet.ibm.com <mailto:vishwa at linux.vnet.ibm.com>> wrote:
>>
>>         Team,
>>
>>         Please help look into this document that describes what I
>>         think the TimeManager on openBMC systems should look like.
>>
>>         Please weigh in your thoughts.
>>
>>         Thanks.
>>
>>         Vishwanath.
>>
>>         _______________________________________________
>>         openbmc mailing list
>>         openbmc at lists.ozlabs.org <mailto:openbmc at lists.ozlabs.org>
>>         https://lists.ozlabs.org/listinfo/openbmc
>>         <https://lists.ozlabs.org/listinfo/openbmc>
>>
>>
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/openbmc/attachments/20160906/108e4b5a/attachment-0001.html>


More information about the openbmc mailing list