<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<p><font size="+1">hello Rick,</font></p>
<p><font size="+1">Sorry on the delay. Please find my comments.</font><br>
</p>
<br>
<div class="moz-cite-prefix">On 08/25/2016 10:49 PM, Rick Altherr
wrote:<br>
</div>
<blockquote
cite="mid:CAPLgG=nXS87g8cnkb3ZG0rxxTWarcowMc4gyhJx1KotRz_8uFw@mail.gmail.com"
type="cite">
<div dir="ltr"><br>
<div class="gmail_extra"><br>
<div class="gmail_quote">On Thu, Aug 25, 2016 at 4:59 AM,
vishwa <span dir="ltr"><<a moz-do-not-send="true"
href="mailto:vishwa@linux.vnet.ibm.com" target="_blank">vishwa@linux.vnet.ibm.com</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000">
<p><font size="+1">Rick,</font></p>
<p><font size="+1">Thanks for the thoughts. I have put
my response in-line. Please let me know if you have
any feedback on this.</font><br>
</p>
<span class=""> <br>
<div class="m_-5857520047155842178moz-cite-prefix">On
08/23/2016 05:14 AM, Rick Altherr wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">In the future, please just send the
text inline instead of as an attachment.
<div><br>
</div>
<div>I don't understand the setting hierarchy. To
me, /org/openbmc/settings/Host/<wbr>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.</div>
<div><br>
</div>
</div>
</blockquote>
</span> 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.<span class=""><br>
<blockquote type="cite">
<div dir="ltr">
<div>1. I like the clear separation of NTP vs
manual.</div>
<div><br>
</div>
<div>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.</div>
<div><br>
</div>
</div>
</blockquote>
</span> We had one of the customers ask for "Both" and
that is why we are putting this in there.<br>
</div>
</blockquote>
<div><br>
</div>
<div>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.</div>
</div>
</div>
</div>
</blockquote>
One of the OpenPower customers had specifically requested for this.
I will dig into what the expectation from them was and let you know.<br>
<blockquote
cite="mid:CAPLgG=nXS87g8cnkb3ZG0rxxTWarcowMc4gyhJx1KotRz_8uFw@mail.gmail.com"
type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<div> </div>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000"> For the *split*
mode, whether it is NTP / Manual, it still demands an
update to offset.</div>
</blockquote>
<div><br>
</div>
<div>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.</div>
<div> </div>
</div>
</div>
</div>
</blockquote>
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.<br>
<blockquote
cite="mid:CAPLgG=nXS87g8cnkb3ZG0rxxTWarcowMc4gyhJx1KotRz_8uFw@mail.gmail.com"
type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000"><span class=""><br>
<blockquote type="cite">
<div dir="ltr">
<div>APIs:</div>
<div>1. What time zone does SetTime assume? If
UTC, make sure to add tests for a valid leap
second and leap year.</div>
</div>
</blockquote>
<br>
</span> It does not handle Time Zones. Its a mere
seconds since epoch. Okay.. I will add the tests.</div>
</blockquote>
<div><br>
</div>
<div>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.</div>
<div> </div>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000"><span class=""><br>
<blockquote type="cite">
<div dir="ltr">
<div>2. I really dislike APIs that change
behavior. Provide separate GetBmcTime and
GetHostTime APIs if you must.</div>
</div>
</blockquote>
</span> I like this idea. Implemented it.<span class=""><br>
<blockquote type="cite">
<div dir="ltr">
<div>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.</div>
</div>
</blockquote>
<br>
</span> 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.<span
class=""><br>
<blockquote type="cite">
<div dir="ltr">
<div><br>
</div>
<div>Changes to NetworkManager:</div>
<div>- 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.</div>
</div>
</blockquote>
</span> 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.<br>
</div>
</blockquote>
<div><br>
</div>
<div>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. <br>
</div>
</div>
</div>
</div>
</blockquote>
I kind of agree with your thoughts. In my patch-set, I have removed
these options from SetAddress4 and SetDHCP.<br>
<blockquote
cite="mid:CAPLgG=nXS87g8cnkb3ZG0rxxTWarcowMc4gyhJx1KotRz_8uFw@mail.gmail.com"
type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000"> <br>
In addition to this, there are 2 parts to having NTP as
TimeMode. <br>
(1) We can let timesyncd choose NTP settings that have
been given by DHCP <br>
(2) Let timesyncd choose NTP settings that were manually
entered using "SetNtpAddress"</div>
</blockquote>
<div><br>
</div>
<div>Both of those cases can be handled with a single
SetNtpAddress API.</div>
</div>
</div>
</div>
</blockquote>
#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.<br>
<blockquote
cite="mid:CAPLgG=nXS87g8cnkb3ZG0rxxTWarcowMc4gyhJx1KotRz_8uFw@mail.gmail.com"
type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<div> </div>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000"><span class=""><br>
<blockquote type="cite">
<div dir="ltr">
<div>- Add a SetNtpServer API instead of adding to
SetAddress4. NTP is entirely separate from IPv4
address configuration.</div>
</div>
</blockquote>
</span> 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.<span
class=""><br>
<blockquote type="cite">
<div class="gmail_extra"><br>
<div class="gmail_quote">On Thu, Aug 18, 2016 at
4:08 AM, vishwa <span dir="ltr"><<a
moz-do-not-send="true"
href="mailto:vishwa@linux.vnet.ibm.com"
target="_blank">vishwa@linux.vnet.ibm.com</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0
0 0 .8ex;border-left:1px #ccc
solid;padding-left:1ex">Team,<br>
<br>
Please help look into this document that
describes what I think the TimeManager on
openBMC systems should look like.<br>
<br>
Please weigh in your thoughts.<br>
<br>
Thanks.<br>
<br>
Vishwanath.<br>
<br>
______________________________<wbr>_________________<br>
openbmc mailing list<br>
<a moz-do-not-send="true"
href="mailto:openbmc@lists.ozlabs.org"
target="_blank">openbmc@lists.ozlabs.org</a><br>
<a moz-do-not-send="true"
href="https://lists.ozlabs.org/listinfo/openbmc"
rel="noreferrer" target="_blank">https://lists.ozlabs.org/listi<wbr>nfo/openbmc</a><br>
<br>
</blockquote>
</div>
<br>
</div>
</blockquote>
<br>
</span></div>
</blockquote>
</div>
<br>
</div>
</div>
</blockquote>
<br>
</body>
</html>