colin/NetworkManager
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

platform: fix handling of fq_codel's memory limit default value

Browse Source 
The memory-limit is an unsigned integer. It is ugly (if not wrong) to compare unsigned
values with "-1". When comparing with the default value we must also use an u32 type.
Instead add a define NM_PLATFORM_FQ_CODEL_MEMORY_LIMIT_UNSET.

Note that like iproute2 we treat NM_PLATFORM_FQ_CODEL_MEMORY_LIMIT_UNSET
to indicate to not set TCA_FQ_CODEL_MEMORY_LIMIT in RTM_NEWQDISC. This
special value is entirely internal to NetworkManager (or iproute2) and
kernel will then choose a default memory limit (of 32MB). So setting
NM_PLATFORM_FQ_CODEL_MEMORY_LIMIT_UNSET means to leave it to kernel to
choose a value (which then chooses 32MB).

See kernel's net/sched/sch_fq_codel.c:

    static int fq_codel_init(struct Qdisc *sch, struct nlattr *opt,
                             struct netlink_ext_ack *extack)
    {
    ...
            q->memory_limit = 32 << 20; /* 32 MBytes */

    static int fq_codel_change(struct Qdisc *sch, struct nlattr *opt,
                               struct netlink_ext_ack *extack)
    ...
            if (tb[TCA_FQ_CODEL_MEMORY_LIMIT])
                    q->memory_limit = min(1U << 31, nla_get_u32(tb[TCA_FQ_CODEL_MEMORY_LIMIT]));

Note that not having zero as default value is problematic. In fields like
"NMPlatformIP4Route.table_coerced" and "NMPlatformRoutingRule.suppress_prefixlen_inverse"
we avoid this problem by storing a coerced value in the structure so that zero is still
the default. We don't do that here for memory-limit, so the caller must always explicitly
set the value.
This commit is contained in:
Thomas Haller
2019-05-01 08:43:31 +02:00
parent b658e3da08
commit 46a904389b
5 changed files with 19 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
src/platform/nm-linux-platform.c
View File

@@ -3515,6 +3515,9 @@ _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"))
obj->qdisc.fq_codel.memory = NM_PLATFORM_FQ_CODEL_MEMORY_LIMIT_UNSET;
if (tb[TCA_OPTIONS]) {
struct nlattr *options_attr;
int remaining;
@@ -4241,7 +4244,7 @@ _nl_msg_new_qdisc (int nlmsg_type,
NLA_PUT_U32 (msg, TCA_FQ_CODEL_QUANTUM, qdisc->fq_codel.quantum);
if (qdisc->fq_codel.ce_threshold != -1)
NLA_PUT_U32 (msg, TCA_FQ_CODEL_CE_THRESHOLD, qdisc->fq_codel.ce_threshold);
if (qdisc->fq_codel.memory != -1)
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);
if (qdisc->fq_codel.ecn)
NLA_PUT_U32 (msg, TCA_FQ_CODEL_ECN, qdisc->fq_codel.ecn);