From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jiri Pirko Subject: Re: [PATCH net] net: sched: Remove TCA_OPTIONS from policy Date: Thu, 25 Oct 2018 08:31:48 +0200 Message-ID: <20181025063148.GA2143@nanopsycho> References: <20181024153249.15374-1-dsahern@kernel.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: netdev@vger.kernel.org, davem@davemloft.net, pupilla@libero.it, David Ahern To: David Ahern Return-path: Received: from mail-wr1-f66.google.com ([209.85.221.66]:36692 "EHLO mail-wr1-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726374AbeJYPJB (ORCPT ); Thu, 25 Oct 2018 11:09:01 -0400 Received: by mail-wr1-f66.google.com with SMTP id y16so8002716wrw.3 for ; Wed, 24 Oct 2018 23:37:41 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20181024153249.15374-1-dsahern@kernel.org> Sender: netdev-owner@vger.kernel.org List-ID: Wed, Oct 24, 2018 at 05:32:49PM CEST, dsahern@kernel.org wrote: >From: David Ahern > >Marco reported an error with hfsc: >root@Calimero:~# tc qdisc add dev eth0 root handle 1:0 hfsc default 1 >Error: Attribute failed policy validation. > >Apparently a few implementations pass TCA_OPTIONS as a binary instead >of nested attribute, so drop TCA_OPTIONS from the policy. Yeah, this is nice example of a case, where I think it wouldn't hurt to be a bit more strict. Apparently, the userspace app is buggy. It should be fixed. Note that I'm aware of the bw compatibility. > >Fixes: 8b4c3cdd9dd8 ("net: sched: Add policy validation for tc attributes") >Reported-by: Marco Berizzi >Signed-off-by: David Ahern >--- > net/sched/sch_api.c | 1 - > 1 file changed, 1 deletion(-) > >diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c >index 3dc0acf54245..be7cd140b2a3 100644 >--- a/net/sched/sch_api.c >+++ b/net/sched/sch_api.c >@@ -1309,7 +1309,6 @@ check_loop_fn(struct Qdisc *q, unsigned long cl, struct qdisc_walker *w) > > const struct nla_policy rtm_tca_policy[TCA_MAX + 1] = { > [TCA_KIND] = { .type = NLA_STRING }, >- [TCA_OPTIONS] = { .type = NLA_NESTED }, A future developer might add this again. A comment why not would be good here. > [TCA_RATE] = { .type = NLA_BINARY, > .len = sizeof(struct tc_estimator) }, > [TCA_STAB] = { .type = NLA_NESTED }, >-- >2.11.0 >