All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH iproute2 net-next 0/3] ss: Allow selection of columns to be displayed
@ 2018-10-30 15:05 Stefano Brivio
  2018-10-30 15:05 ` [PATCH iproute2 net-next 1/3] ss: Discard empty descriptor at the end of buffer, if any, before rendering Stefano Brivio
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Stefano Brivio @ 2018-10-30 15:05 UTC (permalink / raw)
  To: David Ahern; +Cc: Yoann P., Stephen Hemminger, netdev

Now that we have an abstraction for columns, it's relatively easy to
selectively display only some of them, and Yoann has a use case for it.

Patch 1/3 fixes a rendering issue that shows up only when display of
arbitrary columns is disabled. Patch 2/3 implements the relevant option,
and patch 3/3 makes the output more readable when some columns are
disabled.

Stefano Brivio (3):
  ss: Discard empty descriptor at the end of buffer, if any, before
    rendering
  ss: Introduce option to display selected columns only
  ss: Beautify output when arbitrary columns are hidden

 man/man8/ss.8 |  5 +++
 misc/ss.c     | 85 +++++++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 77 insertions(+), 13 deletions(-)

-- 
2.19.1

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH iproute2 net-next 1/3] ss: Discard empty descriptor at the end of buffer, if any, before rendering
  2018-10-30 15:05 [PATCH iproute2 net-next 0/3] ss: Allow selection of columns to be displayed Stefano Brivio
@ 2018-10-30 15:05 ` Stefano Brivio
  2018-10-30 15:05 ` [PATCH iproute2 net-next 2/3] ss: Introduce option to display selected columns only Stefano Brivio
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 14+ messages in thread
From: Stefano Brivio @ 2018-10-30 15:05 UTC (permalink / raw)
  To: David Ahern; +Cc: Yoann P., Stephen Hemminger, netdev

This will allow us to disable display of any given column.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 misc/ss.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index c8970438ce73..c3f61ef66258 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -1245,8 +1245,15 @@ static void render(void)
 
 	token = (struct buf_token *)buffer.head->data;
 
-	/* Ensure end alignment of last token, it wasn't necessarily flushed */
-	buffer.tail->end += buffer.cur->len % 2;
+	if (!buffer.cur->len) {
+		/* Last token was flushed, a new empty descriptor was appended:
+		 * discard it
+		 */
+		buffer.tail->end -= sizeof(buffer.cur->len);
+	} else {
+		/* Last token wasn't flushed: ensure end alignment */
+		buffer.tail->end += buffer.cur->len % 2;
+	}
 
 	render_calc_width();
 
-- 
2.19.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH iproute2 net-next 2/3] ss: Introduce option to display selected columns only
  2018-10-30 15:05 [PATCH iproute2 net-next 0/3] ss: Allow selection of columns to be displayed Stefano Brivio
  2018-10-30 15:05 ` [PATCH iproute2 net-next 1/3] ss: Discard empty descriptor at the end of buffer, if any, before rendering Stefano Brivio
