[Git][NTPsec/ntpsec][master] 9 commits: Extracted variables handling from mrulist method into a utility method.

Eric S. Raymond gitlab at mg.gitlab.com
Mon Dec 9 17:20:37 UTC 2019



Eric S. Raymond pushed to branch master at NTPsec / ntpsec


Commits:
71b921cd by Ian Bruene at 2019-12-09T16:58:41Z
Extracted variables handling from mrulist method into a utility method.

- - - - -
27804079 by Ian Bruene at 2019-12-09T16:58:41Z
Extracted analysis from mrulist method into separate method.

- - - - -
b5221c2e by Ian Bruene at 2019-12-09T16:58:41Z
Extracted stitching from mrulist method into a utility method.

- - - - -
9505521c by Ian Bruene at 2019-12-09T16:58:41Z
Extracted query error handling from mrulist method into seperate method.

- - - - -
f1bc50dd by Ian Bruene at 2019-12-09T16:58:41Z
Changed __mru_stitch method into stitch_mru function.

- - - - -
e141931e by Ian Bruene at 2019-12-09T16:58:41Z
Changed __mru_variables method to parse_mru_variables function.

- - - - -
9d4d9781 by Ian Bruene at 2019-12-09T16:58:41Z
Extracted generate_mru_parms function.

- - - - -
ecac8d8f by Ian Bruene at 2019-12-09T16:58:41Z
Changed variables["recent"] check to use "in".

- - - - -
b9bebfc0 by Ian Bruene at 2019-12-09T16:58:41Z
Extracted generate_mru_lastseen function.

- - - - -


1 changed file:

- pylib/packet.py


Changes:

=====================================
pylib/packet.py
=====================================
@@ -1300,6 +1300,91 @@ This combats source address spoofing
         self.logfp.write("## Nonce expected: %s" % resp)
         raise ControlException(SERR_BADNONCE)
 
+    def __mru_analyze(self, variables, span, direct):
+        warndbg = (lambda txt, th: ntp.util.dolog(self.logfp, txt,
+                                                  self.debug, th))
+        curidx = -1
+        mru = None
+        for (tag, val) in variables.items():
+            warndbg("tag=%s, val=%s" % (tag, val), 4)
+            if tag == "nonce":
+                nonce = "%s=%s" % (tag, val)
+            elif tag == "last.older":
+                continue
+            elif tag == "addr.older":
+                continue
+            if tag == "now":
+                # finished marker
+                span.now = ntp.ntpc.lfptofloat(val)
+                continue
+            elif tag == "last.newest":
+                # more finished
+                continue
+            for prefix in ("addr", "last", "first", "ct", "mv", "rs"):
+                if tag.startswith(prefix + "."):
+                    (member, idx) = tag.split(".")
+                    try:
+                        idx = int(idx)
+                    except ValueError:
+                        raise ControlException(SERR_BADTAG % tag)
+                    if idx != curidx:
+                        # This makes duplicates
+                        curidx = idx
+
+                        if mru:
+                            # Can't have partial slots on list
+                            # or printing crashes after ^C
+                            # Append full slot now
+                            span.entries.append(mru)
+                        mru = MRUEntry()
+                        self.slots += 1
+                    setattr(mru, prefix, val)
+        if mru:
+            span.entries.append(mru)
+        if direct is not None:
+            direct(span.entries)
+
+    def __mru_query_error(self, e, restarted_count, cap_frags, limit, frags):
+        warndbg = (lambda txt, th: ntp.util.dolog(self.logfp, txt,
+                                                  self.debug, th))
+        if e.errorcode is None:
+            raise e
+        elif e.errorcode == ntp.control.CERR_UNKNOWNVAR:
+            # None of the supplied prior entries match, so
+            # toss them from our list and try again.
+            warndbg("no overlap between prior entries and "
+                    "server MRU list", 1)
+            restarted_count += 1
+            if restarted_count > 8:
+                raise ControlException(SERR_STALL)
+            warndbg("--->   Restarting from the beginning, "
+                    "retry #%u" % restarted_count, 1)
+        elif e.errorcode == ntp.control.CERR_BADVALUE:
+            if cap_frags:
+                cap_frags = False
+                warndbg("Reverted to row limit from "
+                        "fragments limit.", 1)
+            else:
+                # ntpd has lower cap on row limit
+                self.ntpd_row_limit -= 1
+                limit = min(limit, self.ntpd_row_limit)
+                warndbg("Row limit reduced to %d following "
+                        "CERR_BADVALUE." % limit, 1)
+        elif e.errorcode in (SERR_INCOMPLETE, SERR_TIMEOUT):
+            # Reduce the number of rows/frags requested by
+            # half to recover from lost response fragments.
+            if cap_frags:
+                frags = max(2, frags / 2)
+                warndbg("Frag limit reduced to %d following "
+                        "incomplete response." % frags, 1)
+            else:
+                limit = max(2, limit / 2)
+                warndbg("Row limit reduced to %d following "
+                        " incomplete response." % limit, 1)
+        elif e.errorcode:
+            raise e
+        return restarted_count, cap_frags, limit, frags
+
     def mrulist(self, variables=None, rawhook=None, direct=None):
         "Retrieve MRU list data"
         restarted_count = 0
