[PATCH iproute2 net-next] ip: add a new parameter -Numeric

[PATCH iproute2 net-next] ip: add a new parameter -Numeric

[Hangbin Liu]

When calles rtnl_dsfield_n2a(), we get the dsfield name from
/etc/iproute2/rt_dsfield. But different distribution may have
different names. So add a new parameter '-Numeric' to only show
the dsfield number.

This parameter is only used for tos value at present. We could enable
this for other fields if needed in the future.

Suggested-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 include/utils.h | 1 +
 ip/ip.c         | 6 +++++-
 lib/rt_names.c  | 4 +++-
 man/man8/ip.8   | 6 ++++++
 4 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/include/utils.h b/include/utils.h
index 8a9c3020..0f57ee97 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -34,6 +34,7 @@ extern int timestamp_short;
 extern const char * _SL_;
 extern int max_flush_loops;
 extern int batch_mode;
+extern int numeric;
 extern bool do_all;
 
 #ifndef CONFDIR
diff --git a/ip/ip.c b/ip/ip.c
index e4131714..2db5bbb0 100644
--- a/ip/ip.c
+++ b/ip/ip.c
@@ -36,6 +36,7 @@ int timestamp;
 int force;
 int max_flush_loops = 10;
 int batch_mode;
+int numeric;
 bool do_all;
 
 struct rtnl_handle rth = { .fd = -1 };
@@ -57,7 +58,8 @@ static void usage(void)
 "                    -4 | -6 | -I | -D | -M | -B | -0 |\n"
 "                    -l[oops] { maximum-addr-flush-attempts } | -br[ief] |\n"
 "                    -o[neline] | -t[imestamp] | -ts[hort] | -b[atch] [filename] |\n"
-"                    -rc[vbuf] [size] | -n[etns] name | -a[ll] | -c[olor]}\n");
+"                    -rc[vbuf] [size] | -n[etns] name | -N[umeric]} | -a[ll] |\n"
+"                    -c[olor]}\n");
 	exit(-1);
 }
 
@@ -287,6 +289,8 @@ int main(int argc, char **argv)
 			NEXT_ARG();
 			if (netns_switch(argv[1]))
 				exit(-1);
