All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH iproute2 net-next 0/4] Abstract columns, properly space and wrap fields
@ 2017-12-08 17:07 Stefano Brivio
  2017-12-08 17:07 ` [PATCH iproute2 net-next 1/4] ss: Replace printf() calls for "main" output by calls to helper Stefano Brivio
                   ` (5 more replies)
  0 siblings, 6 replies; 8+ messages in thread
From: Stefano Brivio @ 2017-12-08 17:07 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, Sabrina Dubroca

Currently, 'ss' simply subdivides the whole available screen width
between available columns, starting from a set of hardcoded amount
of spacing and growing column widths.

This makes the output unreadable in several cases, as it doesn't take
into account the actual content width.

Fix this by introducing a simple abstraction for columns, buffering
the output, measuring the width of the fields, grouping fields into
lines as they fit, equally distributing any remaining whitespace, and
finally rendering the result. Some examples are reported below [1].

This implementation doesn't seem to cause any significant performance
issues, as reported in 3/4.

Patch 1/4 replaces all relevant printf() calls by the out() helper,
which simply consists of the usual printf() implementation.

Patch 2/4 implements column abstraction, with configurable column
width and delimiters, and 3/4 splits buffering and rendering phases,
employing a simple buffering mechanism with chunked allocation and
introducing a rendering function.

Up to this point, the output is still unchanged.

Finally, 4/4 introduces field width calculation based on content
length measured while buffering, in order to split fields onto
multiple lines and equally space them within the single lines.

Now that column behaviour is well-defined and more easily
configurable, it should be easier to further improve the output by
splitting logically separable information (e.g. TCP details) into
additional columns. However, this patchset keeps the full "extended"
information into a single column, for the moment being.


[1]

- 80 columns terminal, ss -Z -f netlink
  * before:
Recv-Q Send-Q Local Address:Port                 Peer Address:Port

0      0            rtnl:evolution-calen/2075           *                     pr
oc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
0      0            rtnl:abrt-applet/32700              *                     pr
oc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
0      0            rtnl:firefox/21619                  *                     pr
oc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
0      0            rtnl:evolution-calen/32639           *                     p
roc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
[...]

  * after:
Recv-Q   Send-Q     Local Address:Port                      Peer Address:Port
0        0                   rtnl:evolution-calen/2075                  *
 proc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
0        0                   rtnl:abrt-applet/32700                     *
 proc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
0        0                   rtnl:firefox/21619                         *
 proc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
0        0                   rtnl:evolution-calen/32639                 *
 proc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
[...]

- 80 colums terminal, ss -tunpl
  * before:
Netid  State      Recv-Q Send-Q Local Address:Port               Peer Address:Port
udp    UNCONN     0      0         *:37732                 *:*
udp    UNCONN     0      0         *:5353                  *:*
udp    UNCONN     0      0      192.168.122.1:53                    *:*
udp    UNCONN     0      0      *%virbr0:67                    *:*
[...]

  * after:
Netid   State    Recv-Q   Send-Q     Local Address:Port      Peer Address:Port
udp     UNCONN   0        0                      *:37732                *:*
udp     UNCONN   0        0                      *:5353                 *:*
udp     UNCONN   0        0          192.168.122.1:53                   *:*
udp     UNCONN   0        0               *%virbr0:67                   *:*
[...]

 - 66 columns terminal, ss -tunpl
  * before:
Netid  State      Recv-Q Send-Q Local Address:Port               P
eer Address:Port
udp    UNCONN     0      0       *:37732               *:*

udp    UNCONN     0      0       *:5353                *:*

udp    UNCONN     0      0      192.168.122.1:53
*:*
udp    UNCONN     0      0      *%virbr0:67                  *:*
[...]

  * after:
Netid State  Recv-Q Send-Q Local Address:Port   Peer Address:Port
udp   UNCONN 0      0                  *:37732             *:*
udp   UNCONN 0      0                  *:5353              *:*
udp   UNCONN 0      0      192.168.122.1:53                *:*
udp   UNCONN 0      0           *%virbr0:67                *:*
[...]


Stefano Brivio (4):
  ss: Replace printf() calls for "main" output by calls to helper
  ss: Introduce columns lightweight abstraction
  ss: Buffer raw fields first, then render them as a table
  ss: Implement automatic column width calculation

 misc/ss.c | 893 +++++++++++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 621 insertions(+), 272 deletions(-)

-- 
2.9.4

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

* [PATCH iproute2 net-next 1/4] ss: Replace printf() calls for "main" output by calls to helper
  2017-12-08 17:07 [PATCH iproute2 net-next 0/4] Abstract columns, properly space and wrap fields Stefano Brivio
@ 2017-12-08 17:07 ` Stefano Brivio
  2017-12-08 17:07 ` [PATCH iproute2 net-next 2/4] ss: Introduce columns lightweight abstraction Stefano Brivio
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Stefano Brivio @ 2017-12-08 17:07 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, Sabrina Dubroca

This is preparation work for output buffering, which will allow
us to use optimal spacing and alignment of logical "columns".

The new out() function is just a re-implementation of a typical
libc's printf(), except that the return value of vfprintf() is
ignored as no callers use it. This implementation will be
replaced in the next patches to provide column width adjustment
and adequate spacing.

All printf() calls that output parts of the socket list are now
replaced by calls to out(). Output of summary and version is
excluded from this.

No functional differences here, output not affected.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>
---
 misc/ss.c | 397 ++++++++++++++++++++++++++++++++------------------------------
 1 file changed, 204 insertions(+), 193 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index b5099d1e0e66..72e117ca9405 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -26,6 +26,7 @@
 #include <getopt.h>
 #include <stdbool.h>
 #include <limits.h>
+#include <stdarg.h>
 
 #include "utils.h"
 #include "rt_names.h"
@@ -822,6 +823,15 @@ static const char *vsock_netid_name(int type)
 	}
 }
 
+static void out(const char *fmt, ...)
+{
+	va_list args;
+
+	va_start(args, fmt);
+	vfprintf(stdout, fmt, args);
+	va_end(args);
+}
+
 static void sock_state_print(struct sockstat *s)
 {
 	const char *sock_name;
@@ -862,39 +872,39 @@ static void sock_state_print(struct sockstat *s)
 	}
 
 	if (netid_width)
-		printf("%-*s ", netid_width,
-		       is_sctp_assoc(s, sock_name) ? "" : sock_name);
+		out("%-*s ", netid_width,
+		    is_sctp_assoc(s, sock_name) ? "" : sock_name);
 	if (state_width) {
 		if (is_sctp_assoc(s, sock_name))
-			printf("`- %-*s ", state_width - 3,
-			       sctp_sstate_name[s->state]);
+			out("`- %-*s ", state_width - 3,
+			    sctp_sstate_name[s->state]);
 		else
-			printf("%-*s ", state_width, sstate_name[s->state]);
+			out("%-*s ", state_width, sstate_name[s->state]);
 	}
 
-	printf("%-6d %-6d %s", s->rq, s->wq, odd_width_pad);
+	out("%-6d %-6d %s", s->rq, s->wq, odd_width_pad);
 }
 
 static void sock_details_print(struct sockstat *s)
 {
 	if (s->uid)
-		printf(" uid:%u", s->uid);
+		out(" uid:%u", s->uid);
 
-	printf(" ino:%u", s->ino);
-	printf(" sk:%llx", s->sk);
+	out(" ino:%u", s->ino);
+	out(" sk:%llx", s->sk);
 
 	if (s->mark)
-		printf(" fwmark:0x%x", s->mark);
+		out(" fwmark:0x%x", s->mark);
 }
 
 static void sock_addr_print_width(int addr_len, const char *addr, char *delim,
 		int port_len, const char *port, const char *ifname)
 {
 	if (ifname) {
-		printf("%*s%%%s%s%-*s ", addr_len, addr, ifname, delim,
-				port_len, port);
+		out("%*s%%%s%s%-*s ", addr_len, addr, ifname, delim,
+		    port_len, port);
 	} else {
-		printf("%*s%s%-*s ", addr_len, addr, delim, port_len, port);
+		out("%*s%s%-*s ", addr_len, addr, delim, port_len, port);
 	}
 }
 
@@ -1792,12 +1802,12 @@ static void proc_ctx_print(struct sockstat *s)
 		if (find_entry(s->ino, &buf,
 				(show_proc_ctx & show_sock_ctx) ?
 				PROC_SOCK_CTX : PROC_CTX) > 0) {
-			printf(" users:(%s)", buf);
+			out(" users:(%s)", buf);
 			free(buf);
 		}
 	} else if (show_users) {
 		if (find_entry(s->ino, &buf, USERS) > 0) {
-			printf(" users:(%s)", buf);
+			out(" users:(%s)", buf);
 			free(buf);
 		}
 	}
@@ -1877,51 +1887,51 @@ static char *sprint_bw(char *buf, double bw)
 static void sctp_stats_print(struct sctp_info *s)
 {
 	if (s->sctpi_tag)
-		printf(" tag:%x", s->sctpi_tag);
+		out(" tag:%x", s->sctpi_tag);
 	if (s->sctpi_state)
-		printf(" state:%s", sctp_sstate_name[s->sctpi_state]);
+		out(" state:%s", sctp_sstate_name[s->sctpi_state]);
 	if (s->sctpi_rwnd)
-		printf(" rwnd:%d", s->sctpi_rwnd);
+		out(" rwnd:%d", s->sctpi_rwnd);
 	if (s->sctpi_unackdata)
-		printf(" unackdata:%d", s->sctpi_unackdata);
+		out(" unackdata:%d", s->sctpi_unackdata);
 	if (s->sctpi_penddata)
-		printf(" penddata:%d", s->sctpi_penddata);
+		out(" penddata:%d", s->sctpi_penddata);
 	if (s->sctpi_instrms)
-		printf(" instrms:%d", s->sctpi_instrms);
+		out(" instrms:%d", s->sctpi_instrms);
 	if (s->sctpi_outstrms)
-		printf(" outstrms:%d", s->sctpi_outstrms);
+		out(" outstrms:%d", s->sctpi_outstrms);
 	if (s->sctpi_inqueue)
-		printf(" inqueue:%d", s->sctpi_inqueue);
+		out(" inqueue:%d", s->sctpi_inqueue);
 	if (s->sctpi_outqueue)
-		printf(" outqueue:%d", s->sctpi_outqueue);
+		out(" outqueue:%d", s->sctpi_outqueue);
 	if (s->sctpi_overall_error)
-		printf(" overerr:%d", s->sctpi_overall_error);
+		out(" overerr:%d", s->sctpi_overall_error);
 	if (s->sctpi_max_burst)
-		printf(" maxburst:%d", s->sctpi_max_burst);
+		out(" maxburst:%d", s->sctpi_max_burst);
 	if (s->sctpi_maxseg)
-		printf(" maxseg:%d", s->sctpi_maxseg);
+		out(" maxseg:%d", s->sctpi_maxseg);
 	if (s->sctpi_peer_rwnd)
-		printf(" prwnd:%d", s->sctpi_peer_rwnd);
+		out(" prwnd:%d", s->sctpi_peer_rwnd);
 	if (s->sctpi_peer_tag)
-		printf(" ptag:%x", s->sctpi_peer_tag);
+		out(" ptag:%x", s->sctpi_peer_tag);
 	if (s->sctpi_peer_capable)
-		printf(" pcapable:%d", s->sctpi_peer_capable);
+		out(" pcapable:%d", s->sctpi_peer_capable);
 	if (s->sctpi_peer_sack)
-		printf(" psack:%d", s->sctpi_peer_sack);
+		out(" psack:%d", s->sctpi_peer_sack);
 	if (s->sctpi_s_autoclose)
-		printf(" autoclose:%d", s->sctpi_s_autoclose);
+		out(" autoclose:%d", s->sctpi_s_autoclose);
 	if (s->sctpi_s_adaptation_ind)
-		printf(" adapind:%d", s->sctpi_s_adaptation_ind);
+		out(" adapind:%d", s->sctpi_s_adaptation_ind);
 	if (s->sctpi_s_pd_point)
-		printf(" pdpoint:%d", s->sctpi_s_pd_point);
+		out(" pdpoint:%d", s->sctpi_s_pd_point);
 	if (s->sctpi_s_nodelay)
-		printf(" nodealy:%d", s->sctpi_s_nodelay);
+		out(" nodealy:%d", s->sctpi_s_nodelay);
 	if (s->sctpi_s_disable_fragments)
-		printf(" nofrag:%d", s->sctpi_s_disable_fragments);
+		out(" nofrag:%d", s->sctpi_s_disable_fragments);
 	if (s->sctpi_s_v4mapped)
-		printf(" v4mapped:%d", s->sctpi_s_v4mapped);
+		out(" v4mapped:%d", s->sctpi_s_v4mapped);
 	if (s->sctpi_s_frag_interleave)
-		printf(" fraginl:%d", s->sctpi_s_frag_interleave);
+		out(" fraginl:%d", s->sctpi_s_frag_interleave);
 }
 
 static void tcp_stats_print(struct tcpstat *s)
