<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 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><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><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. </div><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><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 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 href="mailto:openbmc@lists.ozlabs.org" target="_blank">openbmc@lists.ozlabs.org</a><br>
            <a 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>