@@ -1307,63 +1392,14 @@ This combats source address spoofing
         warndbg = (lambda txt, th: ntp.util.dolog(self.logfp, txt,
                                                   self.debug, th))
         sorter = None
+        sortkey = None
         frags = MAXFRAGS
         self.slots = 0
         if variables is None:
             variables = {}
 
         if variables:
-            if "sort" in variables:
-                sortkey = variables["sort"]
-                del variables["sort"]
-                # Slots are retrieved oldest first.
-                # Slots are printed in reverse so the normal/no-sort
-                #  case prints youngest first.
-                # That means sort functions are backwards.
-                # Note lstint is backwards again (aka normal/forward)
-                #  since we really want to sort on now-last rather than last.
-                sortdict = {
-                    # lstint ascending
-                    "lstint": lambda e: ntp.ntpc.lfptofloat(e.last),
-                    # lstint descending
-                    "-lstint": lambda e: -ntp.ntpc.lfptofloat(e.last),
-                    # avgint ascending
-                    "avgint": lambda e: -e.avgint(),
-                    # avgint descending
-                    "-avgint": lambda e: e.avgint(),
-                    # IPv4 asc. then IPv6 asc.
-                    "addr": lambda e: e.sortaddr(),
-                    # IPv6 desc. then IPv4 desc.
-                    "-addr": lambda e: e.sortaddr(),
-                    # hit count ascending
-                    "count": lambda e: -e.ct,
-                    # hit count descending
-                    "-count": lambda e: e.ct,
-                }
-                if sortkey == "lstint":
-                    sortkey = None   # normal/default case, no need to sort
-                if sortkey is not None:
-                    sorter = sortdict.get(sortkey)
-                    if sorter is None:
-                        raise ControlException(SERR_BADSORT % sortkey)
-            for k in list(variables.keys()):
-                if k in ("mincount", "resall", "resany", "kod", "limited",
-                         "maxlstint", "laddr", "recent", "sort",
-                         "frags", "limit"):
-                    continue
-                else:
-                    raise ControlException(SERR_BADPARAM % k)
-            if 'frags' in variables:
-                frags = int(variables.get('frags'))
-                del variables['frags']
-            if 'kod' in variables:
-                variables['resany'] = variables.get('resany', 0) \
-                    | ntp.magic.RES_KOD
-                del variables['kod']
-            if 'limited' in variables:
-                variables['resany'] = variables.get('resany', 0) \
-                    | ntp.magic.RES_LIMITED
-                del variables['limited']
+            sorter, sortkey, frags = parse_mru_variables(variables)
 
         nonce = self.fetch_nonce()
 