@@ -1929,65 +1939,65 @@ static void tcp_stats_print(struct tcpstat *s)
 	char b1[64];
 
 	if (s->has_ts_opt)
-		printf(" ts");
+		out(" ts");
 	if (s->has_sack_opt)
-		printf(" sack");
+		out(" sack");
 	if (s->has_ecn_opt)
-		printf(" ecn");
+		out(" ecn");
 	if (s->has_ecnseen_opt)
-		printf(" ecnseen");
+		out(" ecnseen");
 	if (s->has_fastopen_opt)
-		printf(" fastopen");
+		out(" fastopen");
 	if (s->cong_alg[0])
-		printf(" %s", s->cong_alg);
+		out(" %s", s->cong_alg);
 	if (s->has_wscale_opt)
-		printf(" wscale:%d,%d", s->snd_wscale, s->rcv_wscale);
+		out(" wscale:%d,%d", s->snd_wscale, s->rcv_wscale);
 	if (s->rto)
-		printf(" rto:%g", s->rto);
+		out(" rto:%g", s->rto);
 	if (s->backoff)
-		printf(" backoff:%u", s->backoff);
+		out(" backoff:%u", s->backoff);
 	if (s->rtt)
-		printf(" rtt:%g/%g", s->rtt, s->rttvar);
+		out(" rtt:%g/%g", s->rtt, s->rttvar);
 	if (s->ato)
-		printf(" ato:%g", s->ato);
+		out(" ato:%g", s->ato);
 
 	if (s->qack)
-		printf(" qack:%d", s->qack);
+		out(" qack:%d", s->qack);
 	if (s->qack & 1)
-		printf(" bidir");
+		out(" bidir");
 
 	if (s->mss)
-		printf(" mss:%d", s->mss);
+		out(" mss:%d", s->mss);
 	if (s->rcv_mss)
-		printf(" rcvmss:%d", s->rcv_mss);
+		out(" rcvmss:%d", s->rcv_mss);
 	if (s->advmss)
-		printf(" advmss:%d", s->advmss);
+		out(" advmss:%d", s->advmss);
 	if (s->cwnd)
-		printf(" cwnd:%u", s->cwnd);
+		out(" cwnd:%u", s->cwnd);
 	if (s->ssthresh)
-		printf(" ssthresh:%d", s->ssthresh);
+		out(" ssthresh:%d", s->ssthresh);
 
 	if (s->bytes_acked)
-		printf(" bytes_acked:%llu", s->bytes_acked);
+		out(" bytes_acked:%llu", s->bytes_acked);
 	if (s->bytes_received)
-		printf(" bytes_received:%llu", s->bytes_received);
+		out(" bytes_received:%llu", s->bytes_received);
 	if (s->segs_out)
-		printf(" segs_out:%u", s->segs_out);
+		out(" segs_out:%u", s->segs_out);
 	if (s->segs_in)
-		printf(" segs_in:%u", s->segs_in);
+		out(" segs_in:%u", s->segs_in);
 	if (s->data_segs_out)
-		printf(" data_segs_out:%u", s->data_segs_out);
+		out(" data_segs_out:%u", s->data_segs_out);
 	if (s->data_segs_in)
-		printf(" data_segs_in:%u", s->data_segs_in);
+		out(" data_segs_in:%u", s->data_segs_in);
 
 	if (s->dctcp && s->dctcp->enabled) {
 		struct dctcpstat *dctcp = s->dctcp;
 
-		printf(" dctcp:(ce_state:%u,alpha:%u,ab_ecn:%u,ab_tot:%u)",
-				dctcp->ce_state, dctcp->alpha, dctcp->ab_ecn,
-				dctcp->ab_tot);
+		out(" dctcp:(ce_state:%u,alpha:%u,ab_ecn:%u,ab_tot:%u)",
+			     dctcp->ce_state, dctcp->alpha, dctcp->ab_ecn,
+			     dctcp->ab_tot);
 	} else if (s->dctcp) {
-		printf(" dctcp:fallback_mode");
+		out(" dctcp:fallback_mode");
 	}
 
 	if (s->bbr_info) {
@@ -1997,71 +2007,70 @@ static void tcp_stats_print(struct tcpstat *s)
 		bw <<= 32;
 		bw |= s->bbr_info->bbr_bw_lo;
 
-		printf(" bbr:(bw:%sbps,mrtt:%g",
-		       sprint_bw(b1, bw * 8.0),
-		       (double)s->bbr_info->bbr_min_rtt / 1000.0);
+		out(" bbr:(bw:%sbps,mrtt:%g",
+		    sprint_bw(b1, bw * 8.0),
+		    (double)s->bbr_info->bbr_min_rtt / 1000.0);
 		if (s->bbr_info->bbr_pacing_gain)
-			printf(",pacing_gain:%g",
-			       (double)s->bbr_info->bbr_pacing_gain / 256.0);
+			out(",pacing_gain:%g",
+			    (double)s->bbr_info->bbr_pacing_gain / 256.0);
 		if (s->bbr_info->bbr_cwnd_gain)
-			printf(",cwnd_gain:%g",
-			       (double)s->bbr_info->bbr_cwnd_gain / 256.0);
-		printf(")");
+			out(",cwnd_gain:%g",
+			    (double)s->bbr_info->bbr_cwnd_gain / 256.0);
+		out(")");
 	}
 
 	if (s->send_bps)
-		printf(" send %sbps", sprint_bw(b1, s->send_bps));
+		out(" send %sbps", sprint_bw(b1, s->send_bps));
 	if (s->lastsnd)
-		printf(" lastsnd:%u", s->lastsnd);
+		out(" lastsnd:%u", s->lastsnd);
 	if (s->lastrcv)
-		printf(" lastrcv:%u", s->lastrcv);
+		out(" lastrcv:%u", s->lastrcv);
 	if (s->lastack)
-		printf(" lastack:%u", s->lastack);
+		out(" lastack:%u", s->lastack);
 
 	if (s->pacing_rate) {
-		printf(" pacing_rate %sbps", sprint_bw(b1, s->pacing_rate));
+		out(" pacing_rate %sbps", sprint_bw(b1, s->pacing_rate));
 		if (s->pacing_rate_max)
-				printf("/%sbps", sprint_bw(b1,
-							s->pacing_rate_max));
+			out("/%sbps", sprint_bw(b1, s->pacing_rate_max));
 	}
 
 	if (s->delivery_rate)
-		printf(" delivery_rate %sbps", sprint_bw(b1, s->delivery_rate));
+		out(" delivery_rate %sbps", sprint_bw(b1, s->delivery_rate));
 	if (s->app_limited)
-		printf(" app_limited");
+		out(" app_limited");
 
 	if (s->busy_time) {
-		printf(" busy:%llums", s->busy_time / 1000);
+		out(" busy:%llums", s->busy_time / 1000);
 		if (s->rwnd_limited)
-			printf(" rwnd_limited:%llums(%.1f%%)",
-			       s->rwnd_limited / 1000,
-			       100.0 * s->rwnd_limited / s->busy_time);
+			out(" rwnd_limited:%llums(%.1f%%)",
+			    s->rwnd_limited / 1000,
+			    100.0 * s->rwnd_limited / s->busy_time);
 		if (s->sndbuf_limited)
-			printf(" sndbuf_limited:%llums(%.1f%%)",
-			       s->sndbuf_limited / 1000,
-			       100.0 * s->sndbuf_limited / s->busy_time);
+			out(" sndbuf_limited:%llums(%.1f%%)",
+			    s->sndbuf_limited / 1000,
+			    100.0 * s->sndbuf_limited / s->busy_time);
 	}
 
 	if (s->unacked)
-		printf(" unacked:%u", s->unacked);
+		out(" unacked:%u", s->unacked);
 	if (s->retrans || s->retrans_total)
-		printf(" retrans:%u/%u", s->retrans, s->retrans_total);
+		out(" retrans:%u/%u", s->retrans, s->retrans_total);
 	if (s->lost)
-		printf(" lost:%u", s->lost);
+		out(" lost:%u", s->lost);
 	if (s->sacked && s->ss.state != SS_LISTEN)
-		printf(" sacked:%u", s->sacked);
+		out(" sacked:%u", s->sacked);
 	if (s->fackets)
-		printf(" fackets:%u", s->fackets);
+		out(" fackets:%u", s->fackets);
 	if (s->reordering != 3)
-		printf(" reordering:%d", s->reordering);
+		out(" reordering:%d", s->reordering);
 	if (s->rcv_rtt)
-		printf(" rcv_rtt:%g", s->rcv_rtt);
+		out(" rcv_rtt:%g", s->rcv_rtt);
 	if (s->rcv_space)
-		printf(" rcv_space:%d", s->rcv_space);
+		out(" rcv_space:%d", s->rcv_space);
 	if (s->not_sent)
-		printf(" notsent:%u", s->not_sent);
+		out(" notsent:%u", s->not_sent);
 	if (s->min_rtt)
-		printf(" minrtt:%g", s->min_rtt);
+		out(" minrtt:%g", s->min_rtt);
 }
 
 static void tcp_timer_print(struct tcpstat *s)
@@ -2078,18 +2087,18 @@ static void tcp_timer_print(struct tcpstat *s)
 	if (s->timer) {
 		if (s->timer > 4)
 			s->timer = 5;
-		printf(" timer:(%s,%s,%d)",
-				tmr_name[s->timer],
-				print_ms_timer(s->timeout),
-				s->retrans);
+		out(" timer:(%s,%s,%d)",
+			     tmr_name[s->timer],
+			     print_ms_timer(s->timeout),
+			     s->retrans);
 	}
 }
 
 static void sctp_timer_print(struct tcpstat *s)
 {
 	if (s->timer)
-		printf(" timer:(T3_RTX,%s,%d)",
-		       print_ms_timer(s->timeout), s->retrans);
+		out(" timer:(T3_RTX,%s,%d)",
+		    print_ms_timer(s->timeout), s->retrans);
 }
 
 static int tcp_show_line(char *line, const struct filter *f, int family)