@ 2018-10-30 15:05 ` Stefano Brivio
  2018-10-30 15:05 ` [PATCH iproute2 net-next 3/3] ss: Beautify output when arbitrary columns are hidden Stefano Brivio
  2018-10-30 16:34 ` [PATCH iproute2 net-next 0/3] ss: Allow selection of columns to be displayed David Ahern
  3 siblings, 0 replies; 14+ messages in thread
From: Stefano Brivio @ 2018-10-30 15:05 UTC (permalink / raw)
  To: David Ahern; +Cc: Yoann P., Stephen Hemminger, netdev

The new option --columns (short: -c) allows to select columns to be
displayed. Note that this doesn't affect the order in which columns are
displayed.

Reported-by: Yoann P. <yoann.p.public@gmail.com>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 man/man8/ss.8 |  5 +++++
 misc/ss.c     | 62 ++++++++++++++++++++++++++++++++++++++++++---------
 2 files changed, 57 insertions(+), 10 deletions(-)

diff --git a/man/man8/ss.8 b/man/man8/ss.8
index 7a6572b17364..c987dec6bcd7 100644
--- a/man/man8/ss.8
+++ b/man/man8/ss.8
@@ -24,6 +24,11 @@ Output version information.
 .B \-H, \-\-no-header
 Suppress header line.
 .TP
+.B \-c COLS, \-\-columns=COLS
+Only display selected columns, separated by commas. The following column names
+are understood: netid, state, local, lport, peer, pport, ext. This does not
+define the order of columns.
+.TP
 .B \-n, \-\-numeric
 Do not try to resolve service names.
 .TP
diff --git a/misc/ss.c b/misc/ss.c
index c3f61ef66258..91be3c6db151 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -132,6 +132,7 @@ enum col_align {
 
 struct column {
 	const enum col_align align;
+	const char *optname;
 	const char *header;
 	const char *ldelim;
 	int disabled;
@@ -140,15 +141,15 @@ struct column {
 };
 
 static struct column columns[] = {
-	{ ALIGN_LEFT,	"Netid",		"",	0, 0, 0 },
-	{ ALIGN_LEFT,	"State",		" ",	0, 0, 0 },
-	{ ALIGN_LEFT,	"Recv-Q",		" ",	0, 0, 0 },
-	{ ALIGN_LEFT,	"Send-Q",		" ",	0, 0, 0 },
-	{ ALIGN_RIGHT,	"Local Address:",	" ",	0, 0, 0 },
-	{ ALIGN_LEFT,	"Port",			"",	0, 0, 0 },
-	{ ALIGN_RIGHT,	"Peer Address:",	" ",	0, 0, 0 },
-	{ ALIGN_LEFT,	"Port",			"",	0, 0, 0 },
-	{ ALIGN_LEFT,	"",			"",	0, 0, 0 },
+	{ ALIGN_LEFT,	"netid",	"Netid",		"",  0, 0, 0 },
+	{ ALIGN_LEFT,	"state",	"State",		" ", 0, 0, 0 },
+	{ ALIGN_LEFT,	"recvq",	"Recv-Q",		" ", 0, 0, 0 },
+	{ ALIGN_LEFT,	"sendq",	"Send-Q",		" ", 0, 0, 0 },
+	{ ALIGN_RIGHT,	"local",	"Local Address:",	" ", 0, 0, 0 },
+	{ ALIGN_LEFT,	"lport",	"Port",			"",  0, 0, 0 },
+	{ ALIGN_RIGHT,	"peer",		"Peer Address:",	" ", 0, 0, 0 },
+	{ ALIGN_LEFT,	"pport",	"Port",			"",  0, 0, 0 },
+	{ ALIGN_LEFT,	"ext",		"",			"",  0, 0, 0 },
 };
 
 static struct column *current_field = columns;
@@ -1073,6 +1074,11 @@ static int field_is_last(struct column *f)
 	return f - columns == COL_MAX - 1;
 }
 
+static int field_is_valid(struct column *f)
+{
+	return f >= columns && f - columns < COL_MAX;
+}
+
 static void field_next(void)
 {
 	field_flush(current_field);
@@ -4666,6 +4672,8 @@ static void _usage(FILE *dest)
 "\n"
 "   -K, --kill          forcibly close sockets, display what was closed\n"
 "   -H, --no-header     Suppress header line\n"
+"   -c, --columns=COLS  display only COLS columns\n"
+"       COLS := {netid|state|local|lport|peer|pport|ext}[,COLS]\n"
 "\n"
 "   -A, --query=QUERY, --socket=QUERY\n"
 "       QUERY := {all|inet|tcp|udp|raw|unix|unix_dgram|unix_stream|unix_seqpacket|packet|netlink|vsock_stream|vsock_dgram|tipc}[,QUERY]\n"
@@ -4785,6 +4793,7 @@ static const struct option long_opts[] = {
 	{ "tipcinfo", 0, 0, OPT_TIPCINFO},
 	{ "kill", 0, 0, 'K' },
 	{ "no-header", 0, 0, 'H' },
+	{ "columns", 1, 0, 'c' },
 	{ 0 }
 
 };
@@ -4800,7 +4809,7 @@ int main(int argc, char *argv[])
 	int state_filter = 0;
 
 	while ((ch = getopt_long(argc, argv,
-				 "dhaletuwxnro460spbEf:miA:D:F:vVzZN:KHS",
+				 "dhaletuwxnro460spbEf:miA:D:F:vVzZN:KHc:S",
 				 long_opts, NULL)) != EOF) {
 		switch (ch) {
 		case 'n':
@@ -4966,6 +4975,39 @@ int main(int argc, char *argv[])
 		case 'H':
 			show_header = 0;
 			break;
+		case 'c':
+		{
+			struct column *f;
+			char *p, *p1;
+
+			if (!optarg) {
+				fprintf(stderr, "ss: No columns given.\n");
+				usage();
+			}
+
+			for (f = columns; field_is_valid(f); f++)
+				f->disabled = 1;
+
+			p = optarg;
+			do {
+				p1 = strchr(p, ',');
+				if (p1)
+					*p1 = 0;
+				for (f = columns; field_is_valid(f); f++) {
+					if (!strcmp(f->optname, p)) {
+						f->disabled = 0;
+						break;
+					}
+				}
+				if (!field_is_valid(f)) {
+					fprintf(stderr, "ss: No column %s\n",
+						p);
+					usage();
+				}
+				p = p1 + 1;
+			} while (p1);
+			break;
+		}
 		case 'h':
 			help();
 		case '?':
-- 
2.19.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH iproute2 net-next 3/3] ss: Beautify output when arbitrary columns are hidden
  2018-10-30 15:05 [PATCH iproute2 net-next 0/3] ss: Allow selection of columns to be displayed Stefano Brivio
  2018-10-30 15:05 ` [PATCH iproute2 net-next 1/3] ss: Discard empty descriptor at the end of buffer, if any, before rendering Stefano Brivio
  2018-10-30 15:05 ` [PATCH iproute2 net-next 2/3] ss: Introduce option to display selected columns only Stefano Brivio
@ 2018-10-30 15:05 ` Stefano Brivio
  2018-10-30 16:34 ` [PATCH iproute2 net-next 0/3] ss: Allow selection of columns to be displayed David Ahern
  3 siblings, 0 replies; 14+ messages in thread
From: Stefano Brivio @ 2018-10-30 15:05 UTC (permalink / raw)
  To: David Ahern; +Cc: Yoann P., Stephen Hemminger, netdev

Define a secondary alignment for columns in case the next column is
hidden, this avoids awkward outputs if e.g. the local address is shown,
but not the local port.

Omit embedded delimiter in socket specifiers if the port or service field
is hidden.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 misc/ss.c | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index 91be3c6db151..d489233681e9 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -131,7 +131,8 @@ enum col_align {
 };
 
 struct column {
-	const enum col_align align;
+	enum col_align align;
+	const enum col_align align_without_next;
 	const char *optname;
 	const char *header;
 	const char *ldelim;
@@ -141,15 +142,15 @@ struct column {
 };
 
 static struct column columns[] = {
-	{ ALIGN_LEFT,	"netid",	"Netid",		"",  0, 0, 0 },
-	{ ALIGN_LEFT,	"state",	"State",		" ", 0, 0, 0 },
-	{ ALIGN_LEFT,	"recvq",	"Recv-Q",		" ", 0, 0, 0 },
-	{ ALIGN_LEFT,	"sendq",	"Send-Q",		" ", 0, 0, 0 },
-	{ ALIGN_RIGHT,	"local",	"Local Address:",	" ", 0, 0, 0 },
-	{ ALIGN_LEFT,	"lport",	"Port",			"",  0, 0, 0 },
-	{ ALIGN_RIGHT,	"peer",		"Peer Address:",	" ", 0, 0, 0 },
-	{ ALIGN_LEFT,	"pport",	"Port",			"",  0, 0, 0 },
-	{ ALIGN_LEFT,	"ext",		"",			"",  0, 0, 0 },
+	{ ALIGN_LEFT,  ALIGN_LEFT, "netid", "Netid",          "",  0, 0, 0 },
+	{ ALIGN_LEFT,  ALIGN_LEFT, "state", "State",          " ", 0, 0, 0 },
+	{ ALIGN_LEFT,  ALIGN_LEFT, "recvq", "Recv-Q",         " ", 0, 0, 0 },
+	{ ALIGN_LEFT,  ALIGN_LEFT, "sendq", "Send-Q",         " ", 0, 0, 0 },
+	{ ALIGN_RIGHT, ALIGN_LEFT, "local", "Local Address:", " ", 0, 0, 0 },
+	{ ALIGN_LEFT,  ALIGN_LEFT, "lport", "Port",           "",  0, 0, 0 },
+	{ ALIGN_RIGHT, ALIGN_LEFT, "peer",  "Peer Address:",  " ", 0, 0, 0 },
+	{ ALIGN_LEFT,  ALIGN_LEFT, "pport", "Port",           "",  0, 0, 0 },
+	{ ALIGN_LEFT,  ALIGN_LEFT, "ext",   "",               "",  0, 0, 0 },
 };
 
 static struct column *current_field = columns;
@@ -1374,6 +1375,9 @@ static void sock_details_print(struct sockstat *s)
 static void sock_addr_print(const char *addr, char *delim, const char *port,
 		const char *ifname)
 {
+	if ((current_field + 1)->disabled)
+		delim = "";
+
 	if (ifname)
 		out("%s" "%%" "%s%s", addr, ifname, delim);
 	else
@@ -5006,6 +5010,12 @@ int main(int argc, char *argv[])
 				}
 				p = p1 + 1;
 			} while (p1);
+
+			for (f = columns; field_is_valid(f + 1); f++) {
+				if ((f + 1)->disabled)
+					f->align = f->align_without_next;
+			}
+
 			break;
 		}
 		case 'h':
-- 
2.19.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH iproute2 net-next 0/3] ss: Allow selection of columns to be displayed
  2018-10-30 15:05 [PATCH iproute2 net-next 0/3] ss: Allow selection of columns to be displayed Stefano Brivio
                   ` (2 preceding siblings ...)
  2018-10-30 15:05 ` [PATCH iproute2 net-next 3/3] ss: Beautify output when arbitrary columns are hidden Stefano Brivio
@ 2018-10-30 16:34 ` David Ahern
  2018-10-30 16:38   ` Stephen Hemminger
  2018-10-30 17:34   ` Stefano Brivio
  3 siblings, 2 replies; 14+ messages in thread
