netdev - Re: [PATCH iproute2 net-next v2 3/4] ss: Buffer raw fields first, then render them as a table

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] [day] [month] [year] [list]

Message-ID: <182981ee-4aec-135c-ab10-4445b6f84435@gmail.com>
Date:   Wed, 13 Feb 2019 16:47:30 -0700
From:   David Ahern <dsahern@...il.com>
To:     Phil Sutter <phil@....cc>,
        Stephen Hemminger <stephen@...workplumber.org>,
        Stefano Brivio <sbrivio@...hat.com>,
        Eric Dumazet <eric.dumazet@...il.com>, netdev@...r.kernel.org,
        Sabrina Dubroca <sd@...asysnail.net>
Subject: Re: [PATCH iproute2 net-next v2 3/4] ss: Buffer raw fields first,
 then render them as a table

On 2/13/19 4:39 PM, Phil Sutter wrote:
>> What I would favor:
>> 	* use big enough columns that for the common case everything lines up fine
>> 	* if column is to wide just print that element wider (which is what print %Ns does)
> 
> This is pretty much the situation Stefano attempted to improve, minus
> scaling the columns to max terminal width. ss output formatting being
> quirky and unreadable with either small or large terminals was the
> number one reason I heard so far why people prefer netstat.

+1.

prior to Stefano's change ss was a PITA trying to read in an xterm. I
for one would run the command and then have to adjust the terminal to
get it to display an actual readable format.

> 
>> and
>> 	* add json output for programs that want to parse
>> 	* use print_uint etc for that
> 
> For Eric's use-case, skipping any buffering and tabular output if stdout
> is not a TTY suffices. In fact, iproute2 does this already for colored
> output (see check_enable_color() for reference).
> 
> Adding JSON output support everywhere is a nice feature when it comes to
> scripting, but it won't help console users. Unless you expect CLI
> frontends to come turning that JSON into human-readable output.
> 
> IMHO, JSON output wouldn't even help in this case - unless Eric indeed
> prefers to write/use a JSON parser for his analysis instead of something
> along 'ss | grep'.

I agree. json has its uses, console/xterm for humans is not one and
piping into something like jq to selectively pick columns is not a user
friendly solution.

> 
>> The buffering patch (in iproute2-next) can/will be reverted.
> 
> It's not fair to claim that despite Stefano's commitment to fix the
> reported issues. His ss output rewrite is there since v4.15.0 and
> according to git history it needed only two fixes so far. I've had
> one-liners which required more follow-ups than that! Also, we're still
> discovering issues introduced by all the jsonify patches. Allowing for
> people to get things right not the first time but after a few tries is
> important. If you want to revert something, start with features which
> have a fundamental design issue in the exact situation they tried to
> improve, like the MSG_PEEK | MSG_TRUNC thing Hangbin and me wrote.

I was just looking at the overhead of that. While it is deceiving to
twice as many recvmsg calls as you expect, the overhead of the peek in
reading 700k+ routes is on the order of 3% with the 32k min buffer size.
The true overhead of the dump functions for ip is the device index to
name mapping (just like the overhead of a batch is the name to index
mapping). I will send a v2 of my patches soon.