@@ -2148,13 +2157,13 @@ static int tcp_show_line(char *line, const struct filter *f, int family)
 	if (show_details) {
 		sock_details_print(&s.ss);
 		if (opt[0])
-			printf(" opt:\"%s\"", opt);
+			out(" opt:\"%s\"", opt);
 	}
 
 	if (show_tcpinfo)
 		tcp_stats_print(&s);
 
-	printf("\n");
+	out("\n");
 	return 0;
 }
 
@@ -2197,44 +2206,44 @@ static void print_skmeminfo(struct rtattr *tb[], int attrtype)
 			const struct inet_diag_meminfo *minfo =
 				RTA_DATA(tb[INET_DIAG_MEMINFO]);
 
-			printf(" mem:(r%u,w%u,f%u,t%u)",
-					minfo->idiag_rmem,
-					minfo->idiag_wmem,
-					minfo->idiag_fmem,
-					minfo->idiag_tmem);
+			out(" mem:(r%u,w%u,f%u,t%u)",
+				   minfo->idiag_rmem,
+				   minfo->idiag_wmem,
+				   minfo->idiag_fmem,
+				   minfo->idiag_tmem);
 		}
 		return;
 	}
 
 	skmeminfo = RTA_DATA(tb[attrtype]);
 
-	printf(" skmem:(r%u,rb%u,t%u,tb%u,f%u,w%u,o%u",
-	       skmeminfo[SK_MEMINFO_RMEM_ALLOC],
-	       skmeminfo[SK_MEMINFO_RCVBUF],
-	       skmeminfo[SK_MEMINFO_WMEM_ALLOC],
-	       skmeminfo[SK_MEMINFO_SNDBUF],
-	       skmeminfo[SK_MEMINFO_FWD_ALLOC],
-	       skmeminfo[SK_MEMINFO_WMEM_QUEUED],
-	       skmeminfo[SK_MEMINFO_OPTMEM]);
+	out(" skmem:(r%u,rb%u,t%u,tb%u,f%u,w%u,o%u",
+		     skmeminfo[SK_MEMINFO_RMEM_ALLOC],
+		     skmeminfo[SK_MEMINFO_RCVBUF],
+		     skmeminfo[SK_MEMINFO_WMEM_ALLOC],
+		     skmeminfo[SK_MEMINFO_SNDBUF],
+		     skmeminfo[SK_MEMINFO_FWD_ALLOC],
+		     skmeminfo[SK_MEMINFO_WMEM_QUEUED],
+		     skmeminfo[SK_MEMINFO_OPTMEM]);
 
 	if (RTA_PAYLOAD(tb[attrtype]) >=
 		(SK_MEMINFO_BACKLOG + 1) * sizeof(__u32))
-		printf(",bl%u", skmeminfo[SK_MEMINFO_BACKLOG]);
+		out(",bl%u", skmeminfo[SK_MEMINFO_BACKLOG]);
 
 	if (RTA_PAYLOAD(tb[attrtype]) >=
 		(SK_MEMINFO_DROPS + 1) * sizeof(__u32))
-		printf(",d%u", skmeminfo[SK_MEMINFO_DROPS]);
+		out(",d%u", skmeminfo[SK_MEMINFO_DROPS]);
 
-	printf(")");
+	out(")");
 }
 
 static void print_md5sig(struct tcp_diag_md5sig *sig)
 {
-	printf("%s/%d=",
-	       format_host(sig->tcpm_family,
-			   sig->tcpm_family == AF_INET6 ? 16 : 4,
-			   &sig->tcpm_addr),
-	       sig->tcpm_prefixlen);
+	out("%s/%d=",
+	    format_host(sig->tcpm_family,
+			sig->tcpm_family == AF_INET6 ? 16 : 4,
+			&sig->tcpm_addr),
+	    sig->tcpm_prefixlen);
 	print_escape_buf(sig->tcpm_key, sig->tcpm_keylen, " ,");
 }
 
@@ -2379,10 +2388,10 @@ static void tcp_show_info(const struct nlmsghdr *nlh, struct inet_diag_msg *r,
 		struct tcp_diag_md5sig *sig = RTA_DATA(tb[INET_DIAG_MD5SIG]);
 		int len = RTA_PAYLOAD(tb[INET_DIAG_MD5SIG]);
 
-		printf(" md5keys:");
+		out(" md5keys:");
 		print_md5sig(sig++);
 		for (len -= sizeof(*sig); len > 0; len -= sizeof(*sig)) {
-			printf(",");
+			out(",");
 			print_md5sig(sig++);
 		}
 	}
@@ -2417,18 +2426,18 @@ static void sctp_show_info(const struct nlmsghdr *nlh, struct inet_diag_msg *r,
 		len = RTA_PAYLOAD(tb[INET_DIAG_LOCALS]);
 		sa = RTA_DATA(tb[INET_DIAG_LOCALS]);
 
-		printf("locals:%s", format_host_sa(sa));
+		out("locals:%s", format_host_sa(sa));
 		for (sa++, len -= sizeof(*sa); len > 0; sa++, len -= sizeof(*sa))
-			printf(",%s", format_host_sa(sa));
+			out(",%s", format_host_sa(sa));
 
 	}
 	if (tb[INET_DIAG_PEERS]) {
 		len = RTA_PAYLOAD(tb[INET_DIAG_PEERS]);
 		sa = RTA_DATA(tb[INET_DIAG_PEERS]);
 
-		printf(" peers:%s", format_host_sa(sa));
+		out(" peers:%s", format_host_sa(sa));
 		for (sa++, len -= sizeof(*sa); len > 0; sa++, len -= sizeof(*sa))
-			printf(",%s", format_host_sa(sa));
+			out(",%s", format_host_sa(sa));
 	}
 	if (tb[INET_DIAG_INFO]) {
 		struct sctp_info *info;
@@ -2515,18 +2524,19 @@ static int inet_show_sock(struct nlmsghdr *nlh,
 	if (show_details) {
 		sock_details_print(s);
 		if (s->local.family == AF_INET6 && tb[INET_DIAG_SKV6ONLY])
-			printf(" v6only:%u", v6only);
+			out(" v6only:%u", v6only);
 
 		if (tb[INET_DIAG_SHUTDOWN]) {
 			unsigned char mask;
 
 			mask = rta_getattr_u8(tb[INET_DIAG_SHUTDOWN]);
-			printf(" %c-%c", mask & 1 ? '-' : '<', mask & 2 ? '-' : '>');
+			out(" %c-%c",
+			    mask & 1 ? '-' : '<', mask & 2 ? '-' : '>');
 		}
 	}
 
 	if (show_mem || (show_tcpinfo && s->type != IPPROTO_UDP)) {
-		printf("\n\t");
+		out("\n\t");
 		if (s->type == IPPROTO_SCTP)
 			sctp_show_info(nlh, r, tb);
 		else
@@ -2534,7 +2544,7 @@ static int inet_show_sock(struct nlmsghdr *nlh,
 	}
 	sctp_ino = s->ino;
 
-	printf("\n");
+	out("\n");
 	return 0;
 }
 
@@ -2990,9 +3000,9 @@ static int dgram_show_line(char *line, const struct filter *f, int family)
 	inet_stats_print(&s, false);
 
 	if (show_details && opt[0])
-		printf(" opt:\"%s\"", opt);
+		out(" opt:\"%s\"", opt);
 
-	printf("\n");
+	out("\n");
 	return 0;
 }
 
@@ -3167,10 +3177,11 @@ static int unix_show_sock(const struct sockaddr_nl *addr, struct nlmsghdr *nlh,
 			unsigned char mask;
 
 			mask = rta_getattr_u8(tb[UNIX_DIAG_SHUTDOWN]);
-			printf(" %c-%c", mask & 1 ? '-' : '<', mask & 2 ? '-' : '>');
+			out(" %c-%c",
+			    mask & 1 ? '-' : '<', mask & 2 ? '-' : '>');
 		}
 	}
-	printf("\n");
+	out("\n");
 
 	return 0;
 }
@@ -3327,7 +3338,7 @@ static int unix_show(struct filter *f)
 		if (++cnt > MAX_UNIX_REMEMBER) {
 			while (list) {
 				unix_stats_print(list, f);
-				printf("\n");
+				out("\n");
 
 				unix_list_drop_first(&list);
 			}
@@ -3337,7 +3348,7 @@ static int unix_show(struct filter *f)
 	fclose(fp);
 	while (list) {
 		unix_stats_print(list, f);
-		printf("\n");
+		out("\n");
 
 		unix_list_drop_first(&list);
 	}
@@ -3383,12 +3394,12 @@ static int packet_stats_print(struct sockstat *s, const struct filter *f)
 
 static void packet_show_ring(struct packet_diag_ring *ring)
 {
-	printf("blk_size:%d", ring->pdr_block_size);
-	printf(",blk_nr:%d", ring->pdr_block_nr);
-	printf(",frm_size:%d", ring->pdr_frame_size);
-	printf(",frm_nr:%d", ring->pdr_frame_nr);
-	printf(",tmo:%d", ring->pdr_retire_tmo);
-	printf(",features:0x%x", ring->pdr_features);
+	out("blk_size:%d", ring->pdr_block_size);
+	out(",blk_nr:%d", ring->pdr_block_nr);
+	out(",frm_size:%d", ring->pdr_frame_size);
+	out(",frm_nr:%d", ring->pdr_frame_nr);
+	out(",tmo:%d", ring->pdr_retire_tmo);
+	out(",features:0x%x", ring->pdr_features);
 }
 
 static int packet_show_sock(const struct sockaddr_nl *addr,
@@ -3446,56 +3457,56 @@ static int packet_show_sock(const struct sockaddr_nl *addr,
 
 	if (show_details) {
 		if (pinfo) {
-			printf("\n\tver:%d", pinfo->pdi_version);
-			printf(" cpy_thresh:%d", pinfo->pdi_copy_thresh);
-			printf(" flags( ");
+			out("\n\tver:%d", pinfo->pdi_version);
+			out(" cpy_thresh:%d", pinfo->pdi_copy_thresh);
+			out(" flags( ");
 			if (pinfo->pdi_flags & PDI_RUNNING)
-				printf("running");
+				out("running");
 			if (pinfo->pdi_flags & PDI_AUXDATA)
-				printf(" auxdata");
+				out(" auxdata");
 			if (pinfo->pdi_flags & PDI_ORIGDEV)
-				printf(" origdev");
+				out(" origdev");
 			if (pinfo->pdi_flags & PDI_VNETHDR)
-				printf(" vnethdr");
+				out(" vnethdr");
 			if (pinfo->pdi_flags & PDI_LOSS)
-				printf(" loss");
+				out(" loss");
 			if (!pinfo->pdi_flags)
-				printf("0");
-			printf(" )");
+				out("0");
+			out(" )");
 		}
 		if (ring_rx) {
-			printf("\n\tring_rx(");
+			out("\n\tring_rx(");
 			packet_show_ring(ring_rx);
-			printf(")");
+			out(")");
 		}
 		if (ring_tx) {
-			printf("\n\tring_tx(");
+			out("\n\tring_tx(");
 			packet_show_ring(ring_tx);
-			printf(")");
+			out(")");
 		}
 		if (has_fanout) {
 			uint16_t type = (fanout >> 16) & 0xffff;
 
-			printf("\n\tfanout(");
-			printf("id:%d,", fanout & 0xffff);
-			printf("type:");
+			out("\n\tfanout(");
+			out("id:%d,", fanout & 0xffff);
+			out("type:");
 
 			if (type == 0)
-				printf("hash");
+				out("hash");
 			else if (type == 1)
-				printf("lb");
+				out("lb");
 			else if (type == 2)
-				printf("cpu");
+				out("cpu");
 			else if (type == 3)
-				printf("roll");
+				out("roll");
 			else if (type == 4)
-				printf("random");
+				out("random");
 			else if (type == 5)
-				printf("qm");
+				out("qm");
 			else
-				printf("0x%x", type);
+				out("0x%x", type);
 
-			printf(")");
+			out(")");
 		}
 	}
 
@@ -3505,15 +3516,15 @@ static int packet_show_sock(const struct sockaddr_nl *addr,
 		int num = RTA_PAYLOAD(tb[PACKET_DIAG_FILTER]) /
 			  sizeof(struct sock_filter);
 
-		printf("\n\tbpf filter (%d): ", num);
+		out("\n\tbpf filter (%d): ", num);
 		while (num) {
-			printf(" 0x%02x %u %u %u,",
-			      fil->code, fil->jt, fil->jf, fil->k);
+			out(" 0x%02x %u %u %u,",
+			    fil->code, fil->jt, fil->jf, fil->k);
 			num--;
 			fil++;
 		}
 	}
-	printf("\n");
+	out("\n");
 	return 0;
 }
 
@@ -3556,7 +3567,7 @@ static int packet_show_line(char *buf, const struct filter *f, int fam)
 	if (packet_stats_print(&stat, f))
 		return 0;
 
-	printf("\n");
+	out("\n");
 	return 0;
 }
 
@@ -3669,14 +3680,14 @@ static int netlink_show_one(struct filter *f,
 		else if (pid > 0)
 			getpidcon(pid, &pid_context);
 
-		printf(" proc_ctx=%s", pid_context ? : "unavailable");
+		out(" proc_ctx=%s", pid_context ? : "unavailable");
 		free(pid_context);
 	}
 
 	if (show_details) {
-		printf(" sk=%llx cb=%llx groups=0x%08x", sk, cb, groups);
+		out(" sk=%llx cb=%llx groups=0x%08x", sk, cb, groups);
 	}
-	printf("\n");
+	out("\n");
 
 	return 0;
 }
