Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 22 Aug 2018 20:53:10 -0400
From:      Patrick Kelsey <pkelsey@freebsd.org>
To:        Gleb Smirnoff <glebius@freebsd.org>
Cc:        src-committers@freebsd.org, svn-src-all@freebsd.org,  svn-src-head@freebsd.org
Subject:   Re: svn commit: r338209 - in head: sbin/ipfw sbin/pfctl sys/net sys/net/altq sys/netpfil/pf usr.sbin/bsnmpd/modules/snmp_pf
Message-ID:  <CAD44qMVCW%2BKP1xNez3ahm1vmzwPpAN3%2BfhNXm5JuVdNNzJpWZQ@mail.gmail.com>
In-Reply-To: <20180822214029.GX27633@FreeBSD.org>
References:  <201808221938.w7MJcmtu064088@repo.freebsd.org> <20180822214029.GX27633@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help
Gleb,

Thanks for taking a look.

I don't think there is all that much complication in the compatibility
layer, and I think what is there is worth the properties it yields.  The
approach taken to backwards compatibility has a clear and regular
structure, it allows out of tree consumers of the interface to be upgraded
asynchronously to the evolution of the interface, it allows out of tree
consumers to be written such that they can be successfully compiled against
multiple versions of the interface (using #ifdef of the various structure
version macros), it doesn't require constructing a FreeBSD version chart to
figure out what each struct looks like and when, and it anticipates that
additional revisions of interface data structures may be made at multiple
unknown points in the future as other altq algorithms are upgraded to
support higher bandwidths.

There are at least two consumers in ports that use the altq portions of the
pf ioctl interface - pftop and pfstat.  These are both named in the review,
having been found pretty easily by looking at the short list of ports with
'pf' in the name and doing a make extract and grep of the obvious potential
users.

I don't think taking a COMPAT_FREEBSDXX approach would have reduced the
diff much or at all, and I think it would introduce breakage scenarios
(that don't exist now) between ports/packages and stable branches if
additional interface changes are made in the future.  By having any sort of
backwards compat, we are guaranteeing that all versions of these structures
are supported by the kernel interface, so all the versions have to be
present in headers somewhere.  There is already pretty poor separation of
public/private in the pf and altq headers, such that hiding prior versions
of data structures in some of the headers consumed by userland programs
won't really improve the situation (I have improved it a little bit by
separating struct pf_altq into the user facing version and the
kernel-internal version).

Best,
Patrick

On Wed, Aug 22, 2018 at 5:40 PM Gleb Smirnoff <glebius@freebsd.org> wrote:

