From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.toke.dk (mail.toke.dk [52.28.52.200]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.bufferbloat.net (Postfix) with ESMTPS id 7832D3BA8E for ; Thu, 19 Jul 2018 06:53:55 -0400 (EDT) From: Toke =?utf-8?Q?H=C3=B8iland-J=C3=B8rgensen?= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=toke.dk; s=20161023; t=1531997634; bh=OUtJUTL9BXCDhEEJEonUv+ZrJs+F7u2B54vInaPNQgE=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=battn64eK3v5TYC6u6H8zz3k1DNJAmTUZy7kwsPC1TNNhTt1Yxr+/jCJDXcA8zBHR jfmfd5KMdDbyG5CTUYb6ogiBVqs4Uek/uNqIM2bFCs9W20kyx1dV4JccUVio6pBJHN vvanOFhZWqMFE4sDxkMoR3geRQXclFXudkBoZ9j4JB12u9lcNiAh1IXwPrxdAwpXLn 4eM97vrHxZFEAqQcy+sDYAxxasDI33funoXwtrDiW7DC2ePkxICmmYhbvs860aWIEl ZNl5Q/P3G0FMbkuNWOY6tyizf0NIoI+AL8rNYpiRN7MrXQeGsP9hCiIoJwIDJyGKLe 3A1Q+pdrON51w== To: David Ahern , netdev@vger.kernel.org Cc: cake@lists.bufferbloat.net, Dave Taht In-Reply-To: References: <20180716163926.4826-1-toke@toke.dk> Date: Thu, 19 Jul 2018 12:53:44 +0200 X-Clacks-Overhead: GNU Terry Pratchett Message-ID: <87bmb34htz.fsf@toke.dk> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Cake] [PATCH iproute2-next v10] Add support for CAKE qdisc X-BeenThere: cake@lists.bufferbloat.net X-Mailman-Version: 2.1.20 Precedence: list List-Id: Cake - FQ_codel the next generation List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 19 Jul 2018 10:53:55 -0000 A few comments below; will fix the rest. >> + print_uint(PRINT_JSON, "bandwidth", NULL, bandwidth); >> + print_string(PRINT_FP, NULL, "bandwidth %s ", sprint_rate(bandwidth, b1)); >> + } else >> + print_string(PRINT_ANY, "bandwidth", "bandwidth %s ", "unlimited"); >> + } >> + if (tb[TCA_CAKE_AUTORATE] && >> + RTA_PAYLOAD(tb[TCA_CAKE_AUTORATE]) >= sizeof(__u32)) { >> + autorate = rta_getattr_u32(tb[TCA_CAKE_AUTORATE]); >> + if(autorate == 1) >> + print_string(PRINT_ANY, "autorate", "autorate_%s ", "ingress"); >> + else if(autorate) >> + print_string(PRINT_ANY, "autorate", "(?autorate?) ", "unknown"); > > Why the '(?' and '?)'? here and the diffserv below. The (? ?) indicates that a value was present, but it was not understood by tc. This has been quite useful to discover mismatch between kernel and userspace versions of CAKE as we have been developing it. >> + } >> + >> + if (tb[TCA_CAKE_NAT] && >> + RTA_PAYLOAD(tb[TCA_CAKE_NAT]) >= sizeof(__u32)) { >> + nat = rta_getattr_u32(tb[TCA_CAKE_NAT]); >> + } >> + >> + if(nat) >> + print_string(PRINT_FP, NULL, "nat ", NULL); >> + print_bool(PRINT_JSON, "nat", NULL, nat); > > why is the fp print under the if check but the json one is not? you > have this in a number of places. Why not be consistent in the output? Because JSON can actually express booleans properly, and thus we can be explicit about its value instead of just omitting it. -Toke