@@ -3712,9 +3723,9 @@ static int netlink_show_sock(const struct sockaddr_nl *addr,
 	}
 
 	if (show_mem) {
-		printf("\t");
+		out("\t");
 		print_skmeminfo(tb, NETLINK_DIAG_MEMINFO);
-		printf("\n");
+		out("\n");
 	}
 
 	return 0;
@@ -3805,7 +3816,7 @@ static void vsock_stats_print(struct sockstat *s, struct filter *f)
 
 	proc_ctx_print(s);
 
-	printf("\n");
+	out("\n");
 }
 
 static int vsock_show_sock(const struct sockaddr_nl *addr,
@@ -4570,10 +4581,10 @@ int main(int argc, char *argv[])
 
 	if (show_header) {
 		if (netid_width)
-			printf("%-*s ", netid_width, "Netid");
+			out("%-*s ", netid_width, "Netid");
 		if (state_width)
-			printf("%-*s ", state_width, "State");
-		printf("%-6s %-6s %s", "Recv-Q", "Send-Q", odd_width_pad);
+			out("%-*s ", state_width, "State");
+		out("%-6s %-6s %s", "Recv-Q", "Send-Q", odd_width_pad);
 	}
 
 	/* Make enough space for the local/remote port field */
@@ -4581,9 +4592,9 @@ int main(int argc, char *argv[])
 	serv_width += 13;
 
 	if (show_header) {
-		printf("%*s:%-*s %*s:%-*s\n",
-		       addr_width, "Local Address", serv_width, "Port",
-		       addr_width, "Peer Address", serv_width, "Port");
+		out("%*s:%-*s %*s:%-*s\n",
+		    addr_width, "Local Address", serv_width, "Port",
+		    addr_width, "Peer Address", serv_width, "Port");
 	}
 
 	fflush(stdout);
-- 
2.9.4

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

* [PATCH iproute2 net-next 2/4] ss: Introduce columns lightweight abstraction
  2017-12-08 17:07 [PATCH iproute2 net-next 0/4] Abstract columns, properly space and wrap fields Stefano Brivio
  2017-12-08 17:07 ` [PATCH iproute2 net-next 1/4] ss: Replace printf() calls for "main" output by calls to helper Stefano Brivio
@ 2017-12-08 17:07 ` Stefano Brivio
  2017-12-08 17:07 ` [PATCH iproute2 net-next 3/4] ss: Buffer raw fields first, then render them as a table Stefano Brivio
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Stefano Brivio @ 2017-12-08 17:07 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, Sabrina Dubroca

Instead of embedding spacing directly while printing contents,
logically declare columns and functions to buffer their content,
to print left and right spacing around fields, to flush them to
screen, and to print headers.

This makes it a bit easier to handle layout changes and prepares
for full output buffering, needed for optimal spacing in field
output layout.

Columns are currently set up to retain exactly the same output
as before. This needs some slight adjustments of the values
previously calculated in main(), as the width value introduced
here already includes the width of left delimiters and spacing
is not explicitly printed anymore whenever a field is printed.
These calculations will go away altogether once automatic width
calculation is implemented.

We can also remove explicit printing of newlines after the final
content for a given line is printed, flushing the last field on
a line will cause field_flush() to print newlines where
appropriate.

No changes in output expected here.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>
---
 misc/ss.c | 289 ++++++++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 198 insertions(+), 91 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index 72e117ca9405..9dbcfd514d48 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -103,11 +103,48 @@ int show_header = 1;
 int follow_events;
 int sctp_ino;
 
-int netid_width;
-int state_width;
-int addr_width;
-int serv_width;
-char *odd_width_pad = "";
+enum col_id {
+	COL_NETID,
+	COL_STATE,
+	COL_RECVQ,
+	COL_SENDQ,
+	COL_ADDR,
+	COL_SERV,
+	COL_RADDR,
+	COL_RSERV,
+	COL_EXT,
+	COL_MAX
+};
+
+enum col_align {
+	ALIGN_LEFT,
+	ALIGN_CENTER,
+	ALIGN_RIGHT
+};
+
+struct column {
+	const enum col_align align;
+	const char *header;
+	const char *ldelim;
+	int width;	/* Including delimiter. -1: fit to content, 0: hide */
+	int stored;	/* Characters buffered */
+	int printed;	/* Characters printed so far */
+};
+
+static struct column columns[] = {
+	{ ALIGN_LEFT,	"Netid",		"",	0,	0,	0 },
+	{ ALIGN_LEFT,	"State",		" ",	0,	0,	0 },
+	{ ALIGN_LEFT,	"Recv-Q",		" ",	7,	0,	0 },
+	{ ALIGN_LEFT,	"Send-Q",		" ",	7,	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,	"",			"",	-1,	0,	0 },
+};
+
+static struct column *current_field = columns;
+static char field_buf[BUFSIZ];
 
 static const char *TCP_PROTO = "tcp";
 static const char *SCTP_PROTO = "sctp";
@@ -825,13 +862,113 @@ static const char *vsock_netid_name(int type)
 
 static void out(const char *fmt, ...)
 {
+	struct column *f = current_field;
 	va_list args;
 
 	va_start(args, fmt);
-	vfprintf(stdout, fmt, args);
+	f->stored += vsnprintf(field_buf + f->stored, BUFSIZ - f->stored,
+			       fmt, args);
 	va_end(args);
 }
 
+static int print_left_spacing(struct column *f)
+{
+	int s;
+
+	if (f->width < 0 || f->align == ALIGN_LEFT)
+		return 0;
+
+	s = f->width - f->stored - f->printed;
+	if (f->align == ALIGN_CENTER)
+		/* If count of total spacing is odd, shift right by one */
+		s = (s + 1) / 2;
+
+	if (s > 0)
+		return printf("%*c", s, ' ');
+
+	return 0;
+}
+
+static void print_right_spacing(struct column *f)
+{
+	int s;
+
+	if (f->width < 0 || f->align == ALIGN_RIGHT)
+		return;
+
+	s = f->width - f->printed;
+	if (f->align == ALIGN_CENTER)
+		s /= 2;
+
+	if (s > 0)
+		printf("%*c", s, ' ');
+}
+
+static int field_needs_delimiter(struct column *f)
+{
+	if (!f->stored)
+		return 0;
+
+	/* Was another field already printed on this line? */
+	for (f--; f >= columns; f--)
+		if (f->width)
+			return 1;
+
+	return 0;
+}
+
+/* Flush given field to screen together with delimiter and spacing */
+static void field_flush(struct column *f)
+{
+	if (!f->width)
+		return;
+
+	if (field_needs_delimiter(f))
+		f->printed = printf("%s", f->ldelim);
+
+	f->printed += print_left_spacing(f);
+	f->printed += printf("%s", field_buf);
+	print_right_spacing(f);
+
+	*field_buf = 0;
+	f->printed = 0;
+	f->stored = 0;
+}
+
+static int field_is_last(struct column *f)
+{
+	return f - columns == COL_MAX - 1;
+}
+
+static void field_next(void)
+{
+	field_flush(current_field);
+
+	if (field_is_last(current_field)) {
+		printf("\n");
+		current_field = columns;
+	} else {
+		current_field++;
+	}
+}
+
+/* Walk through fields and flush them until we reach the desired one */
+static void field_set(enum col_id id)
+{
+	while (id != current_field - columns)
+		field_next();
+}
+
+/* Print header for all non-empty columns */
+static void print_header(void)
+{
+	while (!field_is_last(current_field)) {
+		if (current_field->width)
+			out(current_field->header);
+		field_next();
+	}
+}
+
 static void sock_state_print(struct sockstat *s)
 {
 	const char *sock_name;
@@ -871,18 +1008,21 @@ static void sock_state_print(struct sockstat *s)
 		sock_name = "unknown";
 	}
 
-	if (netid_width)
-		out("%-*s ", netid_width,
-		    is_sctp_assoc(s, sock_name) ? "" : sock_name);
-	if (state_width) {
-		if (is_sctp_assoc(s, sock_name))
-			out("`- %-*s ", state_width - 3,
-			    sctp_sstate_name[s->state]);
-		else
-			out("%-*s ", state_width, sstate_name[s->state]);
+	if (is_sctp_assoc(s, sock_name)) {
+		field_set(COL_STATE);		/* Empty Netid field */
+		out("`- %s", sctp_sstate_name[s->state]);
+	} else {
+		field_set(COL_NETID);
+		out("%s", sock_name);
+		field_set(COL_STATE);
+		out("%s", sstate_name[s->state]);
 	}
 
-	out("%-6d %-6d %s", s->rq, s->wq, odd_width_pad);
+	field_set(COL_RECVQ);
+	out("%-6d", s->rq);
+	field_set(COL_SENDQ);
+	out("%-6d", s->wq);
+	field_set(COL_ADDR);
 }
 
 static void sock_details_print(struct sockstat *s)
@@ -897,21 +1037,17 @@ static void sock_details_print(struct sockstat *s)
 		out(" fwmark:0x%x", s->mark);
 }
 
-static void sock_addr_print_width(int addr_len, const char *addr, char *delim,
-		int port_len, const char *port, const char *ifname)
-{
-	if (ifname) {
-		out("%*s%%%s%s%-*s ", addr_len, addr, ifname, delim,
-		    port_len, port);
-	} else {
-		out("%*s%s%-*s ", addr_len, addr, delim, port_len, port);
-	}
-}
-
 static void sock_addr_print(const char *addr, char *delim, const char *port,
 		const char *ifname)
 {
-	sock_addr_print_width(addr_width, addr, delim, serv_width, port, ifname);
+	if (ifname)
+		out("%s" "%%" "%s%s", addr, ifname, delim);
+	else
+		out("%s%s", addr, delim);
+
+	field_next();
+	out("%s", port);
+	field_next();
 }
 
 static const char *print_ms_timer(unsigned int timeout)