From: David Ahern @ 2018-10-30 16:34 UTC (permalink / raw)
  To: Stefano Brivio; +Cc: Yoann P., Stephen Hemminger, netdev

On 10/30/18 9:05 AM, Stefano Brivio wrote:
> Now that we have an abstraction for columns, it's relatively easy to
> selectively display only some of them, and Yoann has a use case for it.
> 
> Patch 1/3 fixes a rendering issue that shows up only when display of
> arbitrary columns is disabled. Patch 2/3 implements the relevant option,
> and patch 3/3 makes the output more readable when some columns are
> disabled.
> 
>

I like the intent, and I have prototyped something similar for 'ip'.

A more flexible approach is to use format strings to allow users to
customize the output order and whitespace as well. So for ss and your
column list (winging it here):

    netid          = %N
    state          = %S
    recv Q         = %Qr
    send Q         = %Qs
    local address  = %Al
    lport port     = %Pl
    remote address = %Ar
    remote port    = %Pr
    process data   = %p
    ...

then a format string could be: "%S  %Qr %Qs  %Al:%Pl %Ar:%Pr  %p\n"

or for csv output: "%S,%Qr,%Qs,%Al,%Pl,%Ar,%Pr,%p\n"

I have not had time to look into an implementation for ip. Conceptually
- and scanning the kernel's vsprintf code - it does not look that
difficult, just time consuming on the frontend with the initial setup.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH iproute2 net-next 0/3] ss: Allow selection of columns to be displayed
  2018-10-30 16:34 ` [PATCH iproute2 net-next 0/3] ss: Allow selection of columns to be displayed David Ahern
@ 2018-10-30 16:38   ` Stephen Hemminger
  2018-10-30 16:45     ` David Ahern
  2018-10-30 17:34   ` Stefano Brivio
  1 sibling, 1 reply; 14+ messages in thread
