netdev - Re: [PATCH iproute2 v3 1/2] tc: u32: add support for json output

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Ye8abWbX5TZngvIS@tc2>
Date:   Mon, 24 Jan 2022 22:30:21 +0100
From:   Andrea Claudi <aclaudi@...hat.com>
To:     Stephen Hemminger <stephen@...workplumber.org>
Cc:     netdev@...r.kernel.org, Wen Liang <wenliang@...hat.com>,
        David Ahern <dsahern@...nel.org>
Subject: Re: [PATCH iproute2 v3 1/2] tc: u32: add support for json output

On Mon, Jan 24, 2022 at 10:50:16AM -0800, Stephen Hemminger wrote:
> On Mon, 24 Jan 2022 19:25:06 +0100
> Andrea Claudi <aclaudi@...hat.com> wrote:
> 
> > On Thu, Jan 06, 2022 at 02:30:13PM -0800, Stephen Hemminger wrote:
> > > On Thu,  6 Jan 2022 13:45:51 -0500
> > > Wen Liang <liangwen12year@...il.com> wrote:
> > >   
> > > >  	} else if (sel && sel->flags & TC_U32_TERMINAL) {
> > > > -		fprintf(f, "terminal flowid ??? ");
> > > > +		print_bool(PRINT_ANY, "terminal_flowid", "terminal flowid ??? ", true);  
> > > 
> > > This looks like another error (ie to stderr) like the earlier case
> > >  
> > 
> > Hi Stephen,
> > Sorry for coming to this so late, but this doesn't look like an error to me.
> > 
> > As far as I can see, TC_U32_TERMINAL is set in this file together with
> > CLASSID or when "action" or "policy" are used. The latter case should be
> > the one that this else branch should catch.
> > 
> > Now, "terminal flowid ???" looks to me like a message printed when we
> > don't actually have a flowid to show, and indeed that is specified when
> > this flag is set (see the comment at line 1169). As such this is
> > probably more a useless log message, than an error one.
> > 
> > If this is the case, we can probably maintain this message on the
> > PRINT_FP output (only to not break script parsing this bit of info out
> > there), and disregard this bit of info on the JSON output.
> > 
> > What do you think?
> > 
> > Regards,
> > Andrea
> > 
> 
> Just always put the same original message on stderr.
> 

Let me phrase my case better: I think the "terminal flowid" message
should not be on stderr, as I don't think this is an error message.

Indeed, "terminal flowid ???" is printed every time we use "action" or
"policy" (see my previous email for details), even when no error is
present and cls_u32 is working ok. In these cases, not having a flowid
is legitimate and not an error.

As this is the case, I think the proper course of action is to have this
message printed out only in non-json output to preserve the same output
of older iproute versions. It would be even better if we decide to
remove this message altogether, as it is not adding any valuable info to
the user.