@@ -1092,7 +1228,6 @@ static void inet_addr_print(const inet_prefix *a, int port,
 {
 	char buf[1024];
 	const char *ap = buf;
-	int est_len = addr_width;
 	const char *ifname = NULL;
 
 	if (a->family == AF_INET) {
@@ -1111,24 +1246,13 @@ static void inet_addr_print(const inet_prefix *a, int port,
 					 "[%s]", ap);
 				ap = buf;
 			}
-
-			est_len = strlen(ap);
-			if (est_len <= addr_width)
-				est_len = addr_width;
-			else
-				est_len = addr_width + ((est_len-addr_width+3)/4)*4;
 		}
 	}
 
-	if (ifindex) {
-		ifname   = ll_index_to_name(ifindex);
-		est_len -= strlen(ifname) + 1;  /* +1 for percent char */
-		if (est_len < 0)
-			est_len = 0;
-	}
+	if (ifindex)
+		ifname = ll_index_to_name(ifindex);
 
-	sock_addr_print_width(est_len, ap, ":", serv_width, resolve_service(port),
-			ifname);
+	sock_addr_print(ap, ":", resolve_service(port), ifname);
 }
 
 struct aafilter {
@@ -2163,7 +2287,6 @@ static int tcp_show_line(char *line, const struct filter *f, int family)
 	if (show_tcpinfo)
 		tcp_stats_print(&s);
 
-	out("\n");
 	return 0;
 }
 
@@ -2544,7 +2667,6 @@ static int inet_show_sock(struct nlmsghdr *nlh,
 	}
 	sctp_ino = s->ino;
 
-	out("\n");
 	return 0;
 }
 
@@ -3002,7 +3124,6 @@ static int dgram_show_line(char *line, const struct filter *f, int family)
 	if (show_details && opt[0])
 		out(" opt:\"%s\"", opt);
 
-	out("\n");
 	return 0;
 }
 
@@ -3181,7 +3302,6 @@ static int unix_show_sock(const struct sockaddr_nl *addr, struct nlmsghdr *nlh,
 			    mask & 1 ? '-' : '<', mask & 2 ? '-' : '>');
 		}
 	}
-	out("\n");
 
 	return 0;
 }
@@ -3338,8 +3458,6 @@ static int unix_show(struct filter *f)
 		if (++cnt > MAX_UNIX_REMEMBER) {
 			while (list) {
 				unix_stats_print(list, f);
-				out("\n");
-
 				unix_list_drop_first(&list);
 			}
 			cnt = 0;
@@ -3348,8 +3466,6 @@ static int unix_show(struct filter *f)
 	fclose(fp);
 	while (list) {
 		unix_stats_print(list, f);
-		out("\n");
-
 		unix_list_drop_first(&list);
 	}
 
@@ -3524,7 +3640,6 @@ static int packet_show_sock(const struct sockaddr_nl *addr,
 			fil++;
 		}
 	}
-	out("\n");
 	return 0;
 }
 
@@ -3567,7 +3682,6 @@ static int packet_show_line(char *buf, const struct filter *f, int fam)
 	if (packet_stats_print(&stat, f))
 		return 0;
 
-	out("\n");
 	return 0;
 }
 
@@ -3687,7 +3801,6 @@ static int netlink_show_one(struct filter *f,
 	if (show_details) {
 		out(" sk=%llx cb=%llx groups=0x%08x", sk, cb, groups);
 	}
-	out("\n");
 
 	return 0;
 }
@@ -3725,7 +3838,6 @@ static int netlink_show_sock(const struct sockaddr_nl *addr,
 	if (show_mem) {
 		out("\t");
 		print_skmeminfo(tb, NETLINK_DIAG_MEMINFO);
-		out("\n");
 	}
 
 	return 0;
@@ -4536,13 +4648,17 @@ int main(int argc, char *argv[])
 	if (ssfilter_parse(&current_filter.f, argc, argv, filter_fp))
 		usage();
 
-	netid_width = 0;
 	if (current_filter.dbs&(current_filter.dbs-1))
-		netid_width = 5;
+		columns[COL_NETID].width = 6;
 
-	state_width = 0;
 	if (current_filter.states&(current_filter.states-1))
-		state_width = 10;
+		columns[COL_STATE].width = 10;
+
+	/* If Netid or State are hidden, no delimiter before next column */
+	if (!columns[COL_NETID].width)
+		columns[COL_STATE].width--;
+	else if (!columns[COL_STATE].width)
+		columns[COL_RECVQ].width--;
 
 	if (isatty(STDOUT_FILENO)) {
 		struct winsize w;
@@ -4553,49 +4669,38 @@ int main(int argc, char *argv[])
 		}
 	}
 
-	addrp_width = screen_width;
-	if (netid_width)
-		addrp_width -= netid_width + 1;
-	if (state_width)
-		addrp_width -= state_width + 1;
-	addrp_width -= 14;
+	addrp_width = screen_width -
+		      columns[COL_NETID].width -
+		      columns[COL_STATE].width -
+		      columns[COL_RECVQ].width -
+		      columns[COL_SENDQ].width;
 
 	if (addrp_width&1) {
-		if (netid_width)
-			netid_width++;
-		else if (state_width)
-			state_width++;
+		if (columns[COL_NETID].width)
+			columns[COL_NETID].width++;
+		else if (columns[COL_STATE].width)
+			columns[COL_STATE].width++;
 		else
-			odd_width_pad = " ";
+			columns[COL_SENDQ].width++;
 	}
 
 	addrp_width /= 2;
-	addrp_width--;
-
-	serv_width = resolve_services ? 7 : 5;
 
-	if (addrp_width < 15+serv_width+1)
-		addrp_width = 15+serv_width+1;
+	columns[COL_SERV].width = resolve_services ? 8 : 6;
+	if (addrp_width < 15 + columns[COL_SERV].width)
+		addrp_width = 15 + columns[COL_SERV].width;
 
-	addr_width = addrp_width - serv_width - 1;
-
-	if (show_header) {
-		if (netid_width)
-			out("%-*s ", netid_width, "Netid");
-		if (state_width)
-			out("%-*s ", state_width, "State");
-		out("%-6s %-6s %s", "Recv-Q", "Send-Q", odd_width_pad);
-	}
+	columns[COL_ADDR].width = addrp_width - columns[COL_SERV].width;
 
 	/* Make enough space for the local/remote port field */
-	addr_width -= 13;
-	serv_width += 13;
+	columns[COL_ADDR].width -= 13;
+	columns[COL_SERV].width += 13;
 
-	if (show_header) {
-		out("%*s:%-*s %*s:%-*s\n",
-		    addr_width, "Local Address", serv_width, "Port",
-		    addr_width, "Peer Address", serv_width, "Port");
-	}
+	columns[COL_RADDR].width = columns[COL_ADDR].width;
+	columns[COL_RSERV].width = columns[COL_SERV].width;
+
+	if (show_header)
+		print_header();
 
 	fflush(stdout);
 
@@ -4624,5 +4729,7 @@ int main(int argc, char *argv[])
 	if (show_users || show_proc_ctx || show_sock_ctx)
 		user_ent_destroy();
 
+	field_next();
+
 	return 0;
 }
-- 
2.9.4

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

* [PATCH iproute2 net-next 3/4] ss: Buffer raw fields first, then render them as a table
  2017-12-08 17:07 [PATCH iproute2 net-next 0/4] Abstract columns, properly space and wrap fields Stefano Brivio
  2017-12-08 17:07 ` [PATCH iproute2 net-next 1/4] ss: Replace printf() calls for "main" output by calls to helper Stefano Brivio
  2017-12-08 17:07 ` [PATCH iproute2 net-next 2/4] ss: Introduce columns lightweight abstraction Stefano Brivio
@ 2017-12-08 17:07 ` Stefano Brivio
  2017-12-08 17:07 ` [PATCH iproute2 net-next 4/4] ss: Implement automatic column width calculation Stefano Brivio
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Stefano Brivio @ 2017-12-08 17:07 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, Sabrina Dubroca

This allows us to measure the maximum field length for each
column before printing fields and will permit us to apply
optimal field spacing and distribution. Structure of the output
buffer with chunked allocation is described in comments.

Output is still unchanged, original spacing is used.

Running over one million sockets with -tul options by simply
modifying main() to loop 50,000 times over the *_show()
functions, buffering the whole output and rendering it at the
end, with 10 UDP sockets, 10 TCP sockets, while throwing
output away, doesn't show significant changes in execution time
on my laptop with an Intel i7-6600U CPU:

- before this patch:
$ time ./ss -tul > /dev/null
real	0m29.899s
user	0m2.017s
sys	0m27.801s

- after this patch:
$ time ./ss -tul > /dev/null
real	0m29.827s
user	0m1.942s
sys	0m27.812s

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>
---
 misc/ss.c | 271 +++++++++++++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 225 insertions(+), 46 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index 9dbcfd514d48..abc0da7fa8fe 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -47,6 +47,8 @@
 #include <linux/vm_sockets_diag.h>
 
 #define MAGIC_SEQ 123456
+#define BUF_CHUNK (1024 * 1024)
+#define LEN_ALIGN(x) (((x) + 1) & ~1)
 
 #define DIAG_REQUEST(_req, _r)						    \
 	struct {							    \
@@ -127,24 +129,45 @@ struct column {
 	const char *header;
 	const char *ldelim;
 	int width;	/* Including delimiter. -1: fit to content, 0: hide */
-	int stored;	/* Characters buffered */
-	int printed;	/* Characters printed so far */
 };
 
 static struct column columns[] = {
-	{ ALIGN_LEFT,	"Netid",		"",	0,	0,	0 },
-	{ ALIGN_LEFT,	"State",		" ",	0,	0,	0 },
-	{ ALIGN_LEFT,	"Recv-Q",		" ",	7,	0,	0 },
-	{ ALIGN_LEFT,	"Send-Q",		" ",	7,	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,	"",			"",	-1,	0,	0 },
+	{ ALIGN_LEFT,	"Netid",		"",	0 },
+	{ ALIGN_LEFT,	"State",		" ",	0 },
+	{ ALIGN_LEFT,	"Recv-Q",		" ",	7 },
+	{ ALIGN_LEFT,	"Send-Q",		" ",	7 },
+	{ ALIGN_RIGHT,	"Local Address:",	" ",	0 },
+	{ ALIGN_LEFT,	"Port",			"",	0 },
+	{ ALIGN_RIGHT,	"Peer Address:",	" ",	0 },
+	{ ALIGN_LEFT,	"Port",			"",	0 },
+	{ ALIGN_LEFT,	"",			"",	-1 },
 };
 
 static struct column *current_field = columns;
-static char field_buf[BUFSIZ];
+
+/* Output buffer: chained chunks of BUF_CHUNK bytes. Each field is written to
+ * the buffer as a variable size token. A token consists of a 16 bits length
+ * field, followed by a string which is not NULL-terminated.
+ *
+ * A new chunk is allocated and linked when the current chunk doesn't have
+ * enough room to store the current token as a whole.
+ */
+struct buf_chunk {
+	struct buf_chunk *next;	/* Next chained chunk */
+	char *end;		/* Current end of content */
+	char data[0];
+};
+
+struct buf_token {
+	uint16_t len;		/* Data length, excluding length descriptor */
+	char data[0];
+};
+
+static struct {
+	struct buf_token *cur;	/* Position of current token in chunk */
+	struct buf_chunk *head;	/* First chunk */
+	struct buf_chunk *tail;	/* Current chunk */
+} buffer;
 
 static const char *TCP_PROTO = "tcp";
 static const char *SCTP_PROTO = "sctp";
@@ -860,25 +883,109 @@ static const char *vsock_netid_name(int type)
 	}
 }
 