From: Stephen Hemminger @ 2018-10-30 16:38 UTC (permalink / raw)
  To: David Ahern; +Cc: Stefano Brivio, Yoann P., netdev

On Tue, 30 Oct 2018 10:34:45 -0600
David Ahern <dsahern@gmail.com> wrote:

> On 10/30/18 9:05 AM, Stefano Brivio wrote:
> > Now that we have an abstraction for columns, it's relatively easy to
> > selectively display only some of them, and Yoann has a use case for it.
> > 
> > Patch 1/3 fixes a rendering issue that shows up only when display of
> > arbitrary columns is disabled. Patch 2/3 implements the relevant option,
> > and patch 3/3 makes the output more readable when some columns are
> > disabled.
> > 
> >  
> 
> I like the intent, and I have prototyped something similar for 'ip'.
> 
> A more flexible approach is to use format strings to allow users to
> customize the output order and whitespace as well. So for ss and your
> column list (winging it here):
> 
>     netid          = %N
>     state          = %S
>     recv Q         = %Qr
>     send Q         = %Qs
>     local address  = %Al
>     lport port     = %Pl
>     remote address = %Ar
>     remote port    = %Pr
>     process data   = %p
>     ...
> 
> then a format string could be: "%S  %Qr %Qs  %Al:%Pl %Ar:%Pr  %p\n"
> 
> or for csv output: "%S,%Qr,%Qs,%Al,%Pl,%Ar,%Pr,%p\n"
> 
> I have not had time to look into an implementation for ip. Conceptually
> - and scanning the kernel's vsprintf code - it does not look that
> difficult, just time consuming on the frontend with the initial setup.

