The short story is that as part of r343287, code was added to get rid of the false-positive warnings from pfctl that the sum of the child queue bandwidths exceeded the parent that could occur in certain HFSC configurations, and that code has a bug if a queue has an upperlimit service curve that is above the linkshare service curve of its parent - in that case, the upperlimit service curve winds up being erroneously pulled down to the parent's linkshare service curve. In practical terms, the result of this bug would be that any affected HFSC queue would wind up functioning subject to a tighter constraint than expected when it wasn't competing for the link.
It turns out that in the HFSC case, this sum-of-child-bandwidths check logic should not be used at all - in HFSC, the conceptually equivalent form of this check is to make sure that the sum of the child linkshare service curves is at or under the parent's linkshare service curves, as those are the constraints that apply when the queues are competing for the link. This check is already implemented in eval_pfqueue_hfsc().
This patch removes the new parameter conditioning logic introduced in r343287, bypasses the sum-of-child-bandwidths check for HFSC, and moves the call to eval_queue_opts() back to where it was prior to r343287.
Here is a more exhaustive discussion:
The previous fix attempted to make the sum-of-child-bandwidths check
logic 'work for HFSC' by overriding the queue bandwidth parameter with
the link share m2 parameter if it was set.
For HFSC, the queue bandwidth parameter is only used in pfctl (not
used by the kernel logic at all) to set an upper limit for the m2
parameters for all three service curves (realtime, linkshare, and
upperlimit) - specifically, a given queue's m2 parameter on any curve
is limited to the parent's bandwidth parameter. As a valid config
can't have a queue whose linkshare m2 parameter exceeds its parent's
linkshare m2 parameter, and likewise for the realtime m2 parameter (as
the realtime m2 parameter logically can't be greater than the
linkshare m2, it thus transitively can't be greater than the parent's
linkshare m2 parameter), limiting a queue's bandwidth parameter to the
supplied linkshare m2 parameter appeared to be a valid way to pass the
sum-of-child-bandwidths check.
The flaw in the reasoning appears when properly considering the
upperlimit service curve. For example, a configuration may have a queue
whose upperlimit m2 parameter is the same as the interface bandwidth,
and thus greater than any parent linkshare m2 parameter that is less
than the interface bandwidth (such a config would express the idea
that when the queue isn't competing for the link, it can use the whole
thing). Such a config would be broken by the above queue bandwidth
parameter = linkshare m2 parameter approach, as that approach will
pull down any upperlimit m2 parameter that is greater than the parent
queue's linkshare m2 parameter to that parent's linkshare m2 value. The
practical result of this pulling down would be that a queue that
otherwise could have the entire link to itself under the configuration
intent would still be limited to the linkshare limitation of its
parent.
Taking a step back, the next key observation is that the
sum-of-child-bandwidths check should not be appeased for HFSC, it
should be skipped. For HFSC, a check of this type only make sense
when the child queues are competing for the link. When that is
happening, resource allocation occurs according to the child linkshare
service curves, so what should be checked is that the sum of the child
linkshare service curves is under the parent linkshare service curve.
That check is already done by eval_pfqueue_hfsc().
So, in eval_pfqueue(), the sum-of-child-bandwidths check is now
skipped for HFSC, and the call to eval_queue_opts() is moved back to
where it used to be (it was moved up in order to evlauate the limit
logic on the linkshare m2 option prior to making a decision based on
it - a decision which has now been removed).