From patchwork Mon Jul 16 17:52:18 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jesus Sanchez-Palencia X-Patchwork-Id: 944565 X-Patchwork-Delegate: shemminger@vyatta.com Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=intel.com Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 41TrjC2Tbdz9rvt for ; Tue, 17 Jul 2018 03:57:35 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728742AbeGPS0D (ORCPT ); Mon, 16 Jul 2018 14:26:03 -0400 Received: from mga07.intel.com ([134.134.136.100]:9672 "EHLO mga07.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727957AbeGPS0D (ORCPT ); Mon, 16 Jul 2018 14:26:03 -0400 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga105.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 16 Jul 2018 10:57:33 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.51,362,1526367600"; d="scan'208";a="71847134" Received: from darjeeling.jf.intel.com ([10.24.15.98]) by fmsmga004.fm.intel.com with ESMTP; 16 Jul 2018 10:57:30 -0700 From: Jesus Sanchez-Palencia To: netdev@vger.kernel.org Cc: jhs@mojatatu.com, xiyou.wangcong@gmail.com, jiri@resnulli.us, serhe.popovych@gmail.com, alexander.duyck@gmail.com, stephen@networkplumber.org, Jesus Sanchez-Palencia Subject: [PATCH v1 iproute2] tc: Do not use addattr_nest_compat on mqprio and netem Date: Mon, 16 Jul 2018 10:52:18 -0700 Message-Id: <20180716175218.407-1-jesus.sanchez-palencia@intel.com> X-Mailer: git-send-email 2.18.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org 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 Signed-off-by: Jesus Sanchez-Palencia --- tc/q_mqprio.c | 5 +++-- tc/q_netem.c | 7 +++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/tc/q_mqprio.c b/tc/q_mqprio.c index 207d6441..89b46002 100644 --- 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 623ec903..9f9a9b3d 100644 --- 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; }