diff options
author | Thomas Haller <thaller@redhat.com> | 2019-05-01 08:43:31 +0200 |
---|---|---|
committer | Thomas Haller <thaller@redhat.com> | 2019-05-07 20:58:17 +0200 |
commit | 46a904389bf13a2cfd40888ab2a843827fda7469 (patch) | |
tree | 8397586223c80b8ef6759d3a534f607be2b1bed6 /src/platform | |
parent | b658e3da0825cf6e62e0850d3508dde1dd5c1914 (diff) | |
download | NetworkManager-46a904389bf13a2cfd40888ab2a843827fda7469.tar.gz |
platform: fix handling of fq_codel's memory limit default value
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.
Diffstat (limited to 'src/platform')
-rw-r--r-- | src/platform/nm-linux-platform.c | 5 | ||||
-rw-r--r-- | src/platform/nm-platform.c | 2 | ||||
-rw-r--r-- | src/platform/nm-platform.h | 9 |
3 files changed, 13 insertions, 3 deletions
diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index 290c31ad13..2c7ba4015a 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -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); diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c index 4d3b61405d..fc49db19a1 100644 --- a/src/platform/nm-platform.c +++ b/src/platform/nm-platform.c @@ -6455,7 +6455,7 @@ nm_platform_qdisc_to_string (const NMPlatformQdisc *qdisc, char *buf, gsize len) nm_utils_strbuf_append (&buf, &len, " quantum %u", qdisc->fq_codel.quantum); if (qdisc->fq_codel.ce_threshold != -1) nm_utils_strbuf_append (&buf, &len, " ce_threshold %u", qdisc->fq_codel.ce_threshold); - if (qdisc->fq_codel.memory != -1) + if (qdisc->fq_codel.memory != NM_PLATFORM_FQ_CODEL_MEMORY_LIMIT_UNSET) nm_utils_strbuf_append (&buf, &len, " memory %u", qdisc->fq_codel.memory); if (qdisc->fq_codel.ecn) nm_utils_strbuf_append (&buf, &len, " ecn"); diff --git a/src/platform/nm-platform.h b/src/platform/nm-platform.h index 16747f093b..9f9fcf5c31 100644 --- a/src/platform/nm-platform.h +++ b/src/platform/nm-platform.h @@ -596,6 +596,8 @@ typedef struct { bool uid_range_has:1; /* has(FRA_UID_RANGE) */ } NMPlatformRoutingRule; +#define NM_PLATFORM_FQ_CODEL_MEMORY_LIMIT_UNSET (~((guint32) 0)) + typedef struct { guint32 limit; guint32 flows; @@ -603,7 +605,12 @@ typedef struct { guint32 interval; guint32 quantum; guint32 ce_threshold; - guint32 memory; + 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 + * to indicate that no explicit limit is set (when we send a RTM_NEWQDISC request). + * This will cause kernel to choose the default (32MB). + * Beware: zero is not the default you must always explicitly set this value. */ bool ecn:1; } NMPlatformQdiscFqCodel; |