From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-io0-x22f.google.com (mail-io0-x22f.google.com [IPv6:2607:f8b0:4001:c06::22f]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by lists.bufferbloat.net (Postfix) with ESMTPS id E602C3CB38 for ; Sat, 10 Mar 2018 11:30:30 -0500 (EST) Received: by mail-io0-x22f.google.com with SMTP id v6so6819271iog.7 for ; Sat, 10 Mar 2018 08:30:30 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bitamins-net.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=QvFz3WD6ZwwKu7Wpzu41g4eUHTDpHDsenBtEg+7ytE8=; b=wTpfdY9ww73jxLkGEmAy9pH9TnqZt8/+fKDxT1qBeCasyfWgW+FE1/EQIfSUdSg+Rb DfGEZC8aSeAHgPdL2Vghh/Zt3c/HO+0hcmi7bYc6Ma2Ar5vD78Zz7ZDFdbHO6dIl7x2T lh13C1xBZsrfxl28uEZlhG6vacTEWrJkUsR1fs+PJwcE6y2AWqqx7i++EvGxsZ4jZ2Sa Cn+d1LDhKWo4uxQICz1FjPak9bYJ0NzESVCTudVlNinlcZdBNrh3wO1p1BBbWJQxryEN 4HF8p9OficfQwxNiUQjpULiJAScTnVaWpJtFvtiTTcc/0PY5ZZ8AU4zEoSzlsOhLacay z+Fg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=QvFz3WD6ZwwKu7Wpzu41g4eUHTDpHDsenBtEg+7ytE8=; b=TsldG/x1gL+zowEVUlD6JUr7IllZrv4FHVwWpibnlBzxudKyUzpc9Io/l1e8IRE1PG XdblMgg3L59JQihXetbP0StsRtoLgtdPJOv922d07KwGG/tq/D6hvZXuNDFQC3OhcsLH 7EWm4Sics+kn2aDtxwElAF5UfLBFSZeSZtH5TQFzY/dxHonggEdutV854DGiTrQ9wtL5 pHPPkliezgbwAWnEeo9HTC6fR8pPabRfravmYFcf0r+O2MU+57Ui4XwfVS8sCcWCzUxI lD+IuQKkOjMkGz4BtK0W/X8RRgnP/FHhgZjFXsVpFcjrH2ssCN50X1RhKIDzw3H1fkfl FTNQ== X-Gm-Message-State: AElRT7HQzoKAge+FLyIQSwRhsL+xquOK2mfGEsLrAoiJ0T5NZToXf3fs Ef9mdFZzmsUpMDXZTWn7EfoVAiidQcrfk3IDUcDSXg== X-Google-Smtp-Source: AG47ELt/+uhaizosRbsMuMoUgbhmhNz+Ogxtop/eX71YUYF0TTOKlRH/5Ha0u6kVmjMKjWZLI4I9AHW4p4vMAjvByC4= X-Received: by 10.107.223.71 with SMTP id d7mr2632601iop.95.1520699430363; Sat, 10 Mar 2018 08:30:30 -0800 (PST) MIME-Version: 1.0 References: <20180127130542.25817-1-toke@toke.dk> <876068nccm.fsf@toke.dk> <0A156FD3-4816-462C-952B-7938EF8C0EA3@gmail.com> <87sh9cl15n.fsf@toke.dk> <879005B6-2334-4F46-9922-4BC2CACBF107@darbyshire-bryant.me.uk> <87vae8yy68.fsf@toke.dk> <87sh9cyxy9.fsf@toke.dk> <340269AF-2325-4657-8939-5E3195D1C0DC@darbyshire-bryant.me.uk> <87muzkyvj7.fsf@toke.dk> <11DDD9A0-BBD7-4DAA-ACE1-EB88CDC26E7C@darbyshire-bryant.me.uk> <87h8psyraq.fsf@toke.dk> <6F0B516E-3244-4A93-8762-3EAF3BD71BF2@darbyshire-bryant.me.uk> <87k1unlgck.fsf@toke.dk> <4654DB0C-74E6-46FB-9F62-119076D0A20B@darbyshire-bryant.me.uk> <87371a3ljk.fsf@toke.dk> <87sh9a25ta.fsf@toke.dk> <8B3F5A39-852D-41CE-B623-C65FCBA037FF@darbyshire-bryant.me.uk> <04044BF0-504A-41DC-8F1B-A9528B4E887A@darbyshire-bryant.me.uk> <87po4e24ph.fsf@toke.dk> In-Reply-To: From: "Luis E. Garcia" Date: Sat, 10 Mar 2018 16:30:19 +0000 Message-ID: To: Kevin Darbyshire-Bryant Cc: =?UTF-8?B?VG9rZSBIw7hpbGFuZC1Kw7hyZ2Vuc2Vu?= , "cake@lists.bufferbloat.net" Content-Type: multipart/alternative; boundary="f4f5e80dac3c5b7c2f0567116da0" Subject: Re: [Cake] [PATCH] q_cake: Update xstats format to use per-tin structure 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: Sat, 10 Mar 2018 16:30:31 -0000 --f4f5e80dac3c5b7c2f0567116da0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Nice work Kevin. On Sat, Mar 10, 2018 at 09:56 Kevin Darbyshire-Bryant < kevin@darbyshire-bryant.me.uk> wrote: > > > > On 8 Mar 2018, at 11:21, Toke H=C3=B8iland-J=C3=B8rgensen wrote: > > > >> > >> Oh and curiously the bad values go away if you ask for json output > >> it=E2=80=99s much better. Which rather points at a =E2=80=98feature= =E2=80=99 of the > >> =E2=80=98print_string=E2=80=99 behaviour. > > > > Right. Well, the print_* functions are behind several levels of > > pre-processor indirection, so not quite obvious what's going on here. > > Don't really see why they should spit out garbage values, though. > > > > > > Stephen, do you have any ideas? > > > > -Toke > > Right, cracked it and it=E2=80=99s horrible! > > print_uint is expanded thus: Note the type of value uint64_t > > void print_color_uint(enum output_type t, enum color_attr > color, const char *key, const char *fmt, uint64_t value); > static inline void print_uint (enum output_type t, > const char *key, const char *fmt, uint64_t value) > { print_color_uint( t, COLOR_NONE, > key, fmt, value); }; > > So far so good. > > print_color_uint expands to: > > void print_color_uint(enum output_type t, enum color_attr > color, const char *key, const char *fmt, uint64_t value) > { > if (((t & PRINT_JSON || t & PRINT_ANY) && _jw)) > { if (!key) jsonw_uint(_jw, value); > else jsonw_uint_field(_jw, key, value); > } > else if ((!_jw && (t & PRINT_FP || t & PRINT_ANY))) > { color_fprintf( (stdout) , color, fmt, value); > } > }; > > Again, no issue and we eventually call color_fprintf > > int color_fprintf(FILE *fp, enum color_attr attr, const char *fmt, ...) > { > int ret =3D 0; > va_list args; > > va_start(args, fmt); > > if (!color_is_enabled || attr =3D=3D COLOR_NONE) { > ret =3D vfprintf(fp, fmt, args); > goto end; > } > > > Now, color_printf is a variable argument list function and as such is > dependent upon being told the correct size of argument variables in the f= mt > variable. And that=E2=80=99s our problem, we=E2=80=99re passing a 64bit = integer but > telling the format string that it=E2=80=99s =E2=80=98int=E2=80=99=E2=80= =A6which I=E2=80=99m guessing can be > variable sizes depending on architecture, as can the endianness. > > If we instead do (in q_cake.c) > > #include > > print_uint(PRINT_ANY, "min_transport_size", " min/max transport layer > size: %10" PRIu64, stnc->min_trnlen); > > it works. This needs sanity checking by a clever person. > > Cheers, > > Kevin D-B > > 012C ACB2 28C6 C53E 9775 9123 B3A2 389B 9DE2 334A > > _______________________________________________ > Cake mailing list > Cake@lists.bufferbloat.net > https://lists.bufferbloat.net/listinfo/cake > --f4f5e80dac3c5b7c2f0567116da0 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Nice work Kevin.

On Sat, Mar 10, 2018 at 09:56 Kevin Darbyshire-Bryant <kevin@darbyshire-bryant.me.uk>= wrote:


> On 8 Mar 2018, at 11:21, Toke H=C3=B8iland-J=C3=B8rgensen <toke@toke.dk> wrote:
>
>>
>> Oh and curiously the bad values go away if you ask for json output=
>> it=E2=80=99s much better.=C2=A0 Which rather points at a =E2=80=98= feature=E2=80=99 of the
>> =E2=80=98print_string=E2=80=99 behaviour.
>
> Right. Well, the print_* functions are behind several levels of
> pre-processor indirection, so not quite obvious what's going on he= re.
> Don't really see why they should spit out garbage values, though.<= br> >
>
> Stephen, do you have any ideas?
>
> -Toke

Right, cracked it and it=E2=80=99s horrible!

print_uint is expanded thus:=C2=A0 Note the type of value uint64_t

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 void print_color_uint(enum= output_type t, enum color_attr color, const char *key, const char *fmt, ui= nt64_t value);
static inline void print_uint=C2=A0 =C2=A0 =C2=A0 (enum output_type t,=C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 const char *key, const char *fmt, uint64_t value)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0{ print_color= _uint(=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0t, COLO= R_NONE,=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 key,=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0fmt,= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 value); };

So far so good.

print_color_uint expands to:

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0void print_color_uint(enum = output_type t, enum color_attr color, const char *key, const char *fmt, uin= t64_t value)
=C2=A0{
=C2=A0 if (((t & PRINT_JSON || t & PRINT_ANY) && _jw))
=C2=A0 =C2=A0 { if (!key) jsonw_uint(_jw, value);
=C2=A0 =C2=A0 =C2=A0 else=C2=A0 =C2=A0 =C2=A0 jsonw_uint_field(_jw, key, va= lue);
=C2=A0 =C2=A0 }
=C2=A0 else if ((!_jw && (t & PRINT_FP || t & PRINT_ANY)))<= br> =C2=A0 =C2=A0 { color_fprintf( (stdout) , color, fmt, value);
=C2=A0 =C2=A0 }
=C2=A0};

Again, no issue and we eventually call color_fprintf

int color_fprintf(FILE *fp, enum color_attr attr, const char *fmt, ...)
{
=C2=A0 =C2=A0 =C2=A0 =C2=A0 int ret =3D 0;
=C2=A0 =C2=A0 =C2=A0 =C2=A0 va_list args;

=C2=A0 =C2=A0 =C2=A0 =C2=A0 va_start(args, fmt);

=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (!color_is_enabled || attr =3D=3D COLOR_NONE= ) {
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ret =3D vfprintf(fp= , fmt, args);
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 goto end;
=C2=A0 =C2=A0 =C2=A0 =C2=A0 }


Now, color_printf is a variable argument list function and as such is depen= dent upon being told the correct size of argument variables in the fmt vari= able.=C2=A0 And that=E2=80=99s our problem, we=E2=80=99re passing a 64bit i= nteger but telling the format string that it=E2=80=99s =E2=80=98int=E2=80= =99=E2=80=A6which I=E2=80=99m guessing can be variable sizes depending on a= rchitecture, as can the endianness.

If we instead do (in q_cake.c)

#include <inttypes.h>

print_uint(PRINT_ANY, "min_transport_size", " min/max transp= ort layer size: %10" PRIu64, stnc->min_trnlen);

it works.=C2=A0 This needs sanity checking by a clever person.

Cheers,

Kevin D-B

012C ACB2 28C6 C53E 9775=C2=A0 9123 B3A2 389B 9DE2 334A

_______________________________________________
Cake mailing list
Cake@lists.= bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake
--f4f5e80dac3c5b7c2f0567116da0--