@@ -1377,12 +1413,8 @@ This combats source address spoofing
                     variables['resall'] = hex(variables['resall'])
                 if 'resany' in variables:
                     variables['resany'] = hex(variables['resany'])
-                parms = ", " + ",".join([("%s=%s" % it)
-                                         for it in list(variables.items())])
-            else:
-                parms = ""
-            req_buf += parms
-            first_time_only = "recent=%s" % variables.get("recent")
+            parms, firstParms = generate_mru_parms(variables)
+            req_buf += firstParms
 
             while True:
                 # Request additions to the MRU list
@@ -1392,42 +1424,9 @@ This combats source address spoofing
                     recoverable_read_errors = False
                 except ControlException as e:
                     recoverable_read_errors = True
-                    if e.errorcode is None:
-                        raise e
-                    elif e.errorcode == ntp.control.CERR_UNKNOWNVAR:
-                        # None of the supplied prior entries match, so
-                        # toss them from our list and try again.
-                        warndbg("no overlap between prior entries and "
-                                "server MRU list", 1)
-                        restarted_count += 1
-                        if restarted_count > 8:
-                            raise ControlException(SERR_STALL)
-                        warndbg("--->   Restarting from the beginning, "
-                                "retry #%u" % restarted_count, 1)
-                    elif e.errorcode == ntp.control.CERR_BADVALUE:
-                        if cap_frags:
-                            cap_frags = False
-                            warndbg("Reverted to row limit from "
-                                    "fragments limit.", 1)
-                        else:
-                            # ntpd has lower cap on row limit
-                            self.ntpd_row_limit -= 1
-                            limit = min(limit, self.ntpd_row_limit)
-                            warndbg("Row limit reduced to %d following "
-                                    "CERR_BADVALUE." % limit, 1)
-                    elif e.errorcode in (SERR_INCOMPLETE, SERR_TIMEOUT):
-                        # Reduce the number of rows/frags requested by
-                        # half to recover from lost response fragments.
-                        if cap_frags:
-                            frags = max(2, frags / 2)
-                            warndbg("Frag limit reduced to %d following "
-                                    "incomplete response." % frags, 1)
-                        else:
-                            limit = max(2, limit / 2)
-                            warndbg("Row limit reduced to %d following "
-                                    " incomplete response." % limit, 1)
-                    elif e.errorcode:
-                        raise e
+                    res = self.__mru_query_error(e, restarted_count,
+                                                 cap_frags, limit, frags)
+                    restarted_count, cap_frags, limit, frags = res
 
                 # Parse the response
                 variables = self.__parse_varlist()
@@ -1444,46 +1443,7 @@ This combats source address spoofing
                     rawhook(variables)
 
                 # Analyze the contents of this response into a span structure
-                curidx = -1
-                mru = None
-                for (tag, val) in variables.items():
-                    warndbg("tag=%s, val=%s" % (tag, val), 4)
-                    if tag == "nonce":
-                        nonce = "%s=%s" % (tag, val)
-                    elif tag == "last.older":
-                        continue
-                    elif tag == "addr.older":
-                        continue
-                    if tag == "now":
-                        # finished marker
-                        span.now = ntp.ntpc.lfptofloat(val)
-                        continue
-                    elif tag == "last.newest":
-                        # more finished
-                        continue
-                    for prefix in ("addr", "last", "first", "ct", "mv", "rs"):
-                        if tag.startswith(prefix + "."):
-                            (member, idx) = tag.split(".")
-                            try:
-                                idx = int(idx)
-                            except ValueError:
-                                raise ControlException(SERR_BADTAG % tag)
-                            if idx != curidx:
-                                # This makes duplicates
-                                curidx = idx
-
-                                if mru:
-                                    # Can't have partial slots on list
-                                    # or printing crashes after ^C
-                                    # Append full slot now
-                                    span.entries.append(mru)
-                                mru = MRUEntry()
-                                self.slots += 1
-                            setattr(mru, prefix, val)
-                if mru:
-                    span.entries.append(mru)
-                if direct is not None:
-                    direct(span.entries)
+                self.__mru_analyze(variables, span, direct)
 
                 # If we've seen the end sentinel on the span, break out
                 if span.is_complete():