+		} else if (matches(opt, "-Numeric") == 0) {
+			++numeric;
 		} else if (matches(opt, "-all") == 0) {
 			do_all = true;
 		} else {
diff --git a/lib/rt_names.c b/lib/rt_names.c
index 66d5f2f0..122bdd74 100644
--- a/lib/rt_names.c
+++ b/lib/rt_names.c
@@ -27,6 +27,8 @@
 
 #define NAME_MAX_LEN 512
 
+int numeric;
+
 struct rtnl_hash_entry {
 	struct rtnl_hash_entry	*next;
 	const char		*name;
@@ -480,7 +482,7 @@ const char *rtnl_dsfield_n2a(int id, char *buf, int len)
 		snprintf(buf, len, "%d", id);
 		return buf;
 	}
-	if (!rtnl_rtdsfield_tab[id]) {
+	if (!rtnl_rtdsfield_tab[id] && !numeric) {
 		if (!rtnl_rtdsfield_init)
 			rtnl_rtdsfield_initialize();
 	}
diff --git a/man/man8/ip.8 b/man/man8/ip.8
index f4cbfc03..c2a8a92d 100644
--- a/man/man8/ip.8
+++ b/man/man8/ip.8
@@ -47,6 +47,7 @@ ip \- show / manipulate routing, network devices, interfaces and tunnels
 \fB\-t\fR[\fIimestamp\fR] |
 \fB\-ts\fR[\fIhort\fR] |
 \fB\-n\fR[\fIetns\fR] name |
+\fB\-N\fR[\fIumeric\fR] |
 \fB\-a\fR[\fIll\fR] |
 \fB\-c\fR[\fIolor\fR] |
 \fB\-br\fR[\fIief\fR] |
@@ -174,6 +175,11 @@ to
 .RI "-n[etns] " NETNS " [ " OPTIONS " ] " OBJECT " { " COMMAND " | "
 .BR help " }"
 
+.TP
+.BR "\-N" , " \-Numeric"
+Print the dsfield's numeric directly instead of converting to the name from
+/etc/iproute2/rt_dsfield.
+
 .TP
 .BR "\-a" , " \-all"
 executes specified command over all objects, it depends if command
-- 
2.19.2

Re: [PATCH iproute2 net-next] ip: add a new parameter -Numeric

[Phil Sutter]

Hi,

On Mon, May 20, 2019 at 03:56:48PM +0800, Hangbin Liu wrote:
> When calles rtnl_dsfield_n2a(), we get the dsfield name from
> /etc/iproute2/rt_dsfield. But different distribution may have
> different names. So add a new parameter '-Numeric' to only show
> the dsfield number.
> 
> This parameter is only used for tos value at present. We could enable
> this for other fields if needed in the future.

Rationale is to ensure expected output irrespective of host
configuration, especially in test scripts. Concrete example motivating
this patch was net/fib_rule_tests.sh in kernel self-tests.

> Suggested-by: Phil Sutter <phil@nwl.cc>
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>

Acked-by: Phil Sutter <phil@nwl.cc>

Thanks for doing this, Hangbin!

Phil

Re: [PATCH iproute2 net-next] ip: add a new parameter -Numeric

[David Ahern]

[-- Attachment #1: Type: text/plain, Size: 666 bytes --]

On 5/20/19 1:56 AM, Hangbin Liu wrote:
> When calles rtnl_dsfield_n2a(), we get the dsfield name from
> /etc/iproute2/rt_dsfield. But different distribution may have
> different names. So add a new parameter '-Numeric' to only show
> the dsfield number.
> 
> This parameter is only used for tos value at present. We could enable
> this for other fields if needed in the future.
> 

It does not make sense to add this flag just for 1 field.

3 years ago I started a patch to apply this across the board. never
finished it. see attached. The numeric variable should be moved to
lib/rt_names.c. It handles all of the conversions in that file - at
least as of May 2016.

[-- Attachment #2: iproute-numeric.patch --]
[-- Type: text/plain, Size: 5363 bytes --]

diff --git a/include/utils.h b/include/utils.h
index 2c690417b721..31c9e792f8bd 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -26,6 +26,7 @@ extern const char * _SL_;
 extern int max_flush_loops;
 extern int batch_mode;
 extern bool do_all;
+extern int numeric;
 
 #ifndef IPPROTO_ESP
 #define IPPROTO_ESP	50
diff --git a/ip/ip.c b/ip/ip.c
index eea00b822088..995c2daed965 100644
--- a/ip/ip.c
+++ b/ip/ip.c
@@ -39,6 +39,7 @@ int force;
 int max_flush_loops = 10;
 int batch_mode;
 bool do_all;
+int numeric;
 
 struct rtnl_handle rth = { .fd = -1 };
 
@@ -236,10 +237,8 @@ int main(int argc, char **argv)
 		} else if (matches(opt, "-tshort") == 0) {
 			++timestamp;
 			++timestamp_short;
-#if 0
 		} else if (matches(opt, "-numeric") == 0) {
-			rtnl_names_numeric++;
-#endif
+			numeric++;
 		} else if (matches(opt, "-Version") == 0) {
 			printf("ip utility, iproute2-ss%s\n", SNAPSHOT);
 			exit(0);
diff --git a/lib/rt_names.c b/lib/rt_names.c
index f68e91d6d046..86066ec2df2c 100644
--- a/lib/rt_names.c
+++ b/lib/rt_names.c
@@ -21,6 +21,7 @@
 
 #include <asm/types.h>
 #include <linux/rtnetlink.h>
+#include <utils.h>
 
 #include "rt_names.h"
 
@@ -151,7 +152,7 @@ static void rtnl_rtprot_initialize(void)
 
 const char * rtnl_rtprot_n2a(int id, char *buf, int len)
 {
-	if (id<0 || id>=256) {
+	if (id<0 || id>=256 || numeric) {
 		snprintf(buf, len, "%u", id);
 		return buf;
 	}
@@ -216,7 +217,7 @@ static void rtnl_rtscope_initialize(void)
 
 const char *rtnl_rtscope_n2a(int id, char *buf, int len)
 {
-	if (id<0 || id>=256) {
+	if (id<0 || id>=256 || numeric) {
 		snprintf(buf, len, "%d", id);
 		return buf;
 	}
@@ -278,7 +279,7 @@ static void rtnl_rtrealm_initialize(void)
 
 const char *rtnl_rtrealm_n2a(int id, char *buf, int len)
 {
-	if (id<0 || id>=256) {
+	if (id<0 || id>=256 || numeric) {
 		snprintf(buf, len, "%d", id);
 		return buf;
 	}
@@ -380,7 +381,7 @@ const char * rtnl_rttable_n2a(__u32 id, char *buf, int len)
 {
 	struct rtnl_hash_entry *entry;
 
-	if (id > RT_TABLE_MAX) {
+	if (id > RT_TABLE_MAX || numeric) {
 		snprintf(buf, len, "%u", id);
 		return buf;
 	}
@@ -446,7 +447,7 @@ static void rtnl_rtdsfield_initialize(void)
 
 const char *rtnl_dsfield_n2a(int id, char *buf, int len)
 {
-	if (id<0 || id>=256) {
+	if (id<0 || id>=256 || numeric) {
 		snprintf(buf, len, "%d", id);
 		return buf;
 	}
@@ -549,6 +550,11 @@ const char *rtnl_group_n2a(int id, char *buf, int len)
 	struct rtnl_hash_entry *entry;
 	int i;
 
+	if (numeric) {
+		snprintf(buf, len, "%d", id);
+		return buf;
+	}
+
 	if (!rtnl_group_init)
 		rtnl_group_initialize();
 
@@ -598,7 +604,7 @@ static void nl_proto_initialize(void)
 
 const char *nl_proto_n2a(int id, char *buf, int len)
 {
-	if (id < 0 || id >= 256) {
+	if (id < 0 || id >= 256 || numeric) {
 		snprintf(buf, len, "%u", id);
 		return buf;
 	}
diff --git a/misc/rtacct.c b/misc/rtacct.c
index bb8c90f98f5a..acecce0c3ecc 100644
--- a/misc/rtacct.c
+++ b/misc/rtacct.c
@@ -42,6 +42,7 @@ int time_constant = 0;
 int dump_zeros = 0;
 unsigned long magic_number = 0;
 double W;
+int numeric;
 
 static int generic_proc_open(const char *env, const char *name)
 {
diff --git a/misc/ss.c b/misc/ss.c
index eca4aa35facc..c6428cc2f19b 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -87,7 +87,6 @@ static int security_get_initial_context(char *name,  char **context)
 #endif
 
 int resolve_hosts = 0;
-int resolve_services = 1;
 int preferred_family = AF_UNSPEC;
 int show_options = 0;
 int show_details = 0;
@@ -100,6 +99,7 @@ int show_sock_ctx = 0;
 /* If show_users & show_proc_ctx only do user_ent_hash_build() once */
 int user_ent_hash_build_init = 0;
 int follow_events = 0;
+int numeric;
 
 int netid_width;
 int state_width;
@@ -963,7 +963,7 @@ static const char *resolve_service(int port)
 		return buf;
 	}
 
-	if (!resolve_services)
+	if (numeric)
 		goto do_numeric;
 
 	if (dg_proto == RAW_PROTO)
@@ -3076,14 +3076,11 @@ static int netlink_show_one(struct filter *f,
 
 	sock_state_print(&st, "nl");
 
-	if (resolve_services)
-		prot_name = nl_proto_n2a(prot, prot_buf, sizeof(prot_buf));
-	else
-		prot_name = int_to_str(prot, prot_buf);
+	prot_name = nl_proto_n2a(prot, prot_buf, sizeof(prot_buf));
 
 	if (pid == -1) {
 		procname[0] = '*';
-	} else if (resolve_services) {
+	} else if (!numeric) {
 		int done = 0;
 		if (!pid) {
 			done = 1;
@@ -3592,7 +3589,7 @@ int main(int argc, char *argv[])
 				 long_opts, NULL)) != EOF) {
 		switch(ch) {
 		case 'n':
-			resolve_services = 0;
+			numeric = 1;
 			break;
 		case 'r':
 			resolve_hosts = 1;
@@ -3814,7 +3811,7 @@ int main(int argc, char *argv[])
 	filter_states_set(&current_filter, state_filter);
 	filter_merge_defaults(&current_filter);
 
-	if (resolve_services && resolve_hosts &&
+	if (!numeric && resolve_hosts &&
 	    (current_filter.dbs&(UNIX_DBM|(1<<TCP_DB)|(1<<UDP_DB)|(1<<DCCP_DB))))
 		init_service_resolver();
 
@@ -3886,7 +3883,7 @@ int main(int argc, char *argv[])
 	addrp_width /= 2;
 	addrp_width--;
 
-	serv_width = resolve_services ? 7 : 5;
+	serv_width = !numeric ? 7 : 5;
 
 	if (addrp_width < 15+serv_width+1)
 		addrp_width = 15+serv_width+1;
diff --git a/tc/tc.c b/tc/tc.c
index 46ff3714a2e9..f23794f03005 100644
--- a/tc/tc.c
+++ b/tc/tc.c
@@ -42,6 +42,7 @@ int resolve_hosts = 0;
 int use_iec = 0;
 int force = 0;
 bool use_names = false;
+int numeric;
 
 static char *conf_file;
 

Re: [PATCH iproute2 net-next] ip: add a new parameter -Numeric

[Stephen Hemminger]

On Mon, 20 May 2019 09:18:08 -0600
David Ahern <dsahern@gmail.com> wrote:

> On 5/20/19 1:56 AM, Hangbin Liu wrote:
> > When calles rtnl_dsfield_n2a(), we get the dsfield name from
> > /etc/iproute2/rt_dsfield. But different distribution may have
> > different names. So add a new parameter '-Numeric' to only show
> > the dsfield number.
> > 
> > This parameter is only used for tos value at present. We could enable
> > this for other fields if needed in the future.
> > 
> 
> It does not make sense to add this flag just for 1 field.
> 
> 3 years ago I started a patch to apply this across the board. never
> finished it. see attached. The numeric variable should be moved to
> lib/rt_names.c. It handles all of the conversions in that file - at
> least as of May 2016.


Agree, if you are going to do it, go all in.
Handle all types and in same manner for ip, tc, bridge, and devlink.
ss already has -numeric option.

Wish that -n wasn't used for netns, should have been -N...

Re: [PATCH iproute2 net-next] ip: add a new parameter -Numeric

[Hangbin Liu]

On Mon, May 20, 2019 at 10:03:22AM -0700, Stephen Hemminger wrote:
> On Mon, 20 May 2019 09:18:08 -0600
> David Ahern <dsahern@gmail.com> wrote:
> 
> > On 5/20/19 1:56 AM, Hangbin Liu wrote:
> > > When calles rtnl_dsfield_n2a(), we get the dsfield name from
> > > /etc/iproute2/rt_dsfield. But different distribution may have
> > > different names. So add a new parameter '-Numeric' to only show
> > > the dsfield number.
> > > 
> > > This parameter is only used for tos value at present. We could enable
> > > this for other fields if needed in the future.
> > > 
> > 
> > It does not make sense to add this flag just for 1 field.
> > 
> > 3 years ago I started a patch to apply this across the board. never
> > finished it. see attached. The numeric variable should be moved to
> > lib/rt_names.c. It handles all of the conversions in that file - at
> > least as of May 2016.
> 
> 
> Agree, if you are going to do it, go all in.
> Handle all types and in same manner for ip, tc, bridge, and devlink.
> ss already has -numeric option.

OK, I will do it.

BTW, for some pre-defined names in iproute2, like rtnl_rtprot_tab,
nl_proto_tab. Should we also print the number directly or just keep
using the human readable names?

I would like to keep them as this is defined in iproute and we can control
them. But this may make people feel confused with the -Numeric parameter.
So what do you think?

Thanks
Hangbin

Re: [PATCH iproute2 net-next] ip: add a new parameter -Numeric

[David Ahern]

On 5/21/19 6:13 AM, Hangbin Liu wrote:
> BTW, for some pre-defined names in iproute2, like rtnl_rtprot_tab,
> nl_proto_tab. Should we also print the number directly or just keep
> using the human readable names?
> 
> I would like to keep them as this is defined in iproute and we can control
> them. But this may make people feel confused with the -Numeric parameter.
> So what do you think?

To me numeric means no number to string conversions, and the protocol is
passed as a number.