Page MenuHomeFreeBSD

Fix flags collision causing inability to enable CBQ in ALTQ
ClosedPublic

Authored by karels on Oct 30 2018, 1:32 AM.

Details

Summary

The CBQ BORROW flag conflicts with the RMCF_CODEL flag; the
two sets of definitions actually define the same things. The symptom
is that a kernel with CBQ support and not CODEL fails to load a QoS
policy with the obscure error "pfctl: DIOCADDALTQ: Cannot allocate memory."
If ALTQ_DEBUG is enabled, the error becomes a little clearer:
"rmc_newclass: CODEL not configured for CBQ!" is printed by the kernel.
There really shouldn't be two sets of macros that have to be defined
consistently, but the include structure isn't right for exporting
CBQ flags to altq_rmclass.h. Re-align the definitions, and add
CTASSERTs in the kernel to ensure that the definitions are consistent.

Test Plan
  1. Configure kernel with ALTQ and ALTQ_CBC (and not ALTQ_CODEL).
  2. Test that a pf configuration such as the one in bug 215716 can be

loaded.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

karels created this revision.Oct 30 2018, 1:32 AM
pkelsey accepted this revision.Nov 14 2018, 4:55 PM

To clarify 'the symptom is that a kernel with CBQ support and not CODEL fails to load a QoS policy with the obscure error "pfctl: DIOCADDALTQ: Cannot allocate memory."', to experience this failure it is also required that the queue configuration include the BORROW flag ('borrow' keyword in pf.conf), as that sets the bit that is misinterpreted by rmclass as requesting CODEL.

I verified that cbq is the only user of rm_class, and that for other disciplines, their queue configuration flags are unrelated - in other words, cbq is the only discipline that requires that it's flag values be consistent with the RMCF_* values.

Also, there is no associated compat work required in the pfctl ioctl API, as changing the value of RMCF_CODEL does not affect the meaning of the cbq flag bits in that API, it only fixes the following bugs:

  1. Users with CODEL configured in the kernel who are cbq queues with the BORROW flag ('borrow' keyword in pf.conf) will no longer wind up with CODEL also being erroneously enabled for that queue
  2. BORROW no longer erroneously requires CODEL to be configured in the kernel in order 'to work'
  3. CODEL will correctly be enabled for cbq queues when the BORROW flag ('borrow' keyword in pf.conf) is not also present
This revision is now accepted and ready to land.Nov 14 2018, 4:55 PM
This revision was automatically updated to reflect the committed changes.