@@ -1513,9 +1473,6 @@ This combats source address spoofing
                                     max(limit + 1,
                                         limit * 33 / 32))
 
-                # Only ship 'recent' on the first request
-                parms = parms.replace(first_time_only, "")
-
                 # Prepare next query with as many address and last-seen
                 # timestamps as will fit in a single packet.  A new nonce
                 # might be required.
@@ -1526,45 +1483,13 @@ This combats source address spoofing
                            "frags" if cap_frags else "limit",
                            frags if cap_frags else limit,
                            parms)
-                for i in range(len(span.entries)):
-                    e = span.entries[len(span.entries) - i - 1]
-                    incr = ", addr.%d=%s, last.%d=%s" % (i, e.addr, i, e.last)
-                    if (len(req_buf) + len(incr) >=
-                            ntp.control.CTL_MAX_DATA_LEN):
-                        break
-                    else:
-                        req_buf += incr
+                req_buf += generate_mru_lastseen(span, len(req_buf))
                 if direct is not None:
                     span.entries = []
         except KeyboardInterrupt:  # pragma: no cover
             pass        # We can test for interruption with is_complete()
 
-        # C ntpq's code for stitching together spans was absurdly
-        # overelaborate - all that dancing with last.older and
-        # addr.older was, as far as I can tell, just pointless.
-        # Much simpler to just run through the final list throwing
-        # out every entry with an IP address that is duplicated
-        # with a later most-recent-transmission time.
-        addrdict = {}
-        deletia = []
-        for (i, entry) in enumerate(span.entries):
-            if entry.addr not in addrdict:
-                addrdict[entry.addr] = []
-            addrdict[entry.addr].append((i, entry.last))
-        for addr in addrdict:
-            deletia += sorted(addrdict[addr], key=lambda x: x[1])[:-1]
-        deletia = [x[0] for x in deletia]
-        deletia.sort(reverse=True)
-        for i in deletia:
-            span.entries.pop(i)
-
-        # Sort for presentation
-        if sorter:
-            span.entries.sort(key=sorter)
-            if sortkey == "addr":
-                # I don't know how to feed a minus sign to text sort
-                span.entries.reverse()
-
+        stitch_mru(span, sorter, sortkey)
         return span
 
     def __ordlist(self, listtype):
@@ -1591,6 +1516,120 @@ This combats source address spoofing
         return self.__ordlist("ifstats")
 
 