The problem with custom formats is that you lose all ability for Gcc
to check format strings.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH iproute2 net-next 0/3] ss: Allow selection of columns to be displayed
  2018-10-30 16:38   ` Stephen Hemminger
@ 2018-10-30 16:45     ` David Ahern
  0 siblings, 0 replies; 14+ messages in thread
From: David Ahern @ 2018-10-30 16:45 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Stefano Brivio, Yoann P., netdev

On 10/30/18 10:38 AM, Stephen Hemminger wrote:
> On Tue, 30 Oct 2018 10:34:45 -0600
> David Ahern <dsahern@gmail.com> wrote:
> 
>> On 10/30/18 9:05 AM, Stefano Brivio wrote:
>>> Now that we have an abstraction for columns, it's relatively easy to
>>> selectively display only some of them, and Yoann has a use case for it.
>>>
>>> Patch 1/3 fixes a rendering issue that shows up only when display of
>>> arbitrary columns is disabled. Patch 2/3 implements the relevant option,
>>> and patch 3/3 makes the output more readable when some columns are
>>> disabled.
>>>
>>>  
>>
>> I like the intent, and I have prototyped something similar for 'ip'.
>>
>> A more flexible approach is to use format strings to allow users to
>> customize the output order and whitespace as well. So for ss and your
>> column list (winging it here):
>>
>>     netid          = %N
>>     state          = %S
>>     recv Q         = %Qr
>>     send Q         = %Qs
>>     local address  = %Al
>>     lport port     = %Pl
>>     remote address = %Ar
>>     remote port    = %Pr
>>     process data   = %p
>>     ...
>>
>> then a format string could be: "%S  %Qr %Qs  %Al:%Pl %Ar:%Pr  %p\n"
>>
>> or for csv output: "%S,%Qr,%Qs,%Al,%Pl,%Ar,%Pr,%p\n"
>>
>> I have not had time to look into an implementation for ip. Conceptually
>> - and scanning the kernel's vsprintf code - it does not look that
>> difficult, just time consuming on the frontend with the initial setup.
> 
> The problem with custom formats is that you lose all ability for Gcc
> to check format strings.
> 

Sure, trade-offs. A custom print string is powerful.

While selecting columns is an improvement, column ordering is also
important - even handling other output formats (csv).

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH iproute2 net-next 0/3] ss: Allow selection of columns to be displayed
  2018-10-30 16:34 ` [PATCH iproute2 net-next 0/3] ss: Allow selection of columns to be displayed David Ahern
  2018-10-30 16:38   ` Stephen Hemminger
@ 2018-10-30 17:34   ` Stefano Brivio
  2018-11-01  2:48     ` David Ahern
  1 sibling, 1 reply; 14+ messages in thread
From: Stefano Brivio @ 2018-10-30 17:34 UTC (permalink / raw)
  To: David Ahern; +Cc: Yoann P., Stephen Hemminger, netdev

On Tue, 30 Oct 2018 10:34:45 -0600
David Ahern <dsahern@gmail.com> wrote:

> A more flexible approach is to use format strings to allow users to
> customize the output order and whitespace as well. So for ss and your
> column list (winging it here):
> 
>     netid          = %N
>     state          = %S
>     recv Q         = %Qr
>     send Q         = %Qs
>     local address  = %Al
>     lport port     = %Pl
>     remote address = %Ar
>     remote port    = %Pr
>     process data   = %p
>     ...
> 
> then a format string could be: "%S  %Qr %Qs  %Al:%Pl %Ar:%Pr  %p\n"

I like the idea indeed, but I see two issues with ss:

- the current column abstraction is rather lightweight, things are
  already buffered in the defined column order so we don't have to jump
  back and forth in the buffer while rendering. Doing that needs some
  extra care to avoid a performance hit, but it's probably doable, I
  can put that on my to-do list

