Commit 973db2d4 authored by Thomas Haller's avatar Thomas Haller

platform: fix handling of default value for TCA_FQ_CODEL_CE_THRESHOLD

iproute2 uses the special value ~0u to indicate not to set
TCA_FQ_CODEL_CE_THRESHOLD in RTM_NEWQDISC. When not explicitly
setting the value, kernel treats the threshold as disabled.

However note that 0xFFFFFFFFu is not an invalid threshold (as far as
kernel is concerned). Thus, we should not use that as value to indicate
that the value is unset. Note that iproute2 uses the special value ~0u
only internally thereby making it impossible to set the threshold to
0xFFFFFFFFu). But kernel does not have this limitation.

Maybe the cleanest way would be to add another field to NMPlatformQDisc:

    guint32 ce_threshold;
    bool ce_threshold_set:1;

that indicates whether the threshold is enable or not.
But note that kernel does:

    static void codel_params_init(struct codel_params *params)
    {
    ...
            params->ce_threshold = CODEL_DISABLED_THRESHOLD;

    static int fq_codel_change(struct Qdisc *sch, struct nlattr *opt,
                               struct netlink_ext_ack *extack)
    {
    ...
            if (tb[TCA_FQ_CODEL_CE_THRESHOLD]) {
                    u64 val = nla_get_u32(tb[TCA_FQ_CODEL_CE_THRESHOLD]);

                    q->cparams.ce_threshold = (val * NSEC_PER_USEC) >> CODEL_SHIFT;
            }

    static int fq_codel_dump(struct Qdisc *sch, struct sk_buff *skb)
    {
    ...
            if (q->cparams.ce_threshold != CODEL_DISABLED_THRESHOLD &&
                nla_put_u32(skb, TCA_FQ_CODEL_CE_THRESHOLD,
                            codel_time_to_us(q->cparams.ce_threshold)))
                    goto nla_put_failure;

This means, kernel internally uses the special value 0x83126E97u to indicate
that the threshold is disabled (WTF). That is because

  (((guint64) 0x83126E97u) * NSEC_PER_USEC) >> CODEL_SHIFT == CODEL_DISABLED_THRESHOLD

So in kernel API this value is reserved (and has a special meaning
to indicate that the threshold is disabled). So, instead of adding a
ce_threshold_set flag, use the same value that kernel anyway uses.
parent 46a90438
......@@ -2332,6 +2332,9 @@ static const NMVariantAttributeSpec *const tc_qdisc_fq_codel_spec[] = {
NM_VARIANT_ATTRIBUTE_SPEC_DEFINE ("target", G_VARIANT_TYPE_UINT32, ),
NM_VARIANT_ATTRIBUTE_SPEC_DEFINE ("interval", G_VARIANT_TYPE_UINT32, ),
NM_VARIANT_ATTRIBUTE_SPEC_DEFINE ("quantum", G_VARIANT_TYPE_UINT32, ),
/* 0x83126E97u is not a valid value (it means "disabled"). We should reject that
* value. Or alternatively, reject all values >= MAX_INT(32). */
NM_VARIANT_ATTRIBUTE_SPEC_DEFINE ("ce_threshold", G_VARIANT_TYPE_UINT32, ),
/* kernel clamps the value at 2^31. Possibly such values should be rejected from configuration
......
......@@ -6542,7 +6542,7 @@ tc_commit (NMDevice *self)
GET_ATTR("target", qdisc->fq_codel.target, UINT32, uint32, 0);
GET_ATTR("interval", qdisc->fq_codel.interval, UINT32, uint32, 0);
GET_ATTR("quantum", qdisc->fq_codel.quantum, UINT32, uint32, 0);
GET_ATTR("ce_threshold", qdisc->fq_codel.ce_threshold, UINT32, uint32, -1);
GET_ATTR("ce_threshold", qdisc->fq_codel.ce_threshold, UINT32, uint32, NM_PLATFORM_FQ_CODEL_CE_THRESHOLD_DISABLED);
GET_ATTR("memory", qdisc->fq_codel.memory, UINT32, uint32, NM_PLATFORM_FQ_CODEL_MEMORY_LIMIT_UNSET);
GET_ATTR("ecn", qdisc->fq_codel.ecn, BOOLEAN, boolean, FALSE);
}
......
......@@ -3515,8 +3515,10 @@ _new_from_nl_qdisc (struct nlmsghdr *nlh, gboolean id_only)
obj->qdisc.parent = tcm->tcm_parent;
obj->qdisc.info = tcm->tcm_info;
if (nm_streq0 (obj->qdisc.kind, "fq_codel"))
if (nm_streq0 (obj->qdisc.kind, "fq_codel")) {
obj->qdisc.fq_codel.memory = NM_PLATFORM_FQ_CODEL_MEMORY_LIMIT_UNSET;
obj->qdisc.fq_codel.ce_threshold = NM_PLATFORM_FQ_CODEL_CE_THRESHOLD_DISABLED;
}
if (tb[TCA_OPTIONS]) {
struct nlattr *options_attr;
......@@ -4242,7 +4244,7 @@ _nl_msg_new_qdisc (int nlmsg_type,
NLA_PUT_U32 (msg, TCA_FQ_CODEL_INTERVAL, qdisc->fq_codel.interval);
if (qdisc->fq_codel.quantum)
NLA_PUT_U32 (msg, TCA_FQ_CODEL_QUANTUM, qdisc->fq_codel.quantum);
if (qdisc->fq_codel.ce_threshold != -1)
if (qdisc->fq_codel.ce_threshold != NM_PLATFORM_FQ_CODEL_CE_THRESHOLD_DISABLED)
NLA_PUT_U32 (msg, TCA_FQ_CODEL_CE_THRESHOLD, qdisc->fq_codel.ce_threshold);
if (qdisc->fq_codel.memory != NM_PLATFORM_FQ_CODEL_MEMORY_LIMIT_UNSET)
NLA_PUT_U32 (msg, TCA_FQ_CODEL_MEMORY_LIMIT, qdisc->fq_codel.memory);
......
......@@ -6453,7 +6453,7 @@ nm_platform_qdisc_to_string (const NMPlatformQdisc *qdisc, char *buf, gsize len)
nm_utils_strbuf_append (&buf, &len, " interval %u", qdisc->fq_codel.interval);
if (qdisc->fq_codel.quantum)
nm_utils_strbuf_append (&buf, &len, " quantum %u", qdisc->fq_codel.quantum);
if (qdisc->fq_codel.ce_threshold != -1)
if (qdisc->fq_codel.ce_threshold != NM_PLATFORM_FQ_CODEL_CE_THRESHOLD_DISABLED)
nm_utils_strbuf_append (&buf, &len, " ce_threshold %u", qdisc->fq_codel.ce_threshold);
if (qdisc->fq_codel.memory != NM_PLATFORM_FQ_CODEL_MEMORY_LIMIT_UNSET)
nm_utils_strbuf_append (&buf, &len, " memory %u", qdisc->fq_codel.memory);
......
......@@ -598,13 +598,23 @@ typedef struct {
#define NM_PLATFORM_FQ_CODEL_MEMORY_LIMIT_UNSET (~((guint32) 0))
#define NM_PLATFORM_FQ_CODEL_CE_THRESHOLD_DISABLED ((guint32) 0x83126E97u)
G_STATIC_ASSERT (((((guint64) NM_PLATFORM_FQ_CODEL_CE_THRESHOLD_DISABLED) * 1000u) >> 10) == (guint64) INT_MAX);
typedef struct {
guint32 limit;
guint32 flows;
guint32 target;
guint32 interval;
guint32 quantum;
guint32 ce_threshold;
guint32 ce_threshold; /* TCA_FQ_CODEL_CE_THRESHOLD: kernel internally stores this value as
* ((val64 * NSEC_PER_USEC) >> CODEL_SHIFT). The default value (in
* the domain with this coersion) is CODEL_DISABLED_THRESHOLD (INT_MAX).
* That means, "disabled" is expressed on RTM_NEWQDISC netlink API by absence of the
* netlink attribute but also as the special value 0x83126E97u
* (NM_PLATFORM_FQ_CODEL_CE_THRESHOLD_DISABLED).
* Beware: zero is not the default you must always explicitly set this value. */
guint32 memory; /* TCA_FQ_CODEL_MEMORY_LIMIT: note that only values <= 2^31 are accepted by kernel
* and kernel defaults to 32MB.
* Note that we use the special value NM_PLATFORM_FQ_CODEL_MEMORY_LIMIT_UNSET
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment