<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>