- how would you model automatic spacing in a format string? Should we
  support width specifiers? Disable automatic spacing if a format
  string is given? It might even make sense to allow partial automatic
  spacing with a special character in the format string, that is:

	"%S.%Qr.%Qs  %Al:%Pl %Ar:%Pr  %p\n"

  would mean "align everything to the right, distribute remaining
  whitespace between %S, %Qr and %Qs". But it looks rather complicated
  at a glance.

-- 
Stefano

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH iproute2 net-next 0/3] ss: Allow selection of columns to be displayed
  2018-10-30 17:34   ` Stefano Brivio
@ 2018-11-01  2:48     ` David Ahern
  2018-11-01 21:06       ` Jakub Kicinski
  2018-11-02  9:58       ` Stefano Brivio
  0 siblings, 2 replies; 14+ messages in thread
From: David Ahern @ 2018-11-01  2:48 UTC (permalink / raw)
  To: Stefano Brivio; +Cc: Yoann P., Stephen Hemminger, netdev


[ sorry, too many distractions and I forgot to respond ]

On 10/30/18 11:34 AM, Stefano Brivio wrote:
> On Tue, 30 Oct 2018 10:34:45 -0600
> David Ahern <dsahern@gmail.com> wrote:
> 
>> A more flexible approach is to use format strings to allow users to
>> customize the output order and whitespace as well. So for ss and your
>> column list (winging it here):
>>
>>     netid          = %N
>>     state          = %S
>>     recv Q         = %Qr
>>     send Q         = %Qs
>>     local address  = %Al
>>     lport port     = %Pl
>>     remote address = %Ar
>>     remote port    = %Pr
>>     process data   = %p
>>     ...
>>
>> then a format string could be: "%S  %Qr %Qs  %Al:%Pl %Ar:%Pr  %p\n"
> 
> I like the idea indeed, but I see two issues with ss:
> 
> - the current column abstraction is rather lightweight, things are
>   already buffered in the defined column order so we don't have to jump
>   back and forth in the buffer while rendering. Doing that needs some
>   extra care to avoid a performance hit, but it's probably doable, I
>   can put that on my to-do list

The ss command is always a pain; it's much better in newer releases but
I am always having to adjust terminal width.

> 
> - how would you model automatic spacing in a format string? Should we
>   support width specifiers? Disable automatic spacing if a format
>   string is given? It might even make sense to allow partial automatic

Follow the format string for whitespace and order, and
yes, on the width specifiers if possible.

>   spacing with a special character in the format string, that is:
> 
> 	"%S.%Qr.%Qs  %Al:%Pl %Ar:%Pr  %p\n"
> 
>   would mean "align everything to the right, distribute remaining
>   whitespace between %S, %Qr and %Qs". But it looks rather complicated
>   at a glance.
> 

My concern here is that once this goes in for 1 command, the others in
iproute2 need to follow suit - meaning same syntax style for all
commands. Given that I'd prefer we get a reasonable consensus on syntax
that will work across commands -- ss, ip, tc. If it is as simple as
column names with a fixed order, that is fine but just give proper
consideration given the impact.

The 'ip' syntax for example gets ugly quick with the various link types
and their options. We don't need to allow every detail of each link type
to be customized, but there are common attributes for links (e.g., mtu,
ifindex, link flags, lladdr), addresses, and link types such as bridges
and bonds where we can improve the amount of data thrown at a user -- a
better, more customizable version of what the brief option targeted.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH iproute2 net-next 0/3] ss: Allow selection of columns to be displayed
  2018-11-01  2:48     ` David Ahern
@ 2018-11-01 21:06       ` Jakub Kicinski
  2018-11-01 21:18         ` David Ahern
  2018-11-02  9:58         ` Stefano Brivio
  2018-11-02  9:58       ` Stefano Brivio
  1 sibling, 2 replies; 14+ messages in thread
From: Jakub Kicinski @ 2018-11-01 21:06 UTC (permalink / raw)
  To: David Ahern; +Cc: Stefano Brivio, Yoann P., Stephen Hemminger, netdev

On Wed, 31 Oct 2018 20:48:05 -0600, David Ahern wrote:
> >   spacing with a special character in the format string, that is:
> > 
> > 	"%S.%Qr.%Qs  %Al:%Pl %Ar:%Pr  %p\n"
> > 
> >   would mean "align everything to the right, distribute remaining
> >   whitespace between %S, %Qr and %Qs". But it looks rather complicated
> >   at a glance.
> >   
> 
> My concern here is that once this goes in for 1 command, the others in
> iproute2 need to follow suit - meaning same syntax style for all
> commands. Given that I'd prefer we get a reasonable consensus on syntax
> that will work across commands -- ss, ip, tc. If it is as simple as
> column names with a fixed order, that is fine but just give proper
> consideration given the impact.

FWIW I just started piping iproute2 commands to jq.  Example:

tc -s -j qdisc show dev em1 | \
	jq -r '.[] |  [.kind,.parent,.handle,.offloaded,.bytes,.packets,.drops,.overlimits,.requeues,.backlog,.qlen,.marked] | @tsv'

JSONification would probably be quite an undertaking for ss :(

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH iproute2 net-next 0/3] ss: Allow selection of columns to be displayed
  2018-11-01 21:06       ` Jakub Kicinski