+/* Allocate and initialize a new buffer chunk */
+static struct buf_chunk *buf_chunk_new(void)
+{
+	struct buf_chunk *new = malloc(BUF_CHUNK);
+
+	if (!new)
+		abort();
+
+	new->next = NULL;
+
+	/* This is also the last block */
+	buffer.tail = new;
+
+	/* Next token will be stored at the beginning of chunk data area, and
+	 * its initial length is zero.
+	 */
+	buffer.cur = (struct buf_token *)new->data;
+	buffer.cur->len = 0;
+
+	new->end = buffer.cur->data;
+
+	return new;
+}
+
+/* Return available tail room in given chunk */
+static int buf_chunk_avail(struct buf_chunk *chunk)
+{
+	return BUF_CHUNK - offsetof(struct buf_chunk, data) -
+	       (chunk->end - chunk->data);
+}
+
+/* Update end pointer and token length, link new chunk if we hit the end of the
+ * current one. Return -EAGAIN if we got a new chunk, caller has to print again.
+ */
+static int buf_update(int len)
+{
+	struct buf_chunk *chunk = buffer.tail;
+	struct buf_token *t = buffer.cur;
+
+	/* Claim success if new content fits in the current chunk, and anyway
+	 * if this is the first token in the chunk: in the latter case,
+	 * allocating a new chunk won't help, so we'll just cut the output.
+	 */
+	if ((len < buf_chunk_avail(chunk) && len != -1 /* glibc < 2.0.6 */) ||
+	    t == (struct buf_token *)chunk->data) {
+		len = min(len, buf_chunk_avail(chunk));
+
+		/* Total field length can't exceed 2^16 bytes, cut as needed */
+		len = min(len, USHRT_MAX - t->len);
+
+		chunk->end += len;
+		t->len += len;
+		return 0;
+	}
+
+	/* Content truncated, time to allocate more */
+	chunk->next = buf_chunk_new();
+
+	/* Copy current token over to new chunk, including length descriptor */
+	memcpy(chunk->next->data, t, sizeof(t->len) + t->len);
+	chunk->next->end += t->len;
+
+	/* Discard partially written field in old chunk */
+	chunk->end -= t->len + sizeof(t->len);
+
+	return -EAGAIN;
+}
+
+/* Append content to buffer as part of the current field */
 static void out(const char *fmt, ...)
 {
 	struct column *f = current_field;
 	va_list args;
+	char *pos;
+	int len;
+
+	if (!f->width)
+		return;
+
+	if (!buffer.head)
+		buffer.head = buf_chunk_new();
+
+again:	/* Append to buffer: if we have a new chunk, print again */
 
+	pos = buffer.cur->data + buffer.cur->len;
 	va_start(args, fmt);
-	f->stored += vsnprintf(field_buf + f->stored, BUFSIZ - f->stored,
-			       fmt, args);
+
+	/* Limit to tail room. If we hit the limit, buf_update() will tell us */
+	len = vsnprintf(pos, buf_chunk_avail(buffer.tail), fmt, args);
 	va_end(args);
+
+	if (buf_update(len))
+		goto again;
 }
 
-static int print_left_spacing(struct column *f)
+static int print_left_spacing(struct column *f, int stored, int printed)
 {
 	int s;
 
 	if (f->width < 0 || f->align == ALIGN_LEFT)
 		return 0;
 
-	s = f->width - f->stored - f->printed;
+	s = f->width - stored - printed;
 	if (f->align == ALIGN_CENTER)
 		/* If count of total spacing is odd, shift right by one */
 		s = (s + 1) / 2;
@@ -889,14 +996,14 @@ static int print_left_spacing(struct column *f)
 	return 0;
 }
 
-static void print_right_spacing(struct column *f)
+static void print_right_spacing(struct column *f, int printed)
 {
 	int s;
 
 	if (f->width < 0 || f->align == ALIGN_RIGHT)
 		return;
 
-	s = f->width - f->printed;
+	s = f->width - printed;
 	if (f->align == ALIGN_CENTER)
 		s /= 2;
 
@@ -904,35 +1011,29 @@ static void print_right_spacing(struct column *f)
 		printf("%*c", s, ' ');
 }
 
-static int field_needs_delimiter(struct column *f)
-{
-	if (!f->stored)
-		return 0;
-
-	/* Was another field already printed on this line? */
-	for (f--; f >= columns; f--)
-		if (f->width)
-			return 1;
-
-	return 0;
-}
-
-/* Flush given field to screen together with delimiter and spacing */
+/* Done with field: update buffer pointer, start new token after current one */
 static void field_flush(struct column *f)
 {
+	struct buf_chunk *chunk = buffer.tail;
+	unsigned int pad = buffer.cur->len % 2;
+
 	if (!f->width)
 		return;
 
-	if (field_needs_delimiter(f))
-		f->printed = printf("%s", f->ldelim);
-
-	f->printed += print_left_spacing(f);
-	f->printed += printf("%s", field_buf);
-	print_right_spacing(f);
+	/* We need a new chunk if we can't store the next length descriptor.
+	 * Mind the gap between end of previous token and next aligned position
+	 * for length descriptor.
+	 */
+	if (buf_chunk_avail(chunk) - pad < sizeof(buffer.cur->len)) {
+		chunk->end += pad;
+		chunk->next = buf_chunk_new();
+		return;
+	}
 
-	*field_buf = 0;
-	f->printed = 0;
-	f->stored = 0;
+	buffer.cur = (struct buf_token *)(buffer.cur->data +
+					  LEN_ALIGN(buffer.cur->len));
+	buffer.cur->len = 0;
+	buffer.tail->end = buffer.cur->data;
 }
 
 static int field_is_last(struct column *f)
@@ -944,12 +1045,10 @@ static void field_next(void)
 {
 	field_flush(current_field);
 
-	if (field_is_last(current_field)) {
-		printf("\n");
+	if (field_is_last(current_field))
 		current_field = columns;
-	} else {
+	else
 		current_field++;
-	}
 }
 
 /* Walk through fields and flush them until we reach the desired one */
@@ -969,6 +1068,86 @@ static void print_header(void)
 	}
 }
 
+/* Get the next available token in the buffer starting from the current token */
+static struct buf_token *buf_token_next(struct buf_token *cur)
+{
+	struct buf_chunk *chunk = buffer.tail;
+
+	/* If we reached the end of chunk contents, get token from next chunk */
+	if (cur->data + LEN_ALIGN(cur->len) == chunk->end) {
+		buffer.tail = chunk = chunk->next;
+		return chunk ? (struct buf_token *)chunk->data : NULL;
+	}
+
+	return (struct buf_token *)(cur->data + LEN_ALIGN(cur->len));
+}
+
+/* Free up all allocated buffer chunks */
+static void buf_free_all(void)
+{
+	struct buf_chunk *tmp;
+
+	for (buffer.tail = buffer.head; buffer.tail; ) {
+		tmp = buffer.tail;
+		buffer.tail = buffer.tail->next;
+		free(tmp);
+	}
+	buffer.head = NULL;
+}
+
+/* Render buffered output with spacing and delimiters, then free up buffers */
+static void render(void)
+{
+	struct buf_token *token = (struct buf_token *)buffer.head->data;
+	int printed, line_started = 0, need_newline = 0;
+	struct column *f;
+
+	/* Ensure end alignment of last token, it wasn't necessarily flushed */
+	buffer.tail->end += buffer.cur->len % 2;
+
+	/* Rewind and replay */
+	buffer.tail = buffer.head;
+
+	f = columns;
+	while (!f->width)
+		f++;
+
+	while (token) {
+		/* Print left delimiter only if we already started a line */
+		if (line_started++)
+			printed = printf("%s", current_field->ldelim);
+		else
+			printed = 0;
+
+		/* Print field content from token data with spacing */
+		printed += print_left_spacing(f, token->len, printed);
+		printed += fwrite(token->data, 1, token->len, stdout);
+		print_right_spacing(f, printed);
+
+		/* Variable field size or overflow, won't align to screen */
+		if (printed > f->width)
+			need_newline = 1;
+
+		/* Go to next non-empty field, deal with end-of-line */
+		do {
+			if (field_is_last(f)) {
+				if (need_newline) {
+					printf("\n");
+					need_newline = 0;
+				}
+				f = columns;
+				line_started = 0;
+			} else {
+				f++;
+			}
+		} while (!f->width);
+
+		token = buf_token_next(token);
+	}
+
+	buf_free_all();
+}
+
 static void sock_state_print(struct sockstat *s)
 {
 	const char *sock_name;
@@ -4729,7 +4908,7 @@ int main(int argc, char *argv[])
 	if (show_users || show_proc_ctx || show_sock_ctx)
 		user_ent_destroy();
 
-	field_next();
+	render();
 
 	return 0;
 }
-- 
2.9.4

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

* [PATCH iproute2 net-next 4/4] ss: Implement automatic column width calculation
  2017-12-08 17:07 [PATCH iproute2 net-next 0/4] Abstract columns, properly space and wrap fields Stefano Brivio
                   ` (2 preceding siblings ...)
  2017-12-08 17:07 ` [PATCH iproute2 net-next 3/4] ss: Buffer raw fields first, then render them as a table Stefano Brivio
