tc: Do not use addattr_nest_compat on mqprio and netem

Here we are partially reverting commit c14f9d92eee107
"treewide: Use addattr_nest()/addattr_nest_end() to handle nested
attributes" .

As discussed in [1], changing from the 'manually' coded version that
used addattr_l() to addattr_nest_compat() wasn't functionally
equivalent, because now the messages have extra fields appended to it.

This introduced a regression since the implementation of parse_attr()
from both mqprio and netem can't handle this new message format.

Without this fix, mqprio returns an error. netem won't return an error
but its internal configuration ends up wrong.

As an example, this can be reproduced by the following commands when
this patch is not applied:

 1) mqprio
$ tc qdisc replace dev enp3s0 parent root handle 100 mqprio \
	num_tc 3 map 2 2 1 0 2 2 2 2 2 2 2 2 2 2 2 2 \
	queues 1@0 1@1 2@2 hw 0

RTNETLINK answers: Numerical result out of range

 2) netem
$ tc qdisc add dev enp3s0 root netem rate 5kbit 20 100 5 \
	distribution normal latency 1 1

$ tc -s qdisc

(...)
qdisc netem 8001: dev enp3s0 root refcnt 9 limit 1000 delay 0us  0us
 Sent 402 bytes 1 pkt (dropped 0, overlimits 0 requeues 0)
 backlog 0b 0p requeues 0
(...)

With this patch applied, the tc -s qdisc command above for netem instead
reads:

(...)
qdisc netem 8002: dev enp3s0 root refcnt 9 limit 1000 delay 0us  0us \
	rate 5Kbit packetoverhead 20 cellsize 100 celloverhead 5
 Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
 backlog 0b 0p requeues 0
(...)

[1] https://patchwork.ozlabs.org/patch/867860/#1893405

Fixes: c14f9d92eee107 ("treewide: Use addattr_nest()/addattr_nest_end() to handle nested attributes")
Reported-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
Signed-off-by: Jesus Sanchez-Palencia <jesus.sanchez-palencia@intel.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

diff --git a/tc/q_mqprio.c b/tc/q_mqprio.c
index 207d6441d8fa35adb8c68dcde68b1ba270702c28..89b460020e27ea8e63e07c2d98eff04ef5819ac3 100644 (file)
--- a/tc/q_mqprio.c
+++ b/tc/q_mqprio.c
@@ -173,7 +173,8 @@ static int mqprio_parse_opt(struct qdisc_util *qu, int argc,
                argc--; argv++;
        }
 
-       tail = addattr_nest_compat(n, 1024, TCA_OPTIONS, &opt, sizeof(opt));
+       tail = NLMSG_TAIL(n);
+       addattr_l(n, 1024, TCA_OPTIONS, &opt, sizeof(opt));
 
        if (flags & TC_MQPRIO_F_MODE)
                addattr_l(n, 1024, TCA_MQPRIO_MODE,
@@ -208,7 +209,7 @@ static int mqprio_parse_opt(struct qdisc_util *qu, int argc,
                addattr_nest_end(n, start);
        }
 
-       addattr_nest_compat_end(n, tail);
+       tail->rta_len = (void *)NLMSG_TAIL(n) - (void *)tail;
 
        return 0;
 }

diff --git a/tc/q_netem.c b/tc/q_netem.c
index 623ec9038ad37ae09b91b7936c8f5b6050761009..9f9a9b3df255f89fac5269c3f1a43ddb86a8cf71 100644 (file)
--- a/tc/q_netem.c
+++ b/tc/q_netem.c
@@ -422,6 +422,8 @@ static int netem_parse_opt(struct qdisc_util *qu, int argc, char **argv,
                }
        }
 
+       tail = NLMSG_TAIL(n);
+
        if (reorder.probability) {
                if (opt.latency == 0) {
                        fprintf(stderr, "reordering not possible without specifying some delay\n");
@@ -450,7 +452,8 @@ static int netem_parse_opt(struct qdisc_util *qu, int argc, char **argv,
                return -1;
        }
 
-       tail = addattr_nest_compat(n, 1024, TCA_OPTIONS, &opt, sizeof(opt));
+       if (addattr_l(n, 1024, TCA_OPTIONS, &opt, sizeof(opt)) < 0)
+               return -1;
 
        if (present[TCA_NETEM_CORR] &&
            addattr_l(n, 1024, TCA_NETEM_CORR, &cor, sizeof(cor)) < 0)
@@ -509,7 +512,7 @@ static int netem_parse_opt(struct qdisc_util *qu, int argc, char **argv,
                        return -1;
                free(dist_data);
        }
-       addattr_nest_compat_end(n, tail);
+       tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
        return 0;
 }