+def parse_mru_variables(variables):
+    sorter = None
+    frags = MAXFRAGS
+    if "sort" in variables:
+        sortkey = variables["sort"]
+        del variables["sort"]
+        # Slots are retrieved oldest first.
+        # Slots are printed in reverse so the normal/no-sort
+        #  case prints youngest first.
+        # That means sort functions are backwards.
+        # Note lstint is backwards again (aka normal/forward)
+        #  since we really want to sort on now-last rather than last.
+        sortdict = {
+            # lstint ascending
+            "lstint": lambda e: ntp.ntpc.lfptofloat(e.last),
+            # lstint descending
+            "-lstint": lambda e: -ntp.ntpc.lfptofloat(e.last),
+            # avgint ascending
+            "avgint": lambda e: -e.avgint(),
+            # avgint descending
+            "-avgint": lambda e: e.avgint(),
+            # IPv4 asc. then IPv6 asc.
+            "addr": lambda e: e.sortaddr(),
+            # IPv6 desc. then IPv4 desc.
+            "-addr": lambda e: e.sortaddr(),
+            # hit count ascending
+            "count": lambda e: -e.ct,
+            # hit count descending
+            "-count": lambda e: e.ct,
+        }
+        if sortkey == "lstint":
+            sortkey = None   # normal/default case, no need to sort
+        if sortkey is not None:
+            sorter = sortdict.get(sortkey)
+            if sorter is None:
+                raise ControlException(SERR_BADSORT % sortkey)
+    for k in list(variables.keys()):
+        if k in ("mincount", "resall", "resany", "kod", "limited",
+                 "maxlstint", "laddr", "recent", "sort",
+                 "frags", "limit"):
+            continue
+        else:
+            raise ControlException(SERR_BADPARAM % k)
+    if 'frags' in variables:
+        frags = int(variables.get('frags'))
+        del variables['frags']
+    if 'kod' in variables:
+        variables['resany'] = variables.get('resany', 0) \
+                              | ntp.magic.RES_KOD
+        del variables['kod']
+    if 'limited' in variables:
+        variables['resany'] = variables.get('resany', 0) \
+                              | ntp.magic.RES_LIMITED
+        del variables['limited']
+    return sorter, sortkey, frags
+
+
+def stitch_mru(span, sorter, sortkey):
+    # C ntpq's code for stitching together spans was absurdly
+    # overelaborate - all that dancing with last.older and
+    # addr.older was, as far as I can tell, just pointless.
+    # Much simpler to just run through the final list throwing
+    # out every entry with an IP address that is duplicated
+    # with a later most-recent-transmission time.
+    addrdict = {}
+    deletia = []
+    for (i, entry) in enumerate(span.entries):
+        if entry.addr not in addrdict:
+            addrdict[entry.addr] = []
+        addrdict[entry.addr].append((i, entry.last))
+    for addr in addrdict:
+        deletia += sorted(addrdict[addr], key=lambda x: x[1])[:-1]
+    deletia = [x[0] for x in deletia]
+    deletia.sort(reverse=True)
+    for i in deletia:
+        span.entries.pop(i)
+
+    # Sort for presentation
+    if sorter:
+        span.entries.sort(key=sorter)
+        if sortkey == "addr":
+            # I don't know how to feed a minus sign to text sort
+            span.entries.reverse()
+
+
+def generate_mru_parms(variables):
+    if not variables:
+        return "", ""
+    # generate all sans recent
+    parmStrs = [("%s=%s" % it)
+                for it in list(variables.items()) if (it[0] != "recent")]
+    parms = ", " + ", ".join(parmStrs)
+    # Only ship 'recent' on the first request
+    if "recent" in variables:
+        firstParms = ", recent=%s" % variables["recent"]
+        firstParms += parms
+    else:
+        firstParms = parms
+    return parms, firstParms
+
+
+def generate_mru_lastseen(span, existingBufferSize):
+    buf = ""
+    for i in range(len(span.entries)):
+        e = span.entries[len(span.entries) - i - 1]
+        incr = ", addr.%d=%s, last.%d=%s" % (i, e.addr, i, e.last)
+        if (existingBufferSize + len(buf) + len(incr) >=
+                ntp.control.CTL_MAX_DATA_LEN):
+            break
+        else:
+            buf += incr
+    return buf
+
+
 class Authenticator:
     "MAC authentication manager for NTP packets."
 



View it on GitLab: https://gitlab.com/NTPsec/ntpsec/compare/22c134c8b20e9a897fc5521df871606167067b2e...b9bebfc0f6b617fb503a02aef6e8902aedfbaa92

-- 
View it on GitLab: https://gitlab.com/NTPsec/ntpsec/compare/22c134c8b20e9a897fc5521df871606167067b2e...b9bebfc0f6b617fb503a02aef6e8902aedfbaa92
You're receiving this email because of your account on gitlab.com.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ntpsec.org/pipermail/vc/attachments/20191209/cea3aecd/attachment-0001.htm>


More information about the vc mailing list