@ 2017-12-08 17:07 ` Stefano Brivio
  2017-12-08 18:29 ` [PATCH iproute2 net-next 0/4] Abstract columns, properly space and wrap fields Stephen Hemminger
  2017-12-12  0:07 ` Stephen Hemminger
  5 siblings, 0 replies; 8+ messages in thread
From: Stefano Brivio @ 2017-12-08 17:07 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, Sabrina Dubroca

Group fitting fields into lines and space them equally using the
remaining screen width for each line. If columns don't fit on
one line, break them into the least possible amount of lines and
keep them aligned across lines.

This is done by:
 - recording the length of the longest item in each column during
   formatting and buffering (which was added in the previous patch)
 - fitting as many fields as possible on each line of output
 - distributing the remaining padding space equally between the
   columns

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>
---
 misc/ss.c | 188 +++++++++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 120 insertions(+), 68 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index abc0da7fa8fe..44ad9587b62c 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -128,19 +128,21 @@ struct column {
 	const enum col_align align;
 	const char *header;
 	const char *ldelim;
-	int width;	/* Including delimiter. -1: fit to content, 0: hide */
+	int disabled;
+	int width;	/* Calculated, including additional layout spacing */
+	int max_len;	/* Measured maximum field length in this column */
 };
 
 static struct column columns[] = {
-	{ ALIGN_LEFT,	"Netid",		"",	0 },
-	{ ALIGN_LEFT,	"State",		" ",	0 },
-	{ ALIGN_LEFT,	"Recv-Q",		" ",	7 },
-	{ ALIGN_LEFT,	"Send-Q",		" ",	7 },
-	{ ALIGN_RIGHT,	"Local Address:",	" ",	0 },
-	{ ALIGN_LEFT,	"Port",			"",	0 },
-	{ ALIGN_RIGHT,	"Peer Address:",	" ",	0 },
-	{ ALIGN_LEFT,	"Port",			"",	0 },
-	{ ALIGN_LEFT,	"",			"",	-1 },
+	{ 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 },
 };
 
 static struct column *current_field = columns;
@@ -959,7 +961,7 @@ static void out(const char *fmt, ...)
 	char *pos;
 	int len;
 
-	if (!f->width)
+	if (f->disabled)
 		return;
 
 	if (!buffer.head)
@@ -982,7 +984,7 @@ static int print_left_spacing(struct column *f, int stored, int printed)
 {
 	int s;
 
-	if (f->width < 0 || f->align == ALIGN_LEFT)
+	if (!f->width || f->align == ALIGN_LEFT)
 		return 0;
 
 	s = f->width - stored - printed;
@@ -1000,7 +1002,7 @@ static void print_right_spacing(struct column *f, int printed)
 {
 	int s;
 
-	if (f->width < 0 || f->align == ALIGN_RIGHT)
+	if (!f->width || f->align == ALIGN_RIGHT)
 		return;
 
 	s = f->width - printed;
@@ -1017,9 +1019,12 @@ static void field_flush(struct column *f)
 	struct buf_chunk *chunk = buffer.tail;
 	unsigned int pad = buffer.cur->len % 2;
 
-	if (!f->width)
+	if (f->disabled)
 		return;
 
+	if (buffer.cur->len > f->max_len)
+		f->max_len = buffer.cur->len;
+
 	/* We need a new chunk if we can't store the next length descriptor.
 	 * Mind the gap between end of previous token and next aligned position
 	 * for length descriptor.
@@ -1062,7 +1067,7 @@ static void field_set(enum col_id id)
 static void print_header(void)
 {
 	while (!field_is_last(current_field)) {
-		if (current_field->width)
+		if (!current_field->disabled)
 			out(current_field->header);
 		field_next();
 	}
@@ -1095,16 +1100,106 @@ static void buf_free_all(void)
 	buffer.head = NULL;
 }
 
+/* Calculate column width from contents length. If columns don't fit on one
+ * line, break them into the least possible amount of lines and keep them
+ * aligned across lines. Available screen space is equally spread between fields
+ * as additional spacing.
+ */
+static void render_calc_width(int screen_width)
+{
+	int first, len = 0, linecols = 0;
+	struct column *c, *eol = columns - 1;
+
+	/* First pass: set width for each column to measured content length */
+	for (first = 1, c = columns; c - columns < COL_MAX; c++) {
+		if (c->disabled)
+			continue;
+
+		if (!first && c->max_len)
+			c->width = c->max_len + strlen(c->ldelim);
+		else
+			c->width = c->max_len;
+
+		/* But don't exceed screen size. If we exceed the screen size
+		 * for even a single field, it will just start on a line of its
+		 * own and then naturally wrap.
+		 */
+		c->width = min(c->width, screen_width);
+
+		if (c->width)
+			first = 0;
+	}
+
+	/* Second pass: find out newlines and distribute available spacing */
+	for (c = columns; c - columns < COL_MAX; c++) {
+		int pad, spacing, rem, last;
+		struct column *tmp;
+
+		if (!c->width)
+			continue;
+
+		linecols++;
+		len += c->width;
+
+		for (last = 1, tmp = c + 1; tmp - columns < COL_MAX; tmp++) {
+			if (tmp->width) {
+				last = 0;
+				break;
+			}
+		}
+
+		if (!last && len < screen_width) {
+			/* Columns fit on screen so far, nothing to do yet */
+			continue;
+		}
+
+		if (len == screen_width) {
+			/* Exact fit, just start with new line */
+			goto newline;
+		}
+
+		if (len > screen_width) {
+			/* Screen width exceeded: go back one column */
+			len -= c->width;
+			c--;
+			linecols--;
+		}
+
+		/* Distribute remaining space to columns on this line */
+		pad = screen_width - len;
+		spacing = pad / linecols;
+		rem = pad % linecols;
+		for (tmp = c; tmp > eol; tmp--) {
+			if (!tmp->width)
+				continue;
+
+			tmp->width += spacing;
+			if (rem) {
+				tmp->width++;
+				rem--;
+			}
+		}
+
+newline:
+		/* Line break: reset line counters, mark end-of-line */
+		eol = c;
+		len = 0;
+		linecols = 0;
+	}
+}
+
 /* Render buffered output with spacing and delimiters, then free up buffers */
-static void render(void)
+static void render(int screen_width)
 {
 	struct buf_token *token = (struct buf_token *)buffer.head->data;
-	int printed, line_started = 0, need_newline = 0;
+	int printed, line_started = 0;
 	struct column *f;
 
 	/* Ensure end alignment of last token, it wasn't necessarily flushed */
 	buffer.tail->end += buffer.cur->len % 2;
 
+	render_calc_width(screen_width);
+
 	/* Rewind and replay */
 	buffer.tail = buffer.head;
 
@@ -1124,23 +1219,16 @@ static void render(void)
 		printed += fwrite(token->data, 1, token->len, stdout);
 		print_right_spacing(f, printed);
 
-		/* Variable field size or overflow, won't align to screen */
-		if (printed > f->width)
-			need_newline = 1;
-
 		/* Go to next non-empty field, deal with end-of-line */
 		do {
 			if (field_is_last(f)) {
-				if (need_newline) {
-					printf("\n");
-					need_newline = 0;
-				}
+				printf("\n");
 				f = columns;
 				line_started = 0;
 			} else {
 				f++;
 			}
-		} while (!f->width);
+		} while (f->disabled);
 
 		token = buf_token_next(token);
 	}
@@ -4531,7 +4619,7 @@ int main(int argc, char *argv[])
 	FILE *filter_fp = NULL;
 	int ch;
 	int state_filter = 0;
-	int addrp_width, screen_width = 80;
+	int screen_width = 80;
 
 	while ((ch = getopt_long(argc, argv,
 				 "dhaletuwxnro460spbEf:miA:D:F:vVzZN:KHS",
@@ -4827,17 +4915,11 @@ int main(int argc, char *argv[])
 	if (ssfilter_parse(&current_filter.f, argc, argv, filter_fp))
 		usage();
 
-	if (current_filter.dbs&(current_filter.dbs-1))
-		columns[COL_NETID].width = 6;
-
-	if (current_filter.states&(current_filter.states-1))
-		columns[COL_STATE].width = 10;
+	if (!(current_filter.dbs & (current_filter.dbs - 1)))
+		columns[COL_NETID].disabled = 1;
 
-	/* If Netid or State are hidden, no delimiter before next column */
-	if (!columns[COL_NETID].width)
-		columns[COL_STATE].width--;
-	else if (!columns[COL_STATE].width)
-		columns[COL_RECVQ].width--;
+	if (!(current_filter.states & (current_filter.states - 1)))
+		columns[COL_STATE].disabled = 1;
 
 	if (isatty(STDOUT_FILENO)) {
 		struct winsize w;
@@ -4848,36 +4930,6 @@ int main(int argc, char *argv[])
 		}
 	}
 
-	addrp_width = screen_width -
-		      columns[COL_NETID].width -
-		      columns[COL_STATE].width -
-		      columns[COL_RECVQ].width -
-		      columns[COL_SENDQ].width;
-
-	if (addrp_width&1) {
-		if (columns[COL_NETID].width)
-			columns[COL_NETID].width++;
-		else if (columns[COL_STATE].width)
-			columns[COL_STATE].width++;
-		else
-			columns[COL_SENDQ].width++;
-	}
-
-	addrp_width /= 2;
-
-	columns[COL_SERV].width = resolve_services ? 8 : 6;
-	if (addrp_width < 15 + columns[COL_SERV].width)
-		addrp_width = 15 + columns[COL_SERV].width;
-
-	columns[COL_ADDR].width = addrp_width - columns[COL_SERV].width;
-
-	/* Make enough space for the local/remote port field */
-	columns[COL_ADDR].width -= 13;
-	columns[COL_SERV].width += 13;
-
-	columns[COL_RADDR].width = columns[COL_ADDR].width;
-	columns[COL_RSERV].width = columns[COL_SERV].width;
-
 	if (show_header)
 		print_header();
 
@@ -4908,7 +4960,7 @@ int main(int argc, char *argv[])
 	if (show_users || show_proc_ctx || show_sock_ctx)
 		user_ent_destroy();
 
-	render();
+	render(screen_width);
 
 	return 0;
 }
-- 
2.9.4

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

* Re: [PATCH iproute2 net-next 0/4] Abstract columns, properly space and wrap fields
  2017-12-08 17:07 [PATCH iproute2 net-next 0/4] Abstract columns, properly space and wrap fields Stefano Brivio
                   ` (3 preceding siblings ...)
  2017-12-08 17:07 ` [PATCH iproute2 net-next 4/4] ss: Implement automatic column width calculation Stefano Brivio
@ 2017-12-08 18:29 ` Stephen Hemminger
  2017-12-09 16:36   ` David Ahern
  2017-12-12  0:07 ` Stephen Hemminger
  5 siblings, 1 reply; 8+ messages in thread
From: Stephen Hemminger @ 2017-12-08 18:29 UTC (permalink / raw)
  To: Stefano Brivio; +Cc: netdev, Sabrina Dubroca

On Fri,  8 Dec 2017 18:07:19 +0100
Stefano Brivio <sbrivio@redhat.com> wrote:

> Currently, 'ss' simply subdivides the whole available screen width
> between available columns, starting from a set of hardcoded amount
> of spacing and growing column widths.
> 
> This makes the output unreadable in several cases, as it doesn't take
> into account the actual content width.
> 
> Fix this by introducing a simple abstraction for columns, buffering
> the output, measuring the width of the fields, grouping fields into
> lines as they fit, equally distributing any remaining whitespace, and
> finally rendering the result. Some examples are reported below [1].
> 
> This implementation doesn't seem to cause any significant performance
> issues, as reported in 3/4.
> 
> Patch 1/4 replaces all relevant printf() calls by the out() helper,
> which simply consists of the usual printf() implementation.
> 
> Patch 2/4 implements column abstraction, with configurable column
> width and delimiters, and 3/4 splits buffering and rendering phases,
> employing a simple buffering mechanism with chunked allocation and
> introducing a rendering function.
> 
> Up to this point, the output is still unchanged.
> 
> Finally, 4/4 introduces field width calculation based on content
> length measured while buffering, in order to split fields onto
> multiple lines and equally space them within the single lines.
> 
> Now that column behaviour is well-defined and more easily
> configurable, it should be easier to further improve the output by
> splitting logically separable information (e.g. TCP details) into
> additional columns. However, this patchset keeps the full "extended"
> information into a single column, for the moment being.
> 
> 
> [1]
> 
> - 80 columns terminal, ss -Z -f netlink
>   * before:
> Recv-Q Send-Q Local Address:Port                 Peer Address:Port
> 
> 0      0            rtnl:evolution-calen/2075           *                     pr
> oc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
> 0      0            rtnl:abrt-applet/32700              *                     pr
> oc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
> 0      0            rtnl:firefox/21619                  *                     pr
> oc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
> 0      0            rtnl:evolution-calen/32639           *                     p
> roc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
> [...]
> 
>   * after:
> Recv-Q   Send-Q     Local Address:Port                      Peer Address:Port
> 0        0                   rtnl:evolution-calen/2075                  *
>  proc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
> 0        0                   rtnl:abrt-applet/32700                     *
>  proc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
> 0        0                   rtnl:firefox/21619                         *
>  proc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
> 0        0                   rtnl:evolution-calen/32639                 *
>  proc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
> [...]
> 
> - 80 colums terminal, ss -tunpl
>   * before:
> Netid  State      Recv-Q Send-Q Local Address:Port               Peer Address:Port
> udp    UNCONN     0      0         *:37732                 *:*
> udp    UNCONN     0      0         *:5353                  *:*
> udp    UNCONN     0      0      192.168.122.1:53                    *:*
> udp    UNCONN     0      0      *%virbr0:67                    *:*
> [...]
> 
>   * after:
> Netid   State    Recv-Q   Send-Q     Local Address:Port      Peer Address:Port
> udp     UNCONN   0        0                      *:37732                *:*
> udp     UNCONN   0        0                      *:5353                 *:*
> udp     UNCONN   0        0          192.168.122.1:53                   *:*
> udp     UNCONN   0        0               *%virbr0:67                   *:*
> [...]
> 
>  - 66 columns terminal, ss -tunpl
>   * before:
> Netid  State      Recv-Q Send-Q Local Address:Port               P
> eer Address:Port
> udp    UNCONN     0      0       *:37732               *:*
> 
> udp    UNCONN     0      0       *:5353                *:*
> 
> udp    UNCONN     0      0      192.168.122.1:53
> *:*
> udp    UNCONN     0      0      *%virbr0:67                  *:*
> [...]
> 
>   * after:
> Netid State  Recv-Q Send-Q Local Address:Port   Peer Address:Port
> udp   UNCONN 0      0                  *:37732             *:*
> udp   UNCONN 0      0                  *:5353              *:*
> udp   UNCONN 0      0      192.168.122.1:53                *:*
> udp   UNCONN 0      0           *%virbr0:67                *:*
> [...]
> 
> 
> Stefano Brivio (4):
>   ss: Replace printf() calls for "main" output by calls to helper
>   ss: Introduce columns lightweight abstraction
>   ss: Buffer raw fields first, then render them as a table
>   ss: Implement automatic column width calculation
> 
>  misc/ss.c | 893 +++++++++++++++++++++++++++++++++++++++++++-------------------
>  1 file changed, 621 insertions(+), 272 deletions(-)
> 