@ 2018-11-01 21:18         ` David Ahern
  2018-11-01 21:38           ` Stephen Hemminger
  2018-11-02  9:58         ` Stefano Brivio
  1 sibling, 1 reply; 14+ messages in thread
From: David Ahern @ 2018-11-01 21:18 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: Stefano Brivio, Yoann P., Stephen Hemminger, netdev

On 11/1/18 3:06 PM, Jakub Kicinski wrote:
> On Wed, 31 Oct 2018 20:48:05 -0600, David Ahern wrote:
>>>   spacing with a special character in the format string, that is:
>>>
>>> 	"%S.%Qr.%Qs  %Al:%Pl %Ar:%Pr  %p\n"
>>>
>>>   would mean "align everything to the right, distribute remaining
>>>   whitespace between %S, %Qr and %Qs". But it looks rather complicated
>>>   at a glance.
>>>   
>>
>> My concern here is that once this goes in for 1 command, the others in
>> iproute2 need to follow suit - meaning same syntax style for all
>> commands. Given that I'd prefer we get a reasonable consensus on syntax
>> that will work across commands -- ss, ip, tc. If it is as simple as
>> column names with a fixed order, that is fine but just give proper
>> consideration given the impact.
> 
> FWIW I just started piping iproute2 commands to jq.  Example:
> 
> tc -s -j qdisc show dev em1 | \
> 	jq -r '.[] |  [.kind,.parent,.handle,.offloaded,.bytes,.packets,.drops,.overlimits,.requeues,.backlog,.qlen,.marked] | @tsv'
> 
> JSONification would probably be quite an undertaking for ss :(
> 

Right, that is used in some of the scripts under
tools/testing/selftests. I would put that in the 'heavyweight solution'
category.

A number of key commands offer the capability to control the output via
command line argument (e.g., ps, perf script). Given the amount of data
iproute2 commands throw at a user by default, it would be a good
usability feature to allow a user to customize the output without having
to pipe it into other commands.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH iproute2 net-next 0/3] ss: Allow selection of columns to be displayed
  2018-11-01 21:18         ` David Ahern
