netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Matthieu Baerts (NGI0)" <matttbe@kernel.org>
To: Stephen Hemminger <stephen@networkplumber.org>
Cc: netdev@vger.kernel.org, Quentin Deslandes <qde@naccy.de>,
	 "Matthieu Baerts (NGI0)" <matttbe@kernel.org>
Subject: [PATCH iproute2] ss: show extra info when '--processes' is not used
Date: Sat, 13 Jan 2024 18:10:21 +0100	[thread overview]
Message-ID: <20240113-ss-fix-ext-col-disabled-v1-1-cf99a7381dec@kernel.org> (raw)

A recent modification broke "extra" options for all protocols showing
info about the processes when '-p' / '--processes' option was not used
as well. In other words, all the additional bits displayed at the end or
at the next line were no longer printed if the user didn't ask to show
info about processes as well.

The reason is that, the "current_field" pointer never switched to the
"Ext" column. If the user didn't ask to display the processes, nothing
happened when trying to print extra bits using the "out()" function,
because the current field was still pointing to the "Process" one, now
marked as disabled.

Before the commit mentioned below, it was not an issue not to switch to
the "Ext" or "Process" columns because they were never marked as
"disabled".

Here is a quick list of options that were no longer displayed if '-p' /
'--processes' was not set:

- AF_INET(6):
  -o, --options
  -e, --extended
  --tos
  --cgroup
  --inet-sockopt
  -m, --memory
  -i, --info

- AF_PACKET:
  -e, --extended

- AF_XDP:
  -e, --extended

- AF_UNIX:
  -m, --memory
  -e, --extended

- TIPC:
  --tipcinfo

That was just by quickly reading the code, I probably missed some. But
this shows that the impact can be quite important for all scripts using
'ss' to monitor connections or to report info.

Fixes: 1607bf53 ("ss: prevent "Process" column from being printed unless requested")
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---

Notes:
    Note that this issue has quite an annoying impact on our side with
    the MPTCP subsystem: because '-p' is not used with 'ss', this commit
    broke 2 selftests (13 subtests). Also, 'ss' is used in case of
    errors to help better understanding issues, and it is not so useful
    if it is missing the most important bits: MPTCP info.

    I know that typically there is no bug-fix version with IPRoute2, but
    could you please consider one in this case? That would avoid
    troubles for those relying on 'ss' for the monitoring or the
    reporting when this specific version of IPRoute2 is used.

    In our case, it means we have to patch our selftests in 20+ places
    to support this "broken" version. Plus making sure this is
    backported correctly, resolving conflicts if needed, etc. It would
    be really nice if we could avoid that by making a v6.7.1 version
    including this fix :)
---
 misc/ss.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/misc/ss.c b/misc/ss.c
index 900fefa4..5296cabe 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -2427,6 +2427,8 @@ static void proc_ctx_print(struct sockstat *s)
 			free(buf);
 		}
 	}
+
+	field_next();
 }
 
 static void inet_stats_print(struct sockstat *s, bool v6only)

---
base-commit: 05a4fc72587fed4ad5a0a93c59394b3e39f30381
change-id: 20240113-ss-fix-ext-col-disabled-3f489367a5e7

Best regards,
-- 
Matthieu Baerts (NGI0) <matttbe@kernel.org>


             reply	other threads:[~2024-01-13 17:11 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-13 17:10 Matthieu Baerts (NGI0) [this message]
2024-01-15 17:07 ` [PATCH iproute2] ss: show extra info when '--processes' is not used Stephen Hemminger
2024-01-17 17:20 ` patchwork-bot+netdevbpf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240113-ss-fix-ext-col-disabled-v1-1-cf99a7381dec@kernel.org \
    --to=matttbe@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=qde@naccy.de \
    --cc=stephen@networkplumber.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).