This looks good, would like some acknowledgment from heavy users such as Google
that this works for them.

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

* Re: [PATCH iproute2 net-next 0/4] Abstract columns, properly space and wrap fields
  2017-12-08 18:29 ` [PATCH iproute2 net-next 0/4] Abstract columns, properly space and wrap fields Stephen Hemminger
@ 2017-12-09 16:36   ` David Ahern
  0 siblings, 0 replies; 8+ messages in thread
From: David Ahern @ 2017-12-09 16:36 UTC (permalink / raw)
  To: Stephen Hemminger, Stefano Brivio; +Cc: netdev, Sabrina Dubroca

On 12/8/17 11:29 AM, Stephen Hemminger wrote:
>> - 80 columns terminal, ss -Z -f netlink
>>   * before:
>> Recv-Q Send-Q Local Address:Port                 Peer Address:Port
>>
>> 0      0            rtnl:evolution-calen/2075           *                     pr
>> oc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
>> 0      0            rtnl:abrt-applet/32700              *                     pr
>> oc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
>> 0      0            rtnl:firefox/21619                  *                     pr
>> oc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
>> 0      0            rtnl:evolution-calen/32639           *                     p
>> roc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
>> [...]
>>
>>   * after:
>> Recv-Q   Send-Q     Local Address:Port                      Peer Address:Port
>> 0        0                   rtnl:evolution-calen/2075                  *
>>  proc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
>> 0        0                   rtnl:abrt-applet/32700                     *
>>  proc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
>> 0        0                   rtnl:firefox/21619                         *
>>  proc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
>> 0        0                   rtnl:evolution-calen/32639                 *
>>  proc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
>> [...]
>>
>> - 80 colums terminal, ss -tunpl
>>   * before:
>> Netid  State      Recv-Q Send-Q Local Address:Port               Peer Address:Port
>> udp    UNCONN     0      0         *:37732                 *:*
>> udp    UNCONN     0      0         *:5353                  *:*
>> udp    UNCONN     0      0      192.168.122.1:53                    *:*
>> udp    UNCONN     0      0      *%virbr0:67                    *:*
>> [...]
>>
>>   * after:
>> Netid   State    Recv-Q   Send-Q     Local Address:Port      Peer Address:Port
>> udp     UNCONN   0        0                      *:37732                *:*
>> udp     UNCONN   0        0                      *:5353                 *:*
>> udp     UNCONN   0        0          192.168.122.1:53                   *:*
>> udp     UNCONN   0        0               *%virbr0:67                   *:*
>> [...]
>>
>>  - 66 columns terminal, ss -tunpl
>>   * before:
>> Netid  State      Recv-Q Send-Q Local Address:Port               P
>> eer Address:Port
>> udp    UNCONN     0      0       *:37732               *:*
>>
>> udp    UNCONN     0      0       *:5353                *:*
>>
>> udp    UNCONN     0      0      192.168.122.1:53
>> *:*
>> udp    UNCONN     0      0      *%virbr0:67                  *:*
>> [...]
>>
>>   * after:
>> Netid State  Recv-Q Send-Q Local Address:Port   Peer Address:Port
>> udp   UNCONN 0      0                  *:37732             *:*
>> udp   UNCONN 0      0                  *:5353              *:*
>> udp   UNCONN 0      0      192.168.122.1:53                *:*
>> udp   UNCONN 0      0           *%virbr0:67                *:*
>> [...]
>>


> 
> 
> This looks good, would like some acknowledgment from heavy users such as Google
> that this works for them.
> 

I'm not a Google'r but 'ss' output has annoyed me for some time, and it
has been on my to-do list to fix it. Thanks for doing that Stefano.

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

* Re: [PATCH iproute2 net-next 0/4] Abstract columns, properly space and wrap fields
  2017-12-08 17:07 [PATCH iproute2 net-next 0/4] Abstract columns, properly space and wrap fields Stefano Brivio
                   ` (4 preceding siblings ...)
  2017-12-08 18:29 ` [PATCH iproute2 net-next 0/4] Abstract columns, properly space and wrap fields Stephen Hemminger
@ 2017-12-12  0:07 ` Stephen Hemminger
  5 siblings, 0 replies; 8+ messages in thread
From: Stephen Hemminger @ 2017-12-12  0:07 UTC (permalink / raw)
  To: Stefano Brivio; +Cc: netdev, Sabrina Dubroca, David Ahern

On Fri,  8 Dec 2017 18:07:19 +0100
Stefano Brivio <sbrivio@redhat.com> wrote:

> Currently, 'ss' simply subdivides the whole available screen width
> between available columns, starting from a set of hardcoded amount
> of spacing and growing column widths.
> 
> This makes the output unreadable in several cases, as it doesn't take
> into account the actual content width.
> 
> Fix this by introducing a simple abstraction for columns, buffering
> the output, measuring the width of the fields, grouping fields into
> lines as they fit, equally distributing any remaining whitespace, and
> finally rendering the result. Some examples are reported below [1].
> 
> This implementation doesn't seem to cause any significant performance
> issues, as reported in 3/4.
> 
> Patch 1/4 replaces all relevant printf() calls by the out() helper,
> which simply consists of the usual printf() implementation.
> 
> Patch 2/4 implements column abstraction, with configurable column
> width and delimiters, and 3/4 splits buffering and rendering phases,
> employing a simple buffering mechanism with chunked allocation and
> introducing a rendering function.
> 
> Up to this point, the output is still unchanged.
> 
> Finally, 4/4 introduces field width calculation based on content
> length measured while buffering, in order to split fields onto
> multiple lines and equally space them within the single lines.
> 
> Now that column behaviour is well-defined and more easily
> configurable, it should be easier to further improve the output by
> splitting logically separable information (e.g. TCP details) into
> additional columns. However, this patchset keeps the full "extended"
> information into a single column, for the moment being.
> 
> 
> [1]
> 
> - 80 columns terminal, ss -Z -f netlink
>   * before:
> Recv-Q Send-Q Local Address:Port                 Peer Address:Port
> 
> 0      0            rtnl:evolution-calen/2075           *                     pr
> oc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
> 0      0            rtnl:abrt-applet/32700              *                     pr
> oc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
> 0      0            rtnl:firefox/21619                  *                     pr
> oc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
> 0      0            rtnl:evolution-calen/32639           *                     p
> roc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
> [...]
> 
>   * after:
> Recv-Q   Send-Q     Local Address:Port                      Peer Address:Port
> 0        0                   rtnl:evolution-calen/2075                  *
>  proc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
> 0        0                   rtnl:abrt-applet/32700                     *
>  proc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
> 0        0                   rtnl:firefox/21619                         *
>  proc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
> 0        0                   rtnl:evolution-calen/32639                 *
>  proc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
> [...]
> 
> - 80 colums terminal, ss -tunpl
>   * before:
> Netid  State      Recv-Q Send-Q Local Address:Port               Peer Address:Port
> udp    UNCONN     0      0         *:37732                 *:*
> udp    UNCONN     0      0         *:5353                  *:*
> udp    UNCONN     0      0      192.168.122.1:53                    *:*
> udp    UNCONN     0      0      *%virbr0:67                    *:*
> [...]
> 
>   * after:
> Netid   State    Recv-Q   Send-Q     Local Address:Port      Peer Address:Port
> udp     UNCONN   0        0                      *:37732                *:*
> udp     UNCONN   0        0                      *:5353                 *:*
> udp     UNCONN   0        0          192.168.122.1:53                   *:*
> udp     UNCONN   0        0               *%virbr0:67                   *:*
> [...]
> 
>  - 66 columns terminal, ss -tunpl
>   * before:
> Netid  State      Recv-Q Send-Q Local Address:Port               P
> eer Address:Port
> udp    UNCONN     0      0       *:37732               *:*
> 
> udp    UNCONN     0      0       *:5353                *:*
> 
> udp    UNCONN     0      0      192.168.122.1:53
> *:*
> udp    UNCONN     0      0      *%virbr0:67                  *:*
> [...]
> 
>   * after:
> Netid State  Recv-Q Send-Q Local Address:Port   Peer Address:Port
> udp   UNCONN 0      0                  *:37732             *:*
> udp   UNCONN 0      0                  *:5353              *:*
> udp   UNCONN 0      0      192.168.122.1:53                *:*
> udp   UNCONN 0      0           *%virbr0:67                *:*
> [...]
> 
> 
> Stefano Brivio (4):
>   ss: Replace printf() calls for "main" output by calls to helper
>   ss: Introduce columns lightweight abstraction
>   ss: Buffer raw fields first, then render them as a table
>   ss: Implement automatic column width calculation

I was going to apply this but it does not apply cleanly to current iproute2 net-next
tree. Please rebase and resubmit.

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

end of thread, other threads:[~2017-12-12  0:07 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-08 17:07 [PATCH iproute2 net-next 0/4] Abstract columns, properly space and wrap fields Stefano Brivio
2017-12-08 17:07 ` [PATCH iproute2 net-next 1/4] ss: Replace printf() calls for "main" output by calls to helper Stefano Brivio
2017-12-08 17:07 ` [PATCH iproute2 net-next 2/4] ss: Introduce columns lightweight abstraction Stefano Brivio
2017-12-08 17:07 ` [PATCH iproute2 net-next 3/4] ss: Buffer raw fields first, then render them as a table Stefano Brivio
2017-12-08 17:07 ` [PATCH iproute2 net-next 4/4] ss: Implement automatic column width calculation Stefano Brivio
2017-12-08 18:29 ` [PATCH iproute2 net-next 0/4] Abstract columns, properly space and wrap fields Stephen Hemminger
2017-12-09 16:36   ` David Ahern
2017-12-12  0:07 ` Stephen Hemminger

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.