* [PATCH iproute2] ss: Don't pad the last (enabled) column
@ 2025-08-21 5:45 Paul Wayper
2025-08-21 10:35 ` Stefano Brivio
0 siblings, 1 reply; 7+ messages in thread
From: Paul Wayper @ 2025-08-21 5:45 UTC (permalink / raw)
To: Stephen Hemminger, Stefano Brivio, netdev; +Cc: paulway, jbainbri
ss will emit spaces on the right hand side of a left-justified, enabled
column even if it's the last column. In situations where one or more
lines are very long - e.g. because of a large PROCESS field value - this
causes a lot of excess output.
Firstly, calculate the last enabled column. Then use this in the check
for whether to emit trailing spaces on the last column.
Also remove the 'EXT' column which does not seem to be used.
Fixes: 59f46b7b5be86 ("ss: Introduce columns lightweight abstraction")
Signed-off-by: Paul Wayper <paulway@redhat.com>
---
misc/ss.c | 34 +++++++++++++++++++++++++++-------
1 file changed, 27 insertions(+), 7 deletions(-)
diff --git a/misc/ss.c b/misc/ss.c
index 989e168ae35026249ccec0e2d4a3df07b0438c7b..27cf7434c616e0030efc8876f00715d4b37dd567 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -123,10 +123,11 @@ enum col_id {
COL_RADDR,
COL_RSERV,
COL_PROC,
- COL_EXT,
COL_MAX
};
+int LAST_COL = COL_MAX;
+
enum col_align {
ALIGN_LEFT,
ALIGN_CENTER,
@@ -152,7 +153,6 @@ static struct column columns[] = {
{ ALIGN_RIGHT, "Peer Address:", " ", 0, 0, 0 },
{ ALIGN_LEFT, "Port", "", 0, 0, 0 },
{ ALIGN_LEFT, "Process", "", 0, 0, 0 },
- { ALIGN_LEFT, "", "", 0, 0, 0 },
};
static struct column *current_field = columns;
@@ -1079,6 +1079,24 @@ static void out(const char *fmt, ...)
va_end(args);
}
+static void check_last_column(void)
+{
+ /* Find the last non-disabled column and set LAST_COL. */
+ for (int i = COL_MAX - 1; i > 0; i--) {
+ struct column c = columns[i];
+
+ if (!c.disabled) {
+ LAST_COL = i;
+ return;
+ }
+ }
+}
+
+static int field_is_last(struct column *f)
+{
+ return f - columns == LAST_COL;
+}
+
static int print_left_spacing(struct column *f, int stored, int printed)
{
int s;
@@ -1104,6 +1122,10 @@ static void print_right_spacing(struct column *f, int printed)
if (!f->width || f->align == ALIGN_RIGHT)
return;
+ /* Don't print trailing space if this is the last column. */
+ if (field_is_last(f))
+ return;
+
s = f->width - printed;
if (f->align == ALIGN_CENTER)
s /= 2;
@@ -1143,11 +1165,6 @@ static void field_flush(struct column *f)
buffer.tail->end = buffer.cur->data;
}
-static int field_is_last(struct column *f)
-{
- return f - columns == COL_MAX - 1;
-}
-
/* Get the next available token in the buffer starting from the current token */
static struct buf_token *buf_token_next(struct buf_token *cur)
{
@@ -1316,6 +1333,9 @@ static void render(void)
if (!buffer.head)
return;
+ /* Find last non-disabled column */
+ check_last_column();
+
token = (struct buf_token *)buffer.head->data;
/* Ensure end alignment of last token, it wasn't necessarily flushed */
--
2.50.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH iproute2] ss: Don't pad the last (enabled) column
2025-08-21 5:45 Paul Wayper
@ 2025-08-21 10:35 ` Stefano Brivio
[not found] ` <137a3493-bbda-490f-8ad4-fa3a511c2742@redhat.com>
0 siblings, 1 reply; 7+ messages in thread
From: Stefano Brivio @ 2025-08-21 10:35 UTC (permalink / raw)
To: Paul Wayper; +Cc: Stephen Hemminger, netdev, paulway, jbainbri
Hi Paul,
On Thu, 21 Aug 2025 15:45:47 +1000
Paul Wayper <pwayper@redhat.com> wrote:
> ss will emit spaces on the right hand side of a left-justified, enabled
> column even if it's the last column. In situations where one or more
> lines are very long - e.g. because of a large PROCESS field value - this
> causes a lot of excess output.
I guess I understand the issue, but having an example would help,
because I'm not quite sure how to reproduce this.
There's a problem with this change though, which I didn't really
investigate. It turns something like this (105 columns):
$ ss -tunl
Netid State Recv-Q Send-Q Local Address:Port Peer Address:Port
udp UNCONN 0 0 10.45.242.29:49047 0.0.0.0:*
udp UNCONN 0 0 192.168.122.1:53 0.0.0.0:*
udp UNCONN 0 0 0.0.0.0%virbr0:67 0.0.0.0:*
udp UNCONN 0 0 0.0.0.0:111 0.0.0.0:*
udp UNCONN 0 0 0.0.0.0:33335 0.0.0.0:*
udp UNCONN 0 0 192.168.1.185:60797 0.0.0.0:*
udp UNCONN 0 0 0.0.0.0:5154 0.0.0.0:*
udp UNCONN 0 0 224.0.0.251:5353 0.0.0.0:*
udp UNCONN 0 0 224.0.0.251:5353 0.0.0.0:*
udp UNCONN 0 0 224.0.0.251:5353 0.0.0.0:*
udp UNCONN 0 0 224.0.0.251:5353 0.0.0.0:*
udp UNCONN 0 0 0.0.0.0:5353 0.0.0.0:*
udp UNCONN 0 0 [::]:111 [::]:*
udp UNCONN 0 0 [fe80::1839:8c7e:5e64:76a4]%wlp4s0:546 [::]:*
udp UNCONN 0 0 [::]:5353 [::]:*
udp UNCONN 0 0 [::]:39164 [::]:*
tcp LISTEN 0 128 0.0.0.0:22 0.0.0.0:*
tcp LISTEN 0 4096 0.0.0.0:111 0.0.0.0:*
tcp LISTEN 0 1024 0.0.0.0:80 0.0.0.0:*
tcp LISTEN 0 5 0.0.0.0:5154 0.0.0.0:*
tcp LISTEN 0 4096 127.0.0.1:631 0.0.0.0:*
tcp LISTEN 0 4096 127.0.0.1:41001 0.0.0.0:*
tcp LISTEN 0 20 127.0.0.1:25 0.0.0.0:*
tcp LISTEN 0 32 192.168.122.1:53 0.0.0.0:*
tcp LISTEN 0 128 [::]:22 [::]:*
tcp LISTEN 0 4096 [::]:111 [::]:*
tcp LISTEN 0 1024 [::]:80 [::]:*
tcp LISTEN 0 4096 [::1]:631 [::]:*
tcp LISTEN 0 1024 *:12865 *:*
tcp LISTEN 0 20 [::1]:25 [::]:*
into this:
$ ./ss -tunl
Netid State Recv-Q Send-Q Local Address:Port Peer Address:Port
udp UNCONN 0 0 192.168.122.1: 530.0.0.0:
* udp UNCONN 0 0 0.0.0.0%virbr0:67
0.0.0.0: * udp UNCONN0 0 0.0.0.0:
111 0.0.0.0: * udpUNCONN 0 0
0.0.0.0: 33335 0.0.0.0: * udp UNCONN0
0 0.0.0.0: 5154 0.0.0.0: * udpUNCONN
0 0 224.0.0.251: 5353 0.0.0.0:* udp
UNCONN 0 0 224.0.0.251: 53530.0.0.0: *
udp UNCONN 0 0 224.0.0.251:5353 0.0.0.0:*
udp UNCONN 0 0 224.0.0.251: 53530.0.0.0:
* udp UNCONN 0 0 0.0.0.0:5353
0.0.0.0: * udp UNCONN0 0 [::]:
111 [::]: * udpUNCONN 0 0
[fe80::1839:8c7e:5e64:76a4]%wlp4s0: 546 [::]: * udp UNCONN0
0 [::]: 5353 [::]: * udpUNCONN
0 0 [::]: 39164 [::]:* tcp
LISTEN 0 128 0.0.0.0: 220.0.0.0: *
tcp LISTEN 0 4096 0.0.0.0:111 0.0.0.0:*
tcp LISTEN 0 1024 0.0.0.0: 800.0.0.0:
* tcp LISTEN 0 5 0.0.0.0:5154
0.0.0.0: * tcp LISTEN0 4096 127.0.0.1:
631 0.0.0.0: * tcpLISTEN 0 4096
127.0.0.1: 41001 0.0.0.0: * tcp LISTEN0
20 127.0.0.1: 25 0.0.0.0: * tcpLISTEN
0 32 192.168.122.1: 53 0.0.0.0:* tcp
LISTEN 0 128 [::]: 22[::]: *
tcp LISTEN 0 4096 [::]:111 [::]:*
tcp LISTEN 0 1024 [::]: 80[::]:
* tcp LISTEN 0 4096 [::1]:631
[::]: * tcp LISTEN0 1024 *:
12865 *: * tcpLISTEN 0 20
[::1]: 25 [::]: *
> Firstly, calculate the last enabled column. Then use this in the check
> for whether to emit trailing spaces on the last column.
>
> Also remove the 'EXT' column which does not seem to be used.
It's not referenced explicitly but it's definitely used, see also commits:
8740ca9dcd3c ss: add support for BPF socket-local storage
84c45b8acb30 Reapply "ss: prevent "Process" column from being printed unless requested"
f22c49730c36 Revert "ss: prevent "Process" column from being printed unless requested"
1607bf531fd2 ss: prevent "Process" column from being printed unless requested
Now, while 5883c6eba517 ("ss: show header for --processes/-p") and consequently
f22c49730c36 are obviously broken (sorry, I didn't review those, nobody Cc'ed
me), they clearly show that COL_EXT is used. It's for stuff like TCP extensions
(say, 'ss -tei') which have no own / specific column header.
--
Stefano
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH iproute2] ss: Don't pad the last (enabled) column
@ 2025-08-26 0:22 Paul Wayper
2025-08-26 6:55 ` Stefano Brivio
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Paul Wayper @ 2025-08-26 0:22 UTC (permalink / raw)
To: Stephen Hemminger, Stefano Brivio, netdev; +Cc: paulway, jbainbri
ss will emit spaces on the right hand side of a left-justified, enabled
column even if it's the last column. In situations where one or more
lines are very long - e.g. because of a large PROCESS field value - this
causes a lot of excess output.
Firstly, calculate the last enabled column. Then use this in the check
for whether to emit trailing spaces on the last column.
Also name the 'EXT' column as 'Details' and mark it as disabled by
default, enabled when the -e or --extended options are supplied.
Fixes: 59f46b7b5be86 ("ss: Introduce columns lightweight abstraction")
Signed-off-by: Paul Wayper <paulway@redhat.com>
---
misc/ss.c | 42 +++++++++++++++++++++++++++++++++---------
1 file changed, 33 insertions(+), 9 deletions(-)
diff --git a/misc/ss.c b/misc/ss.c
index 989e168ae35026249ccec0e2d4a3df07b0438c7b..1c576c1e5997ccbca448b6ed5af3d41d7867ba76 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -127,6 +127,8 @@ enum col_id {
COL_MAX
};
+int LAST_COL = COL_MAX;
+
enum col_align {
ALIGN_LEFT,
ALIGN_CENTER,
@@ -151,8 +153,8 @@ static struct column columns[] = {
{ ALIGN_LEFT, "Port", "", 0, 0, 0 },
{ ALIGN_RIGHT, "Peer Address:", " ", 0, 0, 0 },
{ ALIGN_LEFT, "Port", "", 0, 0, 0 },
- { ALIGN_LEFT, "Process", "", 0, 0, 0 },
- { ALIGN_LEFT, "", "", 0, 0, 0 },
+ { ALIGN_LEFT, "Process", " ", 0, 0, 0 },
+ { ALIGN_LEFT, "Details", " ", 1, 0, 0 },
};
static struct column *current_field = columns;
@@ -1079,6 +1081,22 @@ static void out(const char *fmt, ...)
va_end(args);
}
+static void check_last_column(void)
+{
+ /* Find the last non-disabled column and set LAST_COL. */
+ for (int i = COL_MAX - 1; i > 0; i--) {
+ if (!columns[i].disabled) {
+ LAST_COL = i;
+ return;
+ }
+ }
+}
+
+static int field_is_last(struct column *f)
+{
+ return f - columns == LAST_COL;
+}
+
static int print_left_spacing(struct column *f, int stored, int printed)
{
int s;
@@ -1104,6 +1122,10 @@ static void print_right_spacing(struct column *f, int printed)
if (!f->width || f->align == ALIGN_RIGHT)
return;
+ /* Don't print trailing space if this is the last column. */
+ if (field_is_last(f))
+ return;
+
s = f->width - printed;
if (f->align == ALIGN_CENTER)
s /= 2;
@@ -1143,11 +1165,6 @@ static void field_flush(struct column *f)
buffer.tail->end = buffer.cur->data;
}
-static int field_is_last(struct column *f)
-{
- return f - columns == COL_MAX - 1;
-}
-
/* Get the next available token in the buffer starting from the current token */
static struct buf_token *buf_token_next(struct buf_token *cur)
{
@@ -1316,6 +1333,9 @@ static void render(void)
if (!buffer.head)
return;
+ /* Find last non-disabled column */
+ check_last_column();
+
token = (struct buf_token *)buffer.head->data;
/* Ensure end alignment of last token, it wasn't necessarily flushed */
@@ -2452,12 +2472,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) {
- out(" users:(%s)", buf);
+ out("users:(%s)", buf);
free(buf);
}
} else if (show_processes || show_threads) {
if (find_entry(s->ino, &buf, USERS) > 0) {
- out(" users:(%s)", buf);
+ out("users:(%s)", buf);
free(buf);
}
}
@@ -6241,6 +6261,10 @@ int main(int argc, char *argv[])
if (ssfilter_parse(¤t_filter.f, argc, argv, filter_fp))
usage();
+ /* Details column normally disabled, enable if asked */
+ if (show_details)
+ columns[COL_EXT].disabled = 0;
+
if (!show_processes)
columns[COL_PROC].disabled = 1;
--
2.51.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH iproute2] ss: Don't pad the last (enabled) column
2025-08-26 0:22 [PATCH iproute2] ss: Don't pad the last (enabled) column Paul Wayper
@ 2025-08-26 6:55 ` Stefano Brivio
2025-08-26 23:22 ` Stefano Brivio
2025-08-29 11:09 ` Stefano Brivio
2 siblings, 0 replies; 7+ messages in thread
From: Stefano Brivio @ 2025-08-26 6:55 UTC (permalink / raw)
To: Paul Wayper; +Cc: Stephen Hemminger, netdev, paulway, jbainbri
On Tue, 26 Aug 2025 10:22:37 +1000
Paul Wayper <pwayper@redhat.com> wrote:
> ss will emit spaces on the right hand side of a left-justified, enabled
> column even if it's the last column. In situations where one or more
> lines are very long - e.g. because of a large PROCESS field value - this
> causes a lot of excess output.
>
> Firstly, calculate the last enabled column. Then use this in the check
> for whether to emit trailing spaces on the last column.
>
> Also name the 'EXT' column as 'Details' and mark it as disabled by
> default, enabled when the -e or --extended options are supplied.
>
> Fixes: 59f46b7b5be86 ("ss: Introduce columns lightweight abstraction")
> Signed-off-by: Paul Wayper <paulway@redhat.com>
Thanks for the new version (this should have "v2" in the subject line).
I'll have a look and test in a bit.
--
Stefano
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH iproute2] ss: Don't pad the last (enabled) column
2025-08-26 0:22 [PATCH iproute2] ss: Don't pad the last (enabled) column Paul Wayper
2025-08-26 6:55 ` Stefano Brivio
@ 2025-08-26 23:22 ` Stefano Brivio
2025-08-29 11:09 ` Stefano Brivio
2 siblings, 0 replies; 7+ messages in thread
From: Stefano Brivio @ 2025-08-26 23:22 UTC (permalink / raw)
To: Paul Wayper; +Cc: Stephen Hemminger, netdev, paulway, jbainbri
On Tue, 26 Aug 2025 10:22:37 +1000
Paul Wayper <pwayper@redhat.com> wrote:
> ss will emit spaces on the right hand side of a left-justified, enabled
> column even if it's the last column. In situations where one or more
> lines are very long - e.g. because of a large PROCESS field value - this
> causes a lot of excess output.
>
> Firstly, calculate the last enabled column. Then use this in the check
> for whether to emit trailing spaces on the last column.
>
> Also name the 'EXT' column as 'Details' and mark it as disabled by
> default, enabled when the -e or --extended options are supplied.
>
> Fixes: 59f46b7b5be86 ("ss: Introduce columns lightweight abstraction")
> Signed-off-by: Paul Wayper <paulway@redhat.com>
> ---
> misc/ss.c | 42 +++++++++++++++++++++++++++++++++---------
> 1 file changed, 33 insertions(+), 9 deletions(-)
For some reason I didn't investigate yet, this still breaks ss -tunl as
well as ss -tunap for me. With 115 columns, before:
$ ss -tunl
Netid State Recv-Q Send-Q Local Address:Port Peer Address:Port
udp UNCONN 0 0 192.168.122.1:53 0.0.0.0:*
udp UNCONN 0 0 0.0.0.0%virbr0:67 0.0.0.0:*
udp UNCONN 0 0 0.0.0.0:111 0.0.0.0:*
udp UNCONN 0 0 0.0.0.0:33335 0.0.0.0:*
...
$ ss -tunap
Netid State Recv-Q Send-Q Local Address:Port Peer Address:Port Process
udp UNCONN 0 0 192.168.122.1:53 0.0.0.0:*
udp UNCONN 0 0 0.0.0.0%virbr0:67 0.0.0.0:*
udp ESTAB 0 0 192.168.1.185%wlp4s0:68 192.168.1.1:67
...
and after:
$ ./ss -tunl
Netid State Recv-Q Send-Q Local Address:Port Peer Address:Port
udp UNCONN 0 0 192.168.122.1: 530.0.0.0:
* udp UNCONN 0 0 0.0.0.0%virbr0:67
0.0.0.0: * udp UNCONN0 0 0.0.0.0:
111 0.0.0.0: * udpUNCONN 0 0
0.0.0.0: 33335 0.0.0.0: * udp UNCONN0
0 0.0.0.0: 5154 0.0.0.0: * udpUNCONN
...
$ ./ss -tunap
Netid State Recv-Q Send-Q Local Address:Port Peer Address:Port Process
udp UNCONN 0 0 192.168.122.1: 530.0.0.0: *
udp UNCONN 0 0 0.0.0.0%virbr0:67 0.0.0.0:
* udp ESTAB0 0 192.168.1.185%wlp4s0: 68
192.168.1.1: 67 udpUNCONN 0 0 0.0.0.0:
111 0.0.0.0: * udp UNCONN0 0
0.0.0.0: 33335 0.0.0.0: * udpUNCONN 0
...
I'll look into this soon, give me a couple of days. I still have to
answer some points from 137a3493-bbda-490f-8ad4-fa3a511c2742@redhat.com
as well.
--
Stefano
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH iproute2] ss: Don't pad the last (enabled) column
[not found] ` <137a3493-bbda-490f-8ad4-fa3a511c2742@redhat.com>
@ 2025-08-28 23:49 ` Stefano Brivio
0 siblings, 0 replies; 7+ messages in thread
From: Stefano Brivio @ 2025-08-28 23:49 UTC (permalink / raw)
To: Paul Wayper; +Cc: Stephen Hemminger, netdev, jbainbri
On Mon, 25 Aug 2025 11:51:20 +1000
Paul Wayper <pwayper@redhat.com> wrote:
> On 21/08/2025 20:35, Stefano Brivio wrote:
> > Hi Paul,
> >
> > On Thu, 21 Aug 2025 15:45:47 +1000
> > Paul Wayper<pwayper@redhat.com> wrote:
> >
> >> ss will emit spaces on the right hand side of a left-justified, enabled
> >> column even if it's the last column. In situations where one or more
> >> lines are very long - e.g. because of a large PROCESS field value - this
> >> causes a lot of excess output.
> > I guess I understand the issue, but having an example would help,
> > because I'm not quite sure how to reproduce this.
>
> Hi Stefano,
>
> To reproduce, do a `ss -tunap` and look at the output with something
> like `od -tx1c`:
>
> 0000000 4e 65 74 69 64 20 53 74 61 74 65 20 20 20 20 20
> N e t i d S t a t e
> 0000020 20 52 65 63 76 2d 51 20 53 65 6e 64 2d 51 20 20
> R e c v - Q S e n d - Q
> 0000040 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20
>
> 0000060 20 20 20 20 20 20 20 20 20 20 20 4c 6f 63 61 6c
> L o c a l
> 0000100 20 41 64 64 72 65 73 73 3a 50 6f 72 74 20 20 20
> A d d r e s s : P o r t
> 0000120 20 20 20 20 20 20 20 50 65 65 72 20 41 64 64 72
> P e e r A d d r
> 0000140 65 73 73 3a 50 6f 72 74 50 72 6f 63 65 73 73 20
> e s s : P o r t P r o c e s s
> 0000160 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20
>
> *
> 0000220 20 20 20 20 20 20 0a 75 64 70 20 20 20 55 4e 43
> \n u d p U N C
> 0000240 4f 4e 4e 20 20 20 20 20 30 20 20 20 20 20 20 30
> O N N 0 0
> 0000260 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20
>
> *
> 0000320 20 20 20 20 20 20 20 20 30 2e 30 2e 30 2e 30 3a
> 0 . 0 . 0 . 0 :
> 0000340 34 36 37 36 35 20 20 20 20 20 20 20 20 20 20 20
> 4 6 7 6 5
> 0000360 20 20 20 30 2e 30 2e 30 2e 30 3a 2a 20 20 20 20
> 0 . 0 . 0 . 0 : *
> 0000400 75 73 65 72 73 3a 28 28 22 77 73 64 64 22 2c 70
> u s e r s : ( ( " w s d d " , p
> 0000420 69 64 3d 35 39 35 37 2c 66 64 3d 31 34 29 29 20
> i d = 5 9 5 7 , f d = 1 4 ) )
> 0000440 20 20 20 20 20 20 20 20 20 20 20 20 20 0a 75 64
> \n u d
>
> You can see the spaces after 'Process' and the process name (i.e.
> 'users:(("wsdd",pid=5957,fd=14))'), just before the end-of-line.
Those spaces _should_ be harmless because they _should_ align with the
end of line anyway. We can drop them, but:
> The problem we had was with the output when there was over 1000
> processes all listening on the same port. That pushed the line length
> out to ~35,000 bytes, with almost all of that being space characters.
> Unfortunately on this server there was also ~21,000 connections in
> TIME-WAIT, which -a will show, so we had ~21,000 lines of ~35,000 bytes
> which added up to a very large output.
...I don't understand, from this, if your issue is that there was a
single process with a very long name (which won't be solved by just
dropping spaces), or the waste of bytes (which would be solved instead).
Having the exact line(s) causing you trouble would be helpful.
> What's messing with my head right now though is that in the `ss -tunap`
> output, when the 'Process' column is on the end, the current standard
> 'ss' on my Fedora 42 machine outputs the Process header right up against
> the 'Peer address:Port' header:
>
> $ /bin/ss -tunap | head -4 # trimmed for email neatness... problem
> is v- here
First off, note that if you pipe the output to head(1), isatty(3) in
render_screen_width() will return 0, and that affects the output of
course, because we can't fetch the number of columns.
What did you trim exactly here, and what do you mean by "problem is v-"?
> Netid State Recv-Q Send-Q Local Address:Port Peer
> Address:PortProcess
> udp UNCONN 0 0 0.0.0.0:46765 0.0.0.0:*
> users:(("wsdd",pid=5957,fd=14))
> udp UNCONN 0 0 0.0.0.0:32772 0.0.0.0:*
> users:(("firefox",pid=8059,fd=279))
> udp UNCONN 0 0 127.0.0.1:53 0.0.0.0:*
I can see something like this as well if I pipe the output and I don't
set COLUMNS according to the current columns from my terminal. The
reason is that we try to pack as much output as possible into one line,
and we pack one byte too many, so the space between "Port" and
"Process" is elided.
This should be fixed, but it won't help with the issue you reported.
There seems to be another issue: render_screen_width() calls
getenv("COLUMNS"), but, with $COLUMNS set to 211 for example, for
whatever reason, I get different outputs between 'ss -tunap' and
'COLUMNS=211 ss -tunap', as if environment variables were scrubbed or
something. I didn't debug that yet.
> $ rpm -qf /bin/ss
> iproute-6.12.0-3.fc42.x86_64
>
> Is this something that I just haven't noticed until now? Or is there a
> more fundamental bug in the way the table spacing is calculated? I've
> tried changing the 'ldelim' to be a space, as it is in the non-first
> left-aligned columns (State, Recv-Q, Send-Q, Local address: and Peer
> address:), and that fixes the header but adds an extra space before the
> 'users' process description.
>
> > There's a problem with this change though, which I didn't really
> > investigate. It turns something like this (105 columns):
> >
> > $ ss -tunl
> > Netid State Recv-Q Send-Q Local Address:Port Peer Address:Port
> > udp UNCONN 0 0 10.45.242.29:49047 0.0.0.0:*
> > udp UNCONN 0 0 192.168.122.1:53 0.0.0.0:*
> > udp UNCONN 0 0 0.0.0.0%virbr0:67 0.0.0.0:*
> > [snip]
> > tcp LISTEN 0 1024 *:12865 *:*
> > tcp LISTEN 0 20 [::1]:25 [::]:*
> >
> > into this:
> >
> > $ ./ss -tunl
> > Netid State Recv-Q Send-Q Local Address:Port Peer Address:Port
> > udp UNCONN 0 0 192.168.122.1: 530.0.0.0:
> > * udp UNCONN 0 0 0.0.0.0%virbr0:67
> > 0.0.0.0: * udp UNCONN0 0 0.0.0.0:
> > [snip]
> > * tcp LISTEN 0 4096 [::1]:631
> > [::]: * tcp LISTEN0 1024 *:
> > 12865 *: * tcpLISTEN 0 20
> > [::1]: 25 [::]: *
>
> OK, that's weird. I'm not seeing that:
>
> $ misc/ss -tunl | head
The same note about piping the output applies here, but anyway:
> Netid State Recv-Q Send-Q Local
> Address:Port Peer Address:Port
> udp UNCONN 0 0 0.0.0.0:46765 0.0.0.0:*
> udp UNCONN 0 0 127.0.0.1:53 0.0.0.0:*
> udp UNCONN 0 0 127.0.0.54:53 0.0.0.0:*
> udp UNCONN 0 0 127.0.0.53%lo:53 0.0.0.0:*
> udp UNCONN 0 0 127.0.0.1:323 0.0.0.0:*
> udp UNCONN 0 0 10.215.66.73:3702 0.0.0.0:*
> udp UNCONN 0 0 239.255.255.250:3702 0.0.0.0:*
> udp UNCONN 0 0 10.76.18.168:3702 0.0.0.0:*
> udp UNCONN 0 0 239.255.255.250:3702 0.0.0.0:*
...the alignment is completely broken even in your output. This is
with the second version of your patch, in my environment:
$ ./ss -tunl | head
Netid State Recv-Q Send-Q Local Address:Port Peer Address:Port
udp UNCONN 0 0 192.168.1.185: 564080.0.0.0:
* udp UNCONN 0 0 192.168.122.1:53
0.0.0.0: * udp UNCONN0 0 0.0.0.0%virbr0:
67 0.0.0.0: * udpUNCONN 0 0
0.0.0.0: 111 0.0.0.0: * udp UNCONN0
0 0.0.0.0: 33335 0.0.0.0: * udpUNCONN
0 0 10.45.242.21: 42711 0.0.0.0:* udp
UNCONN 0 0 0.0.0.0: 51540.0.0.0: *
udp UNCONN 0 0 224.0.0.251:5353 0.0.0.0:*
...and without it:
$ ss -tunl | head
Netid State Recv-Q Send-Q Local Address:Port Peer Address:Port
udp UNCONN 0 0 192.168.122.1:53 0.0.0.0:*
udp UNCONN 0 0 0.0.0.0%virbr0:67 0.0.0.0:*
udp UNCONN 0 0 0.0.0.0:111 0.0.0.0:*
udp UNCONN 0 0 0.0.0.0:33335 0.0.0.0:*
udp UNCONN 0 0 0.0.0.0:5154 0.0.0.0:*
udp UNCONN 0 0 224.0.0.251:5353 0.0.0.0:*
udp UNCONN 0 0 224.0.0.251:5353 0.0.0.0:*
udp UNCONN 0 0 224.0.0.251:5353 0.0.0.0:*
udp UNCONN 0 0 224.0.0.251:5353 0.0.0.0:*
> I'm not doubting you got that result, and at a rough guess it looks like
> the count of which field we're aligning is getting out of sync with the
> field we're actually printing. That makes me think I must have
> interfered with the 'render' function's field output loop, but I didn't
> touch it.
>
> I've tried changing the loops in the render_calc_width function to use
> LAST_COL rather than COL_MAX but it didn't change the output.
>
> Stefano, are you using any CC options in your environment I should know
> about that might change the way your binary compiled compared to mine?
Not really, I'm just building with 'make'. The resulting command line
is:
gcc ss.o ssfilter_check.o ssfilter.tab.o -lselinux -ltirpc -lelf -L/usr/lib64 -lcap ../lib/libutil.a ../lib/libnetlink.a -lselinux -ltirpc -lelf -L/usr/lib64 -lcap -o ss
and that's with gcc 13.3.0.
> > It's not referenced explicitly but it's definitely used, see also commits:
>
> Yeah, I should have just left it as is but marked it as explicitly
> disabled. I've put it back.
>
> I really appreciate your help here Stefano, I'll send through an updated
> patch today using git send-email for the right formatting. I'm new to
> submitting patches to the Kernel so all advice is greatly appreciated.
Except for the missing 'PATCH v2' in the subject line, and the missing
description of the changes compared to the first version (check mailing
list archives for examples), your patch submissions look good to me.
It would be nice if you could drop the HTML attachment from your other
emails to the list (it just adds bytes) and the Red Hat logo as well,
by now we all know how it looks like. :)
I still have to go through your second version in detail. I'm not
really sure what exact problem it's meant to fix, though, and it would
be helpful to know that in advance.
--
Stefano
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH iproute2] ss: Don't pad the last (enabled) column
2025-08-26 0:22 [PATCH iproute2] ss: Don't pad the last (enabled) column Paul Wayper
2025-08-26 6:55 ` Stefano Brivio
2025-08-26 23:22 ` Stefano Brivio
@ 2025-08-29 11:09 ` Stefano Brivio
2 siblings, 0 replies; 7+ messages in thread
From: Stefano Brivio @ 2025-08-29 11:09 UTC (permalink / raw)
To: Paul Wayper; +Cc: Stephen Hemminger, netdev, paulway, jbainbri
Here comes a detailed, but partial review.
It's partial because, as I mentioned on the v1 thread, I don't quite
grasp the issue you're facing yet, and I couldn't reproduce it.
On Tue, 26 Aug 2025 10:22:37 +1000
Paul Wayper <pwayper@redhat.com> wrote:
> ss will emit spaces on the right hand side of a left-justified, enabled
> column even if it's the last column. In situations where one or more
> lines are very long - e.g. because of a large PROCESS field value - this
> causes a lot of excess output.
>
> Firstly, calculate the last enabled column. Then use this in the check
> for whether to emit trailing spaces on the last column.
>
> Also name the 'EXT' column as 'Details' and mark it as disabled by
> default, enabled when the -e or --extended options are supplied.
>
> Fixes: 59f46b7b5be86 ("ss: Introduce columns lightweight abstraction")
> Signed-off-by: Paul Wayper <paulway@redhat.com>
> ---
> misc/ss.c | 42 +++++++++++++++++++++++++++++++++---------
> 1 file changed, 33 insertions(+), 9 deletions(-)
>
> diff --git a/misc/ss.c b/misc/ss.c
> index 989e168ae35026249ccec0e2d4a3df07b0438c7b..1c576c1e5997ccbca448b6ed5af3d41d7867ba76 100644
> --- a/misc/ss.c
> +++ b/misc/ss.c
> @@ -127,6 +127,8 @@ enum col_id {
> COL_MAX
> };
>
> +int LAST_COL = COL_MAX;
Uppercase identifiers are typically used for build-time constants
(#define directives). It would be good to have this one together with
all the other global variables (just after 'int oneline').
> +
> enum col_align {
> ALIGN_LEFT,
> ALIGN_CENTER,
> @@ -151,8 +153,8 @@ static struct column columns[] = {
> { ALIGN_LEFT, "Port", "", 0, 0, 0 },
> { ALIGN_RIGHT, "Peer Address:", " ", 0, 0, 0 },
> { ALIGN_LEFT, "Port", "", 0, 0, 0 },
> - { ALIGN_LEFT, "Process", "", 0, 0, 0 },
> - { ALIGN_LEFT, "", "", 0, 0, 0 },
> + { ALIGN_LEFT, "Process", " ", 0, 0, 0 },
This should be a separate patch with its own tag:
Fixes: 5883c6eba517 ("ss: show header for --processes/-p")
> + { ALIGN_LEFT, "Details", " ", 1, 0, 0 },
What does "Details" add? It's very different types of details, and
that's the reason why this column has no header.
> };
>
> static struct column *current_field = columns;
> @@ -1079,6 +1081,22 @@ static void out(const char *fmt, ...)
> va_end(args);
> }
>
> +static void check_last_column(void)
That's not what this function does. It looks for the last column and
sets a variable, so it could be called find_last_column() or
set_last_column_index(), for instance.
> +{
> + /* Find the last non-disabled column and set LAST_COL. */
...and once the name is clear, this comment can go away.
> + for (int i = COL_MAX - 1; i > 0; i--) {
While it's allowed in C99, in this file, variables are never declared in
loop headers.
You don't actually need 'i', you could directly use the variable of the
index you're calculating.
It's not entirely obvious why you would start from the right / last
column, it looks more intuitive to me to start from the left.
> + if (!columns[i].disabled) {
> + LAST_COL = i;
> + return;
This assumes that, if a column is disabled, the previous one is always
the last column, which isn't correct. You could have COL_PROC disabled,
but COL_EXT enabled.
> + }
> + }
> +}
> +
> +static int field_is_last(struct column *f)
> +{
> + return f - columns == LAST_COL;
> +}
> +
> static int print_left_spacing(struct column *f, int stored, int printed)
> {
> int s;
> @@ -1104,6 +1122,10 @@ static void print_right_spacing(struct column *f, int printed)
> if (!f->width || f->align == ALIGN_RIGHT)
> return;
>
> + /* Don't print trailing space if this is the last column. */
> + if (field_is_last(f))
> + return;
The caller, render(), already deals with this kind of considerations,
such as printing newlines if field_is_last(). It would be more natural
to *not* call print_right_spacing() right away.
> +
> s = f->width - printed;
> if (f->align == ALIGN_CENTER)
> s /= 2;
> @@ -1143,11 +1165,6 @@ static void field_flush(struct column *f)
> buffer.tail->end = buffer.cur->data;
> }
>
> -static int field_is_last(struct column *f)
> -{
> - return f - columns == COL_MAX - 1;
> -}
> -
> /* Get the next available token in the buffer starting from the current token */
> static struct buf_token *buf_token_next(struct buf_token *cur)
> {
> @@ -1316,6 +1333,9 @@ static void render(void)
> if (!buffer.head)
> return;
>
> + /* Find last non-disabled column */
> + check_last_column();
You're calling this in render(), but other functions rely on
field_is_last(), and they might be called before render(), or without
render() being called at all.
Ideally, this should be set once the command line is processed, in
main(). See 'oneline', 'show_header', etc. Alternatively,
field_is_last() could just calculate the index of the last column
itself, it shouldn't be significantly more expensive than a subtraction.
That is, unless you want to have it for render() only, I'm not quite
sure about the intention at this point. But then there's no need for a
global variable.
> +
> token = (struct buf_token *)buffer.head->data;
>
> /* Ensure end alignment of last token, it wasn't necessarily flushed */
> @@ -2452,12 +2472,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) {
> - out(" users:(%s)", buf);
> + out("users:(%s)", buf);
Unrelated change.
> free(buf);
> }
> } else if (show_processes || show_threads) {
> if (find_entry(s->ino, &buf, USERS) > 0) {
> - out(" users:(%s)", buf);
> + out("users:(%s)", buf);
Unrelated change.
> free(buf);
> }
> }
> @@ -6241,6 +6261,10 @@ int main(int argc, char *argv[])
> if (ssfilter_parse(¤t_filter.f, argc, argv, filter_fp))
> usage();
>
> + /* Details column normally disabled, enable if asked */
That should be obvious from variable names (it is for me), if it's not
names should be changed instead of adding a comment.
> + if (show_details)
> + columns[COL_EXT].disabled = 0;
> +
> if (!show_processes)
> columns[COL_PROC].disabled = 1;
>
--
Stefano
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-08-29 11:09 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-26 0:22 [PATCH iproute2] ss: Don't pad the last (enabled) column Paul Wayper
2025-08-26 6:55 ` Stefano Brivio
2025-08-26 23:22 ` Stefano Brivio
2025-08-29 11:09 ` Stefano Brivio
-- strict thread matches above, loose matches on Subject: below --
2025-08-21 5:45 Paul Wayper
2025-08-21 10:35 ` Stefano Brivio
[not found] ` <137a3493-bbda-490f-8ad4-fa3a511c2742@redhat.com>
2025-08-28 23:49 ` Stefano Brivio
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).