@ 2018-11-01 21:38           ` Stephen Hemminger
  0 siblings, 0 replies; 14+ messages in thread
From: Stephen Hemminger @ 2018-11-01 21:38 UTC (permalink / raw)
  To: David Ahern; +Cc: Jakub Kicinski, Stefano Brivio, Yoann P., netdev

On Thu, 1 Nov 2018 15:18:03 -0600
David Ahern <dsahern@gmail.com> wrote:

> On 11/1/18 3:06 PM, Jakub Kicinski wrote:
> > On Wed, 31 Oct 2018 20:48:05 -0600, David Ahern wrote:  
> >>>   spacing with a special character in the format string, that is:
> >>>
> >>> 	"%S.%Qr.%Qs  %Al:%Pl %Ar:%Pr  %p\n"
> >>>
> >>>   would mean "align everything to the right, distribute remaining
> >>>   whitespace between %S, %Qr and %Qs". But it looks rather complicated
> >>>   at a glance.
> >>>     
> >>
> >> My concern here is that once this goes in for 1 command, the others in
> >> iproute2 need to follow suit - meaning same syntax style for all
> >> commands. Given that I'd prefer we get a reasonable consensus on syntax
> >> that will work across commands -- ss, ip, tc. If it is as simple as
> >> column names with a fixed order, that is fine but just give proper
> >> consideration given the impact.  
> > 
> > FWIW I just started piping iproute2 commands to jq.  Example:
> > 
> > tc -s -j qdisc show dev em1 | \
> > 	jq -r '.[] |  [.kind,.parent,.handle,.offloaded,.bytes,.packets,.drops,.overlimits,.requeues,.backlog,.qlen,.marked] | @tsv'
> > 
> > JSONification would probably be quite an undertaking for ss :(
> >   
> 
> Right, that is used in some of the scripts under
> tools/testing/selftests. I would put that in the 'heavyweight solution'
> category.
> 
> A number of key commands offer the capability to control the output via
> command line argument (e.g., ps, perf script). Given the amount of data
> iproute2 commands throw at a user by default, it would be a good
> usability feature to allow a user to customize the output without having
> to pipe it into other commands.

I would rather see ss grow json support than having to make the output
formatting of every iproute2 command grow a new format management.


The jq tool looks cool, and I can see how someone could easily have
a bunch of mini-scripts to do what they want.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH iproute2 net-next 0/3] ss: Allow selection of columns to be displayed
  2018-11-01  2:48     ` David Ahern
  2018-11-01 21:06       ` Jakub Kicinski
@ 2018-11-02  9:58       ` Stefano Brivio
  1 sibling, 0 replies; 14+ messages in thread
From: Stefano Brivio @ 2018-11-02  9:58 UTC (permalink / raw)
  To: David Ahern; +Cc: Yoann P., Stephen Hemminger, netdev

On Wed, 31 Oct 2018 20:48:05 -0600
David Ahern <dsahern@gmail.com> wrote:

> On 10/30/18 11:34 AM, Stefano Brivio wrote:
> > 
> > - the current column abstraction is rather lightweight, things are
> >   already buffered in the defined column order so we don't have to jump
> >   back and forth in the buffer while rendering. Doing that needs some
> >   extra care to avoid a performance hit, but it's probably doable, I
> >   can put that on my to-do list  
> 
> The ss command is always a pain; it's much better in newer releases but
> I am always having to adjust terminal width.

Ouch. Do you have some examples?

-- 
Stefano

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH iproute2 net-next 0/3] ss: Allow selection of columns to be displayed
  2018-11-01 21:06       ` Jakub Kicinski
  2018-11-01 21:18         ` David Ahern
@ 2018-11-02  9:58         ` Stefano Brivio
  1 sibling, 0 replies; 14+ messages in thread
From: Stefano Brivio @ 2018-11-02  9:58 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: David Ahern, Yoann P., Stephen Hemminger, netdev

On Thu, 1 Nov 2018 14:06:23 -0700
Jakub Kicinski <kubakici@wp.pl> wrote:

> JSONification would probably be quite an undertaking for ss :(

Probably not too much, and we would skip buffering for JSON as we don't
need to print columns, so we don't have to care about buffering and
rendering performance too much.

All it takes is to extend a bit the out() function and pass the right
arguments to it -- it looks way easier than implementing format
strings, even though I'd like the latter more.

-- 
Stefano

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2018-11-02 19:05 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-30 15:05 [PATCH iproute2 net-next 0/3] ss: Allow selection of columns to be displayed Stefano Brivio
2018-10-30 15:05 ` [PATCH iproute2 net-next 1/3] ss: Discard empty descriptor at the end of buffer, if any, before rendering Stefano Brivio
2018-10-30 15:05 ` [PATCH iproute2 net-next 2/3] ss: Introduce option to display selected columns only Stefano Brivio
2018-10-30 15:05 ` [PATCH iproute2 net-next 3/3] ss: Beautify output when arbitrary columns are hidden Stefano Brivio
2018-10-30 16:34 ` [PATCH iproute2 net-next 0/3] ss: Allow selection of columns to be displayed David Ahern
2018-10-30 16:38   ` Stephen Hemminger
2018-10-30 16:45     ` David Ahern
2018-10-30 17:34   ` Stefano Brivio
2018-11-01  2:48     ` David Ahern
2018-11-01 21:06       ` Jakub Kicinski
2018-11-01 21:18         ` David Ahern
2018-11-01 21:38           ` Stephen Hemminger
2018-11-02  9:58         ` Stefano Brivio
2018-11-02  9:58       ` Stefano Brivio

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.