>   Patrick,
>
> thanks for the change. The compatibility layer seems overcomplicated
> though. IMHO, it would be enough just to provide binary compatibility
> keeping the old structure and ioctl number as compat, not declaring
> them in public headers. And the compat layer should be embraced with
> COMPAT_FREEBSD11.
>
> To estimate number of ports affected a exp-run can be requested. Might
> happen there are very little or even none!
>
> On Wed, Aug 22, 2018 at 07:38:48PM +0000, Patrick Kelsey wrote:
> P> Author: pkelsey
> P> Date: Wed Aug 22 19:38:48 2018
> P> New Revision: 338209
> P> URL: https://svnweb.freebsd.org/changeset/base/338209
> P>
> P> Log:
> P>   Extended pf(4) ioctl interface and pfctl(8) to allow bandwidths of
> P>   2^32 bps or greater to be used.  Prior to this, bandwidth parameters
> P>   would simply wrap at the 2^32 boundary.  The computations in the HFSC
> P>   scheduler and token bucket regulator have been modified to operate
> P>   correctly up to at least 100 Gbps.  No other algorithms have been
> P>   examined or modified for correct operation above 2^32 bps (some may
> P>   have existing computation resolution or overflow issues at rates below
> P>   that threshold).  pfctl(8) will now limit non-HFSC bandwidth
> P>   parameters to 2^32 - 1 before passing them to the kernel.
> P>
> P>   The extensions to the pf(4) ioctl interface have been made in a
> P>   backwards-compatible way by versioning affected data structures,
> P>   supporting all versions in the kernel, and implementing macros that
> P>   will cause existing code that consumes that interface to use version 0
> P>   without source modifications.  If version 0 consumers of the interface
> P>   are used against a new kernel that has had bandwidth parameters of
> P>   2^32 or greater configured by updated tools, such bandwidth parameters
> P>   will be reported as 2^32 - 1 bps by those old consumers.
> P>
> P>   All in-tree consumers of the pf(4) interface have been updated.  To
> P>   update out-of-tree consumers to the latest version of the interface,
> P>   define PFIOC_USE_LATEST ahead of any includes and use the code of
> P>   pfctl(8) as a guide for the ioctls of interest.
> P>
> P>   PR:        211730
> P>   Reviewed by:       jmallett, kp, loos
> P>   MFC after: 2 weeks
> P>   Relnotes:  yes
> P>   Sponsored by:      RG Nets
> P>   Differential Revision:     https://reviews.freebsd.org/D16782
> P>
> P> Modified:
> P>   head/sbin/ipfw/altq.c
> P>   head/sbin/pfctl/parse.y
> P>   head/sbin/pfctl/pfctl.c
> P>   head/sbin/pfctl/pfctl_altq.c
> P>   head/sbin/pfctl/pfctl_parser.h
> P>   head/sbin/pfctl/pfctl_qstats.c
> P>   head/sys/net/altq/altq.h
> P>   head/sys/net/altq/altq_cbq.c
> P>   head/sys/net/altq/altq_cbq.h
> P>   head/sys/net/altq/altq_codel.c
> P>   head/sys/net/altq/altq_codel.h
> P>   head/sys/net/altq/altq_fairq.c
> P>   head/sys/net/altq/altq_fairq.h
> P>   head/sys/net/altq/altq_hfsc.c
> P>   head/sys/net/altq/altq_hfsc.h
> P>   head/sys/net/altq/altq_priq.c
> P>   head/sys/net/altq/altq_priq.h
> P>   head/sys/net/altq/altq_subr.c
> P>   head/sys/net/altq/altq_var.h
> P>   head/sys/net/pfvar.h
> P>   head/sys/netpfil/pf/pf_altq.h
> P>   head/sys/netpfil/pf/pf_ioctl.c
> P>   head/usr.sbin/bsnmpd/modules/snmp_pf/pf_snmp.c
> P>
> P> Modified: head/sbin/ipfw/altq.c
> P>
> ==============================================================================
> P> --- head/sbin/ipfw/altq.c    Wed Aug 22 18:19:56 2018        (r338208)
> P> +++ head/sbin/ipfw/altq.c    Wed Aug 22 19:38:48 2018        (r338209)
> P> @@ -22,6 +22,8 @@
> P>   * altq interface
> P>   */
> P>
> P> +#define PFIOC_USE_LATEST
> P> +
> P>  #include <sys/types.h>
> P>  #include <sys/socket.h>
> P>  #include <sys/sockio.h>
> P> @@ -85,6 +87,7 @@ altq_fetch(void)
> P>              return;
> P>      }
> P>      bzero(&pfioc, sizeof(pfioc));
> P> +    pfioc.version = PFIOC_ALTQ_VERSION;
> P>      if (ioctl(pffd, DIOCGETALTQS, &pfioc) != 0) {
> P>              warn("altq support getting queue list");
> P>              close(pffd);
> P>
> P> Modified: head/sbin/pfctl/parse.y
> P>
> ==============================================================================
> P> --- head/sbin/pfctl/parse.y  Wed Aug 22 18:19:56 2018        (r338208)
> P> +++ head/sbin/pfctl/parse.y  Wed Aug 22 19:38:48 2018        (r338209)
> P> @@ -32,6 +32,8 @@
> P>  #include <sys/cdefs.h>
> P>  __FBSDID("$FreeBSD$");
> P>
> P> +#define PFIOC_USE_LATEST
> P> +
> P>  #include <sys/types.h>
> P>  #include <sys/socket.h>
> P>  #include <sys/stat.h>
> P> @@ -286,7 +288,7 @@ static struct queue_opts {
> P>      struct node_queue_bw    queue_bwspec;
> P>      struct node_queue_opt   scheduler;
> P>      int                     priority;
> P> -    int                     tbrsize;
> P> +    unsigned int            tbrsize;
> P>      int                     qlimit;
> P>  } queue_opts;
> P>
> P> @@ -1623,8 +1625,8 @@ queue_opt      : BANDWIDTH bandwidth   {
> P>                              yyerror("tbrsize cannot be respecified");
> P>                              YYERROR;
> P>                      }
> P> -                    if ($2 < 0 || $2 > 65535) {
> P> -                            yyerror("tbrsize too big: max 65535");
> P> +                    if ($2 < 0 || $2 > UINT_MAX) {
> P> +                            yyerror("tbrsize too big: max %u",
> UINT_MAX);
> P>                              YYERROR;
> P>                      }
> P>                      queue_opts.marker |= QOM_TBRSIZE;
> P> @@ -1673,10 +1675,10 @@ bandwidth    : STRING {
> P>                              }
> P>                      }
> P>                      free($1);
> P> -                    $$.bw_absolute = (u_int32_t)bps;
> P> +                    $$.bw_absolute = (u_int64_t)bps;
> P>              }
> P>              | NUMBER {
> P> -                    if ($1 < 0 || $1 > UINT_MAX) {
> P> +                    if ($1 < 0 || $1 >= LLONG_MAX) {
> P>                              yyerror("bandwidth number too big");
> P>                              YYERROR;
> P>                      }
> P>
> P> Modified: head/sbin/pfctl/pfctl.c
> P>
> ==============================================================================
> P> --- head/sbin/pfctl/pfctl.c  Wed Aug 22 18:19:56 2018        (r338208)
> P> +++ head/sbin/pfctl/pfctl.c  Wed Aug 22 19:38:48 2018        (r338209)
> P> @@ -36,6 +36,8 @@
> P>  #include <sys/cdefs.h>
> P>  __FBSDID("$FreeBSD$");
> P>
> P> +#define PFIOC_USE_LATEST
> P> +
> P>  #include <sys/types.h>
> P>  #include <sys/ioctl.h>
> P>  #include <sys/socket.h>
> P> @@ -1524,6 +1526,7 @@ pfctl_rules(int dev, char *filename, int opts,
> int opt
> P>      }
> P>
> P>      memset(&pa, 0, sizeof(pa));
> P> +    pa.version = PFIOC_ALTQ_VERSION;
> P>      memset(&pf, 0, sizeof(pf));
> P>      memset(&trs, 0, sizeof(trs));
> P>      if ((path = calloc(1, MAXPATHLEN)) == NULL)
> P> @@ -2032,6 +2035,7 @@ pfctl_test_altqsupport(int dev, int opts)
> P>  {
> P>      struct pfioc_altq pa;
> P>
> P> +    pa.version = PFIOC_ALTQ_VERSION;
> P>      if (ioctl(dev, DIOCGETALTQS, &pa)) {
> P>              if (errno == ENODEV) {
> P>                      if (opts & PF_OPT_VERBOSE)
> P>
> P> Modified: head/sbin/pfctl/pfctl_altq.c
> P>
> ==============================================================================
> P> --- head/sbin/pfctl/pfctl_altq.c     Wed Aug 22 18:19:56 2018
> (r338208)
> P> +++ head/sbin/pfctl/pfctl_altq.c     Wed Aug 22 19:38:48 2018
> (r338209)
> P> @@ -21,6 +21,8 @@
> P>  #include <sys/cdefs.h>
> P>  __FBSDID("$FreeBSD$");
> P>
> P> +#define PFIOC_USE_LATEST
> P> +
> P>  #include <sys/types.h>
> P>  #include <sys/ioctl.h>
> P>  #include <sys/socket.h>
> P> @@ -31,6 +33,7 @@ __FBSDID("$FreeBSD$");
> P>
> P>  #include <err.h>
> P>  #include <errno.h>
> P> +#include <inttypes.h>
> P>  #include <limits.h>
> P>  #include <math.h>
> P>  #include <stdio.h>
> P> @@ -88,14 +91,14 @@ static int                gsc_add_seg(struct gen_sc
> *, double, doub
> P>  static double                sc_x2y(struct service_curve *, double);
> P>
> P>  #ifdef __FreeBSD__
> P> -u_int32_t   getifspeed(int, char *);
> P> +u_int64_t   getifspeed(int, char *);
> P>  #else
> P>  u_int32_t    getifspeed(char *);
> P>  #endif
> P>  u_long               getifmtu(char *);
> P>  int          eval_queue_opts(struct pf_altq *, struct node_queue_opt *,
> P> -                 u_int32_t);
> P> -u_int32_t    eval_bwspec(struct node_queue_bw *, u_int32_t);
> P> +                 u_int64_t);
> P> +u_int64_t    eval_bwspec(struct node_queue_bw *, u_int64_t);
> P>  void                 print_hfsc_sc(const char *, u_int, u_int, u_int,
> P>                   const struct node_hfsc_sc *);
> P>  void                 print_fairq_sc(const char *, u_int, u_int, u_int,
> P> @@ -258,7 +261,8 @@ int
> P>  eval_pfaltq(struct pfctl *pf, struct pf_altq *pa, struct node_queue_bw
> *bw,
> P>      struct node_queue_opt *opts)
> P>  {
> P> -    u_int   rate, size, errors = 0;
> P> +    u_int64_t       rate;
> P> +    u_int           size, errors = 0;
> P>
> P>      if (bw->bw_absolute > 0)
> P>              pa->ifbandwidth = bw->bw_absolute;
> P> @@ -275,6 +279,15 @@ eval_pfaltq(struct pfctl *pf, struct pf_altq *pa,
> stru
> P>              } else if ((pa->ifbandwidth = eval_bwspec(bw, rate)) == 0)
> P>                      pa->ifbandwidth = rate;
> P>
> P> +    /*
> P> +     * Limit bandwidth to UINT_MAX for schedulers that aren't 64-bit
> ready.
> P> +     */
> P> +    if ((pa->scheduler != ALTQT_HFSC) && (pa->ifbandwidth > UINT_MAX))
> {
> P> +            pa->ifbandwidth = UINT_MAX;
> P> +            warnx("interface %s bandwidth limited to %" PRIu64 " bps "
> P> +                "because selected scheduler is 32-bit limited\n",
> pa->ifname,
> P> +                pa->ifbandwidth);
> P> +    }
> P>      errors += eval_queue_opts(pa, opts, pa->ifbandwidth);
> P>
> P>      /* if tbrsize is not specified, use heuristics */
> P> @@ -289,8 +302,6 @@ eval_pfaltq(struct pfctl *pf, struct pf_altq *pa,
> stru
> P>              else
> P>                      size = 24;
> P>              size = size * getifmtu(pa->ifname);
> P> -            if (size > 0xffff)
> P> -                    size = 0xffff;
> P>              pa->tbrsize = size;
> P>      }
> P>      return (errors);
> P> @@ -338,7 +349,7 @@ eval_pfqueue(struct pfctl *pf, struct pf_altq *pa,
> str
> P>  {
> P>      /* should be merged with expand_queue */
> P>      struct pf_altq  *if_pa, *parent, *altq;
> P> -    u_int32_t        bwsum;
> P> +    u_int64_t        bwsum;
> P>      int              error = 0;
> P>
> P>      /* find the corresponding interface and copy fields used by queues
> */
> P> @@ -372,7 +383,7 @@ eval_pfqueue(struct pfctl *pf, struct pf_altq *pa,
> str
> P>      if (pa->scheduler == ALTQT_CBQ || pa->scheduler == ALTQT_HFSC ||
> P>              pa->scheduler == ALTQT_FAIRQ) {
> P>              pa->bandwidth = eval_bwspec(bw,
> P> -                parent == NULL ? 0 : parent->bandwidth);
> P> +                parent == NULL ? pa->ifbandwidth : parent->bandwidth);
> P>
> P>              if (pa->bandwidth > pa->ifbandwidth) {
> P>                      fprintf(stderr, "bandwidth for %s higher than "
> P> @@ -403,7 +414,8 @@ eval_pfqueue(struct pfctl *pf, struct pf_altq *pa,
> str
> P>              }
> P>      }
> P>
> P> -    if (eval_queue_opts(pa, opts, parent == NULL? 0 :
> parent->bandwidth))
> P> +    if (eval_queue_opts(pa, opts,
> P> +            parent == NULL ? pa->ifbandwidth : parent->bandwidth))
> P>              return (1);
> P>
> P>      switch (pa->scheduler) {
> P> @@ -709,7 +721,7 @@ static int
> P>  eval_pfqueue_hfsc(struct pfctl *pf, struct pf_altq *pa)
> P>  {
> P>      struct pf_altq          *altq, *parent;
> P> -    struct hfsc_opts        *opts;
> P> +    struct hfsc_opts_v1     *opts;
> P>      struct service_curve     sc;
> P>
> P>      opts = &pa->pq_u.hfsc_opts;
> P> @@ -1001,7 +1013,7 @@ check_commit_fairq(int dev __unused, int opts
> __unused
> P>  static int
> P>  print_hfsc_opts(const struct pf_altq *a, const struct node_queue_opt
> *qopts)
> P>  {
> P> -    const struct hfsc_opts          *opts;
> P> +    const struct hfsc_opts_v1       *opts;
> P>      const struct node_hfsc_sc       *rtsc, *lssc, *ulsc;
> P>
> P>      opts = &a->pq_u.hfsc_opts;
> P> @@ -1316,7 +1328,7 @@ rate2str(double rate)
> P>   * FreeBSD does not have SIOCGIFDATA.
> P>   * To emulate this, DIOCGIFSPEED ioctl added to pf.
> P>   */
> P> -u_int32_t
> P> +u_int64_t
> P>  getifspeed(int pfdev, char *ifname)
> P>  {
> P>      struct pf_ifspeed io;
> P> @@ -1327,7 +1339,7 @@ getifspeed(int pfdev, char *ifname)
> P>              errx(1, "getifspeed: strlcpy");
> P>      if (ioctl(pfdev, DIOCGIFSPEED, &io) == -1)
> P>              err(1, "DIOCGIFSPEED");
> P> -    return ((u_int32_t)io.baudrate);
> P> +    return (io.baudrate);
> P>  }
> P>  #else
> P>  u_int32_t
> P> @@ -1382,7 +1394,7 @@ getifmtu(char *ifname)
> P>
> P>  int
> P>  eval_queue_opts(struct pf_altq *pa, struct node_queue_opt *opts,
> P> -    u_int32_t ref_bw)
> P> +    u_int64_t ref_bw)
> P>  {
> P>      int     errors = 0;
> P>
> P> @@ -1458,11 +1470,21 @@ eval_queue_opts(struct pf_altq *pa, struct
> node_queue_
> P>      return (errors);
> P>  }
> P>
> P> -u_int32_t
> P> -eval_bwspec(struct node_queue_bw *bw, u_int32_t ref_bw)
> P> +/*
> P> + * If absolute bandwidth if set, return the lesser of that value and
> the
> P> + * reference bandwidth.  Limiting to the reference bandwidth allows
> simple
> P> + * limiting of configured bandwidth parameters for schedulers that are
> P> + * 32-bit limited, as the root/interface bandwidth (top-level reference
> P> + * bandwidth) will be properly limited in that case.
> P> + *
> P> + * Otherwise, if the absolute bandwidth is not set, return given
> percentage
> P> + * of reference bandwidth.
> P> + */
> P> +u_int64_t
> P> +eval_bwspec(struct node_queue_bw *bw, u_int64_t ref_bw)
> P>  {
> P>      if (bw->bw_absolute > 0)
> P> -            return (bw->bw_absolute);
> P> +            return (MIN(bw->bw_absolute, ref_bw));
> P>
> P>      if (bw->bw_percent > 0)
> P>              return (ref_bw / 100 * bw->bw_percent);
> P>
> P> Modified: head/sbin/pfctl/pfctl_parser.h
> P>
> ==============================================================================
> P> --- head/sbin/pfctl/pfctl_parser.h   Wed Aug 22 18:19:56 2018
> (r338208)
> P> +++ head/sbin/pfctl/pfctl_parser.h   Wed Aug 22 19:38:48 2018
> (r338209)
> P> @@ -134,7 +134,7 @@ struct node_os {
> P>  };
> P>
> P>  struct node_queue_bw {
> P> -    u_int32_t       bw_absolute;
> P> +    u_int64_t       bw_absolute;
> P>      u_int16_t       bw_percent;
> P>  };
> P>
> P>
> P> Modified: head/sbin/pfctl/pfctl_qstats.c
> P>
> ==============================================================================
> P> --- head/sbin/pfctl/pfctl_qstats.c   Wed Aug 22 18:19:56 2018
> (r338208)
> P> +++ head/sbin/pfctl/pfctl_qstats.c   Wed Aug 22 19:38:48 2018
> (r338209)
> P> @@ -19,6 +19,8 @@
> P>  #include <sys/cdefs.h>
> P>  __FBSDID("$FreeBSD$");
> P>
> P> +#define PFIOC_USE_LATEST
> P> +
> P>  #include <sys/types.h>
> P>  #include <sys/ioctl.h>
> P>  #include <sys/socket.h>
> P> @@ -148,6 +150,7 @@ pfctl_update_qstats(int dev, struct pf_altq_node
> **roo
> P>      memset(&pa, 0, sizeof(pa));
> P>      memset(&pq, 0, sizeof(pq));
> P>      memset(&qstats, 0, sizeof(qstats));
> P> +    pa.version = PFIOC_ALTQ_VERSION;
> P>      if (ioctl(dev, DIOCGETALTQS, &pa)) {
> P>              warn("DIOCGETALTQS");
> P>              return (-1);
> P> @@ -177,6 +180,7 @@ pfctl_update_qstats(int dev, struct pf_altq_node
> **roo
> P>                      pq.ticket = pa.ticket;
> P>                      pq.buf = &qstats.data;
> P>                      pq.nbytes = sizeof(qstats.data);
> P> +                    pq.version = altq_stats_version(pa.altq.scheduler);
> P>                      if (ioctl(dev, DIOCGETQSTATS, &pq)) {
> P>                              warn("DIOCGETQSTATS");
> P>                              return (-1);
> P>
> P> Modified: head/sys/net/altq/altq.h
> P>
> ==============================================================================
> P> --- head/sys/net/altq/altq.h Wed Aug 22 18:19:56 2018        (r338208)
> P> +++ head/sys/net/altq/altq.h Wed Aug 22 19:38:48 2018        (r338209)
> P> @@ -76,8 +76,8 @@ struct     altqreq {
> P>
> P>  /* simple token backet meter profile */
> P>  struct      tb_profile {
> P> -    u_int   rate;   /* rate in bit-per-sec */
> P> -    u_int   depth;  /* depth in bytes */
> P> +    u_int64_t       rate;   /* rate in bit-per-sec */
> P> +    u_int32_t       depth;  /* depth in bytes */
> P>  };
> P>
> P>  #ifdef ALTQ3_COMPAT
> P> @@ -203,4 +203,29 @@ struct pktcntr {
> P>  #include <net/altq/altq_var.h>
> P>  #endif
> P>
> P> +/*
> P> + * Can't put these versions in the scheduler-specific headers and
> include
> P> + * them all here as that will cause build failure due to
> cross-including
> P> + * each other scheduler's private bits into each scheduler's
> P> + * implementation.
> P> + */
> P> +#define CBQ_STATS_VERSION   0       /* Latest version of class_stats_t
> */
> P> +#define CODEL_STATS_VERSION 0       /* Latest version of codel_ifstats
> */
> P> +#define FAIRQ_STATS_VERSION 0       /* Latest version of
> fairq_classstats */
> P> +#define HFSC_STATS_VERSION  1       /* Latest version of
> hfsc_classstats */
> P> +#define PRIQ_STATS_VERSION  0       /* Latest version of
> priq_classstats */
> P> +
> P> +/* Return the latest stats version for the given scheduler. */
> P> +static inline int altq_stats_version(int scheduler)
> P> +{
> P> +    switch (scheduler) {
> P> +    case ALTQT_CBQ:   return (CBQ_STATS_VERSION);
> P> +    case ALTQT_CODEL: return (CODEL_STATS_VERSION);
> P> +    case ALTQT_FAIRQ: return (FAIRQ_STATS_VERSION);
> P> +    case ALTQT_HFSC:  return (HFSC_STATS_VERSION);
> P> +    case ALTQT_PRIQ:  return (PRIQ_STATS_VERSION);
> P> +    default: return (0);
> P> +    }
> P> +}
> P> +
> P>  #endif /* _ALTQ_ALTQ_H_ */
> P>
> P> Modified: head/sys/net/altq/altq_cbq.c
> P>
> ==============================================================================
> P> --- head/sys/net/altq/altq_cbq.c     Wed Aug 22 18:19:56 2018
> (r338208)
> P> +++ head/sys/net/altq/altq_cbq.c     Wed Aug 22 19:38:48 2018
> (r338209)
> P> @@ -452,7 +452,7 @@ cbq_remove_queue(struct pf_altq *a)
> P>  }
> P>
> P>  int
> P> -cbq_getqstats(struct pf_altq *a, void *ubuf, int *nbytes)
> P> +cbq_getqstats(struct pf_altq *a, void *ubuf, int *nbytes, int version)
> P>  {
> P>      cbq_state_t     *cbqp;
> P>      struct rm_class *cl;
> P>
> P> Modified: head/sys/net/altq/altq_cbq.h
> P>
> ==============================================================================
> P> --- head/sys/net/altq/altq_cbq.h     Wed Aug 22 18:19:56 2018
> (r338208)
> P> +++ head/sys/net/altq/altq_cbq.h     Wed Aug 22 19:38:48 2018
> (r338209)
> P> @@ -99,6 +99,12 @@ typedef struct _cbq_class_stats_ {
> P>      struct codel_stats codel;
> P>  } class_stats_t;
> P>
> P> +/*
> P> + * CBQ_STATS_VERSION is defined in altq.h to work around issues
> stemming
> P> + * from mixing of public-API and internal bits in each
> scheduler-specific
> P> + * header.
> P> + */
> P> +
> P>  #ifdef ALTQ3_COMPAT
> P>  /*
> P>   * Define structures associated with IOCTLS for cbq.
> P>
> P> Modified: head/sys/net/altq/altq_codel.c
> P>
> ==============================================================================
> P> --- head/sys/net/altq/altq_codel.c   Wed Aug 22 18:19:56 2018
> (r338208)
> P> +++ head/sys/net/altq/altq_codel.c   Wed Aug 22 19:38:48 2018
> (r338209)
> P> @@ -156,7 +156,7 @@ codel_remove_altq(struct pf_altq *a)
> P>  }
> P>
> P>  int
> P> -codel_getqstats(struct pf_altq *a, void *ubuf, int *nbytes)
> P> +codel_getqstats(struct pf_altq *a, void *ubuf, int *nbytes, int
> version)
> P>  {
> P>      struct codel_if *cif;
> P>      struct codel_ifstats stats;
> P>
> P> Modified: head/sys/net/altq/altq_codel.h
> P>
> ==============================================================================
> P> --- head/sys/net/altq/altq_codel.h   Wed Aug 22 18:19:56 2018
> (r338208)
> P> +++ head/sys/net/altq/altq_codel.h   Wed Aug 22 19:38:48 2018
> (r338209)
> P> @@ -57,6 +57,12 @@ struct codel_ifstats {
> P>      struct pktcntr  cl_dropcnt;     /* dropped packet counter */
> P>  };
> P>
> P> +/*
> P> + * CBQ_STATS_VERSION is defined in altq.h to work around issues
> stemming
> P> + * from mixing of public-API and internal bits in each
> scheduler-specific
> P> + * header.
> P> + */
> P> +
> P>  #ifdef _KERNEL
> P>  #include <net/altq/altq_classq.h>
> P>
> P>
> P> Modified: head/sys/net/altq/altq_fairq.c
> P>
> ==============================================================================
> P> --- head/sys/net/altq/altq_fairq.c   Wed Aug 22 18:19:56 2018
> (r338208)
> P> +++ head/sys/net/altq/altq_fairq.c   Wed Aug 22 19:38:48 2018
> (r338209)
> P> @@ -229,7 +229,7 @@ fairq_remove_queue(struct pf_altq *a)
> P>  }
> P>
> P>  int
> P> -fairq_getqstats(struct pf_altq *a, void *ubuf, int *nbytes)
> P> +fairq_getqstats(struct pf_altq *a, void *ubuf, int *nbytes, int
> version)
> P>  {
> P>      struct fairq_if *pif;
> P>      struct fairq_class *cl;
> P>
> P> Modified: head/sys/net/altq/altq_fairq.h
> P>
> ==============================================================================
> P> --- head/sys/net/altq/altq_fairq.h   Wed Aug 22 18:19:56 2018
> (r338208)
> P> +++ head/sys/net/altq/altq_fairq.h   Wed Aug 22 19:38:48 2018
> (r338209)
> P> @@ -82,6 +82,12 @@ struct fairq_classstats {
> P>      struct codel_stats      codel;
> P>  };
> P>
> P> +/*
> P> + * FAIRQ_STATS_VERSION is defined in altq.h to work around issues
> stemming
> P> + * from mixing of public-API and internal bits in each
> scheduler-specific
> P> + * header.
> P> + */
> P> +
> P>  #ifdef _KERNEL
> P>
> P>  typedef struct fairq_bucket {
> P>
> P> Modified: head/sys/net/altq/altq_hfsc.c
> P>
> ==============================================================================
> P> --- head/sys/net/altq/altq_hfsc.c    Wed Aug 22 18:19:56 2018
> (r338208)
> P> +++ head/sys/net/altq/altq_hfsc.c    Wed Aug 22 19:38:48 2018
> (r338209)
> P> @@ -116,10 +116,10 @@ static struct hfsc_class
>  *actlist_firstfit(struct hfsc
> P>
> P>  static __inline u_int64_t   seg_x2y(u_int64_t, u_int64_t);
> P>  static __inline u_int64_t   seg_y2x(u_int64_t, u_int64_t);
> P> -static __inline u_int64_t   m2sm(u_int);
> P> -static __inline u_int64_t   m2ism(u_int);
> P> +static __inline u_int64_t   m2sm(u_int64_t);
> P> +static __inline u_int64_t   m2ism(u_int64_t);
> P>  static __inline u_int64_t   d2dx(u_int);
> P> -static u_int                        sm2m(u_int64_t);
> P> +static u_int64_t            sm2m(u_int64_t);
> P>  static u_int                        dx2d(u_int64_t);
> P>
> P>  static void         sc2isc(struct service_curve *, struct internal_sc
> *);
> P> @@ -130,8 +130,10 @@ static u_int64_t        rtsc_x2y(struct runtime_sc
> *, u_int64
> P>  static void         rtsc_min(struct runtime_sc *, struct internal_sc *,
> P>                          u_int64_t, u_int64_t);
> P>
> P> -static void                  get_class_stats(struct hfsc_classstats *,
> P> +static void                  get_class_stats_v0(struct
> hfsc_classstats_v0 *,
> P>                                  struct hfsc_class *);
> P> +static void                  get_class_stats_v1(struct
> hfsc_classstats_v1 *,
> P> +                                struct hfsc_class *);
> P>  static struct hfsc_class    *clh_to_clp(struct hfsc_if *, u_int32_t);
> P>
> P>
> P> @@ -158,7 +160,7 @@ altqdev_decl(hfsc);
> P>   */
> P>  #define     is_a_parent_class(cl)   ((cl)->cl_children != NULL)
> P>
> P> -#define     HT_INFINITY     0xffffffffffffffffLL    /* infinite time
> value */
> P> +#define     HT_INFINITY     0xffffffffffffffffULL   /* infinite time
> value */
> P>
> P>  #ifdef ALTQ3_COMPAT
> P>  /* hif_list keeps all hfsc_if's allocated. */
> P> @@ -226,7 +228,7 @@ hfsc_add_queue(struct pf_altq *a)
> P>  {
> P>      struct hfsc_if *hif;
> P>      struct hfsc_class *cl, *parent;
> P> -    struct hfsc_opts *opts;
> P> +    struct hfsc_opts_v1 *opts;
> P>      struct service_curve rtsc, lssc, ulsc;
> P>
> P>      if ((hif = a->altq_disc) == NULL)
> P> @@ -280,11 +282,15 @@ hfsc_remove_queue(struct pf_altq *a)
> P>  }
> P>
> P>  int
> P> -hfsc_getqstats(struct pf_altq *a, void *ubuf, int *nbytes)
> P> +hfsc_getqstats(struct pf_altq *a, void *ubuf, int *nbytes, int version)
> P>  {
> P>      struct hfsc_if *hif;
> P>      struct hfsc_class *cl;
> P> -    struct hfsc_classstats stats;
> P> +    union {
> P> +            struct hfsc_classstats_v0 v0;
> P> +            struct hfsc_classstats_v1 v1;
> P> +    } stats;
> P> +    size_t stats_size;
> P>      int error = 0;
> P>
> P>      if ((hif = altq_lookup(a->ifname, ALTQT_HFSC)) == NULL)
> P> @@ -293,14 +299,27 @@ hfsc_getqstats(struct pf_altq *a, void *ubuf, int
> *nby
> P>      if ((cl = clh_to_clp(hif, a->qid)) == NULL)
> P>              return (EINVAL);
> P>
> P> -    if (*nbytes < sizeof(stats))
> P> +    if (version > HFSC_STATS_VERSION)
> P>              return (EINVAL);
> P>
> P> -    get_class_stats(&stats, cl);
> P> +    memset(&stats, 0, sizeof(stats));
> P> +    switch (version) {
> P> +    case 0:
> P> +            get_class_stats_v0(&stats.v0, cl);
> P> +            stats_size = sizeof(struct hfsc_classstats_v0);
> P> +            break;
> P> +    case 1:
> P> +            get_class_stats_v1(&stats.v1, cl);
> P> +            stats_size = sizeof(struct hfsc_classstats_v1);
> P> +            break;
> P> +    }
> P>
> P> -    if ((error = copyout((caddr_t)&stats, ubuf, sizeof(stats))) != 0)
> P> +    if (*nbytes < stats_size)
> P> +            return (EINVAL);
> P> +
> P> +    if ((error = copyout((caddr_t)&stats, ubuf, stats_size)) != 0)
> P>              return (error);
> P> -    *nbytes = sizeof(stats);
> P> +    *nbytes = stats_size;
> P>      return (0);
> P>  }
> P>
> P> @@ -1357,27 +1376,17 @@ actlist_firstfit(struct hfsc_class *cl,
> u_int64_t cur_
> P>   *  m: bits/sec
> P>   *  d: msec
> P>   *  internal service curve parameters
> P> - *  sm: (bytes/tsc_interval) << SM_SHIFT
> P> - *  ism: (tsc_count/byte) << ISM_SHIFT
> P> - *  dx: tsc_count
> P> + *  sm: (bytes/machclk tick) << SM_SHIFT
> P> + *  ism: (machclk ticks/byte) << ISM_SHIFT
> P> + *  dx: machclk ticks
> P>   *
> P> - * SM_SHIFT and ISM_SHIFT are scaled in order to keep effective digits.
> P> - * we should be able to handle 100K-1Gbps linkspeed with 200Hz-1GHz CPU
> P> - * speed.  SM_SHIFT and ISM_SHIFT are selected to have at least 3
> effective
> P> - * digits in decimal using the following table.
> P> + * SM_SHIFT and ISM_SHIFT are scaled in order to keep effective
> digits.  we
> P> + * should be able to handle 100K-100Gbps linkspeed with 256 MHz machclk
> P> + * frequency and at least 3 effective digits in decimal.
> P>   *
> P> - *  bits/sec    100Kbps     1Mbps     10Mbps     100Mbps    1Gbps
> P> - *  ----------+-------------------------------------------------------
> P> - *  bytes/nsec  12.5e-6    125e-6     1250e-6    12500e-6   125000e-6
> P> - *  sm(500MHz)  25.0e-6    250e-6     2500e-6    25000e-6   250000e-6
> P> - *  sm(200MHz)  62.5e-6    625e-6     6250e-6    62500e-6   625000e-6
> P> - *
> P> - *  nsec/byte   80000      8000       800        80         8
> P> - *  ism(500MHz) 40000      4000       400        40         4
> P> - *  ism(200MHz) 16000      1600       160        16         1.6
> P>   */
> P>  #define     SM_SHIFT        24
> P> -#define     ISM_SHIFT       10
> P> +#define     ISM_SHIFT       14
> P>
> P>  #define     SM_MASK         ((1LL << SM_SHIFT) - 1)
> P>  #define     ISM_MASK        ((1LL << ISM_SHIFT) - 1)
> P> @@ -1413,16 +1422,16 @@ seg_y2x(u_int64_t y, u_int64_t ism)
> P>  }
> P>
> P>  static __inline u_int64_t
> P> -m2sm(u_int m)
> P> +m2sm(u_int64_t m)
> P>  {
> P>      u_int64_t sm;
> P>
> P> -    sm = ((u_int64_t)m << SM_SHIFT) / 8 / machclk_freq;
> P> +    sm = (m << SM_SHIFT) / 8 / machclk_freq;
> P>      return (sm);
> P>  }
> P>
> P>  static __inline u_int64_t
> P> -m2ism(u_int m)
> P> +m2ism(u_int64_t m)
> P>  {
> P>      u_int64_t ism;
> P>
> P> @@ -1442,13 +1451,13 @@ d2dx(u_int d)
> P>      return (dx);
> P>  }
> P>
> P> -static u_int
> P> +static u_int64_t
> P>  sm2m(u_int64_t sm)
> P>  {
> P>      u_int64_t m;
> P>
> P>      m = (sm * 8 * machclk_freq) >> SM_SHIFT;
> P> -    return ((u_int)m);
> P> +    return (m);
> P>  }
> P>
> P>  static u_int
> P> @@ -1597,7 +1606,89 @@ rtsc_min(struct runtime_sc *rtsc, struct
> internal_sc *
> P>  }
> P>
> P>  static void
> P> -get_class_stats(struct hfsc_classstats *sp, struct hfsc_class *cl)
> P> +get_class_stats_v0(struct hfsc_classstats_v0 *sp, struct hfsc_class
> *cl)
> P> +{
> P> +    sp->class_id = cl->cl_id;
> P> +    sp->class_handle = cl->cl_handle;
> P> +
> P> +#define SATU32(x)   (u_int32_t)uqmin((x), UINT_MAX)
> P> +
> P> +    if (cl->cl_rsc != NULL) {
> P> +            sp->rsc.m1 = SATU32(sm2m(cl->cl_rsc->sm1));
> P> +            sp->rsc.d = dx2d(cl->cl_rsc->dx);
> P> +            sp->rsc.m2 = SATU32(sm2m(cl->cl_rsc->sm2));
> P> +    } else {
> P> +            sp->rsc.m1 = 0;
> P> +            sp->rsc.d = 0;
> P> +            sp->rsc.m2 = 0;
> P> +    }
> P> +    if (cl->cl_fsc != NULL) {
> P> +            sp->fsc.m1 = SATU32(sm2m(cl->cl_fsc->sm1));
> P> +            sp->fsc.d = dx2d(cl->cl_fsc->dx);
> P> +            sp->fsc.m2 = SATU32(sm2m(cl->cl_fsc->sm2));
> P> +    } else {
> P> +            sp->fsc.m1 = 0;
> P> +            sp->fsc.d = 0;
> P> +            sp->fsc.m2 = 0;
> P> +    }
> P> +    if (cl->cl_usc != NULL) {
> P> +            sp->usc.m1 = SATU32(sm2m(cl->cl_usc->sm1));
> P> +            sp->usc.d = dx2d(cl->cl_usc->dx);
> P> +            sp->usc.m2 = SATU32(sm2m(cl->cl_usc->sm2));
> P> +    } else {
> P> +            sp->usc.m1 = 0;
> P> +            sp->usc.d = 0;
> P> +            sp->usc.m2 = 0;
> P> +    }
> P> +
> P> +#undef SATU32
> P> +
> P> +    sp->total = cl->cl_total;
> P> +    sp->cumul = cl->cl_cumul;
> P> +
> P> +    sp->d = cl->cl_d;
> P> +    sp->e = cl->cl_e;
> P> +    sp->vt = cl->cl_vt;
> P> +    sp->f = cl->cl_f;
> P> +
> P> +    sp->initvt = cl->cl_initvt;
> P> +    sp->vtperiod = cl->cl_vtperiod;
> P> +    sp->parentperiod = cl->cl_parentperiod;
> P> +    sp->nactive = cl->cl_nactive;
> P> +    sp->vtoff = cl->cl_vtoff;
> P> +    sp->cvtmax = cl->cl_cvtmax;
> P> +    sp->myf = cl->cl_myf;
> P> +    sp->cfmin = cl->cl_cfmin;
> P> +    sp->cvtmin = cl->cl_cvtmin;
> P> +    sp->myfadj = cl->cl_myfadj;
> P> +    sp->vtadj = cl->cl_vtadj;
> P> +
> P> +    sp->cur_time = read_machclk();
> P> +    sp->machclk_freq = machclk_freq;
> P> +
> P> +    sp->qlength = qlen(cl->cl_q);
> P> +    sp->qlimit = qlimit(cl->cl_q);
> P> +    sp->xmit_cnt = cl->cl_stats.xmit_cnt;
> P> +    sp->drop_cnt = cl->cl_stats.drop_cnt;
> P> +    sp->period = cl->cl_stats.period;
> P> +
> P> +    sp->qtype = qtype(cl->cl_q);
> P> +#ifdef ALTQ_RED
> P> +    if (q_is_red(cl->cl_q))
> P> +            red_getstats(cl->cl_red, &sp->red[0]);
> P> +#endif
> P> +#ifdef ALTQ_RIO
> P> +    if (q_is_rio(cl->cl_q))
> P> +            rio_getstats((rio_t *)cl->cl_red, &sp->red[0]);
> P> +#endif
> P> +#ifdef ALTQ_CODEL
> P> +    if (q_is_codel(cl->cl_q))
> P> +            codel_getstats(cl->cl_codel, &sp->codel);
> P> +#endif
> P> +}
> P> +
> P> +static void
> P> +get_class_stats_v1(struct hfsc_classstats_v1 *sp, struct hfsc_class
> *cl)
> P>  {
> P>      sp->class_id = cl->cl_id;
> P>      sp->class_handle = cl->cl_handle;
> P>
> P> Modified: head/sys/net/altq/altq_hfsc.h
> P>
> ==============================================================================
> P> --- head/sys/net/altq/altq_hfsc.h    Wed Aug 22 18:19:56 2018
> (r338208)
> P> +++ head/sys/net/altq/altq_hfsc.h    Wed Aug 22 19:38:48 2018
> (r338209)
> P> @@ -43,12 +43,21 @@
> P>  extern "C" {
> P>  #endif
> P>
> P> -struct service_curve {
> P> +struct service_curve_v0 {
> P>      u_int   m1;     /* slope of the first segment in bits/sec */
> P>      u_int   d;      /* the x-projection of the first segment in msec */
> P>      u_int   m2;     /* slope of the second segment in bits/sec */
> P>  };
> P>
> P> +struct service_curve_v1 {
> P> +    u_int64_t       m1;   /* slope of the first segment in bits/sec */
> P> +    u_int   d;            /* the x-projection of the first segment in
> msec */
> P> +    u_int64_t       m2;   /* slope of the second segment in bits/sec */
> P> +};
> P> +
> P> +/* Latest version of struct service_curve_vX */
> P> +#define HFSC_SERVICE_CURVE_VERSION  1
> P> +
> P>  /* special class handles */
> P>  #define     HFSC_NULLCLASS_HANDLE   0
> P>  #define     HFSC_MAX_CLASSES        64
> P> @@ -67,12 +76,12 @@ struct service_curve {
> P>  #define     HFSC_UPPERLIMITSC       4
> P>  #define     HFSC_DEFAULTSC
> (HFSC_REALTIMESC|HFSC_LINKSHARINGSC)
> P>
> P> -struct hfsc_classstats {
> P> +struct hfsc_classstats_v0 {
> P>      u_int                   class_id;
> P>      u_int32_t               class_handle;
> P> -    struct service_curve    rsc;
> P> -    struct service_curve    fsc;
> P> -    struct service_curve    usc;    /* upper limit service curve */
> P> +    struct service_curve_v0 rsc;
> P> +    struct service_curve_v0 fsc;
> P> +    struct service_curve_v0 usc;    /* upper limit service curve */
> P>
> P>      u_int64_t               total;  /* total work in bytes */
> P>      u_int64_t               cumul;  /* cumulative work in bytes
> P> @@ -110,6 +119,55 @@ struct hfsc_classstats {
> P>      struct codel_stats      codel;
> P>  };
> P>
> P> +struct hfsc_classstats_v1 {
> P> +    u_int                   class_id;
> P> +    u_int32_t               class_handle;
> P> +    struct service_curve_v1 rsc;
> P> +    struct service_curve_v1 fsc;
> P> +    struct service_curve_v1 usc;    /* upper limit service curve */
> P> +
> P> +    u_int64_t               total;  /* total work in bytes */
> P> +    u_int64_t               cumul;  /* cumulative work in bytes
> P> +                                       done by real-time criteria */
> P> +    u_int64_t               d;              /* deadline */
> P> +    u_int64_t               e;              /* eligible time */
> P> +    u_int64_t               vt;             /* virtual time */
> P> +    u_int64_t               f;              /* fit time for
> upper-limit */
> P> +
> P> +    /* info helpful for debugging */
> P> +    u_int64_t               initvt;         /* init virtual time */
> P> +    u_int64_t               vtoff;          /* cl_vt_ipoff */
> P> +    u_int64_t               cvtmax;         /* cl_maxvt */
> P> +    u_int64_t               myf;            /* cl_myf */
> P> +    u_int64_t               cfmin;          /* cl_mincf */
> P> +    u_int64_t               cvtmin;         /* cl_mincvt */
> P> +    u_int64_t               myfadj;         /* cl_myfadj */
> P> +    u_int64_t               vtadj;          /* cl_vtadj */
> P> +    u_int64_t               cur_time;
> P> +    u_int32_t               machclk_freq;
> P> +
> P> +    u_int                   qlength;
> P> +    u_int                   qlimit;
> P> +    struct pktcntr          xmit_cnt;
> P> +    struct pktcntr          drop_cnt;
> P> +    u_int                   period;
> P> +
> P> +    u_int                   vtperiod;       /* vt period sequence no */
> P> +    u_int                   parentperiod;   /* parent's vt period
> seqno */
> P> +    int                     nactive;        /* number of active
> children */
> P> +
> P> +    /* codel, red and rio related info */
> P> +    int                     qtype;
> P> +    struct redstats         red[3];
> P> +    struct codel_stats      codel;
> P> +};
> P> +
> P> +/*
> P> + * HFSC_STATS_VERSION is defined in altq.h to work around issues
> stemming
> P> + * from mixing of public-API and internal bits in each
> scheduler-specific
> P> + * header.
> P> + */
> P> +
> P>  #ifdef ALTQ3_COMPAT
> P>  struct hfsc_interface {
> P>      char    hfsc_ifname[IFNAMSIZ];  /* interface name (e.g., fxp0) */
> P> @@ -309,6 +367,35 @@ struct hfsc_if {
> P>      struct acc_classifier   hif_classifier;
> P>  #endif
> P>  };
> P> +
> P> +/*
> P> + * Kernel code always wants the latest version - avoid a bunch of
> renames in
> P> + * the code to the current latest versioned name.
> P> + */
> P> +#define     service_curve   __CONCAT(service_curve_v,
> HFSC_SERVICE_CURVE_VERSION)
> P> +
> P> +#else /* _KERNEL */
> P> +
> P> +#ifdef PFIOC_USE_LATEST
> P> +/*
> P> + * Maintaining in-tree consumers of the ioctl interface is easier when
> that
> P> + * code can be written in terms old names that refer to the latest
> interface
> P> + * version as that reduces the required changes in the consumers to
> those
> P> + * that are functionally necessary to accommodate a new interface
> version.
> P> + */
> P> +#define     hfsc_classstats __CONCAT(hfsc_classstats_v,
> HFSC_STATS_VERSION)
> P> +#define     service_curve   __CONCAT(service_curve_v,
> HFSC_SERVICE_CURVE_VERSION)
> P> +
> P> +#else
> P> +/*
> P> + * When building out-of-tree code that is written for the old
> interface,
> P> + * such as may exist in ports for example, resolve the old struct tags
> to
> P> + * the v0 versions.
> P> + */
> P> +#define     hfsc_classstats __CONCAT(hfsc_classstats_v, 0)
> P> +#define     service_curve   __CONCAT(service_curve_v, 0)
> P> +
> P> +#endif /* PFIOC_USE_LATEST */
> P>
> P>  #endif /* _KERNEL */
> P>
> P>
> P> Modified: head/sys/net/altq/altq_priq.c
> P>
> ==============================================================================
> P> --- head/sys/net/altq/altq_priq.c    Wed Aug 22 18:19:56 2018
> (r338208)
> P> +++ head/sys/net/altq/altq_priq.c    Wed Aug 22 19:38:48 2018
> (r338209)
> P> @@ -199,7 +199,7 @@ priq_remove_queue(struct pf_altq *a)
> P>  }
> P>
> P>  int
> P> -priq_getqstats(struct pf_altq *a, void *ubuf, int *nbytes)
> P> +priq_getqstats(struct pf_altq *a, void *ubuf, int *nbytes, int version)
> P>  {
> P>      struct priq_if *pif;
> P>      struct priq_class *cl;
> P>
> P> Modified: head/sys/net/altq/altq_priq.h
> P>
> ==============================================================================
> P> --- head/sys/net/altq/altq_priq.h    Wed Aug 22 18:19:56 2018
> (r338208)
> P> +++ head/sys/net/altq/altq_priq.h    Wed Aug 22 19:38:48 2018
> (r338209)
> P> @@ -112,6 +112,12 @@ struct priq_classstats {
> P>      struct codel_stats      codel;
> P>  };
> P>
> P> +/*
> P> + * PRIQ_STATS_VERSION is defined in altq.h to work around issues
> stemming
> P> + * from mixing of public-API and internal bits in each
> scheduler-specific
> P> + * header.
> P> + */
> P> +
> P>  #ifdef ALTQ3_COMPAT
> P>  struct priq_class_stats {
> P>      struct priq_interface   iface;
> P>
> P> Modified: head/sys/net/altq/altq_subr.c
> P>
> ==============================================================================
> P> --- head/sys/net/altq/altq_subr.c    Wed Aug 22 18:19:56 2018
> (r338208)
> P> +++ head/sys/net/altq/altq_subr.c    Wed Aug 22 19:38:48 2018
> (r338209)
> P> @@ -292,12 +292,12 @@ altq_assert(file, line, failedexpr)
> P>
> P>  /*
> P>   * internal representation of token bucket parameters
> P> - *  rate:   byte_per_unittime << 32
> P> - *          (((bits_per_sec) / 8) << 32) / machclk_freq
> P> - *  depth:  byte << 32
> P> + *  rate:   (byte_per_unittime << TBR_SHIFT)  / machclk_freq
> P> + *          (((bits_per_sec) / 8) << TBR_SHIFT) / machclk_freq
> P> + *  depth:  byte << TBR_SHIFT
> P>   *
> P>   */
> P> -#define     TBR_SHIFT       32
> P> +#define     TBR_SHIFT       29
> P>  #define     TBR_SCALE(x)    ((int64_t)(x) << TBR_SHIFT)
> P>  #define     TBR_UNSCALE(x)  ((x) >> TBR_SHIFT)
> P>
> P> @@ -394,7 +394,20 @@ tbr_set(ifq, profile)
> P>      if (tbr->tbr_rate > 0)
> P>              tbr->tbr_filluptime = tbr->tbr_depth / tbr->tbr_rate;
> P>      else
> P> -            tbr->tbr_filluptime = 0xffffffffffffffffLL;
> P> +            tbr->tbr_filluptime = LLONG_MAX;
> P> +    /*
> P> +     *  The longest time between tbr_dequeue() calls will be about 1
> P> +     *  system tick, as the callout that drives it is scheduled once
> per
> P> +     *  tick.  The refill-time detection logic in tbr_dequeue() can
> only
> P> +     *  properly detect the passage of up to LLONG_MAX machclk ticks.
> P> +     *  Therefore, in order for this logic to function properly in the
> P> +     *  extreme case, the maximum value of tbr_filluptime should be
> P> +     *  LLONG_MAX less one system tick's worth of machclk ticks less
> P> +     *  some additional slop factor (here one more system tick's worth
> P> +     *  of machclk ticks).
> P> +     */
> P> +    if (tbr->tbr_filluptime > (LLONG_MAX - 2 * machclk_per_tick))
> P> +            tbr->tbr_filluptime = LLONG_MAX - 2 * machclk_per_tick;
> P>      tbr->tbr_token = tbr->tbr_depth;
> P>      tbr->tbr_last = read_machclk();
> P>      tbr->tbr_lastop = ALTDQ_REMOVE;
> P> @@ -456,29 +469,6 @@ tbr_timeout(arg)
> P>  }
> P>
> P>  /*
> P> - * get token bucket regulator profile
> P> - */
> P> -int
> P> -tbr_get(ifq, profile)
> P> -    struct ifaltq *ifq;
> P> -    struct tb_profile *profile;
> P> -{
> P> -    struct tb_regulator *tbr;
> P> -
> P> -    IFQ_LOCK(ifq);
> P> -    if ((tbr = ifq->altq_tbr) == NULL) {
> P> -            profile->rate = 0;
> P> -            profile->depth = 0;
> P> -    } else {
> P> -            profile->rate =
> P> -                (u_int)TBR_UNSCALE(tbr->tbr_rate * 8 * machclk_freq);
> P> -            profile->depth = (u_int)TBR_UNSCALE(tbr->tbr_depth);
> P> -    }
> P> -    IFQ_UNLOCK(ifq);
> P> -    return (0);
> P> -}
> P> -
> P> -/*
> P>   * attach a discipline to the interface.  if one already exists, it is
> P>   * overridden.
> P>   * Locking is done in the discipline specific attach functions.
> Basically
> P> @@ -733,34 +723,34 @@ altq_remove_queue(struct pf_altq *a)
> P>   * copyout operations, also it is not yet clear which lock to use.
> P>   */
> P>  int
> P> -altq_getqstats(struct pf_altq *a, void *ubuf, int *nbytes)
> P> +altq_getqstats(struct pf_altq *a, void *ubuf, int *nbytes, int version)
> P>  {
> P>      int error = 0;
> P>
> P>      switch (a->scheduler) {
> P>  #ifdef ALTQ_CBQ
> P>      case ALTQT_CBQ:
> P> -            error = cbq_getqstats(a, ubuf, nbytes);
> P> +            error = cbq_getqstats(a, ubuf, nbytes, version);
> P>              break;
> P>  #endif
> P>  #ifdef ALTQ_PRIQ
> P>      case ALTQT_PRIQ:
> P> -            error = priq_getqstats(a, ubuf, nbytes);
> P> +            error = priq_getqstats(a, ubuf, nbytes, version);
> P>              break;
> P>  #endif
> P>  #ifdef ALTQ_HFSC
> P>      case ALTQT_HFSC:
> P> -            error = hfsc_getqstats(a, ubuf, nbytes);
> P> +            error = hfsc_getqstats(a, ubuf, nbytes, version);
> P>              break;
> P>  #endif
> P>  #ifdef ALTQ_FAIRQ
> P>          case ALTQT_FAIRQ:
> P> -                error = fairq_getqstats(a, ubuf, nbytes);
> P> +                error = fairq_getqstats(a, ubuf, nbytes, version);
> P>                  break;
> P>  #endif
> P>  #ifdef ALTQ_CODEL
> P>      case ALTQT_CODEL:
> P> -            error = codel_getqstats(a, ubuf, nbytes);
> P> +            error = codel_getqstats(a, ubuf, nbytes, version);
> P>              break;
> P>  #endif
> P>      default:
> P>
> P> Modified: head/sys/net/altq/altq_var.h
> P>
> ==============================================================================
> P> --- head/sys/net/altq/altq_var.h     Wed Aug 22 18:19:56 2018
> (r338208)
> P> +++ head/sys/net/altq/altq_var.h     Wed Aug 22 19:38:48 2018
> (r338209)
> P> @@ -196,7 +196,6 @@ u_int8_t read_dsfield(struct mbuf *, struct
> altq_pktat
> P>  void        write_dsfield(struct mbuf *, struct altq_pktattr *,
> u_int8_t);
> P>  void        altq_assert(const char *, int, const char *);
> P>  int tbr_set(struct ifaltq *, struct tb_profile *);
> P> -int tbr_get(struct ifaltq *, struct tb_profile *);
> P>
> P>  int altq_pfattach(struct pf_altq *);
> P>  int altq_pfdetach(struct pf_altq *);
> P> @@ -204,40 +203,40 @@ int    altq_add(struct pf_altq *);
> P>  int altq_remove(struct pf_altq *);
> P>  int altq_add_queue(struct pf_altq *);
> P>  int altq_remove_queue(struct pf_altq *);
> P> -int altq_getqstats(struct pf_altq *, void *, int *);
> P> +int altq_getqstats(struct pf_altq *, void *, int *, int);
> P>
> P>  int cbq_pfattach(struct pf_altq *);
> P>  int cbq_add_altq(struct pf_altq *);
> P>  int cbq_remove_altq(struct pf_altq *);
> P>  int cbq_add_queue(struct pf_altq *);
> P>  int cbq_remove_queue(struct pf_altq *);
> P> -int cbq_getqstats(struct pf_altq *, void *, int *);
> P> +int cbq_getqstats(struct pf_altq *, void *, int *, int);
> P>
> P>  int codel_pfattach(struct pf_altq *);
> P>  int codel_add_altq(struct pf_altq *);
> P>  int codel_remove_altq(struct pf_altq *);
> P> -int codel_getqstats(struct pf_altq *, void *, int *);
> P> +int codel_getqstats(struct pf_altq *, void *, int *, int);
> P>
> P>  int priq_pfattach(struct pf_altq *);
> P>  int priq_add_altq(struct pf_altq *);
> P>  int priq_remove_altq(struct pf_altq *);
> P>  int priq_add_queue(struct pf_altq *);
> P>  int priq_remove_queue(struct pf_altq *);
> P> -int priq_getqstats(struct pf_altq *, void *, int *);
> P> +int priq_getqstats(struct pf_altq *, void *, int *, int);
> P>
> P>  int hfsc_pfattach(struct pf_altq *);
> P>  int hfsc_add_altq(struct pf_altq *);
> P>  int hfsc_remove_altq(struct pf_altq *);
> P>  int hfsc_add_queue(struct pf_altq *);
> P>  int hfsc_remove_queue(struct pf_altq *);
> P> -int hfsc_getqstats(struct pf_altq *, void *, int *);
> P> +int hfsc_getqstats(struct pf_altq *, void *, int *, int);
> P>
> P>  int fairq_pfattach(struct pf_altq *);
> P>  int fairq_add_altq(struct pf_altq *);
> P>  int fairq_remove_altq(struct pf_altq *);
> P>  int fairq_add_queue(struct pf_altq *);
> P>
> P> *** DIFF OUTPUT TRUNCATED AT 1000 LINES ***
> P>
>
> --
> Gleb Smirnoff
>



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAD44qMVCW%2BKP1xNez3ahm1vmzwPpAN3%2BfhNXm5JuVdNNzJpWZQ>